Re: [PATCH] leds/of: leds-gpio.c: Use gpio_get_value_cansleep() when initializing.

[David Daney <david.daney@cavium.com>]

On 09/08/2011 10:44 PM, Andrew Morton wrote:
> On Thu, 8 Sep 2011 22:30:58 -0700 Trent Piepho<tpiepho@gmail.com>  wrote:
>
>>>
>>> They're very different. __Why is it OK to replace one with the other??
>>
>> What's supposed to happen is chip->get() will be a method that does
>> "readl(GPIO_GPLR)&  GPIO_GPIO(gpio);" or whatever the inlined bit in
>> gpio_get_value() is.  So calling gpio_get_value_cansleep() should
>> still get the correct value for the gpio.  It just won't be an inlined
>> register read anymore.
>>
>> For instance, all the arch versions that use builtin_constant_p() will
>> not take the inline path, since the gpio number if obviously not a
>> constant when gpio_get_value() is called in this leds function.  So
>> they inline into a call to __gpio_get_value().  Which as you've
>> pointed out is nearly exactly the same as gpio_get_value_cansleep().
>> The only change is debugging related, that of the might_sleep_if() to
>> a WARN_ON().
>>
>> One could have:
>> static inline int __gpio_get_value(gpio) { return
>> _gpio_get_value(gpio, GFP_ATOMIC); }
>> static inline int gpio_get_value_cansleep(gpio) { return
>> _gpio_get_value(gpio, GFP_KERNEL); }
>>
>> Then _gpio_get_value(gpio, context) would be the current code that's
>> common to both __gpio_get_value() and gpio_get_value_cansleep(),
>> except it uses context solely to spit a warning if the gpio can't be
>> done from the requested context or if the context isn't allowable from
>> whence the call was made.
>
> Well, that may be the case with the current in-tree implementations (I
> didn't check), but from a design point of view the core code shouldn't
> "know" how the architecture is implementing gpio_get_value().

Really there are two separate issues here:

1) Should the patch be applied?

2) Is there room to improve the libgpio API?

It is unclear to me if these are currently being conflated.

In any event, from a purely selfish point of view, I would like to see 
the patch applied as I cannot boot my boards with out it.  As for 
improving the GPIO APIs, it seems slightly less urgent, but also a good 
idea.

Thanks,
David Daney