Re: [PATCH regression fix v2] gpiolib: acpi: Make set-debounce-timeout failures non fatal

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-acpi@vger.kernel.org
Subject: Re: [PATCH regression fix v2] gpiolib: acpi: Make set-debounce-timeout failures non fatal
Date: Mon, 23 Aug 2021 12:41:45 +0300	[thread overview]
Message-ID: <YSNtWe6oEpULLMg7@smile.fi.intel.com> (raw)
In-Reply-To: <e7c1b41a-4d11-21f4-0add-9b6dd00a8da6@redhat.com>

On Mon, Aug 16, 2021 at 02:28:07PM +0200, Hans de Goede wrote:
> On 8/16/21 2:15 PM, Andy Shevchenko wrote:
> > On Mon, Aug 16, 2021 at 12:41:19PM +0200, Hans de Goede wrote:
> >> Commit 8dcb7a15a585 ("gpiolib: acpi: Take into account debounce settings")
> >> made the gpiolib-acpi code call gpio_set_debounce_timeout() when requesting
> >> GPIOs.
> >>
> >> This in itself is fine, but it also made gpio_set_debounce_timeout()
> >> errors fatal, causing the requesting of the GPIO to fail. This is causing
> >> regressions. E.g. on a HP ElitePad 1000 G2 various _AEI specified GPIO
> >> ACPI event sources specify a debouncy timeout of 20 ms, but the
> >> pinctrl-baytrail.c only supports certain fixed values, the closest
> >> ones being 12 or 24 ms and pinctrl-baytrail.c responds with -EINVAL
> >> when specified a value which is not one of the fixed values.
> >>
> >> This is causing the acpi_request_own_gpiod() call to fail for 3
> >> ACPI event sources on the HP ElitePad 1000 G2, which in turn is causing
> >> e.g. the battery charging vs discharging status to never get updated,
> >> even though a charger has been plugged-in or unplugged.
> >>
> >> Make gpio_set_debounce_timeout() errors non fatal, warning about the
> >> failure instead, to fix this regression.
> >>
> >> Note we should probably also fix various pinctrl drivers to just
> >> pick the first bigger discrete value rather then returning -EINVAL but
> >> this will need to be done on a per driver basis, where as this fix
> >> at least gets us back to where things were before and thus restores
> >> functionality on devices where this was lost due to
> >> gpio_set_debounce_timeout() errors.
> > 
> > Yes, I also think that we need to choose upper debounce instead of rejecting
> > the settings. And yes, I agree that for now it's not suitable as a fix.
> > 
> > That said,
> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Thank you.
> 
> FYI, I've prepared a patch to choose the upper debounce time for
> pintctrl-baytrail . I'll test it when I'm back home tonight and
> then submit it upstream.

Bart, can you pick this up? Or do you expect me to send a PR with this one?

-- 
With Best Regards,
Andy Shevchenko