[PATCH v4] backlight: corgi_lcd: Use gpio_set_value_cansleep() to avoid WARN_ON

Marko Katić dromede at gmail.com
Wed Dec 5 13:20:00 EST 2012


On Wed, Dec 5, 2012 at 10:30 AM, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Wed, Dec 05, 2012 at 09:59:07AM +0900, Jingoo Han wrote:
>> -     if (gpio_is_valid(lcd->gpio_backlight_cont))
>> -             gpio_set_value(lcd->gpio_backlight_cont, cont);
>> +     if (gpio_is_valid(lcd->gpio_backlight_cont)) {
>> +             if (gpio_cansleep(lcd->gpio_backlight_cont))
>> +                     gpio_set_value_cansleep(lcd->gpio_backlight_cont, cont);
>> +             else
>> +                     gpio_set_value(lcd->gpio_backlight_cont, cont);
>> +     }
>
> Why not simply:
>
> +       if (gpio_is_valid(lcd->gpio_backlight_cont))
> +               gpio_set_value_cansleep(lcd->gpio_backlight_cont, cont);

My first patch did exactly this but there were complains about it's
commit message.

And i just found out that Marek Vasut posted the exact same patch more
than a year ago.

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-April/046955.html

It was not applied for various reasons.

>
> If you read the gpiolib code and documentation, what you will realise is
> that the two calls are identical except for the "might_sleep_if()" in
> gpio_set_value_cansleep().  You will also note that gpiolib itself _only_
> calls gpio_set_value_cansleep() without checking gpio_cansleep() in
> contexts where sleeping is possible.  So if it's good enough for gpiolib,
> it should be good enough here.

The documentation tells which calls to use when you don't need to sleep
and which calls to use when you might sleep. And here we have a case
where the same call to gpio_set_value might sleep or doesn't have to,
depending on the model.
In this case, i'd rather use gpio_cansleep check as Andrew proposed.

I will also say that the distinction between gpio_set_value and
gpio_set_value_cansleep.
is rather confusing at this point. Is it really necessary to have both ?



More information about the linux-arm-kernel mailing list