[PATCH] USB: bcma: Make GPIO explicitly optional

[PATCH] USB: bcma: Make GPIO explicitly optional

[Linus Walleij]

What the code does is to not check the return value from
devm_gpiod_get() and then avoid using an erroneous GPIO descriptor
with IS_ERR_OR_NULL().

This will miss real errors from the GPIO core that should not be
ignored, such as probe deferral.

Instead request the GPIO as explicitly optional, which means that
if it doesn't exist, the descriptor returned will be NULL.

Then we can add error handling and also avoid just doing this on
the device tree path, and simplify the site where the optional
GPIO descriptor is used.

There were some problems with cleaning up this GPIO descriptor
use in the past, but this is the proper way to deal with it.

Cc: Rafał Miłecki <rafal@milecki.pl>
Cc: Chuhong Yuan <hslester96@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/host/bcma-hcd.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/bcma-hcd.c b/drivers/usb/host/bcma-hcd.c
index 2df52f75f6b3..7558cc4d90cc 100644
--- a/drivers/usb/host/bcma-hcd.c
+++ b/drivers/usb/host/bcma-hcd.c
@@ -285,7 +285,7 @@ static void bcma_hci_platform_power_gpio(struct bcma_device *dev, bool val)
 {
 	struct bcma_hcd_device *usb_dev = bcma_get_drvdata(dev);
 
-	if (IS_ERR_OR_NULL(usb_dev->gpio_desc))
+	if (!usb_dev->gpio_desc)
 		return;
 
 	gpiod_set_value(usb_dev->gpio_desc, val);
@@ -406,9 +406,11 @@ static int bcma_hcd_probe(struct bcma_device *core)
 		return -ENOMEM;
 	usb_dev->core = core;
 
-	if (core->dev.of_node)
-		usb_dev->gpio_desc = devm_gpiod_get(&core->dev, "vcc",
-						    GPIOD_OUT_HIGH);
+	usb_dev->gpio_desc = devm_gpiod_get_optional(&core->dev, "vcc",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(usb_dev->gpio_desc))
+		return dev_err_probe(&core->dev, PTR_ERR(usb_dev->gpio_desc),
+				     "error obtaining VCC GPIO");
 
 	switch (core->id.id) {
 	case BCMA_CORE_USB20_HOST:
-- 
2.34.1

Re: [PATCH] USB: bcma: Make GPIO explicitly optional

[Linus Walleij]

On Mon, Nov 7, 2022 at 10:07 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> What the code does is to not check the return value from
> devm_gpiod_get() and then avoid using an erroneous GPIO descriptor
> with IS_ERR_OR_NULL().
>
> This will miss real errors from the GPIO core that should not be
> ignored, such as probe deferral.
>
> Instead request the GPIO as explicitly optional, which means that
> if it doesn't exist, the descriptor returned will be NULL.
>
> Then we can add error handling and also avoid just doing this on
> the device tree path, and simplify the site where the optional
> GPIO descriptor is used.
>
> There were some problems with cleaning up this GPIO descriptor
> use in the past, but this is the proper way to deal with it.
>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Chuhong Yuan <hslester96@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Actually I think this is something of a regression that needs
a Cc: stable tag.

On my router (DIR-890L) this is needed for the USB ports to probe
and work, I think due to recent refactorings involving device links
and whatnot, i.e. probe deferral happens all the time and the
deferral error has to be handled. I think many BCM systems are
affected.

Yours,
Linus Walleij

Re: [PATCH] USB: bcma: Make GPIO explicitly optional

[Greg Kroah-Hartman]

On Mon, Nov 07, 2022 at 02:38:35PM +0100, Linus Walleij wrote:
> On Mon, Nov 7, 2022 at 10:07 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> > What the code does is to not check the return value from
> > devm_gpiod_get() and then avoid using an erroneous GPIO descriptor
> > with IS_ERR_OR_NULL().
> >
> > This will miss real errors from the GPIO core that should not be
> > ignored, such as probe deferral.
> >
> > Instead request the GPIO as explicitly optional, which means that
> > if it doesn't exist, the descriptor returned will be NULL.
> >
> > Then we can add error handling and also avoid just doing this on
> > the device tree path, and simplify the site where the optional
> > GPIO descriptor is used.
> >
> > There were some problems with cleaning up this GPIO descriptor
> > use in the past, but this is the proper way to deal with it.
> >
> > Cc: Rafał Miłecki <rafal@milecki.pl>
> > Cc: Chuhong Yuan <hslester96@gmail.com>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Actually I think this is something of a regression that needs
> a Cc: stable tag.
> 
> On my router (DIR-890L) this is needed for the USB ports to probe
> and work, I think due to recent refactorings involving device links
> and whatnot, i.e. probe deferral happens all the time and the
> deferral error has to be handled. I think many BCM systems are
> affected.

Now queued up for the next -rc release, thanks.

greg k-h