Re: [PATCH v4] backlight: corgi_lcd: Use gpio_set_value_cansleep() toavoid WARN_ON

[Marko KatiÄ]

On Wed, Dec 5, 2012 at 10:30 AM, Russell King - ARM Linux
<linux@xxxxxxxxxxxxxxxx> 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 ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/