On August 30, 2019 7:55:05 PM GMT+08:00, Andy Shevchenko [off-list ref] wrote:

On Fri, Aug 30, 2019 at 08:00:24AM +0800, Peter Cai wrote:

quoted

The firmware of GPD P2 Max could not handle panel resets although

code

quoted

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

quoted

find the respective GPIO pins.

Fortunately, this time GPD has proper DMI vendor / product strings

that

quoted

we could match against. We simply apply an acpi_gpio_mapping table

when

quoted

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.

quoted

+#ifdef CONFIG_ACPI

I guess most of these #ifdef:s makes code less readable for exchange of
saving
few bytes in the module footprint.

quoted

+	{ "irq-gpios", &irq_gpios_default, 1,
+		ACPI_GPIO_QUIRK_OVERRIDE_POLARITY },

One line?

quoted

+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX")

Comma at the end?

quoted

+		},
+		.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