Re: [PATCH] gpio: pisosr: Use devm_gpiod_get_optional for gpio->load_gpio

["Andrew F. Davis" <afd@ti.com>]

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>