Hi Patrick, > From: Patrick Brünn <p.bru...@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.delau...@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.bru...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
