* [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio
@ 2016-02-22 12:16 Axel Lin
2016-02-22 15:43 ` Andrew F. Davis
2016-02-25 13:17 ` Linus Walleij
0 siblings, 2 replies; 5+ messages in thread
From: Axel Lin @ 2016-02-22 12:16 UTC (permalink / raw)
To: Linus Walleij; +Cc: Andrew F. Davis, Alexandre Courbot, linux-gpio
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio
2016-02-22 12:16 [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio Axel Lin
@ 2016-02-22 15:43 ` Andrew F. Davis
2016-02-23 1:09 ` Axel Lin
2016-02-25 13:17 ` Linus Walleij
1 sibling, 1 reply; 5+ messages in thread
From: Andrew F. Davis @ 2016-02-22 15:43 UTC (permalink / raw)
To: Axel Lin, Linus Walleij; +Cc: Alexandre Courbot, linux-gpio
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio
2016-02-22 15:43 ` Andrew F. Davis
@ 2016-02-23 1:09 ` Axel Lin
2016-02-23 13:48 ` Andrew F. Davis
0 siblings, 1 reply; 5+ messages in thread
From: Axel Lin @ 2016-02-23 1:09 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Linus Walleij, Alexandre Courbot, linux-gpio@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio
2016-02-23 1:09 ` Axel Lin
@ 2016-02-23 13:48 ` Andrew F. Davis
0 siblings, 0 replies; 5+ messages in thread
From: Andrew F. Davis @ 2016-02-23 13:48 UTC (permalink / raw)
To: Axel Lin; +Cc: Linus Walleij, Alexandre Courbot, linux-gpio@vger.kernel.org
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio
2016-02-22 12:16 [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio Axel Lin
2016-02-22 15:43 ` Andrew F. Davis
@ 2016-02-25 13:17 ` Linus Walleij
1 sibling, 0 replies; 5+ messages in thread
From: Linus Walleij @ 2016-02-25 13:17 UTC (permalink / raw)
To: Axel Lin; +Cc: Andrew F. Davis, Alexandre Courbot, linux-gpio@vger.kernel.org
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-25 13:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-22 12:16 [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio Axel Lin
2016-02-22 15:43 ` Andrew F. Davis
2016-02-23 1:09 ` Axel Lin
2016-02-23 13:48 ` Andrew F. Davis
2016-02-25 13:17 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.