linux-kernel - Re: [PATCH v4] backlight: corgi_lcd: Use gpio_set_value_cansleep() to avoid WARN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHod+Gdb_LEnhx5cz9j=q9PyS4L=Yjxs303dHon9i-tgjJRtDA@mail.gmail.com>
Date:	Wed, 5 Dec 2012 19:20:00 +0100
From:	Marko Katić <dromede@...il.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Jingoo Han <jg1.han@...sung.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Florian Tobias Schandinat <FlorianSchandinat@....de>,
	Grant Likely <grant.likely@...retlab.ca>,
	Richard Purdie <rpurdie@...ys.net>,
	linux-arm-kernel@...ts.infradead.org,
	Marek Vašut <marek.vasut@...il.com>
Subject: Re: [PATCH v4] backlight: corgi_lcd: Use gpio_set_value_cansleep() to
 avoid WARN_ON

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