linux-kernel - Re: [RFC/RFT PATCH] gpiolib: reverse-assign the fwnode to struct gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMRc=MdrLSPCEsQ6OEgRX-7Wh7ka+Rczja=QjY-srozj3cz68w@mail.gmail.com>
Date:   Fri, 6 Oct 2023 21:07:54 +0200
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Andy Shevchenko <andy@...nel.org>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Dipen Patel <dipenp@...dia.com>,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [RFC/RFT PATCH] gpiolib: reverse-assign the fwnode to struct gpio_chip

On Fri, Oct 6, 2023 at 3:15 PM Andy Shevchenko <andy@...nel.org> wrote:
>
> On Fri, Oct 06, 2023 at 01:51:47PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >
> > struct gpio_chip is not only used to carry the information needed to
> > set-up a GPIO device but is also used in all GPIOLIB callbacks and is
> > passed to the matching functions of lookup helpers.
> >
> > In that last case, it is currently impossible to match a GPIO device by
> > fwnode unless it was explicitly assigned to the chip in the provider
> > code.
>
> That's expected behaviour.
>

Is it though? We now have a GPIO device that represents a piece of
physical hardware that has an fwnode assigned and the associated GPIO
chip (tied to that device) that has none. How is that logical? It's
not coherent. I'm not surprised users of that code will be confused -
like Dipen in this case.

Bart

> > If the fwnode is taken from the parent device, the pointer in
> > struct gpio_chip will remain NULL.
>
> > If we have a parent device but gc->fwnode was not assigned by the
> > provider, let's assign it ourselves so that lookup by fwnode can work in
> > all cases.
>
> I don't think this is a good change. We paper over the real issue where
> we and callers need to understand what they are looking for.
>
> ...
>
> > This is something that Dipen reported with one of the tegra drivers where
> > a GPIO lookup by fwnode does not work because the fwnode pointer in struct
> > gpio_chip is NULL. This patch addresses this use-case.
>
> I am not sure I understand the problem here. All these should have been
> addressed already, no?
>
> So, the GPIOLIB should use dev_fwnode(&gdev->dev) inside it, outside it
> the GPIO drivers are free to use gc->fwnode as long as they understand
> the lifetime of the respective object.
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>