On Sun, Sep 11, 2011 at 2:41 PM, Joel A Fernandes agnel.joel@gmail.com wrote:

Remove userbutton command and do the detection in board config file using the gpio command

Can you split these into two or three patches? I'm not sure what is normal, but I suspect that this is one of those things where you should remove a feature in one patch then add a replacement with another, rather than doing both in one patch. In general, I would add the replacement functionality before removing the old command.

More comments below...

Signed-off-by: Joel A Fernandes agnel.joel@gmail.com Signed-off-by: Jason Kridner jkridner@beagleboard.org

---

This addresses the suggestions at: http://patchwork.ozlabs.org/patch/110123/ Also, with the removal of the userbutton command, all checkpatch errors are automatically fixed.

v2 changes: Corrected comment about userbutton

board/ti/beagle/beagle.c       |   55 ----------------------------------------  include/configs/omap3_beagle.h |   11 ++++++-  2 files changed, 9 insertions(+), 57 deletions(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c index 8cdceaf..9ce8aeb 100644 --- a/board/ti/beagle/beagle.c +++ b/board/ti/beagle/beagle.c @@ -479,58 +479,3 @@ int ehci_hcd_init(void)  }

#endif /* CONFIG_USB_EHCI */

-/*

• • This command returns the status of the user button on beagle xM
• • Input - none
• • Returns -   1 if button is held down
• • 0 if button is not held down
• */

-int do_userbutton (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) -{

• int     button = 0;
• int     gpio;
• /*
• * pass address parameter as argv[0] (aka command name),
• * and all remaining args
• */
• switch (get_board_revision()) {
• case REVISION_AXBX:
• case REVISION_CX:
• case REVISION_C4:
• gpio = 7;
• break;
• case REVISION_XM_A:
• case REVISION_XM_B:
• case REVISION_XM_C:
• default:
• gpio = 4;
• break;
• }
• gpio_request(gpio, "");
• gpio_direction_input(gpio);
• printf("The user button is currently ");
• if (gpio_get_value(gpio))
• {
• button = 1;
• printf("PRESSED.\n");
• }
• else
• {
• button = 0;
• printf("NOT pressed.\n");
• }
• gpio_free(gpio);
• return !button;

-}

-/* -------------------------------------------------------------------- */

-U_BOOT_CMD(

• userbutton, CONFIG_SYS_MAXARGS, 1,      do_userbutton,
• "Return the status of the BeagleBoard USER button",
• ""

-); diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 1369c89..0cf81a6 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -164,6 +164,7 @@  #define CONFIG_CMD_PING  #define CONFIG_CMD_DHCP  #define CONFIG_CMD_SETEXPR     /* Evaluate expressions         */ +#define CONFIG_CMD_GPIO     /* Enable gpio command */

#undef CONFIG_CMD_FLASH                /* flinfo, erase, protect       */  #undef CONFIG_CMD_FPGA         /* FPGA configuration Support   */ @@ -277,10 +278,16 @@        "ramboot=echo Booting from ramdisk ...; " \                "run ramargs; " \                "bootm ${loadaddr}\0" \

• "userbutton=if gpio input 173; then run userbutton_xm; " \
• "else run userbutton_nonxm; fi;\0" \

There is an environment variable called 'beaglerev'. Is there an easy way to use it?

• "userbutton_xm=gpio input 4;\0" \
• "userbutton_nonxm=gpio input 7;\0"

+/* "run userbutton" will return 1 (false) if is pressed and 0 (true) if not */  #define CONFIG_BOOTCOMMAND \        "if mmc rescan ${mmcdev}; then " \

• "if userbutton; then " \

• "if run userbutton; then " \
• "setenv bootenv uEnv.txt;" \
• "else " \

"setenv bootenv user.txt;" \                "fi;" \                "echo SD/MMC found on device ${mmcdev};" \ -- 1.7.1