| Message ID | 1456143411.23008.1.camel@ingics.com |
|---|---|
| State | New |
| Headers | show |
On 02/22/2016 06:16 AM, Axel Lin wrote: > gpio->load_gpio is optional, so use devm_gpiod_get_optional instead. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> > --- > drivers/gpio/gpio-pisosr.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c > index f9f1074..8b8bf8f 100644 > --- a/drivers/gpio/gpio-pisosr.c > +++ b/drivers/gpio/gpio-pisosr.c > @@ -125,15 +125,12 @@ static int pisosr_gpio_probe(struct spi_device *spi) > if (!gpio->buffer) > return -ENOMEM; > > - gpio->load_gpio = devm_gpiod_get(dev, "load", GPIOD_OUT_LOW); > + gpio->load_gpio = devm_gpiod_get_optional(dev, "load", GPIOD_OUT_LOW); > if (IS_ERR(gpio->load_gpio)) { > ret = PTR_ERR(gpio->load_gpio); > - if (ret != -ENOENT && ret != -ENOSYS) { > - if (ret != -EPROBE_DEFER) > - dev_err(dev, "Unable to allocate load GPIO\n"); > - return ret; > - } > - gpio->load_gpio = NULL; > + if (ret != -EPROBE_DEFER) > + dev_err(dev, "Unable to allocate load GPIO\n"); > + return ret; > } > > mutex_init(&gpio->lock); > How does this work when the GPIO subsystem is disabled? It looks like we get will get -ENOSYS and fail probe. The issue is then that devm_gpiod_get_optional returns an error when it cant get the optional GPIO in that case, when it should probably just return NULL, I would think all the optional functions should. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-02-22 23:43 GMT+08:00 Andrew F. Davis <afd@ti.com>: > On 02/22/2016 06:16 AM, Axel Lin wrote: >> >> gpio->load_gpio is optional, so use devm_gpiod_get_optional instead. >> >> Signed-off-by: Axel Lin <axel.lin@ingics.com> >> --- >> drivers/gpio/gpio-pisosr.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c >> index f9f1074..8b8bf8f 100644 >> --- a/drivers/gpio/gpio-pisosr.c >> +++ b/drivers/gpio/gpio-pisosr.c >> @@ -125,15 +125,12 @@ static int pisosr_gpio_probe(struct spi_device *spi) >> if (!gpio->buffer) >> return -ENOMEM; >> >> - gpio->load_gpio = devm_gpiod_get(dev, "load", GPIOD_OUT_LOW); >> + gpio->load_gpio = devm_gpiod_get_optional(dev, "load", >> GPIOD_OUT_LOW); >> if (IS_ERR(gpio->load_gpio)) { >> ret = PTR_ERR(gpio->load_gpio); >> - if (ret != -ENOENT && ret != -ENOSYS) { >> - if (ret != -EPROBE_DEFER) >> - dev_err(dev, "Unable to allocate load >> GPIO\n"); >> - return ret; >> - } >> - gpio->load_gpio = NULL; >> + if (ret != -EPROBE_DEFER) >> + dev_err(dev, "Unable to allocate load GPIO\n"); >> + return ret; >> } >> >> mutex_init(&gpio->lock); >> > > How does this work when the GPIO subsystem is disabled? It looks like > we get will get -ENOSYS and fail probe. The issue is then that I think it's no problem for this driver as it is in drivers/gpio/ folder, it's always build with GPIOLIB. > devm_gpiod_get_optional returns an error when it cant get the optional > GPIO in that case, when it should probably just return NULL, I would > think all the optional functions should. I had similar question when I mad this change. When !GPIOLIB, all the gpio APIs return error. But for (devm_)gpiod_get_optional, it seems reasonable to return NULL in !GPIOLIB case because it's optional anyway. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/22/2016 07:09 PM, Axel Lin wrote: > 2016-02-22 23:43 GMT+08:00 Andrew F. Davis <afd@ti.com>: >> On 02/22/2016 06:16 AM, Axel Lin wrote: >>> >>> gpio->load_gpio is optional, so use devm_gpiod_get_optional instead. >>> >>> Signed-off-by: Axel Lin <axel.lin@ingics.com> >>> --- >>> drivers/gpio/gpio-pisosr.c | 11 ++++------- >>> 1 file changed, 4 insertions(+), 7 deletions(-) >>> >>> diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c >>> index f9f1074..8b8bf8f 100644 >>> --- a/drivers/gpio/gpio-pisosr.c >>> +++ b/drivers/gpio/gpio-pisosr.c >>> @@ -125,15 +125,12 @@ static int pisosr_gpio_probe(struct spi_device *spi) >>> if (!gpio->buffer) >>> return -ENOMEM; >>> >>> - gpio->load_gpio = devm_gpiod_get(dev, "load", GPIOD_OUT_LOW); >>> + gpio->load_gpio = devm_gpiod_get_optional(dev, "load", >>> GPIOD_OUT_LOW); >>> if (IS_ERR(gpio->load_gpio)) { >>> ret = PTR_ERR(gpio->load_gpio); >>> - if (ret != -ENOENT && ret != -ENOSYS) { >>> - if (ret != -EPROBE_DEFER) >>> - dev_err(dev, "Unable to allocate load >>> GPIO\n"); >>> - return ret; >>> - } >>> - gpio->load_gpio = NULL; >>> + if (ret != -EPROBE_DEFER) >>> + dev_err(dev, "Unable to allocate load GPIO\n"); >>> + return ret; >>> } >>> >>> mutex_init(&gpio->lock); >>> >> >> How does this work when the GPIO subsystem is disabled? It looks like >> we get will get -ENOSYS and fail probe. The issue is then that > > I think it's no problem for this driver as it is in drivers/gpio/ folder, > it's always build with GPIOLIB. > >> devm_gpiod_get_optional returns an error when it cant get the optional >> GPIO in that case, when it should probably just return NULL, I would >> think all the optional functions should. > > I had similar question when I mad this change. > When !GPIOLIB, all the gpio APIs return error. > But for (devm_)gpiod_get_optional, it seems reasonable to return NULL > in !GPIOLIB case > because it's optional anyway. > Well I guess that's work for another patch, for this one I see no issues. Acked-by: Andrew F. Davis <afd@ti.com> -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 22, 2016 at 1:16 PM, Axel Lin <axel.lin@ingics.com> wrote: > gpio->load_gpio is optional, so use devm_gpiod_get_optional instead. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Patch applied with Andrew's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-pisosr.c b/drivers/gpio/gpio-pisosr.c index f9f1074..8b8bf8f 100644 --- a/drivers/gpio/gpio-pisosr.c +++ b/drivers/gpio/gpio-pisosr.c @@ -125,15 +125,12 @@ static int pisosr_gpio_probe(struct spi_device *spi) if (!gpio->buffer) return -ENOMEM; - gpio->load_gpio = devm_gpiod_get(dev, "load", GPIOD_OUT_LOW); + gpio->load_gpio = devm_gpiod_get_optional(dev, "load", GPIOD_OUT_LOW); if (IS_ERR(gpio->load_gpio)) { ret = PTR_ERR(gpio->load_gpio); - if (ret != -ENOENT && ret != -ENOSYS) { - if (ret != -EPROBE_DEFER) - dev_err(dev, "Unable to allocate load GPIO\n"); - return ret; - } - gpio->load_gpio = NULL; + if (ret != -EPROBE_DEFER) + dev_err(dev, "Unable to allocate load GPIO\n"); + return ret; } mutex_init(&gpio->lock);
gpio->load_gpio is optional, so use devm_gpiod_get_optional instead. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- drivers/gpio/gpio-pisosr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)