[U-Boot] [PATCH v3 09/12] samsung: misc: Add LCD download menu.

[Minkyu Kang <mk7.kang@samsung.com>]

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.marczak@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.