Messages in this thread | | | From | Bartosz Golaszewski <> | Date | Tue, 9 Jul 2019 16:20:24 +0200 | Subject | Re: [PATCH] gpio: don't WARN() on NULL descs if gpiolib is disabled |
| |
wt., 9 lip 2019 o 15:30 Linus Walleij <linus.walleij@linaro.org> napisał(a): > > Hi Bartosz, > > On Mon, Jul 8, 2019 at 10:25 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > > > If gpiolib is disabled, we use the inline stubs from gpio/consumer.h > > instead of regular definitions of GPIO API. The stubs for 'optional' > > variants of gpiod_get routines return NULL in this case as if the > > relevant GPIO wasn't found. This is correct so far. > > > > Calling other (non-gpio_get) stubs from this header triggers a warning > > because the GPIO descriptor couldn't have been requested. The warning > > however is unconditional (WARN_ON(1)) and is emitted even if the passed > > descriptor pointer is NULL. > > > > We don't want to force the users of 'optional' gpio_get to check the > > returned pointer before calling e.g. gpiod_set_value() so let's only > > WARN on non-NULL descriptors. > > > > Reported-by: Claus H. Stovgaard <cst@phaseone.com> > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > I remember I had this discussion in the past, and I made a large > refactoring to make it possible for drivers that need gpiod_* > calls to simply do: > > select GPIOLIB > > in Kconfig. > > This should solve also this problem I think. > > However I do realize that there may be situations where people > simply want to make GPIO support entirely optional without > having to e.g. create custom stubs and encapsulate things > inside if IS_ENABLED(CONFIG_GPIOLIB). >
In this case the board doesn't provide any GPIO controller at all so there's simply no need to select GPIOLIB - it would only add bloat.
> I was thinking something like this in the stubs: > > gpiod_get[_index]() { > return POISON; > } > > gpiod_get[_index]_optional() { > return NULL; > }
This is already being done.
> > This way all gpiod_get() and optional calls are properly > handled and the semantic that only _optional calls > can return NULL is preserved. (Your patch would > violate this.) >
Maybe I'm missing something, but I don't quite see how my patch violates this behavior. :(
> Then other stubs can do: > > gpiod_set_value() { > WARN_ON(desc); > } > > As in your patch, and all will be smooth provided the > _optional calls have been used to obtain the desc. > > Yours, > Linus Walleij
Bart
| |