linux-kernel - Re: [PATCH] pinctrl: cherryview: Add quirk with custom translation of ACPI GPIO numbers

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 10 Mar 2020 16:49:13 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Michał Stanek <mst@...ihalf.com>
Cc:     linux-gpio@...r.kernel.org, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org, stanekm@...gle.com,
        stable@...r.kernel.org, Marcin Wojtas <mw@...ihalf.com>,
        levinale@...omium.org, andriy.shevchenko@...ux.intel.com,
        Linus Walleij <linus.walleij@...aro.org>,
        bgolaszewski@...libre.com, rafael.j.wysocki@...el.com,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Subject: Re: [PATCH] pinctrl: cherryview: Add quirk with custom translation
 of ACPI GPIO numbers

On Tue, Mar 10, 2020 at 03:12:00PM +0100, Michał Stanek wrote:
> On Mon, Feb 10, 2020 at 11:14 AM Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> >
> > On Sat, Feb 08, 2020 at 07:43:24PM +0100, Michał Stanek wrote:
> > > > >
> > > > > Hi Mika,
> > > > >
> > > > > The previous patches from Dmitry handled IRQ numbering, here we have a
> > > > > similar issue with GPIO to pin translation - hardcoded values in FW
> > > > > which do not agree with the (non-consecutive) numbering in newer
> > > > > kernels.
> > > >
> > > > Hmm, so instead of passing GpioIo/GpioInt resources to devices the
> > > > firmware uses some hard-coded Linux GPIO numbering scheme? Would you
> > > > able to share the exact firmware description where this happens?
> > >
> > > Actually it is a GPIO offset in ACPI tables for Braswell that was
> > > hardcoded in the old firmware to match the previous (consecutive)
> > > Linux GPIO numbering.
> >
> > Can you share the ACPI tables and point me to the GPIO that is using
> > Linux number?
> 
> I think this is the one:
> https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/%2B/286534/2/src/mainboard/google/cyan/acpi/chromeos.asl
> 
> On Kefka the sysfs GPIO number for wpsw_cur was gpio392 before the
> translation change occurred in Linux.

But that table does not seem to have any GPIO numbers in it.

> > > > > > What GPIO(s) we are talking about and how does it show up to the user?
> > > > >
> > > > > As an example, the issue manifests itself when you run 'crossystem
> > > > > wpsw_cur'. On my Kefka it incorrectly reports the value as 1 instead
> > > > > of 0 when the write protect screw is removed.
> > > >
> > > > Is it poking GPIOs directly through sysfs relying the Linux GPIO
> > > > numbering (which can change and is fragile anyway)?
> > >
> > > I believe so, yes.
> >
> > This is something that should be fixed in userspace. Using global Linux
> > GPIO or IRQ numbers is fragile and source of issues like this. There are
> > correct ways of using GPIOs from userspace: in case of sysfs, you can
> > find the base of the chip and then user relative numbering against it or
> > switch to use libgpiod that does the same but uses the newer char
> > device. Both cases the GPIO number are relative against the GPIO chip so
> > they work even if global Linux GPIO numbering changes.
> 
> I analyzed crossystem source code and it looks like it is doing
> exactly what you're saying without any hardcoded assumptions. It gets
> the absolute offset of the GPIO pin from sysfs using its ACPI
> identifier, then it subtracts the base offset of the GPIO bank from it
> and the result is added to the bank's gpiochip%d number as it shows up
> in sysfs. The result is what is used to export and read the state of
> the pin.
> 
> With the newer kernel the gpiochip%d number is different so crossystem
> ends up reading the wrong pin.

Hmm, so gpiochipX is also not considered a stable number. It is based on
ARCH_NR_GPIOS which may change. So if the userspace is relaying certain GPIO
chip is always gpichip200 for example then it is wrong.