[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <D5227099-6120-446B-A39D-6AE437F5E11E@typeblog.net>
Date: Fri, 30 Aug 2019 23:32:53 +0800
From: Peter Cai <peter@...eblog.net>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: Mika Westerberg <mika.westerberg@...ux.intel.com>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
Bastien Nocera <hadess@...ess.net>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Len Brown <lenb@...nel.org>, linux-gpio@...r.kernel.org,
linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-input@...r.kernel.org
Subject: 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 <andriy.shevchenko@...ux.intel.com> 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
Powered by blists - more mailing lists