* [PATCH v2 00/15] gpiolib: indicate errors in value setters
@ 2025-02-20 9:56 Bartosz Golaszewski
2025-02-20 9:56 ` [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants Bartosz Golaszewski
` (16 more replies)
0 siblings, 17 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:56 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski,
Lee Jones, Pavel Machek, linux-leds, kernel test robot
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>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
@ 2025-02-20 9:56 ` Bartosz Golaszewski
2025-03-06 22:51 ` (subset) " Lee Jones
2025-02-20 9:56 ` [PATCH v2 02/15] gpiolib: make value setters have return values Bartosz Golaszewski
` (15 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:56 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski,
Lee Jones, Pavel Machek, linux-leds, kernel test robot
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
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 02/15] gpiolib: make value setters have return values
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
2025-02-20 9:56 ` [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants Bartosz Golaszewski
@ 2025-02-20 9:56 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 03/15] gpiolib: wrap gpio_chip::set() Bartosz Golaszewski
` (14 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:56 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Change the in-kernel consumer interface for GPIOs: make all variants of
value setters that don't have a return value, return a signed integer
instead. That will allow these routines to indicate failures to callers.
This doesn't change the implementation just yet, we'll do it in
subsequent commits.
We need to update the gpio-latch module as it passes the address of
value setters as a function pointer argument and thus cares about its
type.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-latch.c | 2 +-
drivers/gpio/gpiolib.c | 53 ++++++++++++++++++++++++-------------------
include/linux/gpio.h | 4 ++--
include/linux/gpio/consumer.h | 22 ++++++++++--------
4 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/drivers/gpio/gpio-latch.c b/drivers/gpio/gpio-latch.c
index 46cdfb08747a..64174ea7d008 100644
--- a/drivers/gpio/gpio-latch.c
+++ b/drivers/gpio/gpio-latch.c
@@ -73,7 +73,7 @@ static int gpio_latch_get_direction(struct gpio_chip *gc, unsigned int offset)
}
static void gpio_latch_set_unlocked(struct gpio_latch_priv *priv,
- void (*set)(struct gpio_desc *desc, int value),
+ int (*set)(struct gpio_desc *desc, int value),
unsigned int offset, bool val)
{
int latch = offset / priv->n_latched_gpios;
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 704452fd94bb..0a47fb38dd61 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3405,13 +3405,13 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_value);
* @desc: gpio descriptor whose state need to be set.
* @value: Non-zero for setting it HIGH otherwise it will set to LOW.
*/
-static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
+static int gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
{
int ret = 0, offset = gpio_chip_hwgpio(desc);
CLASS(gpio_chip_guard, guard)(desc);
if (!guard.gc)
- return;
+ return -ENODEV;
if (value) {
ret = guard.gc->direction_input(guard.gc, offset);
@@ -3425,6 +3425,8 @@ static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
gpiod_err(desc,
"%s: Error in set_value for open drain err %d\n",
__func__, ret);
+
+ return ret;
}
/*
@@ -3432,13 +3434,13 @@ static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
* @desc: gpio descriptor whose state need to be set.
* @value: Non-zero for setting it HIGH otherwise it will set to LOW.
*/
-static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
+static int gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
{
int ret = 0, offset = gpio_chip_hwgpio(desc);
CLASS(gpio_chip_guard, guard)(desc);
if (!guard.gc)
- return;
+ return -ENODEV;
if (value) {
ret = guard.gc->direction_output(guard.gc, offset, 1);
@@ -3452,16 +3454,20 @@ static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value
gpiod_err(desc,
"%s: Error in set_value for open source err %d\n",
__func__, ret);
+
+ return ret;
}
-static void gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
+static int gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
{
CLASS(gpio_chip_guard, guard)(desc);
if (!guard.gc)
- return;
+ return -ENODEV;
trace_gpio_value(desc_to_gpio(desc), 0, value);
guard.gc->set(guard.gc, gpio_chip_hwgpio(desc), value);
+
+ return 0;
}
/*
@@ -3619,12 +3625,12 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
* This function can be called from contexts where we cannot sleep, and will
* complain if the GPIO chip functions potentially sleep.
*/
-void gpiod_set_raw_value(struct gpio_desc *desc, int value)
+int gpiod_set_raw_value(struct gpio_desc *desc, int value)
{
- VALIDATE_DESC_VOID(desc);
+ VALIDATE_DESC(desc);
/* Should be using gpiod_set_raw_value_cansleep() */
WARN_ON(desc->gdev->can_sleep);
- gpiod_set_raw_value_commit(desc, value);
+ return gpiod_set_raw_value_commit(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
@@ -3637,16 +3643,17 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_value);
* different semantic quirks like active low and open drain/source
* handling.
*/
-static void gpiod_set_value_nocheck(struct gpio_desc *desc, int value)
+static int gpiod_set_value_nocheck(struct gpio_desc *desc, int value)
{
if (test_bit(FLAG_ACTIVE_LOW, &desc->flags))
value = !value;
+
if (test_bit(FLAG_OPEN_DRAIN, &desc->flags))
- gpio_set_open_drain_value_commit(desc, value);
+ return gpio_set_open_drain_value_commit(desc, value);
else if (test_bit(FLAG_OPEN_SOURCE, &desc->flags))
- gpio_set_open_source_value_commit(desc, value);
- else
- gpiod_set_raw_value_commit(desc, value);
+ return gpio_set_open_source_value_commit(desc, value);
+
+ return gpiod_set_raw_value_commit(desc, value);
}
/**
@@ -3660,12 +3667,12 @@ static void gpiod_set_value_nocheck(struct gpio_desc *desc, int value)
* This function can be called from contexts where we cannot sleep, and will
* complain if the GPIO chip functions potentially sleep.
*/
-void gpiod_set_value(struct gpio_desc *desc, int value)
+int gpiod_set_value(struct gpio_desc *desc, int value)
{
- VALIDATE_DESC_VOID(desc);
+ VALIDATE_DESC(desc);
/* Should be using gpiod_set_value_cansleep() */
WARN_ON(desc->gdev->can_sleep);
- gpiod_set_value_nocheck(desc, value);
+ return gpiod_set_value_nocheck(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_value);
@@ -4084,11 +4091,11 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_value_cansleep);
*
* This function is to be called from contexts that can sleep.
*/
-void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value)
+int gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value)
{
might_sleep();
- VALIDATE_DESC_VOID(desc);
- gpiod_set_raw_value_commit(desc, value);
+ VALIDATE_DESC(desc);
+ return gpiod_set_raw_value_commit(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_raw_value_cansleep);
@@ -4102,11 +4109,11 @@ EXPORT_SYMBOL_GPL(gpiod_set_raw_value_cansleep);
*
* This function is to be called from contexts that can sleep.
*/
-void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
+int gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
{
might_sleep();
- VALIDATE_DESC_VOID(desc);
- gpiod_set_value_nocheck(desc, value);
+ VALIDATE_DESC(desc);
+ return gpiod_set_value_nocheck(desc, value);
}
EXPORT_SYMBOL_GPL(gpiod_set_value_cansleep);
diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 6270150f4e29..c1ec62c11ed3 100644
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -91,7 +91,7 @@ static inline int gpio_get_value_cansleep(unsigned gpio)
}
static inline void gpio_set_value_cansleep(unsigned gpio, int value)
{
- return gpiod_set_raw_value_cansleep(gpio_to_desc(gpio), value);
+ gpiod_set_raw_value_cansleep(gpio_to_desc(gpio), value);
}
static inline int gpio_get_value(unsigned gpio)
@@ -100,7 +100,7 @@ static inline int gpio_get_value(unsigned gpio)
}
static inline void gpio_set_value(unsigned gpio, int value)
{
- return gpiod_set_raw_value(gpio_to_desc(gpio), value);
+ gpiod_set_raw_value(gpio_to_desc(gpio), value);
}
static inline int gpio_to_irq(unsigned gpio)
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index 0b2b56199c36..51ab6728a998 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -121,7 +121,7 @@ int gpiod_get_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
unsigned long *value_bitmap);
-void gpiod_set_value(struct gpio_desc *desc, int value);
+int gpiod_set_value(struct gpio_desc *desc, int value);
int gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
@@ -131,7 +131,7 @@ int gpiod_get_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
unsigned long *value_bitmap);
-void gpiod_set_raw_value(struct gpio_desc *desc, int value);
+int gpiod_set_raw_value(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
@@ -143,7 +143,7 @@ int gpiod_get_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
unsigned long *value_bitmap);
-void gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
+int gpiod_set_value_cansleep(struct gpio_desc *desc, int value);
int gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
@@ -153,7 +153,7 @@ int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
unsigned long *value_bitmap);
-void gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
+int gpiod_set_raw_value_cansleep(struct gpio_desc *desc, int value);
int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
struct gpio_array *array_info,
@@ -360,10 +360,11 @@ static inline int gpiod_get_array_value(unsigned int array_size,
WARN_ON(desc_array);
return 0;
}
-static inline void gpiod_set_value(struct gpio_desc *desc, int value)
+static inline int gpiod_set_value(struct gpio_desc *desc, int value)
{
/* GPIO can never have been requested */
WARN_ON(desc);
+ return 0;
}
static inline int gpiod_set_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
@@ -389,10 +390,11 @@ static inline int gpiod_get_raw_array_value(unsigned int array_size,
WARN_ON(desc_array);
return 0;
}
-static inline void gpiod_set_raw_value(struct gpio_desc *desc, int value)
+static inline int gpiod_set_raw_value(struct gpio_desc *desc, int value)
{
/* GPIO can never have been requested */
WARN_ON(desc);
+ return 0;
}
static inline int gpiod_set_raw_array_value(unsigned int array_size,
struct gpio_desc **desc_array,
@@ -419,10 +421,11 @@ static inline int gpiod_get_array_value_cansleep(unsigned int array_size,
WARN_ON(desc_array);
return 0;
}
-static inline void gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
+static inline int gpiod_set_value_cansleep(struct gpio_desc *desc, int value)
{
/* GPIO can never have been requested */
WARN_ON(desc);
+ return 0;
}
static inline int gpiod_set_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
@@ -448,11 +451,12 @@ static inline int gpiod_get_raw_array_value_cansleep(unsigned int array_size,
WARN_ON(desc_array);
return 0;
}
-static inline void gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
- int value)
+static inline int gpiod_set_raw_value_cansleep(struct gpio_desc *desc,
+ int value)
{
/* GPIO can never have been requested */
WARN_ON(desc);
+ return 0;
}
static inline int gpiod_set_raw_array_value_cansleep(unsigned int array_size,
struct gpio_desc **desc_array,
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 03/15] gpiolib: wrap gpio_chip::set()
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
2025-02-20 9:56 ` [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants Bartosz Golaszewski
2025-02-20 9:56 ` [PATCH v2 02/15] gpiolib: make value setters have return values Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 04/15] gpiolib: rework the wrapper around gpio_chip::set_multiple() Bartosz Golaszewski
` (13 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We have three places where we dereference the gpio_chip::set() callback.
In order to make it easier to incorporate the upcoming new variant of
this callback (one returning an integer value), wrap it in a helper so
that the dereferencing only happens once.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0a47fb38dd61..040b4689eb8e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2755,6 +2755,17 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
return ret;
}
+static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
+{
+ lockdep_assert_held(&gc->gpiodev->srcu);
+
+ if (WARN_ON(unlikely(!gc->set)))
+ return -EOPNOTSUPP;
+
+ gc->set(gc, offset, value);
+ return 0;
+}
+
static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
{
int val = !!value, ret = 0;
@@ -2797,7 +2808,9 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
* If we can't actively set the direction, we are some
* output-only chip, so just drive the output as desired.
*/
- guard.gc->set(guard.gc, gpio_chip_hwgpio(desc), val);
+ ret = gpiochip_set(guard.gc, gpio_chip_hwgpio(desc), val);
+ if (ret)
+ return ret;
}
if (!ret)
@@ -3465,9 +3478,7 @@ static int gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
return -ENODEV;
trace_gpio_value(desc_to_gpio(desc), 0, value);
- guard.gc->set(guard.gc, gpio_chip_hwgpio(desc), value);
-
- return 0;
+ return gpiochip_set(guard.gc, gpio_chip_hwgpio(desc), value);
}
/*
@@ -3492,7 +3503,7 @@ static void gpio_chip_set_multiple(struct gpio_chip *gc,
/* set outputs if the corresponding mask bit is set */
for_each_set_bit(i, mask, gc->ngpio)
- gc->set(gc, i, test_bit(i, bits));
+ gpiochip_set(gc, i, test_bit(i, bits));
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 04/15] gpiolib: rework the wrapper around gpio_chip::set_multiple()
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (2 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 03/15] gpiolib: wrap gpio_chip::set() Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 05/15] gpiolib: introduce gpio_chip setters that return values Bartosz Golaszewski
` (12 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Make the existing wrapper around gpio_chip::set_multiple() consistent
with the one for gpio_chip::set(): make it return int, add a lockdep
assertion, warn on missing set callback and move the code a bit for
better readability.
Add return value checks in all call places.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib.c | 42 ++++++++++++++++++++++++++++++------------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 040b4689eb8e..b1e7d368bc7d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -3490,21 +3490,33 @@ static int gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
* defines which outputs are to be changed
* @bits: bit value array; one bit per output; BITS_PER_LONG bits per word
* defines the values the outputs specified by mask are to be set to
+ *
+ * Returns: 0 on success, negative error number on failure.
*/
-static void gpio_chip_set_multiple(struct gpio_chip *gc,
- unsigned long *mask, unsigned long *bits)
+static int gpiochip_set_multiple(struct gpio_chip *gc,
+ unsigned long *mask, unsigned long *bits)
{
+ unsigned int i;
+ int ret;
+
lockdep_assert_held(&gc->gpiodev->srcu);
+ if (WARN_ON(unlikely(!gc->set_multiple && !gc->set)))
+ return -EOPNOTSUPP;
+
if (gc->set_multiple) {
gc->set_multiple(gc, mask, bits);
- } else {
- unsigned int i;
-
- /* set outputs if the corresponding mask bit is set */
- for_each_set_bit(i, mask, gc->ngpio)
- gpiochip_set(gc, i, test_bit(i, bits));
+ return 0;
}
+
+ /* set outputs if the corresponding mask bit is set */
+ for_each_set_bit(i, mask, gc->ngpio) {
+ ret = gpiochip_set(gc, i, test_bit(i, bits));
+ if (ret)
+ break;
+ }
+
+ return ret;
}
int gpiod_set_array_value_complex(bool raw, bool can_sleep,
@@ -3514,7 +3526,7 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
unsigned long *value_bitmap)
{
struct gpio_chip *gc;
- int i = 0;
+ int i = 0, ret;
/*
* Validate array_info against desc_array and its size.
@@ -3537,7 +3549,10 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
bitmap_xor(value_bitmap, value_bitmap,
array_info->invert_mask, array_size);
- gpio_chip_set_multiple(gc, array_info->set_mask, value_bitmap);
+ ret = gpiochip_set_multiple(gc, array_info->set_mask,
+ value_bitmap);
+ if (ret)
+ return ret;
i = find_first_zero_bit(array_info->set_mask, array_size);
if (i == array_size)
@@ -3614,8 +3629,11 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
} while ((i < array_size) &&
gpio_device_chip_cmp(desc_array[i]->gdev, guard.gc));
/* push collected bits to outputs */
- if (count != 0)
- gpio_chip_set_multiple(guard.gc, mask, bits);
+ if (count != 0) {
+ ret = gpiochip_set_multiple(guard.gc, mask, bits);
+ if (ret)
+ return ret;
+ }
if (mask != fastpath_mask)
bitmap_free(mask);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 05/15] gpiolib: introduce gpio_chip setters that return values
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (3 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 04/15] gpiolib: rework the wrapper around gpio_chip::set_multiple() Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
[not found] ` <CGME20250227140054eucas1p2bf6e3f2416e11e3c62a704682bf052bf@eucas1p2.samsung.com>
2025-02-20 9:57 ` [PATCH v2 06/15] gpio: sim: use value returning setters Bartosz Golaszewski
` (11 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add new variants of the set() and set_multiple() callbacks that have
integer return values allowing to indicate failures to users of the GPIO
consumer API. Until we convert all GPIO providers treewide to using
them, they will live in parallel to the existing ones.
Make sure that providers cannot define both. Prefer the new ones and
only use the old ones as fallback.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpiolib.c | 27 +++++++++++++++++++++++++--
include/linux/gpio/driver.h | 10 ++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b1e7d368bc7d..19f78ffcc3c1 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -926,6 +926,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
int base = 0;
int ret = 0;
+ /* Only allow one set() and one set_multiple(). */
+ if ((gc->set && gc->set_rv) ||
+ (gc->set_multiple && gc->set_multiple_rv))
+ return -EINVAL;
+
/*
* First: allocate and populate the internal stat container, and
* set up the struct device.
@@ -2757,11 +2762,21 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
{
+ int ret;
+
lockdep_assert_held(&gc->gpiodev->srcu);
- if (WARN_ON(unlikely(!gc->set)))
+ if (WARN_ON(unlikely(!gc->set && !gc->set_rv)))
return -EOPNOTSUPP;
+ if (gc->set_rv) {
+ ret = gc->set_rv(gc, offset, value);
+ if (ret > 0)
+ ret = -EBADE;
+
+ return ret;
+ }
+
gc->set(gc, offset, value);
return 0;
}
@@ -3501,9 +3516,17 @@ static int gpiochip_set_multiple(struct gpio_chip *gc,
lockdep_assert_held(&gc->gpiodev->srcu);
- if (WARN_ON(unlikely(!gc->set_multiple && !gc->set)))
+ if (WARN_ON(unlikely(!gc->set_multiple && !gc->set_multiple_rv)))
return -EOPNOTSUPP;
+ if (gc->set_multiple_rv) {
+ ret = gc->set_multiple_rv(gc, mask, bits);
+ if (ret > 0)
+ ret = -EBADE;
+
+ return ret;
+ }
+
if (gc->set_multiple) {
gc->set_multiple(gc, mask, bits);
return 0;
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 10544f4a03e5..b2627c8ed513 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -347,6 +347,10 @@ struct gpio_irq_chip {
* stores them in "bits", returns 0 on success or negative error
* @set: assigns output value for signal "offset"
* @set_multiple: assigns output values for multiple signals defined by "mask"
+ * @set_rv: assigns output value for signal "offset", returns 0 on success or
+ * negative error value
+ * @set_multiple_rv: assigns output values for multiple signals defined by
+ * "mask", returns 0 on success or negative error value
* @set_config: optional hook for all kinds of settings. Uses the same
* packed config format as generic pinconf.
* @to_irq: optional hook supporting non-static gpiod_to_irq() mappings;
@@ -442,6 +446,12 @@ struct gpio_chip {
void (*set_multiple)(struct gpio_chip *gc,
unsigned long *mask,
unsigned long *bits);
+ int (*set_rv)(struct gpio_chip *gc,
+ unsigned int offset,
+ int value);
+ int (*set_multiple_rv)(struct gpio_chip *gc,
+ unsigned long *mask,
+ unsigned long *bits);
int (*set_config)(struct gpio_chip *gc,
unsigned int offset,
unsigned long config);
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 06/15] gpio: sim: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (4 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 05/15] gpiolib: introduce gpio_chip setters that return values Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 07/15] gpio: regmap: " Bartosz Golaszewski
` (10 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-sim.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c
index b6c230fab840..b3baa7e872bd 100644
--- a/drivers/gpio/gpio-sim.c
+++ b/drivers/gpio/gpio-sim.c
@@ -119,12 +119,14 @@ static int gpio_sim_get(struct gpio_chip *gc, unsigned int offset)
return !!test_bit(offset, chip->value_map);
}
-static void gpio_sim_set(struct gpio_chip *gc, unsigned int offset, int value)
+static int gpio_sim_set(struct gpio_chip *gc, unsigned int offset, int value)
{
struct gpio_sim_chip *chip = gpiochip_get_data(gc);
scoped_guard(mutex, &chip->lock)
__assign_bit(offset, chip->value_map, value);
+
+ return 0;
}
static int gpio_sim_get_multiple(struct gpio_chip *gc,
@@ -138,14 +140,16 @@ static int gpio_sim_get_multiple(struct gpio_chip *gc,
return 0;
}
-static void gpio_sim_set_multiple(struct gpio_chip *gc,
- unsigned long *mask, unsigned long *bits)
+static int gpio_sim_set_multiple(struct gpio_chip *gc,
+ unsigned long *mask, unsigned long *bits)
{
struct gpio_sim_chip *chip = gpiochip_get_data(gc);
scoped_guard(mutex, &chip->lock)
bitmap_replace(chip->value_map, chip->value_map, bits, mask,
gc->ngpio);
+
+ return 0;
}
static int gpio_sim_direction_output(struct gpio_chip *gc,
@@ -481,9 +485,9 @@ static int gpio_sim_add_bank(struct fwnode_handle *swnode, struct device *dev)
gc->parent = dev;
gc->fwnode = swnode;
gc->get = gpio_sim_get;
- gc->set = gpio_sim_set;
+ gc->set_rv = gpio_sim_set;
gc->get_multiple = gpio_sim_get_multiple;
- gc->set_multiple = gpio_sim_set_multiple;
+ gc->set_multiple_rv = gpio_sim_set_multiple;
gc->direction_output = gpio_sim_direction_output;
gc->direction_input = gpio_sim_direction_input;
gc->get_direction = gpio_sim_get_direction;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 07/15] gpio: regmap: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (5 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 06/15] gpio: sim: use value returning setters Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 10:08 ` Michael Walle
2025-02-20 9:57 ` [PATCH v2 08/15] gpio: pca953x: " Bartosz Golaszewski
` (9 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-regmap.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 05f8781b5204..10d043596247 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -81,33 +81,43 @@ static int gpio_regmap_get(struct gpio_chip *chip, unsigned int offset)
return !!(val & mask);
}
-static void gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
- int val)
+static int gpio_regmap_set(struct gpio_chip *chip, unsigned int offset,
+ int val)
{
struct gpio_regmap *gpio = gpiochip_get_data(chip);
unsigned int base = gpio_regmap_addr(gpio->reg_set_base);
unsigned int reg, mask;
+ int ret;
+
+ ret = gpio->reg_mask_xlate(gpio, base, offset, ®, &mask);
+ if (ret)
+ return ret;
- gpio->reg_mask_xlate(gpio, base, offset, ®, &mask);
if (val)
- regmap_update_bits(gpio->regmap, reg, mask, mask);
+ ret = regmap_update_bits(gpio->regmap, reg, mask, mask);
else
- regmap_update_bits(gpio->regmap, reg, mask, 0);
+ ret = regmap_update_bits(gpio->regmap, reg, mask, 0);
+
+ return ret;
}
-static void gpio_regmap_set_with_clear(struct gpio_chip *chip,
- unsigned int offset, int val)
+static int gpio_regmap_set_with_clear(struct gpio_chip *chip,
+ unsigned int offset, int val)
{
struct gpio_regmap *gpio = gpiochip_get_data(chip);
unsigned int base, reg, mask;
+ int ret;
if (val)
base = gpio_regmap_addr(gpio->reg_set_base);
else
base = gpio_regmap_addr(gpio->reg_clr_base);
- gpio->reg_mask_xlate(gpio, base, offset, ®, &mask);
- regmap_write(gpio->regmap, reg, mask);
+ ret = gpio->reg_mask_xlate(gpio, base, offset, ®, &mask);
+ if (ret)
+ return ret;
+
+ return regmap_write(gpio->regmap, reg, mask);
}
static int gpio_regmap_get_direction(struct gpio_chip *chip,
@@ -266,9 +276,9 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
chip->free = gpiochip_generic_free;
chip->get = gpio_regmap_get;
if (gpio->reg_set_base && gpio->reg_clr_base)
- chip->set = gpio_regmap_set_with_clear;
+ chip->set_rv = gpio_regmap_set_with_clear;
else if (gpio->reg_set_base)
- chip->set = gpio_regmap_set;
+ chip->set_rv = gpio_regmap_set;
chip->get_direction = gpio_regmap_get_direction;
if (gpio->reg_dir_in_base || gpio->reg_dir_out_base) {
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 08/15] gpio: pca953x: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (6 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 07/15] gpio: regmap: " Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 09/15] gpio: mockup: " Bartosz Golaszewski
` (8 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-pca953x.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d63c1030e6ac..442435ded020 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -570,7 +570,8 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
return !!(reg_val & bit);
}
-static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
+static int pca953x_gpio_set_value(struct gpio_chip *gc, unsigned int off,
+ int val)
{
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 outreg = chip->recalc_addr(chip, chip->regs->output, off);
@@ -578,7 +579,7 @@ static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
guard(mutex)(&chip->i2c_lock);
- regmap_write_bits(chip->regmap, outreg, bit, val ? bit : 0);
+ return regmap_write_bits(chip->regmap, outreg, bit, val ? bit : 0);
}
static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off)
@@ -616,8 +617,8 @@ static int pca953x_gpio_get_multiple(struct gpio_chip *gc,
return 0;
}
-static void pca953x_gpio_set_multiple(struct gpio_chip *gc,
- unsigned long *mask, unsigned long *bits)
+static int pca953x_gpio_set_multiple(struct gpio_chip *gc,
+ unsigned long *mask, unsigned long *bits)
{
struct pca953x_chip *chip = gpiochip_get_data(gc);
DECLARE_BITMAP(reg_val, MAX_LINE);
@@ -627,11 +628,11 @@ static void pca953x_gpio_set_multiple(struct gpio_chip *gc,
ret = pca953x_read_regs(chip, chip->regs->output, reg_val);
if (ret)
- return;
+ return ret;
bitmap_replace(reg_val, reg_val, bits, mask, gc->ngpio);
- pca953x_write_regs(chip, chip->regs->output, reg_val);
+ return pca953x_write_regs(chip, chip->regs->output, reg_val);
}
static int pca953x_gpio_set_pull_up_down(struct pca953x_chip *chip,
@@ -693,10 +694,10 @@ static void pca953x_setup_gpio(struct pca953x_chip *chip, int gpios)
gc->direction_input = pca953x_gpio_direction_input;
gc->direction_output = pca953x_gpio_direction_output;
gc->get = pca953x_gpio_get_value;
- gc->set = pca953x_gpio_set_value;
+ gc->set_rv = pca953x_gpio_set_value;
gc->get_direction = pca953x_gpio_get_direction;
gc->get_multiple = pca953x_gpio_get_multiple;
- gc->set_multiple = pca953x_gpio_set_multiple;
+ gc->set_multiple_rv = pca953x_gpio_set_multiple;
gc->set_config = pca953x_gpio_set_config;
gc->can_sleep = true;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 09/15] gpio: mockup: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (7 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 08/15] gpio: pca953x: " Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 10/15] gpio: aggregator: " Bartosz Golaszewski
` (7 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-mockup.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index d39c6618bade..266c0953d914 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -122,7 +122,7 @@ static void __gpio_mockup_set(struct gpio_mockup_chip *chip,
chip->lines[offset].value = !!value;
}
-static void gpio_mockup_set(struct gpio_chip *gc,
+static int gpio_mockup_set(struct gpio_chip *gc,
unsigned int offset, int value)
{
struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
@@ -130,10 +130,12 @@ static void gpio_mockup_set(struct gpio_chip *gc,
guard(mutex)(&chip->lock);
__gpio_mockup_set(chip, offset, value);
+
+ return 0;
}
-static void gpio_mockup_set_multiple(struct gpio_chip *gc,
- unsigned long *mask, unsigned long *bits)
+static int gpio_mockup_set_multiple(struct gpio_chip *gc,
+ unsigned long *mask, unsigned long *bits)
{
struct gpio_mockup_chip *chip = gpiochip_get_data(gc);
unsigned int bit;
@@ -142,6 +144,8 @@ static void gpio_mockup_set_multiple(struct gpio_chip *gc,
for_each_set_bit(bit, mask, gc->ngpio)
__gpio_mockup_set(chip, bit, test_bit(bit, bits));
+
+ return 0;
}
static int gpio_mockup_apply_pull(struct gpio_mockup_chip *chip,
@@ -445,9 +449,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
gc->owner = THIS_MODULE;
gc->parent = dev;
gc->get = gpio_mockup_get;
- gc->set = gpio_mockup_set;
+ gc->set_rv = gpio_mockup_set;
gc->get_multiple = gpio_mockup_get_multiple;
- gc->set_multiple = gpio_mockup_set_multiple;
+ gc->set_multiple_rv = gpio_mockup_set_multiple;
gc->direction_output = gpio_mockup_dirout;
gc->direction_input = gpio_mockup_dirin;
gc->get_direction = gpio_mockup_get_direction;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 10/15] gpio: aggregator: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (8 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 09/15] gpio: mockup: " Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 11/15] gpio: max77650: " Bartosz Golaszewski
` (6 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-aggregator.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
index 65f41cc3eafc..e799599a06a1 100644
--- a/drivers/gpio/gpio-aggregator.c
+++ b/drivers/gpio/gpio-aggregator.c
@@ -358,25 +358,30 @@ static void gpio_fwd_delay(struct gpio_chip *chip, unsigned int offset, int valu
udelay(delay_us);
}
-static void gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value)
+static int gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value)
{
struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
+ int ret;
if (chip->can_sleep)
- gpiod_set_value_cansleep(fwd->descs[offset], value);
+ ret = gpiod_set_value_cansleep(fwd->descs[offset], value);
else
- gpiod_set_value(fwd->descs[offset], value);
+ ret = gpiod_set_value(fwd->descs[offset], value);
+ if (ret)
+ return ret;
if (fwd->delay_timings)
gpio_fwd_delay(chip, offset, value);
+
+ return ret;
}
-static void gpio_fwd_set_multiple(struct gpiochip_fwd *fwd, unsigned long *mask,
- unsigned long *bits)
+static int gpio_fwd_set_multiple(struct gpiochip_fwd *fwd, unsigned long *mask,
+ unsigned long *bits)
{
struct gpio_desc **descs = fwd_tmp_descs(fwd);
unsigned long *values = fwd_tmp_values(fwd);
- unsigned int i, j = 0;
+ unsigned int i, j = 0, ret;
for_each_set_bit(i, mask, fwd->chip.ngpio) {
__assign_bit(j, values, test_bit(i, bits));
@@ -384,26 +389,31 @@ static void gpio_fwd_set_multiple(struct gpiochip_fwd *fwd, unsigned long *mask,
}
if (fwd->chip.can_sleep)
- gpiod_set_array_value_cansleep(j, descs, NULL, values);
+ ret = gpiod_set_array_value_cansleep(j, descs, NULL, values);
else
- gpiod_set_array_value(j, descs, NULL, values);
+ ret = gpiod_set_array_value(j, descs, NULL, values);
+
+ return ret;
}
-static void gpio_fwd_set_multiple_locked(struct gpio_chip *chip,
- unsigned long *mask, unsigned long *bits)
+static int gpio_fwd_set_multiple_locked(struct gpio_chip *chip,
+ unsigned long *mask, unsigned long *bits)
{
struct gpiochip_fwd *fwd = gpiochip_get_data(chip);
unsigned long flags;
+ int ret;
if (chip->can_sleep) {
mutex_lock(&fwd->mlock);
- gpio_fwd_set_multiple(fwd, mask, bits);
+ ret = gpio_fwd_set_multiple(fwd, mask, bits);
mutex_unlock(&fwd->mlock);
} else {
spin_lock_irqsave(&fwd->slock, flags);
- gpio_fwd_set_multiple(fwd, mask, bits);
+ ret = gpio_fwd_set_multiple(fwd, mask, bits);
spin_unlock_irqrestore(&fwd->slock, flags);
}
+
+ return ret;
}
static int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset,
@@ -533,8 +543,8 @@ static struct gpiochip_fwd *gpiochip_fwd_create(struct device *dev,
chip->direction_output = gpio_fwd_direction_output;
chip->get = gpio_fwd_get;
chip->get_multiple = gpio_fwd_get_multiple_locked;
- chip->set = gpio_fwd_set;
- chip->set_multiple = gpio_fwd_set_multiple_locked;
+ chip->set_rv = gpio_fwd_set;
+ chip->set_multiple_rv = gpio_fwd_set_multiple_locked;
chip->to_irq = gpio_fwd_to_irq;
chip->base = -1;
chip->ngpio = ngpios;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 11/15] gpio: max77650: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (9 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 10/15] gpio: aggregator: " Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 12/15] gpio: latch: use lock guards Bartosz Golaszewski
` (5 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-max77650.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/gpio-max77650.c b/drivers/gpio/gpio-max77650.c
index 3075f2513c6f..a553e141059f 100644
--- a/drivers/gpio/gpio-max77650.c
+++ b/drivers/gpio/gpio-max77650.c
@@ -62,18 +62,16 @@ static int max77650_gpio_direction_output(struct gpio_chip *gc,
MAX77650_REG_CNFG_GPIO, mask, regval);
}
-static void max77650_gpio_set_value(struct gpio_chip *gc,
- unsigned int offset, int value)
+static int max77650_gpio_set_value(struct gpio_chip *gc,
+ unsigned int offset, int value)
{
struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
- int rv, regval;
+ int regval;
regval = value ? MAX77650_GPIO_OUT_HIGH : MAX77650_GPIO_OUT_LOW;
- rv = regmap_update_bits(chip->map, MAX77650_REG_CNFG_GPIO,
- MAX77650_GPIO_OUTVAL_MASK, regval);
- if (rv)
- dev_err(gc->parent, "cannot set GPIO value: %d\n", rv);
+ return regmap_update_bits(chip->map, MAX77650_REG_CNFG_GPIO,
+ MAX77650_GPIO_OUTVAL_MASK, regval);
}
static int max77650_gpio_get_value(struct gpio_chip *gc,
@@ -168,7 +166,7 @@ static int max77650_gpio_probe(struct platform_device *pdev)
chip->gc.direction_input = max77650_gpio_direction_input;
chip->gc.direction_output = max77650_gpio_direction_output;
- chip->gc.set = max77650_gpio_set_value;
+ chip->gc.set_rv = max77650_gpio_set_value;
chip->gc.get = max77650_gpio_get_value;
chip->gc.get_direction = max77650_gpio_get_direction;
chip->gc.set_config = max77650_gpio_set_config;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 12/15] gpio: latch: use lock guards
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (10 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 11/15] gpio: max77650: " Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 13/15] gpio: latch: use value returning setters Bartosz Golaszewski
` (4 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use lock guards from linux/cleanup.h. This will make the subsequent
commit that switches to using value returning GPIO line setters much
simpler as we'll be able to return values without caring about releasing
the locks.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-latch.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpio-latch.c b/drivers/gpio/gpio-latch.c
index 64174ea7d008..f227966c50d5 100644
--- a/drivers/gpio/gpio-latch.c
+++ b/drivers/gpio/gpio-latch.c
@@ -38,6 +38,7 @@
* in the corresponding device tree properties.
*/
+#include <linux/cleanup.h>
#include <linux/err.h>
#include <linux/gpio/consumer.h>
#include <linux/gpio/driver.h>
@@ -94,24 +95,19 @@ static void gpio_latch_set_unlocked(struct gpio_latch_priv *priv,
static void gpio_latch_set(struct gpio_chip *gc, unsigned int offset, int val)
{
struct gpio_latch_priv *priv = gpiochip_get_data(gc);
- unsigned long flags;
- spin_lock_irqsave(&priv->spinlock, flags);
+ guard(spinlock_irqsave)(&priv->spinlock);
gpio_latch_set_unlocked(priv, gpiod_set_value, offset, val);
-
- spin_unlock_irqrestore(&priv->spinlock, flags);
}
static void gpio_latch_set_can_sleep(struct gpio_chip *gc, unsigned int offset, int val)
{
struct gpio_latch_priv *priv = gpiochip_get_data(gc);
- mutex_lock(&priv->mutex);
+ guard(mutex)(&priv->mutex);
gpio_latch_set_unlocked(priv, gpiod_set_value_cansleep, offset, val);
-
- mutex_unlock(&priv->mutex);
}
static bool gpio_latch_can_sleep(struct gpio_latch_priv *priv, unsigned int n_latches)
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 13/15] gpio: latch: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (11 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 12/15] gpio: latch: use lock guards Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 14/15] gpio: davinci: " Bartosz Golaszewski
` (3 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-latch.c | 33 +++++++++++++++++++--------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-latch.c b/drivers/gpio/gpio-latch.c
index f227966c50d5..3d0ff09284fb 100644
--- a/drivers/gpio/gpio-latch.c
+++ b/drivers/gpio/gpio-latch.c
@@ -73,41 +73,46 @@ static int gpio_latch_get_direction(struct gpio_chip *gc, unsigned int offset)
return GPIO_LINE_DIRECTION_OUT;
}
-static void gpio_latch_set_unlocked(struct gpio_latch_priv *priv,
- int (*set)(struct gpio_desc *desc, int value),
- unsigned int offset, bool val)
+static int gpio_latch_set_unlocked(struct gpio_latch_priv *priv,
+ int (*set)(struct gpio_desc *desc, int value),
+ unsigned int offset, bool val)
{
- int latch = offset / priv->n_latched_gpios;
- int i;
+ int latch = offset / priv->n_latched_gpios, i, ret;
assign_bit(offset, priv->shadow, val);
- for (i = 0; i < priv->n_latched_gpios; i++)
- set(priv->latched_gpios->desc[i],
- test_bit(latch * priv->n_latched_gpios + i, priv->shadow));
+ for (i = 0; i < priv->n_latched_gpios; i++) {
+ ret = set(priv->latched_gpios->desc[i],
+ test_bit(latch * priv->n_latched_gpios + i,
+ priv->shadow));
+ if (ret)
+ return ret;
+ }
ndelay(priv->setup_duration_ns);
set(priv->clk_gpios->desc[latch], 1);
ndelay(priv->clock_duration_ns);
set(priv->clk_gpios->desc[latch], 0);
+
+ return 0;
}
-static void gpio_latch_set(struct gpio_chip *gc, unsigned int offset, int val)
+static int gpio_latch_set(struct gpio_chip *gc, unsigned int offset, int val)
{
struct gpio_latch_priv *priv = gpiochip_get_data(gc);
guard(spinlock_irqsave)(&priv->spinlock);
- gpio_latch_set_unlocked(priv, gpiod_set_value, offset, val);
+ return gpio_latch_set_unlocked(priv, gpiod_set_value, offset, val);
}
-static void gpio_latch_set_can_sleep(struct gpio_chip *gc, unsigned int offset, int val)
+static int gpio_latch_set_can_sleep(struct gpio_chip *gc, unsigned int offset, int val)
{
struct gpio_latch_priv *priv = gpiochip_get_data(gc);
guard(mutex)(&priv->mutex);
- gpio_latch_set_unlocked(priv, gpiod_set_value_cansleep, offset, val);
+ return gpio_latch_set_unlocked(priv, gpiod_set_value_cansleep, offset, val);
}
static bool gpio_latch_can_sleep(struct gpio_latch_priv *priv, unsigned int n_latches)
@@ -161,11 +166,11 @@ static int gpio_latch_probe(struct platform_device *pdev)
if (gpio_latch_can_sleep(priv, n_latches)) {
priv->gc.can_sleep = true;
- priv->gc.set = gpio_latch_set_can_sleep;
+ priv->gc.set_rv = gpio_latch_set_can_sleep;
mutex_init(&priv->mutex);
} else {
priv->gc.can_sleep = false;
- priv->gc.set = gpio_latch_set;
+ priv->gc.set_rv = gpio_latch_set;
spin_lock_init(&priv->spinlock);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 14/15] gpio: davinci: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (12 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 13/15] gpio: latch: use value returning setters Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 15/15] gpio: mvebu: " Bartosz Golaszewski
` (2 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-davinci.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 8c033e8cf3c9..63fc7888c1d4 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -139,7 +139,7 @@ static int davinci_gpio_get(struct gpio_chip *chip, unsigned offset)
/*
* Assuming the pin is muxed as a gpio output, set its output value.
*/
-static void
+static int
davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
{
struct davinci_gpio_controller *d = gpiochip_get_data(chip);
@@ -150,6 +150,8 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
writel_relaxed(__gpio_mask(offset),
value ? &g->set_data : &g->clr_data);
+
+ return 0;
}
static int davinci_gpio_probe(struct platform_device *pdev)
@@ -209,7 +211,7 @@ static int davinci_gpio_probe(struct platform_device *pdev)
chips->chip.direction_input = davinci_direction_in;
chips->chip.get = davinci_gpio_get;
chips->chip.direction_output = davinci_direction_out;
- chips->chip.set = davinci_gpio_set;
+ chips->chip.set_rv = davinci_gpio_set;
chips->chip.ngpio = ngpio;
chips->chip.base = -1;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 15/15] gpio: mvebu: use value returning setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (13 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 14/15] gpio: davinci: " Bartosz Golaszewski
@ 2025-02-20 9:57 ` Bartosz Golaszewski
2025-02-21 9:33 ` [PATCH v2 00/15] gpiolib: indicate errors in value setters Uwe Kleine-König
2025-02-26 10:19 ` Bartosz Golaszewski
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-20 9:57 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
struct gpio_chip now has additional variants of the set(_multiple)
driver callbacks that return an integer to indicate success or failure.
Convert the driver to using them.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-mvebu.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 363bad286c32..3604abcb6fec 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -298,12 +298,12 @@ static unsigned int mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *mvpwm)
/*
* Functions implementing the gpio_chip methods
*/
-static void mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
+static int mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
{
struct mvebu_gpio_chip *mvchip = gpiochip_get_data(chip);
- regmap_update_bits(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,
- BIT(pin), value ? BIT(pin) : 0);
+ return regmap_update_bits(mvchip->regs, GPIO_OUT_OFF + mvchip->offset,
+ BIT(pin), value ? BIT(pin) : 0);
}
static int mvebu_gpio_get(struct gpio_chip *chip, unsigned int pin)
@@ -1173,7 +1173,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
mvchip->chip.direction_input = mvebu_gpio_direction_input;
mvchip->chip.get = mvebu_gpio_get;
mvchip->chip.direction_output = mvebu_gpio_direction_output;
- mvchip->chip.set = mvebu_gpio_set;
+ mvchip->chip.set_rv = mvebu_gpio_set;
if (have_irqs)
mvchip->chip.to_irq = mvebu_gpio_to_irq;
mvchip->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
--
2.45.2
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 07/15] gpio: regmap: use value returning setters
2025-02-20 9:57 ` [PATCH v2 07/15] gpio: regmap: " Bartosz Golaszewski
@ 2025-02-20 10:08 ` Michael Walle
0 siblings, 0 replies; 24+ messages in thread
From: Michael Walle @ 2025-02-20 10:08 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
[-- Attachment #1: Type: text/plain, Size: 505 bytes --]
On Thu Feb 20, 2025 at 10:57 AM CET, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> struct gpio_chip now has additional variants of the set(_multiple)
> driver callbacks that return an integer to indicate success or failure.
> Convert the driver to using them.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Michael Walle <mwalle@kernel.org>
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/15] gpiolib: indicate errors in value setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (14 preceding siblings ...)
2025-02-20 9:57 ` [PATCH v2 15/15] gpio: mvebu: " Bartosz Golaszewski
@ 2025-02-21 9:33 ` Uwe Kleine-König
2025-02-26 10:19 ` Bartosz Golaszewski
16 siblings, 0 replies; 24+ messages in thread
From: Uwe Kleine-König @ 2025-02-21 9:33 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Michael Walle, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, linux-gpio, linux-kernel, linux-pwm,
Bartosz Golaszewski, Lee Jones, Pavel Machek, linux-leds,
kernel test robot
[-- 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 --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 00/15] gpiolib: indicate errors in value setters
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
` (15 preceding siblings ...)
2025-02-21 9:33 ` [PATCH v2 00/15] gpiolib: indicate errors in value setters Uwe Kleine-König
@ 2025-02-26 10:19 ` Bartosz Golaszewski
16 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-26 10:19 UTC (permalink / raw)
To: Linus Walleij, Michael Walle, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, Uwe Kleine-König,
Bartosz Golaszewski
Cc: Bartosz Golaszewski, linux-gpio, linux-kernel, linux-pwm,
Lee Jones, Pavel Machek, linux-leds, kernel test robot
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>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/15] gpiolib: introduce gpio_chip setters that return values
[not found] ` <CGME20250227140054eucas1p2bf6e3f2416e11e3c62a704682bf052bf@eucas1p2.samsung.com>
@ 2025-02-27 14:00 ` Marek Szyprowski
2025-02-27 14:10 ` Bartosz Golaszewski
0 siblings, 1 reply; 24+ messages in thread
From: Marek Szyprowski @ 2025-02-27 14:00 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Michael Walle,
Bamvor Jian Zhang, Geert Uytterhoeven, Keerthy,
Uwe Kleine-König
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski
On 20.02.2025 10:57, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Add new variants of the set() and set_multiple() callbacks that have
> integer return values allowing to indicate failures to users of the GPIO
> consumer API. Until we convert all GPIO providers treewide to using
> them, they will live in parallel to the existing ones.
>
> Make sure that providers cannot define both. Prefer the new ones and
> only use the old ones as fallback.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> drivers/gpio/gpiolib.c | 27 +++++++++++++++++++++++++--
> include/linux/gpio/driver.h | 10 ++++++++++
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index b1e7d368bc7d..19f78ffcc3c1 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -926,6 +926,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> int base = 0;
> int ret = 0;
>
> + /* Only allow one set() and one set_multiple(). */
> + if ((gc->set && gc->set_rv) ||
> + (gc->set_multiple && gc->set_multiple_rv))
> + return -EINVAL;
> +
> /*
> * First: allocate and populate the internal stat container, and
> * set up the struct device.
> @@ -2757,11 +2762,21 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
>
> static int gpiochip_set(struct gpio_chip *gc, unsigned int offset, int value)
> {
> + int ret;
> +
> lockdep_assert_held(&gc->gpiodev->srcu);
>
> - if (WARN_ON(unlikely(!gc->set)))
> + if (WARN_ON(unlikely(!gc->set && !gc->set_rv)))
> return -EOPNOTSUPP;
>
> + if (gc->set_rv) {
> + ret = gc->set_rv(gc, offset, value);
> + if (ret > 0)
> + ret = -EBADE;
> +
> + return ret;
> + }
> +
> gc->set(gc, offset, value);
> return 0;
> }
> @@ -3501,9 +3516,17 @@ static int gpiochip_set_multiple(struct gpio_chip *gc,
>
> lockdep_assert_held(&gc->gpiodev->srcu);
>
> - if (WARN_ON(unlikely(!gc->set_multiple && !gc->set)))
> + if (WARN_ON(unlikely(!gc->set_multiple && !gc->set_multiple_rv)))
> return -EOPNOTSUPP;
The above change issues a warning on gpio controllers that doesn't
support set_multiple() callbacks at all. I think that this wasn't intended.
>
> + if (gc->set_multiple_rv) {
> + ret = gc->set_multiple_rv(gc, mask, bits);
> + if (ret > 0)
> + ret = -EBADE;
> +
> + return ret;
> + }
> +
> if (gc->set_multiple) {
> gc->set_multiple(gc, mask, bits);
> return 0;
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 10544f4a03e5..b2627c8ed513 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -347,6 +347,10 @@ struct gpio_irq_chip {
> * stores them in "bits", returns 0 on success or negative error
> * @set: assigns output value for signal "offset"
> * @set_multiple: assigns output values for multiple signals defined by "mask"
> + * @set_rv: assigns output value for signal "offset", returns 0 on success or
> + * negative error value
> + * @set_multiple_rv: assigns output values for multiple signals defined by
> + * "mask", returns 0 on success or negative error value
> * @set_config: optional hook for all kinds of settings. Uses the same
> * packed config format as generic pinconf.
> * @to_irq: optional hook supporting non-static gpiod_to_irq() mappings;
> @@ -442,6 +446,12 @@ struct gpio_chip {
> void (*set_multiple)(struct gpio_chip *gc,
> unsigned long *mask,
> unsigned long *bits);
> + int (*set_rv)(struct gpio_chip *gc,
> + unsigned int offset,
> + int value);
> + int (*set_multiple_rv)(struct gpio_chip *gc,
> + unsigned long *mask,
> + unsigned long *bits);
> int (*set_config)(struct gpio_chip *gc,
> unsigned int offset,
> unsigned long config);
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/15] gpiolib: introduce gpio_chip setters that return values
2025-02-27 14:00 ` Marek Szyprowski
@ 2025-02-27 14:10 ` Bartosz Golaszewski
0 siblings, 0 replies; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-02-27 14:10 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Linus Walleij, Michael Walle, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, Uwe Kleine-König, linux-gpio,
linux-kernel, linux-pwm, Bartosz Golaszewski
On Thu, Feb 27, 2025 at 3:00 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> On 20.02.2025 10:57, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Add new variants of the set() and set_multiple() callbacks that have
> > integer return values allowing to indicate failures to users of the GPIO
> > consumer API. Until we convert all GPIO providers treewide to using
> > them, they will live in parallel to the existing ones.
> >
> > Make sure that providers cannot define both. Prefer the new ones and
> > only use the old ones as fallback.
> >
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> > lockdep_assert_held(&gc->gpiodev->srcu);
> >
> > - if (WARN_ON(unlikely(!gc->set_multiple && !gc->set)))
> > + if (WARN_ON(unlikely(!gc->set_multiple && !gc->set_multiple_rv)))
> > return -EOPNOTSUPP;
>
> The above change issues a warning on gpio controllers that doesn't
> support set_multiple() callbacks at all. I think that this wasn't intended.
>
Eek, not at all, thanks for the report, I'll fix it.
Bartosz
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: (subset) [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
2025-02-20 9:56 ` [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants Bartosz Golaszewski
@ 2025-03-06 22:51 ` Lee Jones
2025-03-07 7:03 ` Bartosz Golaszewski
0 siblings, 1 reply; 24+ messages in thread
From: Lee Jones @ 2025-03-06 22:51 UTC (permalink / raw)
To: Linus Walleij, Michael Walle, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, Uwe Kleine-König,
Bartosz Golaszewski
Cc: linux-gpio, linux-kernel, linux-pwm, Bartosz Golaszewski,
Lee Jones, Pavel Machek, linux-leds, kernel test robot
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 [李琼斯]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: (subset) [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
2025-03-06 22:51 ` (subset) " Lee Jones
@ 2025-03-07 7:03 ` Bartosz Golaszewski
2025-03-13 15:29 ` Lee Jones
0 siblings, 1 reply; 24+ messages in thread
From: Bartosz Golaszewski @ 2025-03-07 7:03 UTC (permalink / raw)
To: Lee Jones
Cc: Linus Walleij, Michael Walle, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, Uwe Kleine-König, linux-gpio,
linux-kernel, linux-pwm, Bartosz Golaszewski, Pavel Machek,
linux-leds, kernel test robot
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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: (subset) [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants
2025-03-07 7:03 ` Bartosz Golaszewski
@ 2025-03-13 15:29 ` Lee Jones
0 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2025-03-13 15:29 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Michael Walle, Bamvor Jian Zhang,
Geert Uytterhoeven, Keerthy, Uwe Kleine-König, linux-gpio,
linux-kernel, linux-pwm, Bartosz Golaszewski, Pavel Machek,
linux-leds, kernel test robot
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 [李琼斯]
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2025-03-13 15:29 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-20 9:56 [PATCH v2 00/15] gpiolib: indicate errors in value setters Bartosz Golaszewski
2025-02-20 9:56 ` [PATCH v2 01/15] leds: aw200xx: don't use return with gpiod_set_value() variants Bartosz Golaszewski
2025-03-06 22:51 ` (subset) " Lee Jones
2025-03-07 7:03 ` Bartosz Golaszewski
2025-03-13 15:29 ` Lee Jones
2025-02-20 9:56 ` [PATCH v2 02/15] gpiolib: make value setters have return values Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 03/15] gpiolib: wrap gpio_chip::set() Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 04/15] gpiolib: rework the wrapper around gpio_chip::set_multiple() Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 05/15] gpiolib: introduce gpio_chip setters that return values Bartosz Golaszewski
[not found] ` <CGME20250227140054eucas1p2bf6e3f2416e11e3c62a704682bf052bf@eucas1p2.samsung.com>
2025-02-27 14:00 ` Marek Szyprowski
2025-02-27 14:10 ` Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 06/15] gpio: sim: use value returning setters Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 07/15] gpio: regmap: " Bartosz Golaszewski
2025-02-20 10:08 ` Michael Walle
2025-02-20 9:57 ` [PATCH v2 08/15] gpio: pca953x: " Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 09/15] gpio: mockup: " Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 10/15] gpio: aggregator: " Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 11/15] gpio: max77650: " Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 12/15] gpio: latch: use lock guards Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 13/15] gpio: latch: use value returning setters Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 14/15] gpio: davinci: " Bartosz Golaszewski
2025-02-20 9:57 ` [PATCH v2 15/15] gpio: mvebu: " Bartosz Golaszewski
2025-02-21 9:33 ` [PATCH v2 00/15] gpiolib: indicate errors in value setters Uwe Kleine-König
2025-02-26 10:19 ` Bartosz Golaszewski
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).