| Message ID | 20211206164747.197309-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Input: goodix - pen support + misc patches | expand |
Hi Hans, On Mon, Dec 06, 2021 at 05:47:45PM +0100, Hans de Goede wrote: > goodix_get_gpio_config() errors are fatal (abort probe()) so log them > at KERN_ERR level rather then as debug messages. > > This change uses dev_err_probe() to automatically suppress the errors > in case of -EPROBE_DEFER. I really believe that dev_err_probe() is wrong API as the providers should be setting the reason for deferred probe failures. Could you simply swap dev_dbg() for dev_err()? Thanks.
Hi Dmitry, On 12/7/21 08:31, Dmitry Torokhov wrote: > Hi Hans, > > On Mon, Dec 06, 2021 at 05:47:45PM +0100, Hans de Goede wrote: >> goodix_get_gpio_config() errors are fatal (abort probe()) so log them >> at KERN_ERR level rather then as debug messages. >> >> This change uses dev_err_probe() to automatically suppress the errors >> in case of -EPROBE_DEFER. > > I really believe that dev_err_probe() is wrong API as the providers > should be setting the reason for deferred probe failures. > > Could you simply swap dev_dbg() for dev_err()? Done for v2, which I'm sending out right away. Note I've dropped the first patch for v2 since you said you applied this. Regards, Hans p.s. Any chance you could also take a look at this 2 patch patch-series, this has been pending for a while now: https://lore.kernel.org/linux-input/20211122220637.11386-1-hdegoede@redhat.com/T/#t
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index aaa3c455e01e..73f3b24f7f1e 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -854,13 +854,10 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts) retry_get_irq_gpio: /* Get the interrupt GPIO pin number */ gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN); - if (IS_ERR(gpiod)) { - error = PTR_ERR(gpiod); - if (error != -EPROBE_DEFER) - dev_dbg(dev, "Failed to get %s GPIO: %d\n", - GOODIX_GPIO_INT_NAME, error); - return error; - } + if (IS_ERR(gpiod)) + return dev_err_probe(dev, PTR_ERR(gpiod), "getting %s GPIO\n", + GOODIX_GPIO_INT_NAME); + if (!gpiod && has_acpi_companion(dev) && !added_acpi_mappings) { added_acpi_mappings = true; if (goodix_add_acpi_gpio_mappings(ts) == 0) @@ -871,13 +868,9 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts) /* Get the reset line GPIO pin number */ gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, ts->gpiod_rst_flags); - if (IS_ERR(gpiod)) { - error = PTR_ERR(gpiod); - if (error != -EPROBE_DEFER) - dev_dbg(dev, "Failed to get %s GPIO: %d\n", - GOODIX_GPIO_RST_NAME, error); - return error; - } + if (IS_ERR(gpiod)) + return dev_err_probe(dev, PTR_ERR(gpiod), "getting %s GPIO\n", + GOODIX_GPIO_RST_NAME); ts->gpiod_rst = gpiod;
goodix_get_gpio_config() errors are fatal (abort probe()) so log them at KERN_ERR level rather then as debug messages. This change uses dev_err_probe() to automatically suppress the errors in case of -EPROBE_DEFER. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/input/touchscreen/goodix.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)