* [RFC] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled
@ 2017-07-19 22:34 Fabio Estevam
2017-07-19 23:08 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Fabio Estevam @ 2017-07-19 22:34 UTC (permalink / raw)
To: linus.walleij
Cc: sergei.shtylyov, andrew, dmitry.torokhov, linux-gpio,
Fabio Estevam
From: Fabio Estevam <fabio.estevam@nxp.com>
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 descriptior is requested via gpiod_get_optional().
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>
---
include/linux/gpio/consumer.h | 59 -------------------------------------------
1 file changed, 59 deletions(-)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 8f702fc..40c1be5 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -197,17 +197,11 @@ gpiod_get_array_optional(struct device *dev, const char *con_id,
static inline void gpiod_put(struct gpio_desc *desc)
{
might_sleep();
-
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline void gpiod_put_array(struct gpio_descs *descs)
{
might_sleep();
-
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline struct gpio_desc *__must_check
@@ -254,150 +248,99 @@ devm_gpiod_get_array_optional(struct device *dev, const char *con_id,
{
return NULL;
}
-
static inline void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
{
might_sleep();
-
- /* GPIO can never have been requested */
- WARN_ON(1);
}
-
static inline void devm_gpiod_put_array(struct device *dev,
struct gpio_descs *descs)
{
might_sleep();
-
- /* GPIO can never have been requested */
- WARN_ON(1);
}
-
-
static inline int gpiod_get_direction(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -ENOSYS;
}
static inline int gpiod_direction_input(struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -ENOSYS;
}
static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -ENOSYS;
}
static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -ENOSYS;
}
-
static inline int gpiod_get_value(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return 0;
}
static inline void gpiod_set_value(struct gpio_desc *desc, int value)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline void gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline int gpiod_get_raw_value(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return 0;
}
static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline void gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return 0;
}
static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline void gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return 0;
}
static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
int value)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
int *value_array)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
}
static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -ENOSYS;
}
static inline int gpiod_is_active_low(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return 0;
}
static inline int gpiod_cansleep(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return 0;
}
static inline int gpiod_to_irq(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -EINVAL;
}
@@ -408,8 +351,6 @@ static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
static inline int desc_to_gpio(const struct gpio_desc *desc)
{
- /* GPIO can never have been requested */
- WARN_ON(1);
return -EINVAL;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [RFC] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled
2017-07-19 22:34 [RFC] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled Fabio Estevam
@ 2017-07-19 23:08 ` Dmitry Torokhov
2017-07-27 19:38 ` Fabio Estevam
2017-08-03 13:22 ` Linus Walleij
0 siblings, 2 replies; 4+ messages in thread
From: Dmitry Torokhov @ 2017-07-19 23:08 UTC (permalink / raw)
To: Fabio Estevam
Cc: linus.walleij, sergei.shtylyov, andrew, linux-gpio, Fabio Estevam
On Wed, Jul 19, 2017 at 07:34:25PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> 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 descriptior is requested via gpiod_get_optional().
>
> Remove the WARN_ON(1) so that drivers can silently work fine
> without kernel warnings when GPIOLIB is disabled.
Hmm, I did not realize that GPIO API allowed calls with NULL desc and
they would be silently accepted and return 0... Not sure if that is the
best way for the subsystem to behave, but Linus is the boss ;)
Maybe check for NULL explicitly and keep warning if !NULL is passed?
>
> Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> include/linux/gpio/consumer.h | 59 -------------------------------------------
> 1 file changed, 59 deletions(-)
>
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index 8f702fc..40c1be5 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -197,17 +197,11 @@ gpiod_get_array_optional(struct device *dev, const char *con_id,
> static inline void gpiod_put(struct gpio_desc *desc)
> {
> might_sleep();
> -
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
>
> static inline void gpiod_put_array(struct gpio_descs *descs)
> {
> might_sleep();
> -
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
>
> static inline struct gpio_desc *__must_check
> @@ -254,150 +248,99 @@ devm_gpiod_get_array_optional(struct device *dev, const char *con_id,
> {
> return NULL;
> }
> -
> static inline void devm_gpiod_put(struct device *dev, struct gpio_desc *desc)
> {
> might_sleep();
> -
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> -
> static inline void devm_gpiod_put_array(struct device *dev,
> struct gpio_descs *descs)
> {
> might_sleep();
> -
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> -
> -
> static inline int gpiod_get_direction(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -ENOSYS;
> }
> static inline int gpiod_direction_input(struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -ENOSYS;
> }
> static inline int gpiod_direction_output(struct gpio_desc *desc, int value)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -ENOSYS;
> }
> static inline int gpiod_direction_output_raw(struct gpio_desc *desc, int value)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -ENOSYS;
> }
>
> -
> static inline int gpiod_get_value(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return 0;
> }
> static inline void gpiod_set_value(struct gpio_desc *desc, int value)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> static inline void gpiod_set_array_value(unsigned int array_size,
> struct gpio_desc **desc_array,
> int *value_array)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> static inline int gpiod_get_raw_value(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return 0;
> }
> static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> static inline void gpiod_set_raw_array_value(unsigned int array_size,
> struct gpio_desc **desc_array,
> int *value_array)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
>
> static inline int gpiod_get_value_cansleep(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return 0;
> }
> static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> static inline void gpiod_set_array_value_cansleep(unsigned int array_size,
> struct gpio_desc **desc_array,
> int *value_array)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> static inline int gpiod_get_raw_value_cansleep(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return 0;
> }
> static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
> int value)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
> static inline void gpiod_set_raw_array_value_cansleep(unsigned int array_size,
> struct gpio_desc **desc_array,
> int *value_array)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> }
>
> static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -ENOSYS;
> }
>
> static inline int gpiod_is_active_low(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return 0;
> }
> static inline int gpiod_cansleep(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return 0;
> }
>
> static inline int gpiod_to_irq(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -EINVAL;
> }
>
> @@ -408,8 +351,6 @@ static inline struct gpio_desc *gpio_to_desc(unsigned gpio)
>
> static inline int desc_to_gpio(const struct gpio_desc *desc)
> {
> - /* GPIO can never have been requested */
> - WARN_ON(1);
> return -EINVAL;
> }
>
> --
> 2.7.4
>
--
Dmitry
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled
2017-07-19 23:08 ` Dmitry Torokhov
@ 2017-07-27 19:38 ` Fabio Estevam
2017-08-03 13:22 ` Linus Walleij
1 sibling, 0 replies; 4+ messages in thread
From: Fabio Estevam @ 2017-07-27 19:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linus Walleij, Sergei Shtylyov, Andrew Lunn,
linux-gpio@vger.kernel.org, Fabio Estevam
On Wed, Jul 19, 2017 at 8:08 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hmm, I did not realize that GPIO API allowed calls with NULL desc and
> they would be silently accepted and return 0... Not sure if that is the
> best way for the subsystem to behave, but Linus is the boss ;)
That part seems fine.
> Maybe check for NULL explicitly and keep warning if !NULL is passed?
Not sure if this is needed.
Prior to commit 22c403676dbbb7c6 ("gpio: return NULL from
gpiod_get_optional when
GPIOLIB is disabled") it made sense to have the WARN_ON(1) and the comment:
" /* GPIO can never have been requested */" was true.
After such commit this is no longer true, so that's why IMO we should
just get rid of the WARN_ON and comment.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled
2017-07-19 23:08 ` Dmitry Torokhov
2017-07-27 19:38 ` Fabio Estevam
@ 2017-08-03 13:22 ` Linus Walleij
1 sibling, 0 replies; 4+ messages in thread
From: Linus Walleij @ 2017-08-03 13:22 UTC (permalink / raw)
To: Dmitry Torokhov, Alexandre Courbot
Cc: Fabio Estevam, Sergei Shtylyov, Andrew Lunn,
linux-gpio@vger.kernel.org, Fabio Estevam
On Thu, Jul 20, 2017 at 1:08 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hmm, I did not realize that GPIO API allowed calls with NULL desc and
> they would be silently accepted and return 0... Not sure if that is the
> best way for the subsystem to behave, but Linus is the boss ;)
NULL gpiods are supported for
gpiod_get_[index_]optional() usecasese only.
See commit:
commit 54d77198fdfbc4f0fe11b4252c1d9c97d51a3264
"gpio: bail out silently on NULL descriptors"
> Maybe check for NULL explicitly and keep warning if !NULL is passed?
A better solution is maybe if gpiod_get_[index_]optional() could
return something akin to a dummy regulator if the GPIO is not
there?
Then we can stop allowing NULL gpiods.
Optional regulators also have some tricksy corner cases like this...
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-08-03 13:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-19 22:34 [RFC] gpio: consumer: Remove WARN_ON(1) when GPIOLIB is disabled Fabio Estevam
2017-07-19 23:08 ` Dmitry Torokhov
2017-07-27 19:38 ` Fabio Estevam
2017-08-03 13:22 ` Linus Walleij
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).