[U-Boot,8/8] dm: led: Add a new 'led' command

When driver model is used for LEDs, provide a command to allow LED access.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/Kconfig  |   9 ++++
 cmd/Makefile |   1 +
 cmd/led.c    | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+)
 create mode 100644 cmd/led.c

On 31.03.2017 19:55, Simon Glass wrote:
> When driver model is used for LEDs, provide a command to allow LED access.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Stefan Roese <sr@denx.de>

One minor comment below.

> ---
>
>  cmd/Kconfig  |   9 ++++
>  cmd/Makefile |   1 +
>  cmd/led.c    | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 143 insertions(+)
>  create mode 100644 cmd/led.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 25e3b783a8..3c6924a8dc 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -661,6 +661,15 @@ config CMD_CACHE
>  	help
>  	  Enable the "icache" and "dcache" commands
>
> +config CMD_LED
> +	bool "led"
> +	default y if LED
> +	help
> +	  Enable the 'led' command which allows for control of LEDs supported
> +	  by the board. The LEDs can be listed with 'led list' and controlled
> +	  with led on/off/togle/blink. Any LED drivers can be controlled with
> +	  this command, e.g. led_gpio.
> +
>  config CMD_TIME
>  	bool "time"
>  	help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index c54734a58c..a3797ba904 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_CMD_JFFS2) += jffs2.o
>  obj-$(CONFIG_CMD_CRAMFS) += cramfs.o
>  obj-$(CONFIG_CMD_LDRINFO) += ldrinfo.o
>  obj-$(CONFIG_LED_STATUS_CMD) += legacy_led.o
> +obj-$(CONFIG_CMD_LED) += led.o
>  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
> new file mode 100644
> index 0000000000..6716ccaa1c
> --- /dev/null
> +++ b/cmd/led.c
> @@ -0,0 +1,133 @@
> +/*
> + * Copyright (c) 2017 Google, Inc
> + * Written by Simon Glass <sjg@chromium.org>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <led.h>
> +#include <dm/uclass-internal.h>
> +
> +#define LED_TOGGLE LEDST_COUNT
> +
> +static const char *const state_label[] = {
> +	[LEDST_OFF]	= "off",
> +	[LEDST_ON]	= "on",
> +	[LEDST_TOGGLE]	= "toggle",
> +	[LEDST_BLINK]	= "blink",
> +};
> +
> +enum led_state_t get_led_cmd(char *var)
> +{
> +	int i;
> +
> +	for (i = 0; i < LEDST_COUNT; i++) {
> +		if (!strncmp(var, state_label[i], strlen(var)))
> +			return i;
> +	}
> +
> +	return -1;

Perhaps return an error code here instead?

Thanks,
Stefan

> +}
> +
> +static int show_led_state(struct udevice *dev)
> +{
> +	int ret;
> +
> +	ret = led_get_state(dev);
> +	if (ret >= LEDST_COUNT)
> +		ret = -EINVAL;
> +	if (ret >= 0)
> +		printf("%s\n", state_label[ret]);
> +
> +	return ret;
> +}
> +
> +static int list_leds(void)
> +{
> +	struct udevice *dev;
> +	int ret;
> +
> +	for (uclass_find_first_device(UCLASS_LED, &dev);
> +	     dev;
> +	     uclass_find_next_device(&dev)) {
> +		struct led_uc_plat *plat = dev_get_uclass_platdata(dev);
> +
> +		if (!plat->label)
> +			continue;
> +		printf("%-15s ", plat->label);
> +		if (device_active(dev)) {
> +			ret = show_led_state(dev);
> +			if (ret < 0)
> +				printf("Error %d\n", ret);
> +		} else {
> +			printf("<inactive>\n");
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +int do_led(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	enum led_state_t cmd;
> +	const char *led_label;
> +	struct udevice *dev;
> +	int freq_ms = 0;
> +	int ret;
> +
> +	/* Validate arguments */
> +	if (argc < 2)
> +		return CMD_RET_USAGE;
> +	led_label = argv[1];
> +	if (*led_label == 'l')
> +		return list_leds();
> +
> +	cmd = argc > 2 ? get_led_cmd(argv[2]) : LEDST_COUNT;
> +	if (cmd < 0)
> +		return CMD_RET_USAGE;
> +	if (cmd == LEDST_BLINK) {
> +		if (argc < 4)
> +			return CMD_RET_USAGE;
> +		freq_ms = simple_strtoul(argv[3], NULL, 10);
> +	}
> +
> +	ret = led_get_by_label(led_label, &dev);
> +	if (ret) {
> +		printf("LED '%s' not found (err=%d)\n", led_label, ret);
> +		return CMD_RET_FAILURE;
> +	}
> +	switch (cmd) {
> +	case LEDST_OFF:
> +	case LEDST_ON:
> +	case LEDST_TOGGLE:
> +		ret = led_set_state(dev, cmd);
> +		break;
> +	case LEDST_BLINK:
> +		ret = led_set_period(dev, freq_ms);
> +		if (!ret)
> +			ret = led_set_state(dev, LEDST_BLINK);
> +		break;
> +	case LEDST_COUNT:
> +		printf("LED '%s': ", led_label);
> +		ret = show_led_state(dev);
> +		break;
> +	}
> +	if (ret < 0) {
> +		printf("LED '%s' operation failed (err=%d)\n", led_label, ret);
> +		return CMD_RET_FAILURE;
> +	}
> +
> +	return 0;
> +}
> +
> +U_BOOT_CMD(
> +	led, 4, 1, do_led,
> +	"manage LEDs",
> +	"<led_label> on|off|toggle|blink [blink-freq in ms]\t"
> +		"Change LED state\n"
> +	"led [<led_label>\tGet LED state\n"
> +	"led list\t\tshow a list of LEDs"
> +);
>

Viele Grüße,
Stefan

On Fri, Mar 31, 2017 at 11:55:05AM -0600, Simon Glass wrote:

> When driver model is used for LEDs, provide a command to allow LED access.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
[snip]
> +static const char *const state_label[] = {
> +	[LEDST_OFF]	= "off",
> +	[LEDST_ON]	= "on",
> +	[LEDST_TOGGLE]	= "toggle",
> +	[LEDST_BLINK]	= "blink",

If I recall some tinkering right, "most" LEDs we have available in
U-Boot don't support blinking directly but rather get blinked in a real
OS with timers to toggle them.  So can we make blink optional so that
only when we'd be using it do we get the size increase?  Thanks!