1. Patchew
  2. linux
  3. View series

[PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants

Bartosz Golaszewski posted 1 patch 10 months, 1 week ago 
drivers/leds/leds-aw200xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants
Posted by Bartosz Golaszewski 10 months, 1 week ago 
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

While it now returns void, it will soon be converted to return an
integer instead. Don't do `return gpiod_set...`.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202502121512.CmoMg9Q7-lkp@intel.com/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/leds/leds-aw200xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 08cca128458c..fe223d363a5d 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -379,7 +379,7 @@ static void aw200xx_enable(const struct aw200xx *const chip)
 
 static void aw200xx_disable(const struct aw200xx *const chip)
 {
-	return gpiod_set_value_cansleep(chip->hwen, 0);
+	gpiod_set_value_cansleep(chip->hwen, 0);
 }
 
 static int aw200xx_probe_get_display_rows(struct device *dev,
-- 
2.45.2
Re: [PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants
Posted by Lee Jones 10 months ago 
On Wed, 12 Feb 2025, Bartosz Golaszewski wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> While it now returns void, it will soon be converted to return an
> integer instead. Don't do `return gpiod_set...`.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202502121512.CmoMg9Q7-lkp@intel.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/leds/leds-aw200xx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Lee Jones <lee@kernel.org>

-- 
Lee Jones [李琼斯]
Re: [PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants
Posted by Bartosz Golaszewski 10 months, 1 week ago 
On Wed, Feb 12, 2025 at 9:59 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> While it now returns void, it will soon be converted to return an
> integer instead. Don't do `return gpiod_set...`.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202502121512.CmoMg9Q7-lkp@intel.com/
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/leds/leds-aw200xx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
> index 08cca128458c..fe223d363a5d 100644
> --- a/drivers/leds/leds-aw200xx.c
> +++ b/drivers/leds/leds-aw200xx.c
> @@ -379,7 +379,7 @@ static void aw200xx_enable(const struct aw200xx *const chip)
>
>  static void aw200xx_disable(const struct aw200xx *const chip)
>  {
> -       return gpiod_set_value_cansleep(chip->hwen, 0);
> +       gpiod_set_value_cansleep(chip->hwen, 0);
>  }
>
>  static int aw200xx_probe_get_display_rows(struct device *dev,
> --
> 2.45.2
>

Lee, Pavel:

If this is OK for you, can you please provide me with an immutable
branch so that I can pull it into the GPIO tree? It seems it's the
only such use-case in the tree apart from the gpio.h header that I can
fix locally. Alternatively you can just Ack this and let me take it
through the GPIO tree.

Bartosz
Re: [PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants
Posted by Lee Jones 10 months, 1 week ago 
On Wed, 12 Feb 2025, Bartosz Golaszewski wrote:

> On Wed, Feb 12, 2025 at 9:59 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > While it now returns void, it will soon be converted to return an
> > integer instead. Don't do `return gpiod_set...`.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202502121512.CmoMg9Q7-lkp@intel.com/
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  drivers/leds/leds-aw200xx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
> > index 08cca128458c..fe223d363a5d 100644
> > --- a/drivers/leds/leds-aw200xx.c
> > +++ b/drivers/leds/leds-aw200xx.c
> > @@ -379,7 +379,7 @@ static void aw200xx_enable(const struct aw200xx *const chip)
> >
> >  static void aw200xx_disable(const struct aw200xx *const chip)
> >  {
> > -       return gpiod_set_value_cansleep(chip->hwen, 0);
> > +       gpiod_set_value_cansleep(chip->hwen, 0);
> >  }
> >
> >  static int aw200xx_probe_get_display_rows(struct device *dev,
> > --
> > 2.45.2
> >
> 
> Lee, Pavel:
> 
> If this is OK for you, can you please provide me with an immutable
> branch so that I can pull it into the GPIO tree? It seems it's the
> only such use-case in the tree apart from the gpio.h header that I can
> fix locally. Alternatively you can just Ack this and let me take it
> through the GPIO tree.

I'm okay with it, but why do you need it?

-- 
Lee Jones [李琼斯]
Re: [PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants
Posted by Bartosz Golaszewski 10 months, 1 week ago 
On Wed, Feb 12, 2025 at 4:55 PM Lee Jones <lee@kernel.org> wrote:
>
> On Wed, 12 Feb 2025, Bartosz Golaszewski wrote:
>
> > On Wed, Feb 12, 2025 at 9:59 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > While it now returns void, it will soon be converted to return an
> > > integer instead. Don't do `return gpiod_set...`.
> > >
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202502121512.CmoMg9Q7-lkp@intel.com/
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > ---
> > >  drivers/leds/leds-aw200xx.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
> > > index 08cca128458c..fe223d363a5d 100644
> > > --- a/drivers/leds/leds-aw200xx.c
> > > +++ b/drivers/leds/leds-aw200xx.c
> > > @@ -379,7 +379,7 @@ static void aw200xx_enable(const struct aw200xx *const chip)
> > >
> > >  static void aw200xx_disable(const struct aw200xx *const chip)
> > >  {
> > > -       return gpiod_set_value_cansleep(chip->hwen, 0);
> > > +       gpiod_set_value_cansleep(chip->hwen, 0);
> > >  }
> > >
> > >  static int aw200xx_probe_get_display_rows(struct device *dev,
> > > --
> > > 2.45.2
> > >
> >
> > Lee, Pavel:
> >
> > If this is OK for you, can you please provide me with an immutable
> > branch so that I can pull it into the GPIO tree? It seems it's the
> > only such use-case in the tree apart from the gpio.h header that I can
> > fix locally. Alternatively you can just Ack this and let me take it
> > through the GPIO tree.
>
> I'm okay with it, but why do you need it?
>

For historical reasons gpiod_set_value() and its variants don't have a
return value. However, we now support all kinds of hardware that can
fail to set a line value: I2C, SPI, USB (hot-unpluggable chips), etc.
I want to rework the GPIO subsystem to make these functions return int
and become able to indicate failures. Build-bot complained about my
series[1] and pointed at this driver after the interface for
gpiod_set_value_cansleep() changed in patch 1. This is why I want to
fix it, get it into my tree and then pick up the series.

Sorry for not explaining it in detail earlier.

Bart

[1] https://lore.kernel.org/linux-gpio/20250211-gpio-set-retval-v1-0-52d3d613d7d3@linaro.org/
Re: [PATCH] leds: aw200xx: don't use return with gpiod_set_value() variants
Posted by Bartosz Golaszewski 10 months ago 
On Wed, Feb 12, 2025 at 5:39 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Wed, Feb 12, 2025 at 4:55 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Wed, 12 Feb 2025, Bartosz Golaszewski wrote:
> >
> > > On Wed, Feb 12, 2025 at 9:59 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > >
> > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > >
> > > > While it now returns void, it will soon be converted to return an
> > > > integer instead. Don't do `return gpiod_set...`.
> > > >
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202502121512.CmoMg9Q7-lkp@intel.com/
> > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > > > ---
> > > >  drivers/leds/leds-aw200xx.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
> > > > index 08cca128458c..fe223d363a5d 100644
> > > > --- a/drivers/leds/leds-aw200xx.c
> > > > +++ b/drivers/leds/leds-aw200xx.c
> > > > @@ -379,7 +379,7 @@ static void aw200xx_enable(const struct aw200xx *const chip)
> > > >
> > > >  static void aw200xx_disable(const struct aw200xx *const chip)
> > > >  {
> > > > -       return gpiod_set_value_cansleep(chip->hwen, 0);
> > > > +       gpiod_set_value_cansleep(chip->hwen, 0);
> > > >  }
> > > >
> > > >  static int aw200xx_probe_get_display_rows(struct device *dev,
> > > > --
> > > > 2.45.2
> > > >
> > >
> > > Lee, Pavel:
> > >
> > > If this is OK for you, can you please provide me with an immutable
> > > branch so that I can pull it into the GPIO tree? It seems it's the
> > > only such use-case in the tree apart from the gpio.h header that I can
> > > fix locally. Alternatively you can just Ack this and let me take it
> > > through the GPIO tree.
> >
> > I'm okay with it, but why do you need it?
> >
>
> For historical reasons gpiod_set_value() and its variants don't have a
> return value. However, we now support all kinds of hardware that can
> fail to set a line value: I2C, SPI, USB (hot-unpluggable chips), etc.
> I want to rework the GPIO subsystem to make these functions return int
> and become able to indicate failures. Build-bot complained about my
> series[1] and pointed at this driver after the interface for
> gpiod_set_value_cansleep() changed in patch 1. This is why I want to
> fix it, get it into my tree and then pick up the series.
>
> Sorry for not explaining it in detail earlier.
>
> Bart
>
> [1] https://lore.kernel.org/linux-gpio/20250211-gpio-set-retval-v1-0-52d3d613d7d3@linaro.org/

Is it fine for you if I take it through the GPIO tree? Could you
please leave your Ack under the patch if so?

Thanks in advance,
Bartosz

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.