Re: [PATCH v4 3/9] gpiolib: acpi: Take into account debounce settings

[Hans de Goede <hdegoede@xxxxxxxxxx>]

Hi,

On 11/7/20 4:26 PM, Andy Shevchenko wrote:
> On Sat, Nov 7, 2020 at 4:49 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>> On 11/6/20 8:22 PM, Andy Shevchenko wrote:
> 
> ...
> 
>> I added an older version of this (which only modified acpi_dev_gpio_irq_get())
>> for testing and when booting a kernel with that version applied to it,
>> on a Cherry Trail device I noticed that a whole bunch of devices where no
>> longer seen by the kernel because of acpi_dev_gpio_irq_get() returning
>> errors now (-ENOTSUPP).
>>
>> Quoting from the gpiod_set_debounce docs:
>>
>> /**
>>  * gpiod_set_debounce - sets @debounce time for a GPIO
>>  * @desc: descriptor of the GPIO for which to set debounce time
>>  * @debounce: debounce time in microseconds
>>  *
>>  * Returns:
>>  * 0 on success, %-ENOTSUPP if the controller doesn't support setting the
>>  * debounce time.
>>  */
>>
>> This is expected on GPIO chips where setting the debounce time
>> is not supported. So the error handling should be modified to
>> ignore -ENOTSUPP errors here.
>>
>> This certainly MUST NOT be merged as is because it breaks a lot
>> of things as is.
> 
> Thank you very much for the testing! I remember that I fixed debounce
> for BayTrail, but it seems I have yet to fix Cherry Trail pin control
> as a prerequisite to this patch.
> 
> And like I said this series is definitely not for backporting.

Independent of fixing the CherryTrail pinctrl driver to support this,
I strongly believe that -ENOTSUPP should be ignored (treated as success)
by this patch. Remember ACPI is not only used on x86 but also on ARM
now a days. We simply cannot guarantee that all pinctrls will support
(let alone implement) debounce settings. E.g. I'm pretty sure that
the pinctrl on the popular Allwinner A64 does not support debouncing
and there are builts using a combination of uboot + EDK2 to boot!

The documentation for gpiod_set_debounce even explicitly mentioned that
-ENOTSUPP is an error which one may expect (and thus treat specially).

The same goes for the bias stuff too.

Regards,

Hans