diff mbox series

Input: gpio_keys_polled - Suppress deferred probe error for gpio

Message ID 20240305101042.10953-2-u.kleine-koenig@pengutronix.de (mailing list archive)
State New
Headers show
Series Input: gpio_keys_polled - Suppress deferred probe error for gpio | expand

Commit Message

Uwe Kleine-König March 5, 2024, 10:10 a.m. UTC
On a PC Engines APU our admins are faced with:

	$ dmesg | grep -c "gpio-keys-polled gpio-keys-polled: unable to claim gpio 0, err=-517"
	261

Such a message always appears when e.g. a new USB device is plugged in.

Suppress this message which considerably clutters the kernel log for
EPROBE_DEFER (i.e. -517).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

there are a few other exit paths that could use dev_err_probe(), but IIRC
Dmitry isn't a fan of using dev_err_probe() where the return value cannot be
EPROBE_DEFER, so I'm only changing this one error path.

Best regards
Uwe

 drivers/input/keyboard/gpio_keys_polled.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)


base-commit: 11afac187274a6177a7ac82997f8691c0f469e41

Comments

Linus Walleij March 5, 2024, 10:18 a.m. UTC | #1
On Tue, Mar 5, 2024 at 11:10 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> On a PC Engines APU our admins are faced with:
>
>         $ dmesg | grep -c "gpio-keys-polled gpio-keys-polled: unable to claim gpio 0, err=-517"
>         261
>
> Such a message always appears when e.g. a new USB device is plugged in.
>
> Suppress this message which considerably clutters the kernel log for
> EPROBE_DEFER (i.e. -517).
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Fair enough,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Andy Shevchenko March 5, 2024, 5:01 p.m. UTC | #2
On Tue, Mar 5, 2024 at 12:10 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

...

> there are a few other exit paths that could use dev_err_probe(), but IIRC
> Dmitry isn't a fan of using dev_err_probe() where the return value cannot be
> EPROBE_DEFER, so I'm only changing this one error path.

It's not true anymore. He is fine with that API, and please use it.
Dmitry Torokhov March 5, 2024, 5:11 p.m. UTC | #3
On Tue, Mar 05, 2024 at 07:01:02PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 5, 2024 at 12:10 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> ...
> 
> > there are a few other exit paths that could use dev_err_probe(), but IIRC
> > Dmitry isn't a fan of using dev_err_probe() where the return value cannot be
> > EPROBE_DEFER, so I'm only changing this one error path.
> 
> It's not true anymore. He is fine with that API, and please use it.

I would not get ahead of ourselves ;) I still think we could have done
this better by:

1. Having a format option for printing symbolic errors
2. Recording deferral source at supplier level

But yes, I am accepting patches and new drivers using dev_err_probe().

Thanks.
Dmitry Torokhov March 5, 2024, 5:15 p.m. UTC | #4
On Tue, Mar 05, 2024 at 11:10:42AM +0100, Uwe Kleine-König wrote:
> On a PC Engines APU our admins are faced with:
> 
> 	$ dmesg | grep -c "gpio-keys-polled gpio-keys-polled: unable to claim gpio 0, err=-517"
> 	261
> 
> Such a message always appears when e.g. a new USB device is plugged in.
> 
> Suppress this message which considerably clutters the kernel log for
> EPROBE_DEFER (i.e. -517).

I'll apply this, but that seems to be a misconfiguration somewhere - we
expect deferred probes to succeed eventually, here it looks like it
stays deferred forever and each time a new devices gets plugged in we
try to resolve deferred probe again and again.

Why doesn't gpio 0 become available?

Thanks.
Uwe Kleine-König March 5, 2024, 5:41 p.m. UTC | #5
On Tue, Mar 05, 2024 at 09:15:06AM -0800, Dmitry Torokhov wrote:
> On Tue, Mar 05, 2024 at 11:10:42AM +0100, Uwe Kleine-König wrote:
> > On a PC Engines APU our admins are faced with:
> > 
> > 	$ dmesg | grep -c "gpio-keys-polled gpio-keys-polled: unable to claim gpio 0, err=-517"
> > 	261
> > 
> > Such a message always appears when e.g. a new USB device is plugged in.
> > 
> > Suppress this message which considerably clutters the kernel log for
> > EPROBE_DEFER (i.e. -517).
> 
> I'll apply this, but that seems to be a misconfiguration somewhere - we
> expect deferred probes to succeed eventually, here it looks like it
> stays deferred forever and each time a new devices gets plugged in we
> try to resolve deferred probe again and again.
> 
> Why doesn't gpio 0 become available?

This is an x86 machine and I don't even know why the device actually
exists. Also I have no idea what gpio 0 should be, there seems to be a
gpio controller[1], but its GPIO numbers start at 512.

I found a bug report about this this issue:
https://github.com/pcengines/apu2-documentation/issues/204

Anyhow, I think my patch is fine. I forwarded the bug report to my admin
who might or might not dig deeper.

Best regards and thanks,
Uwe

[1] /sys/devices/pci0000:00/AMD0030:00/gpio/gpiochip512
diff mbox series

Patch

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index ba00ecfbd343..b41fd1240f43 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -315,12 +315,10 @@  static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 			error = devm_gpio_request_one(dev, button->gpio,
 					flags, button->desc ? : DRV_NAME);
-			if (error) {
-				dev_err(dev,
-					"unable to claim gpio %u, err=%d\n",
-					button->gpio, error);
-				return error;
-			}
+			if (error)
+				return dev_err_probe(dev, error,
+						     "unable to claim gpio %u\n",
+						     button->gpio);
 
 			bdata->gpiod = gpio_to_desc(button->gpio);
 			if (!bdata->gpiod) {