lkml.org 
[lkml]   [2011]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] leds/of: leds-gpio.c: Use gpio_get_value_cansleep() when initializing.
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


\
 
 \ /
  Last update: 2011-09-09 18:19    [W:2.300 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site