Hi Patrick,
sorry, for responding so late, I am in the middle of a vacation at the moment.
>From: Patrick Delaunay [mailto:patrick.delau...@st.com]
>Sent: Dienstag, 24. Juli 2018 10:59
>Subject: [PATCH v3 3/5] dm: led: move default state support in led uclass
>
>This patch save common LED property "default-state" value
>in post bind of LED uclass.
>The configuration for this default state is only performed when
>led_default_state() is called;
>It can be called in your board_init()
>or it could added in init_sequence_r[] in future.
>
>Reviewed-by: Simon Glass <s...@chromium.org>
>Signed-off-by: Patrick Delaunay <patrick.delau...@st.com>
>---
>
>Changes in v3: None
>Changes in v2: None
>
> drivers/led/led-uclass.c | 54
>++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/led/led_gpio.c | 8 -------
> include/led.h | 23 +++++++++++++++++++++
> 3 files changed, 77 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
>index 2f4d69e..141401d 100644
>--- a/drivers/led/led-uclass.c
>+++ b/drivers/led/led-uclass.c
>@@ -8,6 +8,7 @@
> #include <dm.h>
> #include <errno.h>
> #include <led.h>
>+#include <dm/device-internal.h>
> #include <dm/root.h>
> #include <dm/uclass-internal.h>
>
>@@ -63,8 +64,61 @@ int led_set_period(struct udevice *dev, int period_ms)
> }
> #endif
>
>+static int led_post_bind(struct udevice *dev)
>+{
>+ struct led_uc_plat *uc_pdata;
>+ const char *default_state;
>+
>+ uc_pdata = dev_get_uclass_platdata(dev);
>+
>+ /* common optional properties */
>+ uc_pdata->default_state = LED_DEF_NO;
>+ default_state = dev_read_string(dev, "default-state");
>+ if (default_state) {
>+ if (!strncmp(default_state, "on", 2))
>+ uc_pdata->default_state = LED_DEF_ON;
>+ else if (!strncmp(default_state, "off", 3))
>+ uc_pdata->default_state = LED_DEF_OFF;
>+ else if (!strncmp(default_state, "keep", 4))
>+ uc_pdata->default_state = LED_DEF_KEEP;
>+ }
>+
>+ return 0;
>+}
>+
>+int led_default_state(void)
>+{
>+ struct udevice *dev;
>+ struct uclass *uc;
>+ struct led_uc_plat *uc_pdata;
>+ int ret;
>+
>+ ret = uclass_get(UCLASS_LED, &uc);
>+ if (ret)
>+ return ret;
>+ for (uclass_find_first_device(UCLASS_LED, &dev);
>+ dev;
>+ uclass_find_next_device(&dev)) {
>+ uc_pdata = dev_get_uclass_platdata(dev);
>+ if (!uc_pdata || uc_pdata->default_state == LED_DEF_NO)
>+ continue;
>+ ret = device_probe(dev);
>+ if (ret)
>+ return ret;
>+ if (uc_pdata->default_state == LED_DEF_ON)
>+ led_set_state(dev, LEDST_ON);
>+ else if (uc_pdata->default_state == LED_DEF_OFF)
>+ led_set_state(dev, LEDST_OFF);
>+ printf("%s: default_state=%d\n",
>+ uc_pdata->label, uc_pdata->default_state);
Do you really want to keep this printf()?
>+ }
>+
>+ return ret;
>+}
>+
> UCLASS_DRIVER(led) = {
> .id = UCLASS_LED,
> .name = "led",
>+ .post_bind = led_post_bind,
> .per_device_platdata_auto_alloc_size = sizeof(struct led_uc_plat),
> };
>diff --git a/drivers/led/led_gpio.c b/drivers/led/led_gpio.c
>index 533587d..93f6b91 100644
>--- a/drivers/led/led_gpio.c
>+++ b/drivers/led/led_gpio.c
>@@ -57,7 +57,6 @@ static int led_gpio_probe(struct udevice *dev)
> {
> struct led_uc_plat *uc_plat = dev_get_uclass_platdata(dev);
> struct led_gpio_priv *priv = dev_get_priv(dev);
>- const char *default_state;
> int ret;
>
> /* Ignore the top-level LED node */
>@@ -68,13 +67,6 @@ static int led_gpio_probe(struct udevice *dev)
> if (ret)
> return ret;
>
>- default_state = dev_read_string(dev, "default-state");
>- if (default_state) {
>- if (!strncmp(default_state, "on", 2))
>- gpio_led_set_state(dev, LEDST_ON);
>- else if (!strncmp(default_state, "off", 3))
>- gpio_led_set_state(dev, LEDST_OFF);
>- }
Is it necessary to move this code out of led_gpio_probe()?
If possible I would keep it here and implement led_default_state()
similar to mmc_initialize(->mmc_probe()). Than we could avoid
enum led_def_state_t and the double evaluation of default_state.
> return 0;
> }
>
>diff --git a/include/led.h b/include/led.h
>index 940b97f..ff45f03 100644
>--- a/include/led.h
>+++ b/include/led.h
>@@ -8,12 +8,27 @@
> #define __LED_H
>
> /**
>+ * enum led_default_state - The initial state of the LED.
>+ * see Documentation/devicetree/bindings/leds/common.txt
>+ */
>+enum led_def_state_t {
>+ LED_DEF_NO,
>+ LED_DEF_ON,
>+ LED_DEF_OFF,
>+ LED_DEF_KEEP
>+};
>+
>+/**
> * struct led_uc_plat - Platform data the uclass stores about each device
> *
> * @label: LED label
>+ * @default_state* - The initial state of the LED.
>+ see Documentation/devicetree/bindings/leds/common.txt
>+ * * - set automatically on device bind by the uclass's '.post_bind' method.
> */
> struct led_uc_plat {
> const char *label;
>+ enum led_def_state_t default_state;
> };
>
> /**
>@@ -106,4 +121,12 @@ enum led_state_t led_get_state(struct udevice
>*dev);
> */
> int led_set_period(struct udevice *dev, int period_ms);
>
>+/**
>+ * led_default_state() - set the default state for all the LED
>+ *
>+ * This enables all leds which have default state.
>+ *
>+ */
>+int led_default_state(void);
>+
> #endif
>--
>2.7.4
>
However, this whole v3 series was:
Tested-by: Patrick Bruenn <p.bru...@beckhoff.com>
Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff
Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
