Messages in this thread | | | Subject | Re: [PATCH v3 1/2] leds: pwm: Allow automatic labels for DT based devices | From | Jacek Anaszewski <> | Date | Wed, 9 Sep 2020 22:47:46 +0200 |
| |
Hi Alexander,
On 9/9/20 10:29 PM, Alexander Dahl wrote: > Hei hei, > > On Wed, Sep 09, 2020 at 11:07:36AM +0200, Pavel Machek wrote: >> Hi! >> >>> pwm_init_state(led_data->pwm, &led_data->pwmstate); >>> >>> - ret = devm_led_classdev_register(dev, &led_data->cdev); >>> + if (fwnode) { >>> + init_data.fwnode = fwnode; >>> + ret = devm_led_classdev_register_ext(dev, &led_data->cdev, >>> + &init_data); >>> + } else { >>> + ret = devm_led_classdev_register(dev, &led_data->cdev); >>> + } >> >> Can you always use _ext version, even with null fwnode? > > I did not try on real hardware, but from reading the code I would say > the following would happen: led_classdev_register_ext() calls > led_compose_name(parent, init_data, composed_name) which itself calls > led_parse_fwnode_props(dev, fwnode, &props); that returns early due to > fwnode==NULL without changing props, thus this stays as initialized > with {}, so led_compose_name() would return -EINVAL which would let > led_classdev_register_ext() fail, too. > >> If not, can you fix the core to accept that? Having that conditional >> in driver is ugly. > > It is ugly, although the approach is inspired by the leds-gpio driver. > I'll see if I can come up with a change to led-core, but I'm also open > for suggestions. ;-)
devm_led_classdev_register() calls devm_led_classdev_register_ext() with NULL passed in place of init_data, so you could do something like below to achieve the same without touching LED core:
struct led_init_data init_data_impl = { .fwnode = fwnode }; struct led_init_data *init_data = NULL;
if (fwnode) init_data = &init_data_impl;
devm_led_classdev_register_ext(dev, &led_data->cdev, init_data);
> fyi: Peter Ujfalusi answered and would give his Ack to the changed > dual license for the yaml file. You can expect that for v4. > > Stay tuned > Alex >
-- Best regards, Jacek Anaszewski
| |