Hello Minkyu,
Thank you for review.
On 01/06/2014 12:38 PM, Minkyu Kang wrote:
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.
I will add mmc_init for sure here. But in most cases mmc is initialized
at misc_init_r stage because of environment initialization.
+
+ 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.
Ok, will be changed.
+ 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?
You're right, I will add checking here.
+ 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?
Actually we don't. Will be removed.
+ 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?
User input can't be ignored because key_pressed(KEY_POWER) is using pmic
interrupt register. It is no matter when user press the power key, if he
does then he will wait at most 1 second.
+
+ 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?
You're right It is unneeded here.
+ 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.
Comments will be applied to next version.
Regards
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marc...@samsung.com
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
