[PATCH v3 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
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
Reviewed-by: Andy Shevchenko
---
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
[PATCH v3 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
Acked-by: Andy Shevchenko
---
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_GPIOIOBIT(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
[PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
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
---
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
[PATCH 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
---
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_GPIOIOBIT(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
Re: [PATCH 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
On August 30, 2019 7:55:05 PM GMT+08:00, Andy Shevchenko
wrote:
>On Fri, Aug 30, 2019 at 08:00:24AM +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.
>
>> +#ifdef CONFIG_ACPI
>
>I guess most of these #ifdef:s makes code less readable for exchange of
>saving
>few bytes in the module footprint.
>
>> +{ "irq-gpios", &irq_gpios_default, 1,
>> +ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },
>
>One line?
>
>> +.matches = {
>> +DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
>> +DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")
>
>Comma at the end?
>
>> +},
>> +.driver_data = &gpio_mapping_force_irq_active_high
>
>Ditto.
> I guess most of these #ifdef:s makes code less readable for exchange of saving
few bytes in the module footprint.
Since they can only be used when ACPI is supported
(devm_acpi_dev_add_driver_gpios does not exist without ACPI defined, thus the
last guard must exist), if they were not guarded then we would be left with a
bunch of unused variables warnings when building without ACPI which doesn't
seem good.
Should we use __maybe_unused here instead of #ifdef guards?
> Comma at the end?
I was trying to follow the style of this driver but it doesn't seem to be
really consistent within itself. Another dmi_system_id definition in the same
file mixed both styles so I was kind of confused.
--
Regards,
Xiyu Cai
[PATCH 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
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
---
drivers/input/touchscreen/goodix.c | 37 ++
1 file changed, 37 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c
b/drivers/input/touchscreen/goodix.c
index 5178ea8b5f30..65b8d04b6dcf 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -144,6 +144,34 @@ static const struct dmi_system_id rotated_screen[] = {
{}
};
+#ifdef CONFIG_ACPI
+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
+ {}
+};
+#endif
+
/**
* goodix_i2c_read - read data from a register of the i2c slave device.
*
@@ -796,6 +824,15 @@ static int goodix_ts_probe(struct i2c_client *client,
struct goodix_ts_data *ts;
int error;
+#ifdef CONFIG_ACPI
+ 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);
+#endif
+
dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client->addr);
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
--
2.23.0
[PATCH 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
---
drivers/gpio/gpiolib-acpi.c | 10 +-
include/linux/acpi.h| 6 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 39f2f9035c11..1a07c79ca2de 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -583,13 +583,21 @@ static int acpi_populate_gpio_lookup(struct acpi_resource
*ares, void *data)
/*
* Polarity and triggering are only specified for GpioInt
* resource.
+* Polarity specified by GpioInt may be ignored if
+* ACPI_GPIO_QUIRK_OVERRIDE_POLARITY is set.
* Note: we expect here:
* - ACPI_ACTIVE_LOW == GPIO_ACTIVE_LOW
* - ACPI_ACTIVE_HIGH == GPIO_ACTIVE_HIGH
*/
if (lookup->info.gpioint) {
lookup->info.flags = GPIOD_IN;
- lookup->info.polarity = agpio->polarity;
+ 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;
+ } else {
+ lookup->info.polarity = agpio->polarity;
+ }
lookup->info.triggering = agpio->triggering;
} else {
lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 9426b9aaed86..6569773ceffd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1014,6 +1014,12 @@ struct acpi_gpio_mapping {
* get GpioIo type explicitly, this quirk may be used.
*/
#define ACPI_GPIO_QUIRK_ONLY_GPIOIOBIT(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
