Re: [PATCH] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]

On Fri, Jul 28, 2017 at 01:21:26PM -0300, Fabio Estevam wrote:
> gpiod_get_optional() returns NULL when GPIOLIB is disabled since
> commit 22c403676dbbb7c6 ("gpio: return NULL from gpiod_get_optional when
> GPIOLIB is disabled").
> 
> However, many gpiod functions still have WARN_ON(1) in their
> GPIOLIB=n stubs, which causes warnings in drivers even if the
> GPIO descriptor is requested via gpiod_get_optional().
> 
> Prior to commit 22c403676dbbb7c6 ("gpio: return NULL from
> gpiod_get_optional when GPIOLIB is disabled") it was indeed true
> the comment: "GPIO can never have been requested" as gpiod_get_optional()
> used to return an error.
> 
> After this commit the returned value is NULL, so the comment and
> WARN_ON(1) are no longer accurate.
> 
> An example of this kernel warning can be see in this report:
> https://www.spinics.net/lists/kernel/msg2563045.html
> 
> Remove the WARN_ON(1) so that drivers can silently work fine
> without kernel warnings when GPIOLIB is disabled.
> 
> Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since RFC:
> - Expand a bit on the commit log
> - Include the kernel warning example
> 
>  include/linux/gpio/consumer.h | 59 -------------------------------------------
>  1 file changed, 59 deletions(-)

How about cases where the GPIO was not optional? Do we still want those
to be silently ignored? I guess in some way that makes sense as well,
but after this patch we're deviating from existing behaviour for non-
optional GPIOs.

Also, if we do decide that we want to silently ignore all GPIO calls if
GPIOLIB is disabled, maybe it would still be a good idea to warn users
about this at least once, so that they don't go chasing wild geese?

In either case, I think the commit message is still confusing because
you're arguing that gpiod_get_optional() now returns NULL and therefore
it is fine to remove WARN_ON(), whereas the WARN_ON() would still be
valid for non-NULL GPIO descriptors (such as requested via gpiod_get()
when GPIOLIB=n.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]