Re: [PATCH RFC 2/2] gpiolib: check the 'ngpios' property in core gpiolib code

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Thu, Nov 11, 2021 at 9:47 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Thu, Nov 11, 2021 at 10:26 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> >
> > Several drivers read the 'ngpios' device property on their own, but
> > since it's defined as a standard GPIO property in the device tree bindings
> > anyway, it's a good candidate for generalization. If the driver didn't
> > set its gc->ngpio, try to read the 'ngpios' property from the GPIO
> > device's firmware node before bailing out.
>
> Thanks!
>
> ...
>
> > +               ret = fwnode_property_read_u32(gdev->dev.fwnode, "ngpios",
> > +                                              &ngpios);
>
> I'm wondering if there is any obstacle to call
>
>                ret = device_property_read_u32(&gdev->dev, "ngpios", &ngpios);
>
> ?
>
> Rationale (the main one) is to avoid direct dereference of fwnode from
> struct device (it might be changed in the future). I really prefer API
> calls here.
>
> > +               if (ret == 0) {
> > +                       gc->ngpio = ngpios;
> > +               } else {
> > +                       chip_err(gc, "tried to insert a GPIO chip with zero lines\n");
> > +                       ret = -EINVAL;
> > +                       goto err_free_descs;
> > +               }
>
> I would prefer the other way around and without 'else' being involved.
>
> >         }
>
>
> --
> With Best Regards,
> Andy Shevchenko

Will do. Just a note: some drivers that parse the ngpios property will
need to continue doing it themselves as they have additional
requirements from the value (limited max value, needs to be multiple
of 8, etc.) on the read value. For those that are obvious, we can
start converting them after this patch lands.

Bart