[v2,1/2] gpio: acpi: add quirk to override GpioInt polarity

On GPD P2 Max, the firmware could not reset the touch panel correctly.
The kernel needs to take on the job instead, but the GpioInt definition
in DSDT specifies ActiveHigh while the GPIO pin should actually be
ActiveLow.

We need to override the polarity defined by DSDT. The GPIO driver
already allows defining polarity in acpi_gpio_params, but the option is
not applied to GpioInt.

This patch adds a new quirk that enables the polarity specified in
acpi_gpio_params to also be applied to GpioInt.

Signed-off-by: Peter Cai <peter@typeblog.net>
---

v2: rebased to gpio/for-next, moved quirk out of the gpioint
conditional.
---
 drivers/gpio/gpiolib-acpi.c   | 9 +++++++++
 include/linux/gpio/consumer.h | 6 ++++++
 2 files changed, 15 insertions(+)

On Sat, Aug 31, 2019 at 11:09:14AM +0800, Peter Cai wrote:
> On GPD P2 Max, the firmware could not reset the touch panel correctly.
> The kernel needs to take on the job instead, but the GpioInt definition
> in DSDT specifies ActiveHigh while the GPIO pin should actually be
> ActiveLow.
> 
> We need to override the polarity defined by DSDT. The GPIO driver
> already allows defining polarity in acpi_gpio_params, but the option is
> not applied to GpioInt.
> 
> This patch adds a new quirk that enables the polarity specified in
> acpi_gpio_params to also be applied to GpioInt.

Thank you for an update!

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

on the condition that Dmitry and other input / Goodix developers are okay with
the approach in general.

> 
> Signed-off-by: Peter Cai <peter@typeblog.net>
> ---
> 
> v2: rebased to gpio/for-next, moved quirk out of the gpioint
> conditional.
> ---
>  drivers/gpio/gpiolib-acpi.c   | 9 +++++++++
>  include/linux/gpio/consumer.h | 6 ++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index fdee8afa5339..ab16ea61a8fa 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -603,6 +603,15 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
>  			lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
>  			lookup->info.polarity = lookup->active_low;
>  		}
> +
> +		/*
> +		 * Override the polarity specified by GpioInt if
> +		 * ACPI_GPIO_QUIRK_OVERRIDE_POLARITY is set.
> +		 */
> +		if (lookup->info.quirks & ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) {
> +			dev_warn(&lookup->info.adev->dev, FW_BUG "Incorrect polarity specified by GpioInt, overriding.\n");
> +			lookup->info.polarity = lookup->active_low;
> +		}
>  	}
>  
>  	return 1;
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index b70af921c614..7e9f24ebb085 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -622,6 +622,12 @@ struct acpi_gpio_mapping {
>   * get GpioIo type explicitly, this quirk may be used.
>   */
>  #define ACPI_GPIO_QUIRK_ONLY_GPIOIO		BIT(1)
> +/*
> + * Use the GPIO polarity (ActiveHigh / ActiveLow) from acpi_gpio_params
> + * for GpioInt as well. The default behavior is to use the one specified
> + * by GpioInt, which can be incorrect on some devices.
> + */
> +#define ACPI_GPIO_QUIRK_OVERRIDE_POLARITY	BIT(2)
>  
>  	unsigned int quirks;
>  };
> -- 
> 2.23.0
>

On Mon, Sep 2, 2019 at 12:01 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Sat, Aug 31, 2019 at 11:09:14AM +0800, Peter Cai wrote:
> > On GPD P2 Max, the firmware could not reset the touch panel correctly.
> > The kernel needs to take on the job instead, but the GpioInt definition
> > in DSDT specifies ActiveHigh while the GPIO pin should actually be
> > ActiveLow.
> >
> > We need to override the polarity defined by DSDT. The GPIO driver
> > already allows defining polarity in acpi_gpio_params, but the option is
> > not applied to GpioInt.
> >
> > This patch adds a new quirk that enables the polarity specified in
> > acpi_gpio_params to also be applied to GpioInt.
>
> Thank you for an update!
>
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> on the condition that Dmitry and other input / Goodix developers are okay with
> the approach in general.

Acked-by: Linus Walleij <linus.walleij@linaro.org>
In case Dmitry needs to merge this.

Or should I simply merge this patch to the GPIO tree?

Yours,
Linus Walleij

On Mon, Sep 02, 2019 at 03:36:24PM +0200, Linus Walleij wrote:
> On Mon, Sep 2, 2019 at 12:01 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, Aug 31, 2019 at 11:09:14AM +0800, Peter Cai wrote:
> > > On GPD P2 Max, the firmware could not reset the touch panel correctly.
> > > The kernel needs to take on the job instead, but the GpioInt definition
> > > in DSDT specifies ActiveHigh while the GPIO pin should actually be
> > > ActiveLow.
> > >
> > > We need to override the polarity defined by DSDT. The GPIO driver
> > > already allows defining polarity in acpi_gpio_params, but the option is
> > > not applied to GpioInt.
> > >
> > > This patch adds a new quirk that enables the polarity specified in
> > > acpi_gpio_params to also be applied to GpioInt.
> >
> > Thank you for an update!
> >
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > on the condition that Dmitry and other input / Goodix developers are okay with
> > the approach in general.
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> In case Dmitry needs to merge this.
> 
> Or should I simply merge this patch to the GPIO tree?

For my opinion Input subsystem should decide how to proceed with this.