diff mbox series

[v3,2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max

Message ID 20190902124352.12291-2-peter@typeblog.net (mailing list archive)
State New, archived
Headers show
Series [v3,1/2] gpio: acpi: add quirk to override GpioInt polarity | expand

Commit Message

Peter Cai Sept. 2, 2019, 12:43 p.m. UTC 
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(+)

Comments

Bastien Nocera March 2, 2020, 11:57 a.m. UTC | #1 
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
Peter Cai March 2, 2020, 12:10 p.m. UTC | #2 
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.
Bastien Nocera March 2, 2020, 1:08 p.m. UTC | #3 
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
diff mbox series

Patch

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)) {