Re: [PATCH 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max

[Peter Cai]

On August 30, 2019 7:55:05 PM GMT+08:00, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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