linux-kernel - Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node

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: <CAMpxmJVUVhpcNOVQCB3p8tNpac5e5c7vRQS=-avA6Cuaag9eRw@mail.gmail.com>
Date:   Mon, 15 Mar 2021 17:47:53 +0100
From:   Bartosz Golaszewski <bgolaszewski@...libre.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Bartosz Golaszewski <brgl@...ev.pl>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Marek Vasut <marex@...x.de>,
        Roman Guskov <rguskov@...electronics.com>
Subject: Re: [PATCH v1 1/1] gpiolib: Read "gpio-line-names" from a firmware node

On Mon, Mar 15, 2021 at 3:34 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Mon, Mar 15, 2021 at 03:04:37PM +0100, Bartosz Golaszewski wrote:
> > On Mon, Mar 15, 2021 at 1:50 PM Andy Shevchenko
> > <andriy.shevchenko@...ux.intel.com> wrote:
> > >
> > > On Mon, Mar 15, 2021 at 12:16:26PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Mar 15, 2021 at 10:01:47AM +0100, Bartosz Golaszewski wrote:
> > > > > On Fri, Mar 5, 2021 at 1:03 PM Andy Shevchenko
> > > > > <andriy.shevchenko@...ux.intel.com> wrote:
> > > >
> > > > > Unfortunately while this may fix the particular use-case on STM32, it
> > > > > breaks all other users as the 'gpio-line-names' property doesn't live
> > > > > on dev_fwnode(&gdev->dev) but on dev_fwnode(chip->parent).
> > > > >
> > > > > How about we first look for this property on the latter and only if
> > > > > it's not present descend down to the former fwnode?
> > > >
> > > > Oops, I have tested on x86 and it worked the same way.
> > > >
> > > > Lemme check this, but I think the issue rather in ordering when we apply fwnode
> > > > to the newly created device and when we actually retrieve gpio-line-names
> > > > property.
> > >
> > > Hmm... I can't see how it's possible can be. Can you provide a platform name
> > > and pointers to the DTS that has been broken by the change?
> > >
> >
> > I noticed it with gpio-mockup (libgpiod tests failed on v5.12-rc3) and
> > the WiP gpio-sim - but it's the same on most DT platforms. The node
> > that contains the `gpio-line-names` is the one associated with the
> > platform device passed to the GPIO driver. The gpiolib then creates
> > another struct device that becomes the child of that node but it
> > doesn't copy the parent's properties to it (nor should it).
> >
> > Every driver that reads device properties does it from the parent
> > device, not the one in gdev - whether it uses of_, fwnode_ or generic
> > device_ properties.
>
> What you are telling contradicts with the idea of copying parent's fwnode
> (or OF node) in the current code.
>

Ha! While the OF node of the parent device is indeed assigned to the
gdev's dev, the same isn't done in the core code for fwnodes and
simulated chips don't have an associated OF node, so this is the
culprit I suppose.

Bart

> Basically to access the properties we have to use either what specific driver
> supplied (by setting gpiochip->of_node or by leaving it NULL and in this case
> gpiochip_add_data_with_key() will copy it from the parent.
>
> That said, we shouldn't care about parent vs. GPIO device fwnode when reading
> properties. So, bug is somewhere else.
>
> In any case, I will test with the gpio-mockup, thanks!
>
> --
> With Best Regards,
> Andy Shevchenko
>
>