2017-04-01 12:22 GMT+08:00 Simon Glass sjg@chromium.org:
Hi,
On 27 March 2017 at 08:38, techping.chan@gmail.com wrote:
From: Ziping Chen techping.chan@gmail.com
Currently the "led" command only supports the old API without DM.
Add DM-based implementation of this command.
Also allow this command to be select with Kconfig.
Signed-off-by: Ziping Chen techping.chan@gmail.com
cmd/Kconfig | 6 ++++ cmd/Makefile | 4 +++ cmd/led.c | 113 ++++++++++++++++++++++++++++++
+++++++++++++++++++++++++----
3 files changed, 116 insertions(+), 7 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 25e3b78..66c94de 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -511,6 +511,12 @@ config CMD_GPIO help GPIO support.
+config CMD_LED
bool "led"depends on LEDhelpLED support.endmenu
diff --git a/cmd/Makefile b/cmd/Makefile index f13bb8c..0817de5 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -79,7 +79,11 @@ obj-$(CONFIG_CMD_ITEST) += itest.o obj-$(CONFIG_CMD_JFFS2) += jffs2.o obj-$(CONFIG_CMD_CRAMFS) += cramfs.o obj-$(CONFIG_CMD_LDRINFO) += ldrinfo.o +ifdef CONFIG_LED +obj-$(CONFIG_CMD_LED) += led.o +else obj-$(CONFIG_LED_STATUS_CMD) += led.o +endif obj-$(CONFIG_CMD_LICENSE) += license.o obj-y += load.o obj-$(CONFIG_LOGBUFFER) += log.o diff --git a/cmd/led.c b/cmd/led.c index d50938a..3849a79 100644 --- a/cmd/led.c +++ b/cmd/led.c @@ -13,8 +13,14 @@ #include <common.h> #include <config.h> #include <command.h> +#ifndef CONFIG_LED #include <status_led.h> +#else +#include <dm.h> +DECLARE_GLOBAL_DATA_PTR; +#endif
I think you can drop those two #ifdefs.
+#ifndef CONFIG_LED struct led_tbl_s { char *string; /* String for use in the command
*/
led_id_t mask; /* Mask used for calling__led_set() */
@@ -62,6 +68,15 @@ static const led_tbl_t led_commands[] = { { NULL, 0, NULL, NULL, NULL } };
+/*
- LED drivers providing a blinking LED functionality, like the
- PCA9551, can override this empty weak function
- */
+void __weak __led_blink(led_id_t mask, int freq) +{ +} +#endif
enum led_cmd { LED_CMD_ERROR = -1, LED_CMD_ON, @@ -78,19 +93,53 @@ enum led_cmd get_led_cmd(char *var) return LED_CMD_ON; if (strcmp(var, "toggle") == 0) return LED_CMD_TOGGLE; +#ifndef CONFIG_LED if (strcmp(var, "blink") == 0) return LED_CMD_BLINK;
+#endif return -1; }
-/*
- LED drivers providing a blinking LED functionality, like the
- PCA9551, can override this empty weak function
- */
-void __weak __led_blink(led_id_t mask, int freq) +#ifdef CONFIG_LED +int dm_led_set(char *label, enum led_cmd cmd) {
struct udevice *dev;char status;if (led_get_by_label(label, &dev)) {printf("Warning: led [ %s ] not found\n",label);return -1;}switch (cmd) {case LED_CMD_ON:led_set_on(dev, 1);if (led_get_status(dev) != 1) {printf("Warning: status of [ %s ] is still 0\n",label);return -1;}break;case LED_CMD_OFF:led_set_on(dev, 0);if (led_get_status(dev) != 0) {printf("Warning: status of [ %s ] is still 1\n",label);return -1;}break;case LED_CMD_TOGGLE:status = led_get_status(dev);led_set_on(dev, !status);if (led_get_status(dev) == status) {printf("Warning: status of [ %s ] is still %d\n",label, status);return -1;}Funny, in my version I moved this down into the uclass...but this seems reasonable also.
break;}return 0;} +#endif
int do_led (cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { @@ -99,14 +148,23 @@ int do_led (cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
int freq; /* Validate arguments */+#ifndef CONFIG_LED if ((argc < 3) || (argc > 4)) +#else
if ((argc < 2) || (argc > 4))+#endif return CMD_RET_USAGE;
cmd = get_led_cmd(argv[2]);+#ifndef CONFIG_LED if (cmd < 0) { +#else
if (argc > 2 && cmd < 0) {+#endif return CMD_RET_USAGE; }
+#ifndef CONFIG_LED for (i = 0; led_commands[i].string; i++) { if ((strcmp("all", argv[1]) == 0) || (strcmp(led_commands[i].string, argv[1]) == 0)) { @@ -144,7 +202,40 @@ int do_led (cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
break; } }
+#else
if (strcmp("all", argv[1]) == 0) {char *label = "";int node, len, error_count = 0;match = 1;node = fdt_path_offset(gd->fdt_blob, "/leds");if (node < 0) {printf("led: null led found\n");return CMD_RET_FAILURE;}node = fdt_first_subnode(gd->fdt_blob, node);Why are you checking the DT here - can this information not come from the uclass? Please see my led command patch. I might be missing a reason.
if (node < 0) {printf("led: null led found\n");return CMD_RET_FAILURE;}label = fdt_getprop(gd->fdt_blob, node, "label", &len);if (dm_led_set(label, cmd) < 0)error_count++;while (1) {node = fdt_next_subnode(gd->fdt_blob, node);if (node < 0)break;label = fdt_getprop(gd->fdt_blob, node, "label",&len);
if (dm_led_set(label, cmd) < 0)error_count++;}if (error_count != 0)return CMD_RET_FAILURE;} else if (argc > 2) {match = 1;if (dm_led_set(argv[1], cmd) < 0)return CMD_RET_FAILURE;}+#endif /* If we ran out of matches, print Usage */ if (!match) { return CMD_RET_USAGE; @@ -153,6 +244,7 @@ int do_led (cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
return 0;}
+#ifndef CONFIG_LED U_BOOT_CMD( led, 4, 1, do_led, "[" @@ -191,3 +283,10 @@ U_BOOT_CMD( "all] [on|off|toggle|blink] [blink-freq in ms]", "[led_name] [on|off|toggle|blink] sets or clears led(s)" ); +#else +U_BOOT_CMD(
led, 4, 1, do_led,"operate led(s)","[all|led_name] [on|off|toggle] - sets or clears led(s)"+);
+#endif
2.7.4
Regards, Simon
Hi, Simon
I have seen your version, and I deem your code is more appropriate. I'll learn from your code.
Thanks.