Greg,
I have a few comments down below. The main thing is that this patch
needs to be broken into a few patches. Another thing is to have one
of the patches be a generic GPIO read command in, even if it starts
out pretty limited.
From: Greg Turner <gregturner@ti.com>
Modified bootcmd to check the staus at boot time and set filename of the boot script.
Signed-off-by: Jason Kridner <jkridner@beagleboard.org>
---
common/cmd_boot.c | 45 ++++++++++++++++++++++++++++++++++++++++
include/configs/omap3_beagle.h | 11 +++++++++
2 files changed, 56 insertions(+), 0 deletions(-)
diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index bfc1db2..4caf674 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -35,6 +35,51 @@ unsigned long do_go_exec (ulong (*entry)(int, char *), int argc, char *argv)
return entry (argc, argv);
}
+/*
+ * 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)
This function really belongs in its own .c file, not cmd_boot. I
understand this was a quick hack, but we'll want to put this
functionality in the mainline, so we want to clean it up. The patch
that adds this command should be different than the device-specific
patch that implements the actual GPIO read for the board, which should
be different than the patch that enables, configures, and uses the
GPIO read.
The name of this function should reflect simple GPIO read functionality.
+{
+ int button = 0;
+
+ //printf ("## Return Status of User button\n");
+
+ /*
+ * pass address parameter as argv[0] (aka command name),
+ * and all remaining args
+ */
+ omap_request_gpio(4);
+ omap_set_gpio_direction(4, 1);
There needs to be a call to a board specific function/implementation,
but the main command should be set here so that it is compatible
across all systems that implement your GPIO read command.
+ printf("The user button is currently ");
This will need to be more generic.
+ if(omap_get_gpio_datain(4))
+ {
+ button = 1;
+ }
+ else
+ {
+ button = 0;
+ printf("NOT ");
+ }
+
+ printf("pressed.\n");
+
+ omap_free_gpio(4);
+
+ return button;
I find it really odd that the behavior seems to be that if the button
is pressed, the 'if' test fails. Since this is a GPIO read function,
perhaps it should take an argument of a mask to test such that a
successful return of "0" happens when the button is pressed.
+}
+
+/* -------------------------------------------------------------------- */
+
+U_BOOT_CMD(
+ userbutton, CONFIG_SYS_MAXARGS, 1, do_userbutton,
The name of the function should be something other than "userbutton",
because that isn't generic at all. If other folks don't have interest
in this function, then the whole file should be removed from the
common folder. Perhaps we should send a copy of this to the u-boot
mailing list to get feedback?
+ "Return the status of the user button",
+ ""
+);
+
+
int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv)
{
ulong addr, rc;
diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
index eaa8779..555b350 100644
--- a/include/configs/omap3_beagle.h
+++ b/include/configs/omap3_beagle.h
@@ -254,8 +254,19 @@
"run ramargs; " \
"bootm ${loadaddr}\0" \
+/*
+ * The default bootcmd checks the status of the user button
+ * and sets the boot script accordingly.
+ * If the user button is NOT pressed: bootscr = boot.scr
+ * If the user button is pressed: bootscr = user.scr
+ */
Most of the BOOTCOMMAND isn't documented with comments. We should
probably document the entire process or not at all (have it be
obvious).