gpiolib: of: fix fallback quirks handling
PATCH 1/1 gpiolib: of: fix fallback quirks handling
We should only try to execute fallback quirks handling when previous call returned -ENOENT, and not when we did not get -EPROBE_DEFER. The other errors should be treated as hard errors: we did find the GPIO description, but for some reason we failed to handle it properly.
The fallbacks should only be executed when previous handlers returned -ENOENT, which means the mapping/description was not found.
Also let's remove the explicit deferral handling when iterating through GPIO suffixes: it is not needed anymore as we will not be calling fallbacks for anything but -ENOENT.
--- drivers/gpio/gpiolib-of.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)
drivers/gpio/gpiolib-of.c +9 -18
@@ -457,36 +457,27 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, desc =of_get_named_gpiod_flags (dev->of_node, prop_name, idx, &of_flags); -/* -* -EPROBE_DEFER in our case means that we found a -* valid GPIO property, but no controller has been -* registered so far. -* -* This means we don't need to look any further for -* alternate name conventions, and we should really -* preserve the return code for our user to be able to -* retry probing later. -*/ -if (IS_ERR (desc) &&PTR_ERR (desc) == -EPROBE_DEFER ) -return desc; -if (!IS_ERR (desc) || (PTR_ERR (desc) != -ENOENT )) +if (!IS_ERR (desc) ||PTR_ERR (desc) != -ENOENT )break ; } -/* Special handling for SPI GPIOs if used */ -if (IS_ERR (desc)) +if (IS_ERR (desc) &&PTR_ERR (desc) == -ENOENT ) { +/* Special handling for SPI GPIOs if used */ desc =of_find_spi_gpio (dev, con_id, &of_flags); -if (IS_ERR (desc) &&PTR_ERR (desc) != -EPROBE_DEFER ) { + } + +if (IS_ERR (desc) &&PTR_ERR (desc) == -ENOENT ) {/* This quirk looks up flags and all */ desc =of_find_spi_cs_gpio (dev, con_id, idx, flags);if (!IS_ERR (desc))return desc; } -/* Special handling for regulator GPIOs if used */ -if (IS_ERR (desc) &&PTR_ERR (desc) != -EPROBE_DEFER ) +if (IS_ERR (desc) &&PTR_ERR (desc) == -ENOENT ) { +/* Special handling for regulator GPIOs if used */ desc =of_find_regulator_gpio (dev, con_id, &of_flags); + }if (IS_ERR (desc))return desc;
Signature · 5 lines
2.23.0.187.g17f5b7556c-goog -- Dmitry
Quoted 5 lines · Dmitry Torokhov
We should only try to execute fallback quirks handling when previous call returned
-ENOENT, and not when we did not get -EPROBE_DEFER. The other errors should be treated as hard errors: we did find the GPIO description, but for some reason we failed to handle it properly.
Quoted 7 lines
The fallbacks should only be executed when previous handlers returned
-ENOENT, which means the mapping/description was not found.Also let's remove the explicit deferral handling when iterating through GPIO suffixes: it is not needed anymore as we will not be calling fallbacks for anything but
-ENOENT.
I would rather leave extra parenthesis and comments untouched, nevertheless, FWIW,
Quoted 62 lines
Fixes df451f83e1fc ("gpio: of: fix Freescale SPI CS quirk handling")Signed-off-by Dmitry Torokhov <[email protected]>--- drivers/gpio/gpiolib-of.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-)drivers/gpio/gpiolib-of.c +9
-18@@ -457,36 +457,27 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id, desc =of_get_named_gpiod_flags (dev->of_node, prop_name, idx, &of_flags); -/* -* -EPROBE_DEFER in our case means that we found a -* valid GPIO property, but no controller has been -* registered so far. -* -* This means we don't need to look any further for -* alternate name conventions, and we should really -* preserve the return code for our user to be able to -* retry probing later. -*/ -if (IS_ERR (desc) &&PTR_ERR (desc) == -EPROBE_DEFER ) -return desc; -if (!IS_ERR (desc) || (PTR_ERR (desc) != -ENOENT )) +if (!IS_ERR (desc) ||PTR_ERR (desc) != -ENOENT )break ; } -/* Special handling for SPI GPIOs if used */ -if (IS_ERR (desc)) +if (IS_ERR (desc) &&PTR_ERR (desc) == -ENOENT ) { +/* Special handling for SPI GPIOs if used */ desc =of_find_spi_gpio (dev, con_id, &of_flags); -if (IS_ERR (desc) &&PTR_ERR (desc) != -EPROBE_DEFER ) { + } + +if (IS_ERR (desc) &&PTR_ERR (desc) == -ENOENT ) {/* This quirk looks up flags and all */ desc =of_find_spi_cs_gpio (dev, con_id, idx, flags);if (!IS_ERR (desc))return desc; } -/* Special handling for regulator GPIOs if used */ -if (IS_ERR (desc) &&PTR_ERR (desc) != -EPROBE_DEFER ) +if (IS_ERR (desc) &&PTR_ERR (desc) == -ENOENT ) { +/* Special handling for regulator GPIOs if used */ desc =of_find_regulator_gpio (dev, con_id, &of_flags); + }if (IS_ERR (desc))return desc;Signature · 5 lines
2.23.0.187.g17f5b7556c-goog -- Dmitry
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 8 lines · Dmitry Torokhov
We should only try to execute fallback quirks handling when previous call returned
-ENOENT, and not when we did not get -EPROBE_DEFER. The other errors should be treated as hard errors: we did find the GPIO description, but for some reason we failed to handle it properly.The fallbacks should only be executed when previous handlers returned
-ENOENT, which means the mapping/description was not found.
Quoted 6 lines
Also let's remove the explicit deferral handling when iterating through GPIO suffixes: it is not needed anymore as we will not be calling fallbacks for anything but
-ENOENT.Fixes df451f83e1fc ("gpio: of: fix Freescale SPI CS quirk handling")Signed-off-by Dmitry Torokhov <[email protected]>
Patch applied with Andy's review tag.
Sorry for not catching this before, the reasoning with the patch is correct and I should have noticed. Overload I guess.
Yours,
Linus Walleij