Messages in this thread | | | Date | Fri, 09 Sep 2011 09:15:58 -0700 | From | David Daney <> | Subject | Re: [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
| |