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

Andy Shevchenko
2021-03-05T10:09:50Z

On Fri, Mar 05, 2021 at 10:10:50AM +0100, Hans de Goede wrote:
> On 3/5/21 2:14 AM, Shawn Guo wrote:
> > On Thu, Mar 04, 2021 at 08:32:14PM +0100, Hans de Goede wrote:
> >> On 3/3/21 10:47 AM, Andy Shevchenko wrote:

...

> > So we reach a consensus that this is not the right solution for Lenovo
> > Flex 5G. But what about for Andy's Galileo Gen 2 case, where the GPIO
> > number in ACPI is truly broken?
>
> Well if the ACPI table truely simply has a wrong number in it, like in
> this case, then we clearly need a workaround.
>
> > ba8c90c61847 ("gpio: pca953x: Override IRQ for one of the expanders on Galileo Gen 2")
>
> And we have one in place, so I'm not sure what the question is?
>
> I guess the question is of your generic GPIO renumber patch would not
> be a better answer to that ?
>
> IMHO no, we want to keep quirks out of the core as much as possible,
> for example the code which Andy added a quirk to is build as a module
> in the generic Fedora distro kernel, so for most users the code will
> not be loaded into memory. Where as if we add it to the core it would
> use up extra memory for everyone.

I guess Shawn is referring to my rework of that quirk [1] due to found a flaw
in the upstreamed variant. I agree, that this is not ideal, but TL;DR: it less
invasive even to the upstreamed approach and it has no use of any hard coded
numbering schemes. The Galileo Gen 2 is "broken" in an *understandable* way,
i.e. ACPI designers put an absolute GPIO numbers (there are two SoC based GPIO
controllers: SCH and DesignWare which numbers starts at 0) instead of be
relative. For the time being only one device has a driver that needs such GPIO
number, but as I explained in the cover letter, to support it as a quirk I have
to copy ~10% of the existing (in gpiolib-acpi.c) code.

I'm all ears for better approach!

[1]: https://lore.kernel.org/linux-gpio/20210225163320.71267-1-andriy.shevchenko@linux.intel.com/T/#u

> Also if, in the future, we were to ever add a generic GPIO renumber quirk
> mechanism to the core, then your code would need more work. Because to be
> truely generic it would need to remap one gpiochip-name:pin-number on
> another gpiochip-name:pin-number pair. There might very well be a case
> with multiple gpiochips with pin number 32 being referenced in the DSDT
> and where we need to remap one of those 32-s to a different number
> (or possibly even to a different chip + number).

--
With Best Regards,
Andy Shevchenko