Re: [PATCH] leds: lp5860: Return an error for an out-of-range 'reg' property

[Mert Seftali]

Hello Mr. Jones,

Thanks, that's clearer. Did it as two checks in v2: if (ret) for the
read failure, then a separate block that returns -EINVAL when the
channel is out of range, each with its own message. Dropped the nested
if (ret >= 0).

Mert


Am Do., 18. Juni 2026 um 13:55 Uhr schrieb Lee Jones <lee@xxxxxxxxxx>:
>
> On Fri, 12 Jun 2026, Mert Seftali wrote:
>
> > When fwnode_property_read_u32() succeeds but the channel number exceeds
> > LP5860_MAX_LED, ret is 0. The error path then passes 0 to dev_err_probe()
> > and returns 0, so an out-of-range "reg" value is silently treated as
> > success instead of being rejected.
> >
> > Set ret to -EINVAL in that case so the invalid channel is reported and
> > propagated as an error.
> >
> > Fixes: 3daf2c4ef82b ("leds: Add support for TI LP5860 LED driver chip")
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> > Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> > Closes: https://lore.kernel.org/r/202605210624.3gcr3prk-lkp@xxxxxxxxx/
> > Signed-off-by: Mert Seftali <mertsftl@xxxxxxxxx>
> > ---
> >  drivers/leds/rgb/leds-lp5860-core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/leds/rgb/leds-lp5860-core.c b/drivers/leds/rgb/leds-lp5860-core.c
> > index fd0e2f6e6e0f..9eeb01b3e56a 100644
> > --- a/drivers/leds/rgb/leds-lp5860-core.c
> > +++ b/drivers/leds/rgb/leds-lp5860-core.c
> > @@ -115,6 +115,8 @@ static int lp5860_iterate_subleds(struct lp5860_led *led, struct led_init_data *
> >
> >               ret = fwnode_property_read_u32(led_node, "reg", &channel);
> >               if (ret < 0 || channel > LP5860_MAX_LED) {
>
> Whilst we're hear, let's split this out.  If 'fwnode_property_read_u32()'
> fails, 'channel' is not populated.  While short-circuit evaluation protects
> us here, separating the error paths would allow us to use the preferred
> 'if (ret)' check instead of 'if (ret < 0)'.  It would also let us provide a
> more accurate error message, as 'reg' is not missing when it is simply out of
> range.
>
> > +                     if (ret >= 0)
> > +                             ret = -EINVAL;
>
> Avoid this nested check entirely by assigning 'ret = -EINVAL' directly
> within a separate block for the out-of-range check.  This would keep the
> flow a bit cleaner and easier to follow.
>
> >                       dev_err_probe(led->chip->dev, ret,
> >                                     "%pfwP: 'reg' property is missing. Skipping.\n", led_node);
> >                       fwnode_handle_put(led_node);
> > --
> > 2.54.0
> >
>
> --
> Lee Jones