[PATCH v2 00/15] gpiolib: indicate errors in value setters

[PATCH v2 00/15] gpiolib: indicate errors in value setters

[Bartosz Golaszewski]

The value setter callbacks (both for single and multiple lines) return
void even though we have many types of controllers that can fail to set
a line's value: i2c, SPI, USB, etc.

For the consumer API: single line setters return void. Multiple line
setters do have an integer return value but due to the above, they still
cannot be used to indicate problems within the driver.

This series proposes to start the process of converting the setters to
returning int thus making it possible to propagate any errors to the
user.

The first patch addresses an existing issue in one of the leds drivers
that will break after we change the GPIO consumer API. The second changes
the consumer interfaces. This has no impact on any user (even though they
don't currently check the retval) except for places where any of the
functions are passed as function pointer arguments in which case we need
to update the affected callers. I only identified one such place - the
gpio-latch module.

The third patch adds a wrapper around gpio_chip::set() to limit the
number of places where this pointer is dereferenced to one making
further work easier. Next we make the existing wrapper around
gpio_chip::set_multiple() consistent with the one for set() in terms of
the return value as well as SRCU and callback checks.

Finally in patch 5 we add new variants of the set callbacks suffixed
with "_rv" indicating that they return a value. We privilege them in the
code only falling back to the old ones if the new ones aren't present.

Patches that follow convert several drivers to using the new callbacks
to start the process.

My long term plan for this rework is the following:
1. Get this intitial series into the GPIO tree and next. Make sure it
   doesn't cause problems.
2. Start to convert drivers under drivers/gpio/ until the end of this
   cycle.
3. After v6.15-rc1 is tagged and the new callbacks are available
   upstream, start converting drivers outside of drivers/gpio/. For most
   part, this concerns drivers/pinctrl/ but we also have GPIO drivers
   scattered in media, sound, iio and old board files.
4. Once all GPIO chips are converted to using the new setters, remove
   the old callbacks and rename the new ones to the old name in one
   swift move across the tree (similarly to how the remove_new() was
   changed back to remove().

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
Changes in v2:
- add a leds patch that addresses an issue that would become visible
  after the consumer API change (reported by build bot after v1)
- check the return values of reg_mask_xlate() in gpio-regmap
- Link to v1: https://lore.kernel.org/r/20250211-gpio-set-retval-v1-0-52d3d613d7d3@linaro.org

---
Bartosz Golaszewski (15):
      leds: aw200xx: don't use return with gpiod_set_value() variants
      gpiolib: make value setters have return values
      gpiolib: wrap gpio_chip::set()
      gpiolib: rework the wrapper around gpio_chip::set_multiple()
      gpiolib: introduce gpio_chip setters that return values
      gpio: sim: use value returning setters
      gpio: regmap: use value returning setters
      gpio: pca953x: use value returning setters
      gpio: mockup: use value returning setters
      gpio: aggregator: use value returning setters
      gpio: max77650: use value returning setters
      gpio: latch: use lock guards
      gpio: latch: use value returning setters
      gpio: davinci: use value returning setters
      gpio: mvebu: use value returning setters

 drivers/gpio/gpio-aggregator.c |  38 +++++++-----
 drivers/gpio/gpio-davinci.c    |   6 +-
 drivers/gpio/gpio-latch.c      |  53 ++++++++--------
 drivers/gpio/gpio-max77650.c   |  14 ++---
 drivers/gpio/gpio-mockup.c     |  14 +++--
 drivers/gpio/gpio-mvebu.c      |   8 +--
 drivers/gpio/gpio-pca953x.c    |  17 +++---
 drivers/gpio/gpio-regmap.c     |  32 ++++++----
 drivers/gpio/gpio-sim.c        |  14 +++--
 drivers/gpio/gpiolib.c         | 133 +++++++++++++++++++++++++++++------------
 drivers/leds/leds-aw200xx.c    |   2 +-
 include/linux/gpio.h           |   4 +-
 include/linux/gpio/consumer.h  |  22 ++++---
 include/linux/gpio/driver.h    |  10 ++++
 14 files changed, 235 insertions(+), 132 deletions(-)
---
base-commit: 50a0c754714aa3ea0b0e62f3765eb666a1579f24
change-id: 20250210-gpio-set-retval-41cd6baeead3

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

Cc: Lee Jones <lee@kernel.org>
Cc: Pavel Machek <pavel@kernel.org>
Cc: linux-leds@vger.kernel.org
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: (subset) [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants

[Lee Jones]

On Thu, 20 Feb 2025 10:56:58 +0100, Bartosz Golaszewski wrote:
> While gpiod_set_value() currently returns void, it will soon be converted
> to return an integer instead. Don't do `return gpiod_set...`.
> 
> 

Applied, thanks!

[01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
        commit: 5d5e2a6f15a6c5e0c431c1388fd90e14b448da1e

--
Lee Jones [李琼斯]

Re: (subset) [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants

[Bartosz Golaszewski]

On Thu, Mar 6, 2025 at 11:51 PM Lee Jones <lee@kernel.org> wrote:
>
> On Thu, 20 Feb 2025 10:56:58 +0100, Bartosz Golaszewski wrote:
> > While gpiod_set_value() currently returns void, it will soon be converted
> > to return an integer instead. Don't do `return gpiod_set...`.
> >
> >
>
> Applied, thanks!
>
> [01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
>         commit: 5d5e2a6f15a6c5e0c431c1388fd90e14b448da1e
>

Hi Lee!

Can you please drop it from your tree? You acked v1 of this patch
after I had already sent v2 (this patch remained unchanged) folded
into a respin of the bigger GPIO series that had triggered build bots
to point this issue out in the first place. I picked the entire series
up and this commit is already in next as
129fdfe25ac513018d5fe85b0c493025193ef19f.

Thanks,
Bartosz

Re: (subset) [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants

[Lee Jones]

On Fri, 07 Mar 2025, Bartosz Golaszewski wrote:

> On Thu, Mar 6, 2025 at 11:51 PM Lee Jones <lee@kernel.org> wrote:
> >
> > On Thu, 20 Feb 2025 10:56:58 +0100, Bartosz Golaszewski wrote:
> > > While gpiod_set_value() currently returns void, it will soon be converted
> > > to return an integer instead. Don't do `return gpiod_set...`.
> > >
> > >
> >
> > Applied, thanks!
> >
> > [01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
> >         commit: 5d5e2a6f15a6c5e0c431c1388fd90e14b448da1e
> >
> 
> Hi Lee!
> 
> Can you please drop it from your tree? You acked v1 of this patch
> after I had already sent v2 (this patch remained unchanged) folded
> into a respin of the bigger GPIO series that had triggered build bots
> to point this issue out in the first place. I picked the entire series
> up and this commit is already in next as
> 129fdfe25ac513018d5fe85b0c493025193ef19f.

Unapplied, thanks.

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 00/15] gpiolib: indicate errors in value setters

[Uwe Kleine-König]

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

Hello Bartosz,

On Thu, Feb 20, 2025 at 10:56:57AM +0100, Bartosz Golaszewski wrote:
> The value setter callbacks (both for single and multiple lines) return
> void even though we have many types of controllers that can fail to set
> a line's value: i2c, SPI, USB, etc.
> 
> For the consumer API: single line setters return void. Multiple line
> setters do have an integer return value but due to the above, they still
> cannot be used to indicate problems within the driver.
> 
> This series proposes to start the process of converting the setters to
> returning int thus making it possible to propagate any errors to the
> user.

The lack of error indication is something that bothered me already a few
times in the past (but never to hard that I invested effort to change
that). Great that you work on that.

Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>

Best regards
Uwe

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

Re: [PATCH v2 00/15] gpiolib: indicate errors in value setters

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Thu, 20 Feb 2025 10:56:57 +0100, Bartosz Golaszewski wrote:
> The value setter callbacks (both for single and multiple lines) return
> void even though we have many types of controllers that can fail to set
> a line's value: i2c, SPI, USB, etc.
> 
> For the consumer API: single line setters return void. Multiple line
> setters do have an integer return value but due to the above, they still
> cannot be used to indicate problems within the driver.
> 
> [...]

Applied, thanks!

[01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
        commit: 129fdfe25ac513018d5fe85b0c493025193ef19f
[02/15] gpiolib: make value setters have return values
        commit: 8ce258f62f90cb2d339cc39fa43e5634594a9dfb
[03/15] gpiolib: wrap gpio_chip::set()
        commit: d36058b89a4aa30865d4cfeb101bbfd1d1dcb22f
[04/15] gpiolib: rework the wrapper around gpio_chip::set_multiple()
        commit: 9b407312755fd5db012413ca005f0f3a661db8dd
[05/15] gpiolib: introduce gpio_chip setters that return values
        commit: 98ce1eb1fd87ea1b016e0913ef6836ab0139b5c4
[06/15] gpio: sim: use value returning setters
        commit: fe69bedc77c119ffd4e27778eec03c89acb8e87b
[07/15] gpio: regmap: use value returning setters
        commit: a458d2309c81902dc6ca19b5037b9d25eb60a4a8
[08/15] gpio: pca953x: use value returning setters
        commit: e32ce8f62dd9c0ec923cfb9c783fc04070edb24e
[09/15] gpio: mockup: use value returning setters
        commit: 66d231b12eb8d39c21835a9bf553299a278ae363
[10/15] gpio: aggregator: use value returning setters
        commit: 468eae4166ab796cd2f9ad2bbb141d914e19c0b1
[11/15] gpio: max77650: use value returning setters
        commit: 97c9b59f6658671f3f13f57de1352ec9d16ad13d
[12/15] gpio: latch: use lock guards
        commit: 14628b692707fa8e61d0a068ef012156d23dc776
[13/15] gpio: latch: use value returning setters
        commit: 4b28762caa7b85609ee1a9a5e1038ae7bbd24892
[14/15] gpio: davinci: use value returning setters
        commit: f01436c2a038fe8d7b69a5fe701ab98028ce5cc4
[15/15] gpio: mvebu: use value returning setters
        commit: 9080b5d1b9c259645cd0e3694ffba85ccdd25352

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>