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

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Mar 02, 2021 at 08:27:26AM +0800, Shawn Guo wrote:
> On Mon, Mar 01, 2021 at 02:17:06PM +0200, Andy Shevchenko wrote:
> > On Sat, Feb 27, 2021 at 11:19:45AM +0800, Shawn Guo wrote:
> > > On Fri, Feb 26, 2021 at 01:19:21PM +0200, Andy Shevchenko 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
> > > > 
> > > > Looking into DSDT I think the problem is much worse. First of all there are
> > > > many cases where pins like 0x140, 0x1c0, etc are being used. On top of that
> > > > there is no GPIO driver in the upstream (as far as I can see by HID, perhaps
> > > > there is a driver but for different HID. And I see that GPIO device consumes a
> > > > lot of Interrupts from GIC as well (it's ARM platfrom as far as I understand).
> > > 
> > > Yes, it's a laptop built on Qualcomm Snapdragon SC8180X SoC.  The GPIO
> > > driver is generic for all Snapdragon SoCs, and has been available in
> > > upstream for many years (for DT though). It can be found as the gpio_chip
> > > implementation in MSM pinctrl driver [1].  The SC8180X specific part can
> > > be found as pinctrl-sc8180x.c [2], and it's already working for DT boot.
> > > The only missing piece is to add "QCOM040D" as the acpi_device_id to
> > > support ACPI boot, and it will be submitted after 5.12-rc1 comes out.
> > > 
> > > > Looking at the Microsoft brain damaged way of understanding GPIOs and hardware
> > > > [1], I am afraid you really want to have a specific GPIO driver for this. So,
> > > > for now until we have better picture of what's going on, NAK to this patch.
> > > 
> > > Thanks for the pointer to Microsoft document.  On Snapdragon, we have
> > > only one GPIO instance that accommodates all GPIO pins, so I'm not sure
> > > that Microsoft GPIOs mapping layer is relevant here at all.
> > > 
> > > Please take a look at the GPIO driver, and feel free to let me know if
> > > you need any further information to understand what's going on.
> > 
> > Yes, I looked into the driver and see that it has 3 blocks of GPIOs (we call
> > them communities, but in the driver the term 'tiles' is used) AFAIU (correct me
> > if I'm wrong). And who knows how many banks in each of them.
> 
> I'm not sure that the 3 'tiles' means 3 blocks of GPIOs.  Maybe, @Bjorn
> can help clarify.  But the ACPI table shows that there is only 'GIO0'
> with 'QCOM040D' HID.

Yeah, I already got that ACPI there is screwed up...

> > I'm afraid that MS does on his way and not yours.
> > 
> > Can we have TRM for GPIO IP used there and any evidence / document from
> > firmware team about the implementation of the GPIO numbering in the ACPI
> > (at Intel we have so called BIOS Writers Guide that is given to the customers
> > where such info can be found)?
> 
> Unfortunately, I do not have the access to any sort of these documents.
> But I looped in Jeffrey who is part of Qualcomm kernel/firmware team,
> and should be able to help clarify GPIO numbering in the ACPI table.

Thanks! Will wait for new information then.

> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/qcom/pinctrl-msm.c#n713
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/qcom/pinctrl-sc8180x.c

-- 
With Best Regards,
Andy Shevchenko