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 <[email protected]>

though one comment below.

> 
> Signed-off-by: Peter Cai <[email protected]>
> ---
> 
> 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


Reply via email to