[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