linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity
@ 2019-08-31  3:09 Peter Cai
  2019-08-31  3:09 ` [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Peter Cai @ 2019-08-31  3:09 UTC (permalink / raw)
  Cc: Rafael J. Wysocki, Len Brown, Peter Cai, Mika Westerberg,
	Andy Shevchenko, Linus Walleij, Bartosz Golaszewski,
	Bastien Nocera, Dmitry Torokhov, linux-gpio, linux-acpi,
	linux-kernel, linux-input

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(+)

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
  2019-08-31  3:09 [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Peter Cai
@ 2019-08-31  3:09 ` Peter Cai
  2019-09-02 10:04   ` Andy Shevchenko
  2019-09-02 10:01 ` [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Andy Shevchenko
  2019-09-02 12:43 ` [PATCH v3 " Peter Cai
  2 siblings, 1 reply; 11+ messages in thread
From: Peter Cai @ 2019-08-31  3:09 UTC (permalink / raw)
  Cc: Rafael J. Wysocki, Len Brown, Peter Cai, Mika Westerberg,
	Andy Shevchenko, Linus Walleij, Bartosz Golaszewski,
	Bastien Nocera, Dmitry Torokhov, linux-gpio, linux-acpi,
	linux-kernel, linux-input

The firmware of GPD P2 Max could not handle panel resets although code
is present in DSDT. The kernel needs to take on this job instead, but
the DSDT does not provide _DSD, rendering kernel helpless when trying to
find the respective GPIO pins.

Fortunately, this time GPD has proper DMI vendor / product strings that
we could match against. We simply apply an acpi_gpio_mapping table when
GPD P2 Max is matched.

Additionally, the DSDT definition of the irq pin specifies a wrong
polarity. The new quirk introduced in the previous patch
(ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.

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

v2: removed '#ifdef CONFIG_ACPI' as per suggestion. The #ifdef guards
for CONFIG_DMI is kept for consistency with the other dmi_system_id
definition in the same file.
---
 drivers/input/touchscreen/goodix.c | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 5178ea8b5f30..df476f7dcd95 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -144,6 +144,31 @@ static const struct dmi_system_id rotated_screen[] = {
 	{}
 };
 
+static const struct acpi_gpio_params irq_gpios_default = { 0, 0, false };
+static const struct acpi_gpio_params reset_gpios_default = { 1, 0, false };
+static const struct acpi_gpio_mapping gpio_mapping_force_irq_active_high[] = {
+	{ "irq-gpios", &irq_gpios_default, 1, ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
+	{ "reset-gpios", &reset_gpios_default, 1 },
+	{}
+};
+
+/*
+ * Devices that need acpi_gpio_mapping to function correctly
+ */
+static const struct dmi_system_id need_gpio_mapping[] = {
+#if defined(CONFIG_DMI) && defined(CONFIG_X86)
+	{
+		.ident = "GPD P2 Max",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
+		},
+		.driver_data = &gpio_mapping_force_irq_active_high
+	},
+#endif
+	{}
+};
+
 /**
  * goodix_i2c_read - read data from a register of the i2c slave device.
  *
@@ -795,6 +820,12 @@ static int goodix_ts_probe(struct i2c_client *client,
 {
 	struct goodix_ts_data *ts;
 	int error;
+	struct dmi_system_id *dmi_match;
+
+	dmi_match = dmi_first_match(need_gpio_mapping);
+	if (dmi_match)
+		devm_acpi_dev_add_driver_gpios(&client->dev,
+					       dmi_match->driver_data);
 
 	dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity
  2019-08-31  3:09 [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Peter Cai
  2019-08-31  3:09 ` [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
@ 2019-09-02 10:01 ` Andy Shevchenko
  2019-09-02 13:36   ` Linus Walleij
  2019-09-02 12:43 ` [PATCH v3 " Peter Cai
  2 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2019-09-02 10:01 UTC (permalink / raw)
  To: Peter Cai
  Cc: Rafael J. Wysocki, Len Brown, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov, linux-gpio,
	linux-acpi, linux-kernel, linux-input

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
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
  2019-08-31  3:09 ` [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
@ 2019-09-02 10:04   ` Andy Shevchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2019-09-02 10:04 UTC (permalink / raw)
  To: Peter Cai
  Cc: Rafael J. Wysocki, Len Brown, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov, linux-gpio,
	linux-acpi, linux-kernel, linux-input

On Sat, Aug 31, 2019 at 11:09:15AM +0800, Peter Cai wrote:
> The firmware of GPD P2 Max could not handle panel resets although code
> is present in DSDT. The kernel needs to take on this job instead, but
> the DSDT does not provide _DSD, rendering kernel helpless when trying to
> find the respective GPIO pins.
> 
> Fortunately, this time GPD has proper DMI vendor / product strings that
> we could match against. We simply apply an acpi_gpio_mapping table when
> GPD P2 Max is matched.
> 
> Additionally, the DSDT definition of the irq pin specifies a wrong
> polarity. The new quirk introduced in the previous patch
> (ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

though one comment below.

> 
> Signed-off-by: Peter Cai <peter@typeblog.net>
> ---
> 
> v2: removed '#ifdef CONFIG_ACPI' as per suggestion. The #ifdef guards
> for CONFIG_DMI is kept for consistency with the other dmi_system_id
> definition in the same file.
> ---
>  drivers/input/touchscreen/goodix.c | 31 ++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index 5178ea8b5f30..df476f7dcd95 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -144,6 +144,31 @@ static const struct dmi_system_id rotated_screen[] = {
>  	{}
>  };
>  
> +static const struct acpi_gpio_params irq_gpios_default = { 0, 0, false };
> +static const struct acpi_gpio_params reset_gpios_default = { 1, 0, false };
> +static const struct acpi_gpio_mapping gpio_mapping_force_irq_active_high[] = {
> +	{ "irq-gpios", &irq_gpios_default, 1, ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
> +	{ "reset-gpios", &reset_gpios_default, 1 },
> +	{}
> +};
> +
> +/*
> + * Devices that need acpi_gpio_mapping to function correctly
> + */
> +static const struct dmi_system_id need_gpio_mapping[] = {
> +#if defined(CONFIG_DMI) && defined(CONFIG_X86)
> +	{
> +		.ident = "GPD P2 Max",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
> +		},
> +		.driver_data = &gpio_mapping_force_irq_active_high
> +	},
> +#endif
> +	{}
> +};
> +
>  /**
>   * goodix_i2c_read - read data from a register of the i2c slave device.
>   *
> @@ -795,6 +820,12 @@ static int goodix_ts_probe(struct i2c_client *client,
>  {
>  	struct goodix_ts_data *ts;
>  	int error;

> +	struct dmi_system_id *dmi_match;

I guess it should be const as kbuild bot complained on v1.

And perhaps you want to keep reverse xmas tree order in definition block:

	const struct dmi_system_id *dmi_match;
	struct goodix_ts_data *ts;
	int error;

> +
> +	dmi_match = dmi_first_match(need_gpio_mapping);
> +	if (dmi_match)
> +		devm_acpi_dev_add_driver_gpios(&client->dev,
> +					       dmi_match->driver_data);
>  
>  	dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
>  
> -- 
> 2.23.0
> 

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] gpio: acpi: add quirk to override GpioInt polarity
  2019-08-31  3:09 [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Peter Cai
  2019-08-31  3:09 ` [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
  2019-09-02 10:01 ` [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Andy Shevchenko
@ 2019-09-02 12:43 ` Peter Cai
  2019-09-02 12:43   ` [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
  2 siblings, 1 reply; 11+ messages in thread
From: Peter Cai @ 2019-09-02 12:43 UTC (permalink / raw)
  Cc: Peter Cai, Andy Shevchenko, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov, linux-gpio,
	linux-acpi, linux-kernel, linux-input

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>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

v2: rebased to gpio/for-next, moved quirk out of the gpioint
conditional.

v3: no change, series update.
---
 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
  2019-09-02 12:43 ` [PATCH v3 " Peter Cai
@ 2019-09-02 12:43   ` Peter Cai
  2020-03-02 11:57     ` Bastien Nocera
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Cai @ 2019-09-02 12:43 UTC (permalink / raw)
  Cc: Peter Cai, Andy Shevchenko, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov, linux-gpio,
	linux-acpi, linux-kernel, linux-input

The firmware of GPD P2 Max could not handle panel resets although code
is present in DSDT. The kernel needs to take on this job instead, but
the DSDT does not provide _DSD, rendering kernel helpless when trying to
find the respective GPIO pins.

Fortunately, this time GPD has proper DMI vendor / product strings that
we could match against. We simply apply an acpi_gpio_mapping table when
GPD P2 Max is matched.

Additionally, the DSDT definition of the irq pin specifies a wrong
polarity. The new quirk introduced in the previous patch
(ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.

Signed-off-by: Peter Cai <peter@typeblog.net>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

v2: removed '#ifdef CONFIG_ACPI' as per suggestion. The #ifdef guards
for CONFIG_DMI is kept for consistency with the other dmi_system_id
definition in the same file.

v3: minor style adjustments. Added 'const' to dmi_match and moved it in
reverse xmas tree order as per suggestion.
---
 drivers/input/touchscreen/goodix.c | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 5178ea8b5f30..49ce478c1134 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -144,6 +144,31 @@ static const struct dmi_system_id rotated_screen[] = {
 	{}
 };
 
+static const struct acpi_gpio_params irq_gpios_default = { 0, 0, false };
+static const struct acpi_gpio_params reset_gpios_default = { 1, 0, false };
+static const struct acpi_gpio_mapping gpio_mapping_force_irq_active_high[] = {
+	{ "irq-gpios", &irq_gpios_default, 1, ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
+	{ "reset-gpios", &reset_gpios_default, 1 },
+	{}
+};
+
+/*
+ * Devices that need acpi_gpio_mapping to function correctly
+ */
+static const struct dmi_system_id need_gpio_mapping[] = {
+#if defined(CONFIG_DMI) && defined(CONFIG_X86)
+	{
+		.ident = "GPD P2 Max",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
+		},
+		.driver_data = &gpio_mapping_force_irq_active_high
+	},
+#endif
+	{}
+};
+
 /**
  * goodix_i2c_read - read data from a register of the i2c slave device.
  *
@@ -793,9 +818,15 @@ static void goodix_disable_regulators(void *arg)
 static int goodix_ts_probe(struct i2c_client *client,
 			   const struct i2c_device_id *id)
 {
+	const struct dmi_system_id *dmi_match;
 	struct goodix_ts_data *ts;
 	int error;
 
+	dmi_match = dmi_first_match(need_gpio_mapping);
+	if (dmi_match)
+		devm_acpi_dev_add_driver_gpios(&client->dev,
+					       dmi_match->driver_data);
+
 	dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity
  2019-09-02 10:01 ` [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Andy Shevchenko
@ 2019-09-02 13:36   ` Linus Walleij
  2019-09-02 16:06     ` Andy Shevchenko
  0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2019-09-02 13:36 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Peter Cai, Rafael J. Wysocki, Len Brown, Mika Westerberg,
	Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov,
	open list:GPIO SUBSYSTEM, ACPI Devel Maling List, linux-kernel,
	Linux Input

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity
  2019-09-02 13:36   ` Linus Walleij
@ 2019-09-02 16:06     ` Andy Shevchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2019-09-02 16:06 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Peter Cai, Rafael J. Wysocki, Len Brown, Mika Westerberg,
	Bartosz Golaszewski, Bastien Nocera, Dmitry Torokhov,
	open list:GPIO SUBSYSTEM, ACPI Devel Maling List, linux-kernel,
	Linux Input

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.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
  2019-09-02 12:43   ` [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
@ 2020-03-02 11:57     ` Bastien Nocera
  2020-03-02 12:10       ` Peter Cai
  0 siblings, 1 reply; 11+ messages in thread
From: Bastien Nocera @ 2020-03-02 11:57 UTC (permalink / raw)
  To: Peter Cai, Hans de Goede
  Cc: Andy Shevchenko, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Dmitry Torokhov, linux-gpio, linux-acpi,
	linux-kernel, linux-input

On Mon, 2019-09-02 at 20:43 +0800, Peter Cai wrote:
> The firmware of GPD P2 Max could not handle panel resets although
> code
> is present in DSDT. The kernel needs to take on this job instead, but
> the DSDT does not provide _DSD, rendering kernel helpless when trying
> to
> find the respective GPIO pins.
> 
> Fortunately, this time GPD has proper DMI vendor / product strings
> that
> we could match against. We simply apply an acpi_gpio_mapping table
> when
> GPD P2 Max is matched.
> 
> Additionally, the DSDT definition of the irq pin specifies a wrong
> polarity. The new quirk introduced in the previous patch
> (ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.

Hans has posted a patchset which reworks GPIO access for ACPI devices.

Could you please check whether you could rebase your patch on top of
that?

I also think the comment in "Input: goodix - Add support for getting
IRQ + reset GPIOs on Cherry Trail devices" might also be of use:

+       case irq_pin_access_acpi_gpio:
+               /*
+                * The IRQ pin triggers on a falling edge, so its gets
marked
+                * as active-low, use output_raw to avoid the value
inversion.
+                */

Cheers


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
  2020-03-02 11:57     ` Bastien Nocera
@ 2020-03-02 12:10       ` Peter Cai
  2020-03-02 13:08         ` Bastien Nocera
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Cai @ 2020-03-02 12:10 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: Hans de Goede, Andy Shevchenko, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Dmitry Torokhov, linux-gpio, linux-acpi,
	linux-kernel, linux-input

On Mon, Mar 2, 2020 at 7:57 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Mon, 2019-09-02 at 20:43 +0800, Peter Cai wrote:
> > The firmware of GPD P2 Max could not handle panel resets although
> > code
> > is present in DSDT. The kernel needs to take on this job instead, but
> > the DSDT does not provide _DSD, rendering kernel helpless when trying
> > to
> > find the respective GPIO pins.
> >
> > Fortunately, this time GPD has proper DMI vendor / product strings
> > that
> > we could match against. We simply apply an acpi_gpio_mapping table
> > when
> > GPD P2 Max is matched.
> >
> > Additionally, the DSDT definition of the irq pin specifies a wrong
> > polarity. The new quirk introduced in the previous patch
> > (ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this.
>
> Hans has posted a patchset which reworks GPIO access for ACPI devices.
>
> Could you please check whether you could rebase your patch on top of
> that?
>
> I also think the comment in "Input: goodix - Add support for getting
> IRQ + reset GPIOs on Cherry Trail devices" might also be of use:
>
> +       case irq_pin_access_acpi_gpio:
> +               /*
> +                * The IRQ pin triggers on a falling edge, so its gets
> marked
> +                * as active-low, use output_raw to avoid the value
> inversion.
> +                */
>
> Cheers
>

Sorry I forgot to notify you earlier, but it turned out that the
manufacturer of the device has already applied a fix in their ACPI
table after this patch was submitted and discussed. This patch is no
longer needed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
  2020-03-02 12:10       ` Peter Cai
@ 2020-03-02 13:08         ` Bastien Nocera
  0 siblings, 0 replies; 11+ messages in thread
From: Bastien Nocera @ 2020-03-02 13:08 UTC (permalink / raw)
  To: Peter Cai
  Cc: Hans de Goede, Andy Shevchenko, Mika Westerberg, Linus Walleij,
	Bartosz Golaszewski, Dmitry Torokhov, linux-gpio, linux-acpi,
	linux-kernel, linux-input

On Mon, 2020-03-02 at 20:10 +0800, Peter Cai wrote:
> 
<snip>
> Sorry I forgot to notify you earlier, but it turned out that the
> manufacturer of the device has already applied a fix in their ACPI
> table after this patch was submitted and discussed. This patch is no
> longer needed.

That's nice when it happens, thanks for letting me know nonetheless :)

Cheers


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-03-02 13:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-31  3:09 [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Peter Cai
2019-08-31  3:09 ` [PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
2019-09-02 10:04   ` Andy Shevchenko
2019-09-02 10:01 ` [PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity Andy Shevchenko
2019-09-02 13:36   ` Linus Walleij
2019-09-02 16:06     ` Andy Shevchenko
2019-09-02 12:43 ` [PATCH v3 " Peter Cai
2019-09-02 12:43   ` [PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max Peter Cai
2020-03-02 11:57     ` Bastien Nocera
2020-03-02 12:10       ` Peter Cai
2020-03-02 13:08         ` Bastien Nocera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).