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. ;-)
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
--
/"\ ASCII RIBBON | »With the first link, the chain is forged. The first
\ / CAMPAIGN | speech censured, the first thought forbidden, the
X AGAINST | first freedom denied, chains us all irrevocably.«
/ \ HTML MAIL | (Jean-Luc Picard, quoting Judge Aaron Satie)
Attachment:
signature.asc
Description: PGP signature
