Re: [PATCH] gpiolib: Defer on non-DT find_chip_by_name() failure

From: Richard Fitzgerald
Date: Fri Jul 06 2018 - 05:06:24 EST


On 05/07/18 21:56, Janusz Krzysztofik wrote:
On Thursday, July 5, 2018 7:50:37 AM CEST Lee Jones wrote:
On Wed, 04 Jul 2018, Janusz Krzysztofik wrote:
On Tuesday, July 3, 2018 7:31:41 PM CEST Boris Brezillon wrote:
Hi Janusz,

On Tue, 3 Jul 2018 19:26:35 +0200

Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx> wrote:
Avoid replication of error code conversion in non-DT GPIO consumers'
code by returning -EPROBE_DEFER from gpiod_find() in case a chip
identified by its label in a registered lookup table is not ready.

See https://lkml.org/lkml/2018/5/30/176 for example case.

Signed-off-by: Janusz Krzysztofik <jmkrzyszt@xxxxxxxxx>
---
If accepted, please add

Suggested-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxx>

if Boris doesn't mind.

Thanks,
Janusz

drivers/gpio/gpiolib.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e11a3bb03820..15dc77c80328 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3639,9 +3639,16 @@ static struct gpio_desc *gpiod_find(struct
device
*dev, const char *con_id,>

chip = find_chip_by_name(p->chip_label);

if (!chip) {

- dev_err(dev, "cannot find GPIO chip %s\n",
- p->chip_label);
- return ERR_PTR(-ENODEV);
+ /*
+ * As the lookup table indicates a chip with
+ * p->chip_label should exist, assume it may
+ * still appear latar and let the interested

^ later

+ * consumer be probed again or let the Deferred
+ * Probe infrastructure handle the error.
+ */
+ dev_warn(dev, "cannot find GPIO chip %s, deferring\n",
+ p->chip_label);
+ return ERR_PTR(-EPROBE_DEFER);

}

if (chip->ngpio <= p->chip_hwnum) {

Looks good otherwise. Let's hope we're not breaking implementations
testing for -ENODEV...

I've reviewed them all and found two which I think may be affected:
- drivers/mfd/arizona-core.c,
- drivers/i2c/busses/i2c-imx.c.
As far as I can understand the code, both depend on error != -EPROBE_DEFER
in order to continue in degraded mode. I'm adding their maintainers to
the loop.
From a quick glance, the -EPROBE_DEFER handing in Arizona Core appears
to be correct. Would you mind explaining what your concerns are in
more detail please?

Hi

That's more about handling -ENODEV rather than -EPROBE_DEFER.

Before the change, if GPIO chip supposed to provide "reset" pin was not ready
during arizona_dev_init(), devm_gpiod_get() returned -ENODEV and device was
initialized in degraded mode, i.e., with no control over the "reset" pin.
After the change, gpiod_get() will return -EPROBE_DEFER in such case and
arizona_dev_init() won't succeed in case the GPIO chip doesn't appear later
for some reason.

Thanks,
Januszz



The intention is that if the DT node is missing, the Arizona driver can run
using only soft reset, though there are limitations in that mode.
This should return -ENOENT so that the Arizona driver will continue without
a GPIO.

If the DT defines a GPIO it is effectively saying that this GPIO is required so
it is valid for the Arizona driver never to come up if the GPIO it is defined to
depend on doesn't come up.