Re: [PATCH v2 21/23] gpio: protect the pointer to gpio_chip in gpio_device with SRCU

[Andy Shevchenko]

On Tue, Feb 06, 2024 at 02:23:35PM +0100, Bartosz Golaszewski wrote:
> On Tue, Feb 6, 2024 at 2:13 PM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Tue, Feb 06, 2024 at 01:57:39PM +0100, Bartosz Golaszewski wrote:
> > > On Tue, Feb 6, 2024 at 1:24 PM Andy Shevchenko
> > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > > > On Mon, Feb 05, 2024 at 08:36:39PM +0100, Bartosz Golaszewski wrote:
> > > > > On Mon, Feb 5, 2024 at 1:31 PM Andy Shevchenko
> > > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

..

> > > > > > >  int gpiod_get_direction(struct gpio_desc *desc)
> > > > > > >  {
> > > > > > > -     struct gpio_chip *gc;
> > > > > > >       unsigned long flags;
> > > > > > >       unsigned int offset;
> > > > > > >       int ret;
> > > > > > >
> > > > > > > -     gc = gpiod_to_chip(desc);
> > > > > > > +     if (!desc)
> > > > > > > +             /* Sane default is INPUT. */
> > > > > > > +             return 1;
> > > > > >
> > > > > > Hmm... I can't imagine how this value may anyhow be used / useful.
> > > > >
> > > > > What else would you return for an optional (NULL) GPIO?
> > > >
> > > > An error. If somebody asks for direction of the non-existing GPIO, there is no
> > > > (valid) answer for that.
> >
> > > All other functions return 0 for desc == NULL to accommodate
> > > gpiod_get_optional(). I think we should stay consistent here.
> >
> > The way you proposed is inconsistent, i.e. you may not return any direction
> > for the unknown / non-existing GPIO. You speculate it will be 1, I may consider
> > that in my (hypothetical for now) case it should be 0.
> >
> > Just don't make all bananas to be oranges. It won't work.
> 
> I don't have a strong conviction here. May make it an error as well.
> It's still inconsistent though - calling gpiod_direction_output(NULL);
> will return 0 and then you get an error when you do
> gpiod_get_direction(NULL). I don't have a good solution though.

Yes, and this is the best what we can have. Because the real code may rely on
the returned value and they should be really aware on the returned values in
some cases.

-- 
With Best Regards,
Andy Shevchenko