[U-Boot] [PATCH v3 3/5] dm: led: move default state support in led uclass

Patrick DELAUNAY patrick.delaunay at st.com
Fri Jul 27 14:13:57 UTC 2018


Hi Patrick,

> From: Patrick BrĂ¼nn <P.Bruenn at beckhoff.com>
> Sent: jeudi 26 juillet 2018 12:40
> 
> Hi Patrick,
> sorry, for responding so late, I am in the middle of a vacation at the moment.

It is normal in summer time, 
I will be also in holiday at end of next week.

> >From: Patrick Delaunay [mailto:patrick.delaunay at st.com]
> >Sent: Dienstag, 24. Juli 2018 10:59
> >Subject: [PATCH v3 3/5] dm: led: move default state support in led
> >uclass
> >
...
> >@@ -63,8 +64,61 @@ int led_set_period(struct udevice *dev, int
> >period_ms)  }  #endif
...
> >+int led_default_state(void)
> >+{
...
> >+                      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()?

No it is debug message keep; I need to remove it

> >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()?

No really needed but better I think.
I add this parsing during bind to avoid  probing of LED drivers
(and so configuration of the associated device as pincontrol, clock, ...)
when the "default-state" node is not present;  That avoid potential issue and it is  faster.

In next function, device_probe() is not called when uc_pdata->default_state = LED_DEF_NO to avoid it.

So I need the information before the probe...

I choose the u_class bind function, because uclass have no ofdata_to_platdata.

> 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.

I want to have only probed driver in "dm tree" only the LED driver really initiliazed
And mmc_initialaze which probe all the mmc drivers

Class      Probed  Driver      Name
----------------------------------------
 led        [ + ]   gpio_led    |-- leds
 led        [   ]   gpio_led    |   |-- iracibble
 led        [   ]   gpio_led    |   |-- martinet
 led        [ + ]   gpio_led    |   |-- default_on
 led        [ + ]   gpio_led    |   `-- default_off

But after double check, I can move all the treatment in led_default_state()
Without changing the behavior and it is is more simple (no enum and double evaluation).

> 
> >       return 0;
> > }

> >
> However, this whole v3 series was:
> Tested-by: Patrick Bruenn <p.bruenn at beckhoff.com> Beckhoff Automation
> GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered
> office: Verl, Germany | Register court: Guetersloh HRA 7075
> 

Thanks for the tests, I will push a v4 with the proposed code simplification.

Patrick



More information about the U-Boot mailing list