| Message ID | 20190326152114.40591-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] gpiolib: Don't WARN on gpiod_put() for optional GPIO | expand |
wt., 26 mar 2019 o 16:21 Andy Shevchenko <andriy.shevchenko@linux.intel.com> napisał(a): > > In case of debug and optional GPIO requested, the gpiod_put() is not aware of > and will WARN, which is not the case. > > Make gpiod_put() NULL-aware to keep silent for optional GPIOs. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/gpio/gpiolib.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > index 144af0733581..36445e24ee89 100644 > --- a/drivers/gpio/gpiolib.c > +++ b/drivers/gpio/gpiolib.c > @@ -4616,7 +4616,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_optional); > */ > void gpiod_put(struct gpio_desc *desc) > { > - gpiod_free(desc); > + if (desc) > + gpiod_free(desc); > } > EXPORT_SYMBOL_GPL(gpiod_put); > > -- > 2.20.1 > In that case I don't think we need to keep checking desc for NULL in gpiod_free(). Bart
On Tue, Mar 26, 2019 at 06:48:25PM +0100, Bartosz Golaszewski wrote: > wt., 26 mar 2019 o 16:21 Andy Shevchenko > <andriy.shevchenko@linux.intel.com> napisał(a): > > > > In case of debug and optional GPIO requested, the gpiod_put() is not aware of > > and will WARN, which is not the case. > > > > Make gpiod_put() NULL-aware to keep silent for optional GPIOs. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/gpio/gpiolib.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > > index 144af0733581..36445e24ee89 100644 > > --- a/drivers/gpio/gpiolib.c > > +++ b/drivers/gpio/gpiolib.c > > @@ -4616,7 +4616,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_optional); > > */ > > void gpiod_put(struct gpio_desc *desc) > > { > > - gpiod_free(desc); > > + if (desc) > > + gpiod_free(desc); > > } > > EXPORT_SYMBOL_GPL(gpiod_put); > > > > -- > > 2.20.1 > > > > In that case I don't think we need to keep checking desc for NULL in > gpiod_free(). I didn't remove the check there because it's called more than just from gpiod_put(). I didn't check all code paths myself. So, if it seems good for you to remove the check I can do it in next version.
On Tue, Mar 26, 2019 at 09:44:20PM +0200, Andy Shevchenko wrote: > On Tue, Mar 26, 2019 at 06:48:25PM +0100, Bartosz Golaszewski wrote: > > wt., 26 mar 2019 o 16:21 Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> napisał(a): > > > > > > In case of debug and optional GPIO requested, the gpiod_put() is not aware of > > > and will WARN, which is not the case. > > > > > > Make gpiod_put() NULL-aware to keep silent for optional GPIOs. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > --- > > > drivers/gpio/gpiolib.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > > > index 144af0733581..36445e24ee89 100644 > > > --- a/drivers/gpio/gpiolib.c > > > +++ b/drivers/gpio/gpiolib.c > > > @@ -4616,7 +4616,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_optional); > > > */ > > > void gpiod_put(struct gpio_desc *desc) > > > { > > > - gpiod_free(desc); > > > + if (desc) > > > + gpiod_free(desc); > > > } > > > EXPORT_SYMBOL_GPL(gpiod_put); > > > > > > -- > > > 2.20.1 > > > > > > > In that case I don't think we need to keep checking desc for NULL in > > gpiod_free(). > > I didn't remove the check there because it's called more than just from > gpiod_put(). I didn't check all code paths myself. So, if it seems good for you > to remove the check I can do it in next version. Any additional comments on this? I have looked briefly at this and found that gpiod_free() is used to support legacy gpio_free() which is used in a plenty of drivers. There are more than dozen are using non static number for these calls. I think the patch as is is the best approach we can do right now.
wt., 2 kwi 2019 o 12:49 Andy Shevchenko <andriy.shevchenko@linux.intel.com> napisał(a): > > On Tue, Mar 26, 2019 at 09:44:20PM +0200, Andy Shevchenko wrote: > > On Tue, Mar 26, 2019 at 06:48:25PM +0100, Bartosz Golaszewski wrote: > > > wt., 26 mar 2019 o 16:21 Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> napisał(a): > > > > > > > > In case of debug and optional GPIO requested, the gpiod_put() is not aware of > > > > and will WARN, which is not the case. > > > > > > > > Make gpiod_put() NULL-aware to keep silent for optional GPIOs. > > > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > --- > > > > drivers/gpio/gpiolib.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c > > > > index 144af0733581..36445e24ee89 100644 > > > > --- a/drivers/gpio/gpiolib.c > > > > +++ b/drivers/gpio/gpiolib.c > > > > @@ -4616,7 +4616,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_optional); > > > > */ > > > > void gpiod_put(struct gpio_desc *desc) > > > > { > > > > - gpiod_free(desc); > > > > + if (desc) > > > > + gpiod_free(desc); > > > > } > > > > EXPORT_SYMBOL_GPL(gpiod_put); > > > > > > > > -- > > > > 2.20.1 > > > > > > > > > > In that case I don't think we need to keep checking desc for NULL in > > > gpiod_free(). > > > > I didn't remove the check there because it's called more than just from > > gpiod_put(). I didn't check all code paths myself. So, if it seems good for you > > to remove the check I can do it in next version. > > Any additional comments on this? I have looked briefly at this and found that > gpiod_free() is used to support legacy gpio_free() which is used in a plenty of > drivers. There are more than dozen are using non static number for these calls. > > I think the patch as is is the best approach we can do right now. > > -- > With Best Regards, > Andy Shevchenko > > Now applied. Bart
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 144af0733581..36445e24ee89 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -4616,7 +4616,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_optional); */ void gpiod_put(struct gpio_desc *desc) { - gpiod_free(desc); + if (desc) + gpiod_free(desc); } EXPORT_SYMBOL_GPL(gpiod_put);
In case of debug and optional GPIO requested, the gpiod_put() is not aware of and will WARN, which is not the case. Make gpiod_put() NULL-aware to keep silent for optional GPIOs. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpiolib.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)