On 04/01/14 01:43, Przemyslaw Marczak wrote:
> This simple LCD menu allows run one of download mode on device
> without writing on console or for fast and easy upgrade.
> This feature check user keys combination at boot:
> - power key + volume up - download menu
> - power key + volume down - thor mode (without menu)
>
> New configs:
> - CONFIG_LCD_MENU
> - CONFIG_LCD_MENU_BOARD
> which depends on: CONFIG_MISC_INIT_R
>
> For proper effect this feature needs following definitions:
>
> Power key:
> - KEY_PWR_PMIC_NAME - (string) pmic which supports power key check
>
> Register address:
> - KEY_PWR_STATUS_REG
> - KEY_PWR_INTERRUPT_REG
>
> Register power key mask:
> - KEY_PWR_STATUS_MASK
> - KEY_PWR_INTERRUPT_MASK
>
> Gpio numbers:
> - KEY_PWR_INTERRUPT_MASK
> - KEY_VOL_DOWN_GPIO
>
> Signed-off-by: Przemyslaw Marczak <p.marc...@samsung.com>
>
> ---
> Changes v2:
> - remove keys.h - definitions should be in boards headers
> - add misc.h
> - code cleanup
> - extend commit msg by more informations
>
> Changes v3:
> - none
>
> board/samsung/common/misc.c | 340
> +++++++++++++++++++++++++++++++++++++++++++
> board/samsung/common/misc.h | 18 +++
> 2 files changed, 358 insertions(+)
> create mode 100644 board/samsung/common/misc.h
>
> diff --git a/board/samsung/common/misc.c b/board/samsung/common/misc.c
> index 3a91d62..93f766c 100644
> --- a/board/samsung/common/misc.c
> +++ b/board/samsung/common/misc.c
> @@ -8,6 +8,341 @@
> #include <common.h>
> #include <lcd.h>
> #include <libtizen.h>
> +#include <errno.h>
> +#include <version.h>
> +#include <asm/sizes.h>
> +#include <asm/arch/cpu.h>
> +#include <asm/arch/gpio.h>
> +#include <asm/gpio.h>
> +#include <linux/input.h>
> +#include <lcd.h>
> +#include <libtizen.h>
> +#include <power/pmic.h>
> +#include <mmc.h>
> +#include "misc.h"
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#ifdef CONFIG_LCD_MENU
> +static int power_key_pressed(int reg)
u32 reg?
> +{
> + struct pmic *pmic = pmic_get(KEY_PWR_PMIC_NAME);
> + u32 status = 0;
= 0 do not need.
please do not use initial value, if possible.
> + u32 mask;
please check return value of pmic_get
pmic = pmic_get(KEY_PWR_PMIC_NAME);
if !(pmic)
return 0;
> +
> + if (pmic_probe(pmic))
> + return 0;
> +
> + if (!pmic) {
> + printf("%s: Not found\n", KEY_PWR_PMIC_NAME);
> + return 0;
> + }
> +
> + if (reg == KEY_PWR_STATUS_REG)
> + mask = KEY_PWR_STATUS_MASK;
> + else
> + mask = KEY_PWR_INTERRUPT_MASK;
> +
> + if (pmic_reg_read(pmic, reg, &status))
> + return 0;
> +
> + return !!(status & mask);
> +}
> +
> +static int key_pressed(int key)
> +{
> + int value = 0;
please do not use initial value, if possible.
instead, please add "value = 0" at default statement.
> +
> + switch (key) {
> + case KEY_POWER:
> + value = power_key_pressed(KEY_PWR_INTERRUPT_REG);
> + break;
> + case KEY_VOLUMEUP:
> + value = !gpio_get_value(KEY_VOL_UP_GPIO);
> + break;
> + case KEY_VOLUMEDOWN:
> + value = !gpio_get_value(KEY_VOL_DOWN_GPIO);
> + break;
> + default:
> + break;
> + }
> +
> + return value;
> +}
> +
> +static int check_keys(void)
> +{
> + int keys = 0;
> +
> + if (key_pressed(KEY_POWER))
> + keys += KEY_POWER;
> + if (key_pressed(KEY_VOLUMEUP))
> + keys += KEY_VOLUMEUP;
> + if (key_pressed(KEY_VOLUMEDOWN))
> + keys += KEY_VOLUMEDOWN;
> +
> + return keys;
> +}
> +
> +/*
> + * 0 BOOT_MODE_INFO
> + * 1 BOOT_MODE_THOR
> + * 2 BOOT_MODE_UMS
> + * 3 BOOT_MODE_DFU
> + * 4 BOOT_MODE_EXIT
> + */
> +static char *
> +mode_name[BOOT_MODE_EXIT + 1] = {"DEVICE",
please move the "DEVICE" to next line.
> + "THOR",
> + "UMS",
> + "DFU",
> + "EXIT"};
please move the brace to next line.
> +
> +static char *
> +mode_info[BOOT_MODE_EXIT + 1] = {"info",
> + "downloader",
> + "mass storage",
> + "firmware update",
> + "and run normal boot"};
ditto.
> +
> +static char *
> +mode_cmd[BOOT_MODE_EXIT + 1] = {"",
> + "thor 0 mmc 0",
> + "ums 0 mmc 0",
> + "dfu 0 mmc 0",
> + ""};
ditto.
> +
> +static void display_board_info(void)
> +{
#ifdef CONFIG_GENERIC_MMC
> + struct mmc *mmc = find_mmc_device(0);
#endif
> + vidinfo_t *vid = &panel_info;
> +
> + lcd_position_cursor(4, 4);
> +
> + lcd_printf("%s\n\t", U_BOOT_VERSION);
> + lcd_puts("\n\t\tBoard Info:\n");
> +#ifdef CONFIG_SYS_BOARD
> + lcd_printf("\tBoard name: %s\n", CONFIG_SYS_BOARD);
> +#endif
> +#ifdef CONFIG_REVISION_TAG
> + lcd_printf("\tBoard rev: %u\n", get_board_rev());
> +#endif
> + lcd_printf("\tDRAM banks: %u\n", CONFIG_NR_DRAM_BANKS);
> + lcd_printf("\tDRAM size: %u MB\n", gd->ram_size / SZ_1M);
> +
#ifdef CONFIG_GENERIC_MMC
> + if (mmc)
> + lcd_printf("\teMMC size: %llu MB\n", mmc->capacity / SZ_1M);
#endif
maybe, you can access mmc->capacity after do mmc_init.
> +
> + if (vid)
> + lcd_printf("\tDisplay resolution: %u x % u\n",
> + vid->vl_col, vid->vl_row);
> +
> + lcd_printf("\tDisplay BPP: %u\n", 1 << vid->vl_bpix);
> +}
> +
> +static int mode_leave_menu(int mode)
> +{
> + int mode_supported = 1;
> + int leave = 0;
> + char *exit_option;
> + char *exit_boot = "boot";
> + char *exit_back = "back";
> +
> + switch (mode) {
> + case BOOT_MODE_INFO:
> +#if !defined(CONFIG_LCD_MENU_BOARD)
> + mode_supported = 0;
> +#endif
> + break;
> + case BOOT_MODE_THOR:
> +#if !defined(CONFIG_CMD_THOR_DOWNLOAD)
> + mode_supported = 0;
> +#endif
> + break;
> + case BOOT_MODE_UMS:
> +#if !defined(CONFIG_CMD_USB_MASS_STORAGE)
> + mode_supported = 0;
> +#endif
> + break;
> + case BOOT_MODE_DFU:
> +#if !defined(CONFIG_CMD_DFU)
> + mode_supported = 0;
> +#endif
> + break;
> + case BOOT_MODE_EXIT:
> + leave = 1;
> + goto exit;
why don't you return directly at here.
goto statement is used here only.
> + default:
then, mode_supported = 0; maybe?
> + break;
> + }
> +
> + lcd_clear();
> +
> + if (mode_supported) {
> + if (mode) {
> + lcd_printf("\n\n\t%s %s\n", mode_name[mode],
> + mode_info[mode]);
> + lcd_puts("\n\tDo not turn off device before finish!\n");
> +
> + run_command(mode_cmd[mode], 0);
do not need to check a return value?
> + printf("Command finished\n");
> + lcd_clear();
> + lcd_printf("\n\n\t%s finished\n", mode_name[mode]);
> + exit_option = exit_boot;
> + leave = 1;
> + } else {
> + display_board_info();
> + exit_option = exit_back;
> + leave = 0;
> + }
> + } else {
> + lcd_puts("\n\n\tThis mode is not supported.\n");
> + exit_option = exit_back;
> + leave = 0;
> + }
> +
> + lcd_printf("\n\n\tPress POWER KEY to %s\n", exit_option);
> +
> + /* Wait for PWR key */
> + while (!key_pressed(KEY_POWER))
> + udelay(1000);
> +exit:
> + lcd_clear();
> + return leave;
> +}
> +
> +static void display_download_menu(int mode)
> +{
> + char *menu_name = "Download Mode Menu";
> + char *indicator = "[=>]";
> + char *blank = "[ ]";
Do we need those three variable?
> + char *selection[BOOT_MODE_EXIT + 1];
> + int i;
> +
> + for (i = 0; i <= BOOT_MODE_EXIT; i++)
> + selection[i] = blank;
> +
> + selection[mode] = indicator;
> +
> + lcd_clear();
> + lcd_printf("\n\t\t%s\n", menu_name);
> +
> + for (i = 0; i <= BOOT_MODE_EXIT; i++)
> + lcd_printf("\t%s %s - %s\n\n", selection[i],
> + mode_name[i],
> + mode_info[i]);
> +}
> +
> +static void download_menu(void)
> +{
> + int mode = 0;
> + int last_mode = 0;
> + int run;
> + int key;
> +
> + display_download_menu(mode);
> +
> + while (1) {
> + run = 0;
> +
> + if (mode != last_mode)
> + display_download_menu(mode);
> +
> + last_mode = mode;
> + udelay(100000);
> +
> + key = check_keys();
> + switch (key) {
> + case KEY_POWER:
> + run = 1;
> + break;
> + case KEY_VOLUMEUP:
> + if (mode > 0)
> + mode--;
> + break;
> + case KEY_VOLUMEDOWN:
> + if (mode < BOOT_MODE_EXIT)
> + mode++;
> + break;
> + default:
> + break;
> + }
> +
> + if (run) {
> + if (mode_leave_menu(mode))
> + break;
> +
> + display_download_menu(mode);
> + }
> + }
> +
> + lcd_clear();
> +}
> +
> +static void display_mode_info(void)
> +{
> + lcd_position_cursor(4, 4);
> + lcd_printf("%s\n", U_BOOT_VERSION);
> + lcd_puts("\nDownload Mode Menu\n");
> +#ifdef CONFIG_SYS_BOARD
> + lcd_printf("Board name: %s\n", CONFIG_SYS_BOARD);
> +#endif
> + lcd_printf("Press POWER KEY to display MENU options.");
> +}
> +
> +static int boot_menu(void)
> +{
> + int key = 0;
> + int timeout = 10;
> +
> + display_mode_info();
> +
> + while (timeout--) {
> + lcd_printf("\rNormal boot will start in: %d seconds.", timeout);
> + udelay(1000000);
I think.. user input can be ignored because too long delay.
1 second? it's too long.
and how about use mdelay instead?
> +
> + key = key_pressed(KEY_POWER);
> + if (key)
> + break;
> + }
> +
> + lcd_clear();
> +
> + /* If PWR pressed - show download menu */
> + if (key) {
> + printf("Power pressed - go to download menu\n");
> + udelay(1000000);
delay for 1 second? is it need?
> + download_menu();
> + printf("Download mode exit.\n");
> + }
> +
> + return 0;
> +}
> +
> +static void check_boot_mode(void)
> +{
> + int pwr_key;
> +
> + pwr_key = power_key_pressed(KEY_PWR_STATUS_REG);
> + if (!pwr_key)
> + return;
> +
> + /* Clear PWR button Rising edge interrupt status flag */
> + power_key_pressed(KEY_PWR_INTERRUPT_REG);
> +
> + if (key_pressed(KEY_VOLUMEUP))
> + boot_menu();
> + else if (key_pressed(KEY_VOLUMEDOWN))
> + mode_leave_menu(BOOT_MODE_THOR);
> +}
> +
> +static void keys_init(void)
> +{
> + /* Set direction to input */
> + gpio_direction_input(KEY_VOL_UP_GPIO);
> + gpio_direction_input(KEY_VOL_DOWN_GPIO);
> +}
> +#endif /* CONFIG_LCD_MENU */
>
> #ifdef CONFIG_CMD_BMP
> static void draw_logo(void)
> @@ -50,9 +385,14 @@ static void draw_logo(void)
> /* Common for Samsung boards */
> int misc_init_r(void)
> {
> +#ifdef CONFIG_LCD_MENU
> + keys_init();
> + check_boot_mode();
> +#endif
> #ifdef CONFIG_CMD_BMP
> if (panel_info.logo_on)
> draw_logo();
> #endif
> +
> return 0;
> }
> diff --git a/board/samsung/common/misc.h b/board/samsung/common/misc.h
> new file mode 100644
> index 0000000..f583552
> --- /dev/null
> +++ b/board/samsung/common/misc.h
> @@ -0,0 +1,18 @@
> +#ifndef __SAMSUNG_COMMON_MISC_H__
> +#define __SAMSUNG_COMMON_MISC_H__
> +
> +#ifdef CONFIG_LCD_MENU
> +enum {
> + BOOT_MODE_INFO,
> + BOOT_MODE_THOR,
> + BOOT_MODE_UMS,
> + BOOT_MODE_DFU,
> + BOOT_MODE_EXIT,
> +};
> +
> +#ifdef CONFIG_REVISION_TAG
> +u32 get_board_rev(void);
> +#endif
> +#endif /* CONFIG_LCD_MENU */
> +
> +#endif /* __SAMSUNG_COMMON_MISC_H__ */
> \ No newline at end of file
>
Thanks,
Minkyu Kang.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
