Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Thu, Sep 8, 2011 at 3:05 AM, Wolfgang Denk wrote: > Dear Joel A Fernandes, > > In message <1313462214-3716-2-git-send-email-agnel.j...@gmail.com> you wrote: >> From: Jason Kridner >> >> Based on commit f1099c7c43caf5bac3bf6a65aa266fade4747072 >> Author: Greg Turner >> Date: Tue May 25 09:19:06 2010 -0500 >> >> New u-boot command for status of USER button on BeagleBoard-xM >> >> Modified bootcmd to check the staus at boot time and set >> filename of the boot script. >> >> * Moved to a BeagleBoard specific file. >> * Removed changes to default boot command from adding userbutton >> command. >> * Made to handle pre-xM boards. >> * Flipped polarity of the return value to avoid confusion. Success (0) >> is when the button is pressed. Failure (1) is when the button is NOT >> pressed. >> * Used latest revision getting function. >> * Used latest macros for board revision. >> * Added xM-C revision definition (optional, since it was default) >> * updated default configuration with UserButton functionality >> * Added a separate bootenv variable to load a user defined .txt file >> * Added an example, showing how a different environment file can be loaded >> with >> the user button pressed > > Your patch has a large number of cding style issues; please always > run checkpatch before submitting patches. Very sorry, I might have missed running it on this patch. I will correct it and resubmit it > > Also, I agree with Albert: there should be no need for a separate > userbutton command. > > Please fix and resubmit. Sure. Thanks, Joel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Thu, Sep 8, 2011 at 10:10 AM, Kridner, Jason wrote: > > > On Sep 8, 2011, at 11:03 AM, "Albert ARIBAUD" > wrote: > >> Hi Joel, >> >> Le 08/09/2011 16:56, Joel A Fernandes a écrit : >> Also, I agree with Albert: there should be no need for a separate userbutton command. Please fix and resubmit. >>> >>> If this patch is to be dropped, then I'm not sure why the need to resubmit? >> >> I gather the "resubmit" means "resubmit the patch set minus this patch and >> renumbered accordingly". > > One thing that would be necessary is to replace the default bootcmd with one > that uses the gpio command and to enable the gpio command. > One of the difficulties with this is the GPIO value for userbutton depends on the board rev which is what your patch does. Hardcoding gpio values for one will break support for other boards Without adding any additional commands, the only way I see this can be done is: 1. Use the gpio and test commands to get the board rev in omap3_beagle.h 2. Based on this, use the right gpio value to pass to another gpio command to check userbutton state This is quite messy IMO and is redundant because the revision detection logic is already in the board/ file. Another way is to add a command to the board/ file like "get_userbutton_gpio" and use the value it returns to pass to the gpio command but this seems to defeat the purpose of the cleanup itself. If you think one of the above methods is acceptable, please ACK them and I will start developing the patch. Thanks, Joel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Fri, Sep 9, 2011 at 9:19 AM, Jason Kridner wrote: > On Thu, Sep 8, 2011 at 9:41 PM, Joel A Fernandes wrote: >> On Thu, Sep 8, 2011 at 10:10 AM, Kridner, Jason wrote: >>> >>> >>> On Sep 8, 2011, at 11:03 AM, "Albert ARIBAUD" >>> wrote: >>> Hi Joel, Le 08/09/2011 16:56, Joel A Fernandes a écrit : >> Also, I agree with Albert: there should be no need for a separate >> userbutton command. >> >> Please fix and resubmit. > > If this patch is to be dropped, then I'm not sure why the need to > resubmit? I gather the "resubmit" means "resubmit the patch set minus this patch and renumbered accordingly". >>> >>> One thing that would be necessary is to replace the default bootcmd with >>> one that uses the gpio command and to enable the gpio command. >>> >> >> Actually the functions in arch/arm/cpu/armv7/omap-common/gpio.c are >> not compatible with the ones used by common/cmd_led.c > > The patches from Sanjeev Premi seemed to me to handle the deltas. Are > you applying those? They are working for me for use with the led > command. > I missed those patches! I have updated my tree and can see them now, thanks. Regards, Joel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Thu, Sep 8, 2011 at 9:41 PM, Joel A Fernandes wrote: > On Thu, Sep 8, 2011 at 10:10 AM, Kridner, Jason wrote: >> >> >> On Sep 8, 2011, at 11:03 AM, "Albert ARIBAUD" >> wrote: >> >>> Hi Joel, >>> >>> Le 08/09/2011 16:56, Joel A Fernandes a écrit : >>> > Also, I agree with Albert: there should be no need for a separate > userbutton command. > > Please fix and resubmit. If this patch is to be dropped, then I'm not sure why the need to resubmit? >>> >>> I gather the "resubmit" means "resubmit the patch set minus this patch and >>> renumbered accordingly". >> >> One thing that would be necessary is to replace the default bootcmd with one >> that uses the gpio command and to enable the gpio command. >> > > Actually the functions in arch/arm/cpu/armv7/omap-common/gpio.c are > not compatible with the ones used by common/cmd_led.c The patches from Sanjeev Premi seemed to me to handle the deltas. Are you applying those? They are working for me for use with the led command. > > What would be the right place to implement the required functions for > the gpio command? > > 1. In the (beagle)board file as wrappers to the omap gpio functions > 2. In the gpio.c file in omap-common > 3. Any other option? Do you mean to replace userbutton? As long as the gpio command returns a status, then you should be able to remove userbutton from the board.c file and add usage of gpio to replace userbutton in the omap3_beagle.h config header file. (Of course, the patches should apply those changes in opposite order to make sure git bisect doesn't show breakage.) If you are saying that gpio.c is missing functionality, please describe the missing functionality. > > Thanks, > Joel > ___ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot > ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Thu, Sep 8, 2011 at 10:10 AM, Kridner, Jason wrote: > > > On Sep 8, 2011, at 11:03 AM, "Albert ARIBAUD" > wrote: > >> Hi Joel, >> >> Le 08/09/2011 16:56, Joel A Fernandes a écrit : >> Also, I agree with Albert: there should be no need for a separate userbutton command. Please fix and resubmit. >>> >>> If this patch is to be dropped, then I'm not sure why the need to resubmit? >> >> I gather the "resubmit" means "resubmit the patch set minus this patch and >> renumbered accordingly". > > One thing that would be necessary is to replace the default bootcmd with one > that uses the gpio command and to enable the gpio command. > Actually the functions in arch/arm/cpu/armv7/omap-common/gpio.c are not compatible with the ones used by common/cmd_led.c What would be the right place to implement the required functions for the gpio command? 1. In the (beagle)board file as wrappers to the omap gpio functions 2. In the gpio.c file in omap-common 3. Any other option? Thanks, Joel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Thu, Sep 8, 2011 at 10:10 AM, Kridner, Jason wrote: > > > On Sep 8, 2011, at 11:03 AM, "Albert ARIBAUD" > wrote: > >> Hi Joel, >> >> Le 08/09/2011 16:56, Joel A Fernandes a écrit : >> Also, I agree with Albert: there should be no need for a separate userbutton command. Please fix and resubmit. >>> >>> If this patch is to be dropped, then I'm not sure why the need to resubmit? >> >> I gather the "resubmit" means "resubmit the patch set minus this patch and >> renumbered accordingly". > > One thing that would be necessary is to replace the default bootcmd with one > that uses the gpio command and to enable the gpio command. > Sounds like a good idea, I will work on this and submit a patch soon. Thanks, Joel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Sep 8, 2011, at 11:03 AM, "Albert ARIBAUD" wrote: > Hi Joel, > > Le 08/09/2011 16:56, Joel A Fernandes a écrit : > >>> Also, I agree with Albert: there should be no need for a separate >>> userbutton command. >>> >>> Please fix and resubmit. >> >> If this patch is to be dropped, then I'm not sure why the need to resubmit? > > I gather the "resubmit" means "resubmit the patch set minus this patch and > renumbered accordingly". One thing that would be necessary is to replace the default bootcmd with one that uses the gpio command and to enable the gpio command. > >> Thanks, >> Joel > > Amicalement, > -- > Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
Hi Joel, Le 08/09/2011 16:56, Joel A Fernandes a écrit : >> Also, I agree with Albert: there should be no need for a separate >> userbutton command. >> >> Please fix and resubmit. > > If this patch is to be dropped, then I'm not sure why the need to resubmit? I gather the "resubmit" means "resubmit the patch set minus this patch and renumbered accordingly". > Thanks, > Joel Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
On Thu, Sep 8, 2011 at 3:05 AM, Wolfgang Denk wrote: > Dear Joel A Fernandes, > > In message <1313462214-3716-2-git-send-email-agnel.j...@gmail.com> you wrote: >> From: Jason Kridner >> >> Based on commit f1099c7c43caf5bac3bf6a65aa266fade4747072 >> Author: Greg Turner >> Date: Tue May 25 09:19:06 2010 -0500 >> >> New u-boot command for status of USER button on BeagleBoard-xM >> .. > > Also, I agree with Albert: there should be no need for a separate > userbutton command. > > Please fix and resubmit. > If this patch is to be dropped, then I'm not sure why the need to resubmit? Thanks, Joel ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
Dear Joel A Fernandes, In message <1313462214-3716-2-git-send-email-agnel.j...@gmail.com> you wrote: > From: Jason Kridner > > Based on commit f1099c7c43caf5bac3bf6a65aa266fade4747072 > Author: Greg Turner > Date: Tue May 25 09:19:06 2010 -0500 > > New u-boot command for status of USER button on BeagleBoard-xM > > Modified bootcmd to check the staus at boot time and set >filename of the boot script. > > * Moved to a BeagleBoard specific file. > * Removed changes to default boot command from adding userbutton > command. > * Made to handle pre-xM boards. > * Flipped polarity of the return value to avoid confusion. Success (0) > is when the button is pressed. Failure (1) is when the button is NOT > pressed. > * Used latest revision getting function. > * Used latest macros for board revision. > * Added xM-C revision definition (optional, since it was default) > * updated default configuration with UserButton functionality > * Added a separate bootenv variable to load a user defined .txt file > * Added an example, showing how a different environment file can be loaded > with > the user button pressed Your patch has a large number of cding style issues; please always run checkpatch before submitting patches. Also, I agree with Albert: there should be no need for a separate userbutton command. Please fix and resubmit. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de The average woman would rather have beauty than brains, because the average man can see better than he can think. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
Le 16/08/2011 04:36, Joel A Fernandes a écrit : > From: Jason Kridner > > Based on commit f1099c7c43caf5bac3bf6a65aa266fade4747072 Can(t find this one in my tree. > Author: Greg Turner > Date: Tue May 25 09:19:06 2010 -0500 > > New u-boot command for status of USER button on BeagleBoard-xM > > Modified bootcmd to check the staus at boot time and set >filename of the boot script. Generally speaking, is a 'userbutton' command' really needed when a gpio command exists? The notion of 'user button' isn't really something fundamentally meaningful to a bootloader -- We don't have a concept of 'user', even. Amicalement, -- Albert. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 2/3] BeagleBoard: Added userbutton command
From: Jason Kridner
Based on commit f1099c7c43caf5bac3bf6a65aa266fade4747072
Author: Greg Turner
Date: Tue May 25 09:19:06 2010 -0500
New u-boot command for status of USER button on BeagleBoard-xM
Modified bootcmd to check the staus at boot time and set
filename of the boot script.
* Moved to a BeagleBoard specific file.
* Removed changes to default boot command from adding userbutton
command.
* Made to handle pre-xM boards.
* Flipped polarity of the return value to avoid confusion. Success (0)
is when the button is pressed. Failure (1) is when the button is NOT
pressed.
* Used latest revision getting function.
* Used latest macros for board revision.
* Added xM-C revision definition (optional, since it was default)
* updated default configuration with UserButton functionality
* Added a separate bootenv variable to load a user defined .txt file
* Added an example, showing how a different environment file can be loaded
with
the user button pressed
Signed-off-by: Jason Kridner
Signed-off-by: Koen Kooi
Signed-off-by: Joel A Fernandes
Cc: Greg Turner
---
board/ti/beagle/beagle.c | 56
include/configs/omap3_beagle.h |7 -
2 files changed, 62 insertions(+), 1 deletions(-)
diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index a958545..8dc8dfa 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -50,6 +50,7 @@ extern struct ehci_hccr *hccr;
extern volatile struct ehci_hcor *hcor;
#endif
#include "beagle.h"
+#include
#define pr_debug(fmt, args...) debug(fmt, ##args)
@@ -446,3 +447,58 @@ 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;
+ }
+ omap_request_gpio(gpio);
+ omap_set_gpio_direction(gpio, 1);
+ printf("The user button is currently ");
+ if(omap_get_gpio_datain(gpio))
+ {
+ button = 1;
+ printf("PRESSED.\n");
+ }
+ else
+ {
+ button = 0;
+ printf("NOT pressed.\n");
+ }
+
+ omap_free_gpio(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 c41d48e..a494f99 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -227,7 +227,8 @@
"omapdss.def_disp=${defaultdisplay} " \
"root=${nandroot} " \
"rootfstype=${nandrootfstype}\0" \
- "loadbootenv=fatload mmc ${mmcdev} ${loadaddr} uEnv.txt\0" \
+ "bootenv=uEnv.txt\0" \
+ "loadbootenv=fatload mmc ${mmcdev} ${loadaddr} ${bootenv}\0" \
"importbootenv=echo Importing environment from mmc ...; " \
"env import -t $loadaddr $filesize\0" \
"loaduimage=fatload mmc ${mmcdev} ${loadaddr} uImage\0" \
@@ -241,8 +242,12 @@
#define CONFIG_BOOTCOMMAND \
"if mmc rescan ${mmcdev}; then " \
+ "if userbutton; then " \
+ "setenv bootenv user.txt;" \
+ "fi;" \
"echo SD/MMC found on device ${mmcdev};" \
"if run loadbootenv; then " \
+ "echo Loaded environment from ${bootenv};" \
"run importbootenv;" \
"fi;" \
"if test -n $uenvcmd; then " \
--
1.7.1
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
