Re: [PATCH] gpiolib: acpi: support override broken GPIO number in ACPI table

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Tue, Mar 2, 2021 at 2:44 AM Shawn Guo <shawn.guo@xxxxxxxxxx> wrote:
> On Mon, Mar 01, 2021 at 02:19:50PM +0200, Andy Shevchenko wrote:
> > On Sat, Feb 27, 2021 at 11:46:42AM +0800, Shawn Guo wrote:
> > > On Fri, Feb 26, 2021 at 12:57:37PM +0200, Andy Shevchenko wrote:
> > > > On Fri, Feb 26, 2021 at 11:39 AM Shawn Guo <shawn.guo@xxxxxxxxxx> wrote:
> > > > > On Fri, Feb 26, 2021 at 11:12:07AM +0200, Andy Shevchenko wrote:
> > > > > > On Fri, Feb 26, 2021 at 5:42 AM Shawn Guo <shawn.guo@xxxxxxxxxx> wrote:
> > > > > > >
> > > > > > > Running kernel with ACPI on Lenovo Flex 5G laptop, touchpad is just
> > > > > > > not working.  That's because the GpioInt number of TSC2 node in ACPI
> > > > > > > table is simply wrong, and the number even exceeds the maximum GPIO
> > > > > > > lines.  As the touchpad works fine with Windows on the same machine,
> > > > > > > presumably this is something Windows-ism.  Although it's obviously
> > > > > > > a specification violation, believe of that Microsoft will fix this in
> > > > > > > the near future is not really realistic.
> > > > > > >
> > > > > > > It adds the support of overriding broken GPIO number in ACPI table
> > > > > > > on particular machines, which are matched using DMI info.  Such
> > > > > > > mechanism for fixing up broken firmware and ACPI table is not uncommon
> > > > > > > in kernel.  And hopefully it can be useful for other machines that get
> > > > > > > broken GPIO number coded in ACPI table.
> > > > > >
> > > > > > Thanks for the report and patch.
> > > > > >
> > > > > > First of all, have you reported the issue to Lenovo? At least they
> > > > > > will know that they did wrong.
> > > > >
> > > > > Yes, we are reporting this to Lenovo, but to be honest, we are not sure
> > > > > how much they will care about it, as they are shipping the laptop with
> > > > > Windows only.
> > > > >
> > > > > > Second, is it possible to have somewhere output of `acpidump -o
> > > > > > flex5g.dat` (the flex5g.dat file)?
> > > > >
> > > > > https://raw.githubusercontent.com/aarch64-laptops/build/master/misc/lenovo-flex-5g/dsdt.dsl
> > > > >
> > > > > > And as Mika said once to one of mine patches "since you know the
> > > > > > number ahead there is no need to pollute GPIO ACPI library core with
> > > > > > this quirk". But in any case I would like to see the ACPI tables
> > > > > > first.
> > > > >
> > > > > Oh, so you had something similar already?  Could you point me to the
> > > > > patch and discussion?
> > > >
> > > > Similar, but might be not the same:
> > > >  - patches in the upstream [1] (v3 applied), discussion [2]
> > > >  - new version with some additional fixes [3]
> > >
> > > Thanks for all the pointers.  It looks to me that it's the same problem
> > > - the GPIO number in ACPI table is broken and needs an override from
> > > kernel.
> >
> > Not exactly. On Galileo Gen 2 platform it's broken in understandable way.
> > In your case it's different and I'm not sure at all that's considered "broken"
> > in the MS' eyes.
>
> At least, I was told by Jeffrey that MS admits this is something needs
> to be fixed in the future.

Yes, but as far as I understand we have already this firmware in the
wild, so we will have users stuck with it...

> > > So I think what we need is a generic solution to a problem
> > > not uncommon.  Rather than asking all different drivers to resolve the
> > > same problem all over the kernel, I believe GPIO ACPI library is just
> > > the right place.
> > >
> > > Looking at your platform and problem, I realise that to be a generic
> > > solution, my patch needs an additional device identification matching,
> > > as one GPIO number that is broken for one device could be correct for
> > > another.  I will improve it, so that your problem can be resolved by
> > > simply adding a new entry to acpi_gpio_pin_override_table[].
> >
> > Before any steps further I really want to see more information about that IP
> > and how firmware applied the numbering scheme.
>
> Deduced by those working GPIO numbers in ACPI table and how Linux kernel
> is working, I think the GPIO is numbered without any bank thing.  All
> available pins are just numbered linearly, and every pin can be
> configured in GPIO mode.

Can you be absolutely sure? If you have banks (MS core code since the
laptop runs Windows uses what is described on their site for GPIOs)
linear and then you test pins at the beginning of the region, you
won't have issues, you need to find a pin in each tile wishfully at
the end of it and test.

Better and easier way of course to ask MS to provide an insight on this.

> > If it's confidential, you may sent any insights privately.
>
> Unfortunately, all those documents are confidential to me as well.

Okay, since we have no solid proof of what's going on there, I prefer
for now not to touch GPIO ACPI core and do this quirk in the driver
(using standard methods: DMI strings / ACPI IDs / etc).

Otherwise I'm not convinced that we need a global quirk for that and
it might be needed in the future.

-- 
With Best Regards,
Andy Shevchenko