[PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[David Lechner]

This series was inspired by some minor annoyance I have experienced a
few times in recent reviews.

Calling gpiod_set_array_value_cansleep() can be quite verbose due to
having so many parameters. In most cases, we already have a struct
gpio_descs that contains the first 3 parameters so we end up with 3 (or
often even 6) pointer indirections at each call site. Also, people have
a tendency to want to hard-code the first argument instead of using
struct gpio_descs.ndescs, often without checking that ndescs >= the
hard-coded value.

So I'm proposing that we add a gpiod_multi_set_value_cansleep()
function that is a wrapper around gpiod_set_array_value_cansleep()
that has struct gpio_descs as the first parameter to make it a bit
easier to read the code and avoid the hard-coding temptation.

I've just done gpiod_multi_set_value_cansleep() for now since there
were over 10 callers of this one. There aren't as many callers of
the get and atomic variants, but we can add those too if this seems
like a useful thing to do.

Maintainers, if you prefer to have this go through the gpio tree, please
give your Acked-by:. Several maintainers have also requested an
immutable branch, so I expect that will be made available. And if there
is anything leftover after the next kernel release, I will resend it.

---
Changes in v3:
- Added IS_ERR_OR_NULL() check to gpiod_multi_set_value_cansleep()
- Added new patches to clean up accessing bitmap directly (ts-nbus, ad2s1210).
- Added function prefix for max3191x.
- Removed unnecessary braces in ad7606 patch.
- Picked up additional trailers.
- Link to v2: https://lore.kernel.org/r/20250206-gpio-set-array-helper-v2-0-1c5f048f79c3@baylibre.com

Changes in v2:
- Renamed new function from gpiods_multi_set_value_cansleep() to
  gpiod_multi_set_value_cansleep()
- Fixed typo in name of replaced function in all commit messages.
- Picked up trailers.
- Link to v1: https://lore.kernel.org/r/20250131-gpio-set-array-helper-v1-0-991c8ccb4d6e@baylibre.com

---
David Lechner (15):
      gpiolib: add gpiod_multi_set_value_cansleep()
      auxdisplay: seg-led-gpio: use gpiod_multi_set_value_cansleep
      bus: ts-nbus: validate ts,data-gpios array size
      bus: ts-nbus: use gpiod_multi_set_value_cansleep
      bus: ts-nbus: use bitmap_get_value8()
      gpio: max3191x: use gpiod_multi_set_value_cansleep
      iio: adc: ad7606: use gpiod_multi_set_value_cansleep
      iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
      iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
      iio: resolver: ad2s1210: use bitmap_write
      mmc: pwrseq_simple: use gpiod_multi_set_value_cansleep
      mux: gpio: use gpiod_multi_set_value_cansleep
      net: mdio: mux-gpio: use gpiod_multi_set_value_cansleep
      phy: mapphone-mdm6600: use gpiod_multi_set_value_cansleep
      ASoC: adau1701: use gpiod_multi_set_value_cansleep

 drivers/auxdisplay/seg-led-gpio.c           |  3 +--
 drivers/bus/ts-nbus.c                       | 15 +++++++++------
 drivers/gpio/gpio-max3191x.c                | 18 +++++++-----------
 drivers/iio/adc/ad7606.c                    |  3 +--
 drivers/iio/adc/ad7606_spi.c                |  7 +++----
 drivers/iio/amplifiers/hmc425a.c            |  3 +--
 drivers/iio/resolver/ad2s1210.c             | 13 +++++--------
 drivers/mmc/core/pwrseq_simple.c            |  3 +--
 drivers/mux/gpio.c                          |  4 +---
 drivers/net/mdio/mdio-mux-gpio.c            |  3 +--
 drivers/phy/motorola/phy-mapphone-mdm6600.c |  4 +---
 include/linux/gpio/consumer.h               | 11 +++++++++++
 sound/soc/codecs/adau1701.c                 |  4 +---
 13 files changed, 43 insertions(+), 48 deletions(-)
---
base-commit: df4b2bbff898227db0c14264ac7edd634e79f755
change-id: 20250131-gpio-set-array-helper-bd4a328370d3

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

[PATCH v3 01/15] gpiolib: add gpiod_multi_set_value_cansleep()

[David Lechner]

Add a new gpiod_multi_set_value_cansleep() helper function with fewer
parameters than gpiod_set_array_value_cansleep().

Calling gpiod_set_array_value_cansleep() can get quite verbose. In many
cases, the first arguments all come from the same struct gpio_descs, so
having a separate function where we can just pass that cuts down on the
boilerplate.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---

FYI, I dropped Linus' Reviewed-by: tag since adding the IS_ERR_OR_NULL()
check isn't exactly trivial.
---
 include/linux/gpio/consumer.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index db2dfbae8edbd12059826183b1c0f73c7a58ff40..5cbd4afd78625367a761e224acc3f7336d310dd0 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -3,6 +3,7 @@
 #define __LINUX_GPIO_CONSUMER_H
 
 #include <linux/bits.h>
+#include <linux/err.h>
 #include <linux/types.h>
 
 struct acpi_device;
@@ -655,4 +656,14 @@ static inline void gpiod_unexport(struct gpio_desc *desc)
 
 #endif /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */
 
+static inline int gpiod_multi_set_value_cansleep(struct gpio_descs *descs,
+						 unsigned long *value_bitmap)
+{
+	if (IS_ERR_OR_NULL(descs))
+		return PTR_ERR_OR_ZERO(descs);
+
+	return gpiod_set_array_value_cansleep(descs->ndescs, descs->desc,
+					      descs->info, value_bitmap);
+}
+
 #endif

-- 
2.43.0

[PATCH v3 02/15] auxdisplay: seg-led-gpio: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Acked-by: Andy Shevchenko <andy@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/auxdisplay/seg-led-gpio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/auxdisplay/seg-led-gpio.c b/drivers/auxdisplay/seg-led-gpio.c
index f10c25e6bf126cfaac3e4c353f8bfc6639d94a60..dfb62e9ce9b407fe356c3a7d2d25319b91a11a75 100644
--- a/drivers/auxdisplay/seg-led-gpio.c
+++ b/drivers/auxdisplay/seg-led-gpio.c
@@ -36,8 +36,7 @@ static void seg_led_update(struct work_struct *work)
 
 	bitmap_set_value8(values, map_to_seg7(&map->map.seg7, linedisp->buf[0]), 0);
 
-	gpiod_set_array_value_cansleep(priv->segment_gpios->ndescs, priv->segment_gpios->desc,
-				       priv->segment_gpios->info, values);
+	gpiod_multi_set_value_cansleep(priv->segment_gpios, values);
 }
 
 static int seg_led_linedisp_get_map_type(struct linedisp *linedisp)

-- 
2.43.0

[PATCH v3 03/15] bus: ts-nbus: validate ts,data-gpios array size

[David Lechner]

Add validation of ts,data-gpios array size during probe. The driver
later hard-codes 8 as the size of the array when using it, so we should
be validating that the array is actually that big to prevent possible
out of bounds accesses.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/bus/ts-nbus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index 2328c48b9b1260e805c631f2aa7379d620084537..d3ee102a13893c83c50e41f7298821f4d7ae3487 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -48,6 +48,10 @@ static int ts_nbus_init_pdata(struct platform_device *pdev,
 		return dev_err_probe(&pdev->dev, PTR_ERR(ts_nbus->data),
 				     "failed to retrieve ts,data-gpio from dts\n");
 
+	if (ts_nbus->data->ndescs != 8)
+		return dev_err_probe(&pdev->dev, -EINVAL,
+				     "invalid number of ts,data-gpios\n");
+
 	ts_nbus->csn = devm_gpiod_get(&pdev->dev, "ts,csn", GPIOD_OUT_HIGH);
 	if (IS_ERR(ts_nbus->csn))
 		return dev_err_probe(&pdev->dev, PTR_ERR(ts_nbus->csn),

-- 
2.43.0

[PATCH v3 04/15] bus: ts-nbus: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

ts_nbus->data->ndescs is validated to be 8 during probe, so will have
the same value as the hard-coded 8 that is removed by this change.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/bus/ts-nbus.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index d3ee102a13893c83c50e41f7298821f4d7ae3487..b4c9308caf0647a3261071d9527fffce77784af2 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -109,8 +109,7 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
 
 	values[0] = 0;
 
-	gpiod_set_array_value_cansleep(8, ts_nbus->data->desc,
-				       ts_nbus->data->info, values);
+	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
 	gpiod_set_value_cansleep(ts_nbus->csn, 0);
 	gpiod_set_value_cansleep(ts_nbus->strobe, 0);
 	gpiod_set_value_cansleep(ts_nbus->ale, 0);
@@ -150,12 +149,11 @@ static int ts_nbus_read_byte(struct ts_nbus *ts_nbus, u8 *val)
  */
 static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
 {
-	struct gpio_descs *gpios = ts_nbus->data;
 	DECLARE_BITMAP(values, 8);
 
 	values[0] = byte;
 
-	gpiod_set_array_value_cansleep(8, gpios->desc, gpios->info, values);
+	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
 }
 
 /*

-- 
2.43.0

[PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[David Lechner]

Use bitmap_get_value8() instead of accessing the bitmap directly.

Accessing the bitmap directly is not considered good practice. We now
have a helper function that can be used instead, so let's use it.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/bus/ts-nbus.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
index b4c9308caf0647a3261071d9527fffce77784af2..beac67f3b820377f8bb1fc4f4ee77e15ee240834 100644
--- a/drivers/bus/ts-nbus.c
+++ b/drivers/bus/ts-nbus.c
@@ -10,6 +10,7 @@
  * TS-4600 SoM.
  */
 
+#include <linux/bitmap.h>
 #include <linux/bitops.h>
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
@@ -107,7 +108,7 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
 {
 	DECLARE_BITMAP(values, 8);
 
-	values[0] = 0;
+	bitmap_set_value8(values, byte, 0);
 
 	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
 	gpiod_set_value_cansleep(ts_nbus->csn, 0);
@@ -151,7 +152,7 @@ static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
 {
 	DECLARE_BITMAP(values, 8);
 
-	values[0] = byte;
+	bitmap_set_value8(values, byte, 8);
 
 	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
 }

-- 
2.43.0

[PATCH v3 06/15] gpio: max3191x: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Also add max3191x_ namespace prefix to the driver's helper function
since we are changing the function signature anyway.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/gpio/gpio-max3191x.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-max3191x.c b/drivers/gpio/gpio-max3191x.c
index bbacc714632b70e672a3d8494636fbc40dfea8ec..fc0708ab5192bd518bb0e6362f737bacbd549d61 100644
--- a/drivers/gpio/gpio-max3191x.c
+++ b/drivers/gpio/gpio-max3191x.c
@@ -309,23 +309,21 @@ static int max3191x_set_config(struct gpio_chip *gpio, unsigned int offset,
 	return 0;
 }
 
-static void gpiod_set_array_single_value_cansleep(unsigned int ndescs,
-						  struct gpio_desc **desc,
-						  struct gpio_array *info,
+static void max3191x_gpiod_multi_set_single_value(struct gpio_descs *descs,
 						  int value)
 {
 	unsigned long *values;
 
-	values = bitmap_alloc(ndescs, GFP_KERNEL);
+	values = bitmap_alloc(descs->ndescs, GFP_KERNEL);
 	if (!values)
 		return;
 
 	if (value)
-		bitmap_fill(values, ndescs);
+		bitmap_fill(values, descs->ndescs);
 	else
-		bitmap_zero(values, ndescs);
+		bitmap_zero(values, descs->ndescs);
 
-	gpiod_set_array_value_cansleep(ndescs, desc, info, values);
+	gpiod_multi_set_value_cansleep(descs, values);
 	bitmap_free(values);
 }
 
@@ -396,10 +394,8 @@ static int max3191x_probe(struct spi_device *spi)
 	max3191x->mode = device_property_read_bool(dev, "maxim,modesel-8bit")
 				 ? STATUS_BYTE_DISABLED : STATUS_BYTE_ENABLED;
 	if (max3191x->modesel_pins)
-		gpiod_set_array_single_value_cansleep(
-				 max3191x->modesel_pins->ndescs,
-				 max3191x->modesel_pins->desc,
-				 max3191x->modesel_pins->info, max3191x->mode);
+		max3191x_gpiod_multi_set_single_value(max3191x->modesel_pins,
+						      max3191x->mode);
 
 	max3191x->ignore_uv = device_property_read_bool(dev,
 						  "maxim,ignore-undervoltage");

-- 
2.43.0

[PATCH v3 07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value().

These are not called in an atomic context, so changing to the cansleep
variant is fine.

Also drop unnecessary braces while we are at it.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7606.c     | 3 +--
 drivers/iio/adc/ad7606_spi.c | 7 +++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index d8e3c7a43678c57470a5118715637a68b39125c1..9a124139924e4a4fbbbd234a8514eb77024442b3 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -818,8 +818,7 @@ static int ad7606_write_os_hw(struct iio_dev *indio_dev, int val)
 
 	values[0] = val & GENMASK(2, 0);
 
-	gpiod_set_array_value(st->gpio_os->ndescs, st->gpio_os->desc,
-			      st->gpio_os->info, values);
+	gpiod_multi_set_value_cansleep(st->gpio_os, values);
 
 	/* AD7616 requires a reset to update value */
 	if (st->chip_info->os_req_reset)
diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
index e2c1475257065c98bf8e2512bda921d6d88a3002..091f31edb6604da3a8ec4d2d5328ac6550faa22c 100644
--- a/drivers/iio/adc/ad7606_spi.c
+++ b/drivers/iio/adc/ad7606_spi.c
@@ -296,10 +296,9 @@ static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
 	 * in the device tree, then they need to be set to high,
 	 * otherwise, they must be hardwired to VDD
 	 */
-	if (st->gpio_os) {
-		gpiod_set_array_value(st->gpio_os->ndescs,
-				      st->gpio_os->desc, st->gpio_os->info, os);
-	}
+	if (st->gpio_os)
+		gpiod_multi_set_value_cansleep(st->gpio_os, os);
+
 	/* OS of 128 and 256 are available only in software mode */
 	st->oversampling_avail = ad7606B_oversampling_avail;
 	st->num_os_ratios = ARRAY_SIZE(ad7606B_oversampling_avail);

-- 
2.43.0

[PATCH v3 08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Passing NULL as the 3rd argument to gpiod_set_array_value_cansleep()
only needs to be done if the array was constructed manually, which is
not the case here. This change effectively replaces that argument with
st->gpios->array_info. The possible side effect of this change is that
it could make setting the GPIOs more efficient.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/amplifiers/hmc425a.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/amplifiers/hmc425a.c b/drivers/iio/amplifiers/hmc425a.c
index 2ee4c0d70281e24c1c818249b86d89ebe06d4876..d9a359e1388a0f3eb5909bf668ff82102286542b 100644
--- a/drivers/iio/amplifiers/hmc425a.c
+++ b/drivers/iio/amplifiers/hmc425a.c
@@ -161,8 +161,7 @@ static int hmc425a_write(struct iio_dev *indio_dev, u32 value)
 
 	values[0] = value;
 
-	gpiod_set_array_value_cansleep(st->gpios->ndescs, st->gpios->desc,
-				       NULL, values);
+	gpiod_multi_set_value_cansleep(st->gpios, values);
 	return 0;
 }
 

-- 
2.43.0

[PATCH v3 09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value().

These are not called in an atomic context, so changing to the cansleep
variant is fine.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/resolver/ad2s1210.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
index b681129a99b6cf399668bf01a1f5a15fbc4f95b8..7f18df790157f1e411fb70de193a49f0677c999f 100644
--- a/drivers/iio/resolver/ad2s1210.c
+++ b/drivers/iio/resolver/ad2s1210.c
@@ -182,8 +182,7 @@ static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
 
 	bitmap[0] = mode;
 
-	return gpiod_set_array_value(gpios->ndescs, gpios->desc, gpios->info,
-				     bitmap);
+	return gpiod_multi_set_value_cansleep(gpios, bitmap);
 }
 
 /*
@@ -1473,10 +1472,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 
 		bitmap[0] = st->resolution;
 
-		ret = gpiod_set_array_value(resolution_gpios->ndescs,
-					    resolution_gpios->desc,
-					    resolution_gpios->info,
-					    bitmap);
+		ret = gpiod_multi_set_value_cansleep(resolution_gpios, bitmap);
 		if (ret < 0)
 			return dev_err_probe(dev, ret,
 					     "failed to set resolution gpios\n");

-- 
2.43.0

[PATCH v3 10/15] iio: resolver: ad2s1210: use bitmap_write

[David Lechner]

Replace bitmap array access with bitmap_write.

Accessing the bitmap array directly is not recommended and now there is
a helper function that can be used.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/resolver/ad2s1210.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
index 7f18df790157f1e411fb70de193a49f0677c999f..04879e6d538bce664469c5f6759d8b1cedea16e9 100644
--- a/drivers/iio/resolver/ad2s1210.c
+++ b/drivers/iio/resolver/ad2s1210.c
@@ -46,6 +46,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/clk.h>
@@ -180,7 +181,7 @@ static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
 	if (!gpios)
 		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
 
-	bitmap[0] = mode;
+	bitmap_write(bitmap, mode, 0, 2);
 
 	return gpiod_multi_set_value_cansleep(gpios, bitmap);
 }
@@ -1470,7 +1471,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 			return dev_err_probe(dev, -EINVAL,
 				      "requires exactly 2 resolution-gpios\n");
 
-		bitmap[0] = st->resolution;
+		bitmap_write(bitmap, st->resolution, 0, 2);
 
 		ret = gpiod_multi_set_value_cansleep(resolution_gpios, bitmap);
 		if (ret < 0)

-- 
2.43.0

[PATCH v3 11/15] mmc: pwrseq_simple: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/mmc/core/pwrseq_simple.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
index 37cd858df0f4d7123683e1fe23a4c3fcd7817d13..4b47e6c3b04b99dc328a8b063665a76340a8e0d0 100644
--- a/drivers/mmc/core/pwrseq_simple.c
+++ b/drivers/mmc/core/pwrseq_simple.c
@@ -54,8 +54,7 @@ static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq,
 		else
 			bitmap_zero(values, nvalues);
 
-		gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
-					       reset_gpios->info, values);
+		gpiod_multi_set_value_cansleep(reset_gpios, values);
 
 		bitmap_free(values);
 	}

-- 
2.43.0

[PATCH v3 12/15] mux: gpio: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Acked-by: Peter Rosin <peda@axentia.se>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/mux/gpio.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/mux/gpio.c b/drivers/mux/gpio.c
index cc5f2c1861d4a22d984bcd37efb98dd3561ee765..5710879cd47f89b6ef4458d6b4419a1fe9ad349f 100644
--- a/drivers/mux/gpio.c
+++ b/drivers/mux/gpio.c
@@ -28,9 +28,7 @@ static int mux_gpio_set(struct mux_control *mux, int state)
 
 	bitmap_from_arr32(values, &value, BITS_PER_TYPE(value));
 
-	gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
-				       mux_gpio->gpios->desc,
-				       mux_gpio->gpios->info, values);
+	gpiod_multi_set_value_cansleep(mux_gpio->gpios, values);
 
 	return 0;
 }

-- 
2.43.0

[PATCH v3 13/15] net: mdio: mux-gpio: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/net/mdio/mdio-mux-gpio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/mdio/mdio-mux-gpio.c b/drivers/net/mdio/mdio-mux-gpio.c
index ef77bd1abae984e5b1e51315de39cae33e0d063d..fefa40ea5227c5a35d89ec2c6f95c6668a2470f6 100644
--- a/drivers/net/mdio/mdio-mux-gpio.c
+++ b/drivers/net/mdio/mdio-mux-gpio.c
@@ -30,8 +30,7 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
 
 	values[0] = desired_child;
 
-	gpiod_set_array_value_cansleep(s->gpios->ndescs, s->gpios->desc,
-				       s->gpios->info, values);
+	gpiod_multi_set_value_cansleep(s->gpios, values);
 
 	return 0;
 }

-- 
2.43.0

[PATCH v3 14/15] phy: mapphone-mdm6600: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

ddata->cmd_gpios->ndescs is validated to be equal to
PHY_MDM6600_NR_CMD_LINES during driver probe, so it will have the same
value as the previously hard-coded argument.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/phy/motorola/phy-mapphone-mdm6600.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/phy/motorola/phy-mapphone-mdm6600.c b/drivers/phy/motorola/phy-mapphone-mdm6600.c
index 152344e4f7e44de0f8ab1cae6ae01a1f1c5408e9..fd0e0cd1c1cfb10fb55ed271e47b6a0bf857028e 100644
--- a/drivers/phy/motorola/phy-mapphone-mdm6600.c
+++ b/drivers/phy/motorola/phy-mapphone-mdm6600.c
@@ -177,9 +177,7 @@ static void phy_mdm6600_cmd(struct phy_mdm6600 *ddata, int val)
 
 	values[0] = val;
 
-	gpiod_set_array_value_cansleep(PHY_MDM6600_NR_CMD_LINES,
-				       ddata->cmd_gpios->desc,
-				       ddata->cmd_gpios->info, values);
+	gpiod_multi_set_value_cansleep(ddata->cmd_gpios, values);
 }
 
 /**

-- 
2.43.0

[PATCH v3 15/15] ASoC: adau1701: use gpiod_multi_set_value_cansleep

[David Lechner]

Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
gpiod_set_array_value_cansleep().

Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 sound/soc/codecs/adau1701.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/codecs/adau1701.c b/sound/soc/codecs/adau1701.c
index 291249e0a2a32df7dde81904dce2f6be143fc2d7..6876462d8bdbb41d551f776c2d7fe6ed46115fa1 100644
--- a/sound/soc/codecs/adau1701.c
+++ b/sound/soc/codecs/adau1701.c
@@ -325,9 +325,7 @@ static int adau1701_reset(struct snd_soc_component *component, unsigned int clkd
 			__assign_bit(1, values, 1);
 			break;
 		}
-		gpiod_set_array_value_cansleep(adau1701->gpio_pll_mode->ndescs,
-				adau1701->gpio_pll_mode->desc, adau1701->gpio_pll_mode->info,
-				values);
+		gpiod_multi_set_value_cansleep(adau1701->gpio_pll_mode, values);
 	}
 
 	adau1701->pll_clkdiv = clkdiv;

-- 
2.43.0

Re: [PATCH v3 01/15] gpiolib: add gpiod_multi_set_value_cansleep()

[Geert Uytterhoeven]

On Mon, 10 Feb 2025 at 23:37, David Lechner <dlechner@baylibre.com> wrote:
> Add a new gpiod_multi_set_value_cansleep() helper function with fewer
> parameters than gpiod_set_array_value_cansleep().
>
> Calling gpiod_set_array_value_cansleep() can get quite verbose. In many
> cases, the first arguments all come from the same struct gpio_descs, so
> having a separate function where we can just pass that cuts down on the
> boilerplate.
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 01/15] gpiolib: add gpiod_multi_set_value_cansleep()

[Andy Shevchenko]

On Mon, Feb 10, 2025 at 04:33:27PM -0600, David Lechner wrote:
> Add a new gpiod_multi_set_value_cansleep() helper function with fewer
> parameters than gpiod_set_array_value_cansleep().
> 
> Calling gpiod_set_array_value_cansleep() can get quite verbose. In many
> cases, the first arguments all come from the same struct gpio_descs, so
> having a separate function where we can just pass that cuts down on the
> boilerplate.

LGTM now,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[Andy Shevchenko]

On Mon, Feb 10, 2025 at 04:33:31PM -0600, David Lechner wrote:
> Use bitmap_get_value8() instead of accessing the bitmap directly.
> 
> Accessing the bitmap directly is not considered good practice. We now
> have a helper function that can be used instead, so let's use it.

Thank you, LGTM (one minor thing you may address,
or keep it as in the current variant of the patch),

Reviewed-by: Andy Shevchenko <andy@kernel.org>

...

> +#include <linux/bitmap.h>
>  #include <linux/bitops.h>

bitmap.h implies bitops.h

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 06/15] gpio: max3191x: use gpiod_multi_set_value_cansleep

[Andy Shevchenko]

On Mon, Feb 10, 2025 at 04:33:32PM -0600, David Lechner wrote:
> Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
> gpiod_set_array_value_cansleep().
> 
> Also add max3191x_ namespace prefix to the driver's helper function
> since we are changing the function signature anyway.

With renamed function looks much better, thanks,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 10/15] iio: resolver: ad2s1210: use bitmap_write

[Jonathan Cameron]

On Mon, 10 Feb 2025 16:33:36 -0600
David Lechner <dlechner@baylibre.com> wrote:

> Replace bitmap array access with bitmap_write.
> 
> Accessing the bitmap array directly is not recommended and now there is
> a helper function that can be used.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> ---
>  drivers/iio/resolver/ad2s1210.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> index 7f18df790157f1e411fb70de193a49f0677c999f..04879e6d538bce664469c5f6759d8b1cedea16e9 100644
> --- a/drivers/iio/resolver/ad2s1210.c
> +++ b/drivers/iio/resolver/ad2s1210.c
> @@ -46,6 +46,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/bitmap.h>
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
>  #include <linux/clk.h>
> @@ -180,7 +181,7 @@ static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
>  	if (!gpios)
>  		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
>  
> -	bitmap[0] = mode;
> +	bitmap_write(bitmap, mode, 0, 2);
>  
>  	return gpiod_multi_set_value_cansleep(gpios, bitmap);
>  }
> @@ -1470,7 +1471,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  			return dev_err_probe(dev, -EINVAL,
>  				      "requires exactly 2 resolution-gpios\n");
>  
> -		bitmap[0] = st->resolution;
> +		bitmap_write(bitmap, st->resolution, 0, 2);
>  
>  		ret = gpiod_multi_set_value_cansleep(resolution_gpios, bitmap);
>  		if (ret < 0)
> 

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

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


On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> This series was inspired by some minor annoyance I have experienced a
> few times in recent reviews.
> 
> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> having so many parameters. In most cases, we already have a struct
> gpio_descs that contains the first 3 parameters so we end up with 3 (or
> often even 6) pointer indirections at each call site. Also, people have
> a tendency to want to hard-code the first argument instead of using
> struct gpio_descs.ndescs, often without checking that ndescs >= the
> hard-coded value.
> 
> [...]

Applied, thanks!

[01/15] gpiolib: add gpiod_multi_set_value_cansleep()
        commit: 91931af18bd22437e08e2471f5484d6fbdd8ab93

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

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

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


On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> This series was inspired by some minor annoyance I have experienced a
> few times in recent reviews.
> 
> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> having so many parameters. In most cases, we already have a struct
> gpio_descs that contains the first 3 parameters so we end up with 3 (or
> often even 6) pointer indirections at each call site. Also, people have
> a tendency to want to hard-code the first argument instead of using
> struct gpio_descs.ndescs, often without checking that ndescs >= the
> hard-coded value.
> 
> [...]

Applied, thanks!

[06/15] gpio: max3191x: use gpiod_multi_set_value_cansleep
        commit: eb2e9c308d2882d9d364af048eb3d8336d41c4bb

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

Re: [PATCH v3 02/15] auxdisplay: seg-led-gpio: use gpiod_multi_set_value_cansleep

[Andy Shevchenko]

On Mon, Feb 10, 2025 at 04:33:28PM -0600, David Lechner wrote:
> Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
> gpiod_set_array_value_cansleep().

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 14/15] phy: mapphone-mdm6600: use gpiod_multi_set_value_cansleep

[Vinod Koul]

On 10-02-25, 16:33, David Lechner wrote:
> Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
> gpiod_set_array_value_cansleep().
> 
> ddata->cmd_gpios->ndescs is validated to be equal to
> PHY_MDM6600_NR_CMD_LINES during driver probe, so it will have the same
> value as the previously hard-coded argument.

Acked-by: Vinod Koul <vkoul@kernel.org>

-- 
~Vinod

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[David Lechner]

On 2/12/25 3:36 AM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> 
> On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
>> This series was inspired by some minor annoyance I have experienced a
>> few times in recent reviews.
>>
>> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
>> having so many parameters. In most cases, we already have a struct
>> gpio_descs that contains the first 3 parameters so we end up with 3 (or
>> often even 6) pointer indirections at each call site. Also, people have
>> a tendency to want to hard-code the first argument instead of using
>> struct gpio_descs.ndescs, often without checking that ndescs >= the
>> hard-coded value.
>>
>> [...]
> 
> Applied, thanks!
> 
> [06/15] gpio: max3191x: use gpiod_multi_set_value_cansleep
>         commit: eb2e9c308d2882d9d364af048eb3d8336d41c4bb
> 
> Best regards,

Hi Bartosz,

Do you plan to pick up the other patches that have been acked
as well? It seems like most folks were OK with everything going
though the gpio tree since the changes are small.

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Andy Shevchenko]

On Thu, Feb 13, 2025 at 11:25:21AM -0600, David Lechner wrote:
> On 2/12/25 3:36 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

[...]

> > Applied, thanks!
> > 
> > [06/15] gpio: max3191x: use gpiod_multi_set_value_cansleep
> >         commit: eb2e9c308d2882d9d364af048eb3d8336d41c4bb
> 
> Do you plan to pick up the other patches that have been acked
> as well? It seems like most folks were OK with everything going
> though the gpio tree since the changes are small.

FWIW, I took auxdisplay one via the respective tree.
But please tell me, if you are going to take them all,
I will drop that in my tree.

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

On Thu, Feb 13, 2025 at 6:25 PM David Lechner <dlechner@baylibre.com> wrote:
>
> On 2/12/25 3:36 AM, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> >
> > On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> >> This series was inspired by some minor annoyance I have experienced a
> >> few times in recent reviews.
> >>
> >> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> >> having so many parameters. In most cases, we already have a struct
> >> gpio_descs that contains the first 3 parameters so we end up with 3 (or
> >> often even 6) pointer indirections at each call site. Also, people have
> >> a tendency to want to hard-code the first argument instead of using
> >> struct gpio_descs.ndescs, often without checking that ndescs >= the
> >> hard-coded value.
> >>
> >> [...]
> >
> > Applied, thanks!
> >
> > [06/15] gpio: max3191x: use gpiod_multi_set_value_cansleep
> >         commit: eb2e9c308d2882d9d364af048eb3d8336d41c4bb
> >
> > Best regards,
>
> Hi Bartosz,
>
> Do you plan to pick up the other patches that have been acked
> as well? It seems like most folks were OK with everything going
> though the gpio tree since the changes are small.
>

Jonathan requested a branch so I made one and sent out a PR. I figured
people would just pick the relevant patches into their respective
trees? For patches that won't be in next by rc5 - I will take them if
Acked - just remind me.

Bart

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Mark Brown]

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

On Thu, Feb 13, 2025 at 06:42:19PM +0100, Bartosz Golaszewski wrote:
> On Thu, Feb 13, 2025 at 6:25 PM David Lechner <dlechner@baylibre.com> wrote:

> > Do you plan to pick up the other patches that have been acked
> > as well? It seems like most folks were OK with everything going
> > though the gpio tree since the changes are small.

> Jonathan requested a branch so I made one and sent out a PR. I figured
> people would just pick the relevant patches into their respective
> trees? For patches that won't be in next by rc5 - I will take them if
> Acked - just remind me.

If people are acking things that generally means they're expecting them
to go along with the rest of the series.  When you didn't apply the ASoC
patch I did actually put into CI but it was a bit surprising that you
seemed to be expecting that.

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

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

On Thu, Feb 13, 2025 at 6:53 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Feb 13, 2025 at 06:42:19PM +0100, Bartosz Golaszewski wrote:
> > On Thu, Feb 13, 2025 at 6:25 PM David Lechner <dlechner@baylibre.com> wrote:
>
> > > Do you plan to pick up the other patches that have been acked
> > > as well? It seems like most folks were OK with everything going
> > > though the gpio tree since the changes are small.
>
> > Jonathan requested a branch so I made one and sent out a PR. I figured
> > people would just pick the relevant patches into their respective
> > trees? For patches that won't be in next by rc5 - I will take them if
> > Acked - just remind me.
>
> If people are acking things that generally means they're expecting them
> to go along with the rest of the series.  When you didn't apply the ASoC
> patch I did actually put into CI but it was a bit surprising that you
> seemed to be expecting that.

There was no clear consensus. Some patches are still not acked.

No worries, I will take the acked ones. I didn't see any b4
notifications from your side yet, so I assume the patches are still
pending?

Bart

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Mark Brown]

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

On Thu, Feb 13, 2025 at 06:58:04PM +0100, Bartosz Golaszewski wrote:
> On Thu, Feb 13, 2025 at 6:53 PM Mark Brown <broonie@kernel.org> wrote:

> > If people are acking things that generally means they're expecting them
> > to go along with the rest of the series.  When you didn't apply the ASoC
> > patch I did actually put into CI but it was a bit surprising that you
> > seemed to be expecting that.

> There was no clear consensus. Some patches are still not acked.

What I would do in that situation is apply the patches that were acked
and leave the rest.

> No worries, I will take the acked ones. I didn't see any b4
> notifications from your side yet, so I assume the patches are still
> pending?

Yes, like I say they're in CI.

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

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Mark Brown]

On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> This series was inspired by some minor annoyance I have experienced a
> few times in recent reviews.
> 
> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> having so many parameters. In most cases, we already have a struct
> gpio_descs that contains the first 3 parameters so we end up with 3 (or
> often even 6) pointer indirections at each call site. Also, people have
> a tendency to want to hard-code the first argument instead of using
> struct gpio_descs.ndescs, often without checking that ndescs >= the
> hard-coded value.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[15/15] ASoC: adau1701: use gpiod_multi_set_value_cansleep
        commit: ad0fbcebb5f6e093d433a0873758a2778d747eb8

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v3 01/15] gpiolib: add gpiod_multi_set_value_cansleep()

[Linus Walleij]

On Mon, Feb 10, 2025 at 11:37 PM David Lechner <dlechner@baylibre.com> wrote:

> Add a new gpiod_multi_set_value_cansleep() helper function with fewer
> parameters than gpiod_set_array_value_cansleep().
>
> Calling gpiod_set_array_value_cansleep() can get quite verbose. In many
> cases, the first arguments all come from the same struct gpio_descs, so
> having a separate function where we can just pass that cuts down on the
> boilerplate.
>
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

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


On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> This series was inspired by some minor annoyance I have experienced a
> few times in recent reviews.
> 
> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> having so many parameters. In most cases, we already have a struct
> gpio_descs that contains the first 3 parameters so we end up with 3 (or
> often even 6) pointer indirections at each call site. Also, people have
> a tendency to want to hard-code the first argument instead of using
> struct gpio_descs.ndescs, often without checking that ndescs >= the
> hard-coded value.
> 
> [...]

Applied, thanks!

[07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
        commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
[08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
        commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
[09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
        commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
[10/15] iio: resolver: ad2s1210: use bitmap_write
        commit: a67e45055ea90048372066811da7c7fe2d91f9aa
[11/15] mmc: pwrseq_simple: use gpiod_multi_set_value_cansleep
        commit: 2a5920429897201f75ba026c8aa3488c792b3bd7
[12/15] mux: gpio: use gpiod_multi_set_value_cansleep
        commit: 47a7c4f58e1f9967eb0ea6c1cb2c29e0ad2edb1a
[14/15] phy: mapphone-mdm6600: use gpiod_multi_set_value_cansleep
        commit: c88aa68297390695b16fd9b7a33612257d8ef548

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

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Andy Shevchenko]

On Fri, Feb 14, 2025 at 12:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> > This series was inspired by some minor annoyance I have experienced a
> > few times in recent reviews.

...

> [07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
>         commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
> [08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
>         commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
> [09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
>         commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
> [10/15] iio: resolver: ad2s1210: use bitmap_write
>         commit: a67e45055ea90048372066811da7c7fe2d91f9aa

FWIW, Jonathan usually takes care of patch queue on weekends.
But whatever, it's not my business after all :-)

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

On Fri, Feb 14, 2025 at 3:35 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, Feb 14, 2025 at 12:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> > > This series was inspired by some minor annoyance I have experienced a
> > > few times in recent reviews.
>
> ...
>
> > [07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
> >         commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
> > [08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
> >         commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
> > [09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
> >         commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
> > [10/15] iio: resolver: ad2s1210: use bitmap_write
> >         commit: a67e45055ea90048372066811da7c7fe2d91f9aa
>
> FWIW, Jonathan usually takes care of patch queue on weekends.
> But whatever, it's not my business after all :-)
>

Too many conflicting suggestions. I just picked up all Acked patches. ¯\_(ツ)_/¯

Bart

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Jonathan Cameron]

On Fri, 14 Feb 2025 15:37:48 +0100
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> On Fri, Feb 14, 2025 at 3:35 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Fri, Feb 14, 2025 at 12:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:  
> > > On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:  
> > > > This series was inspired by some minor annoyance I have experienced a
> > > > few times in recent reviews.  
> >
> > ...
> >  
> > > [07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
> > >         commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
> > > [08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
> > >         commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
> > > [09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
> > >         commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
> > > [10/15] iio: resolver: ad2s1210: use bitmap_write
> > >         commit: a67e45055ea90048372066811da7c7fe2d91f9aa  
> >
> > FWIW, Jonathan usually takes care of patch queue on weekends.
> > But whatever, it's not my business after all :-)
> >  
> 
> Too many conflicting suggestions. I just picked up all Acked patches. ¯\_(ツ)_/¯

Resolution of any issues 'should' be easy enough. Let's keep an eye on how
it goes as other series hit Linux next.  Might be a little work to be done there
and by Linus in next merge window.

Jonathan

> 
> Bart

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

On Sun, Feb 16, 2025 at 3:23 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Fri, 14 Feb 2025 15:37:48 +0100
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > On Fri, Feb 14, 2025 at 3:35 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > >
> > > On Fri, Feb 14, 2025 at 12:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> > > > > This series was inspired by some minor annoyance I have experienced a
> > > > > few times in recent reviews.
> > >
> > > ...
> > >
> > > > [07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
> > > >         commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
> > > > [08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
> > > >         commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
> > > > [09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
> > > >         commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
> > > > [10/15] iio: resolver: ad2s1210: use bitmap_write
> > > >         commit: a67e45055ea90048372066811da7c7fe2d91f9aa
> > >
> > > FWIW, Jonathan usually takes care of patch queue on weekends.
> > > But whatever, it's not my business after all :-)
> > >
> >
> > Too many conflicting suggestions. I just picked up all Acked patches. ¯\_(ツ)_/¯
>
> Resolution of any issues 'should' be easy enough. Let's keep an eye on how
> it goes as other series hit Linux next.  Might be a little work to be done there
> and by Linus in next merge window.
>
> Jonathan
>

I'm totally fine with removing the iio commits from my queue if you
prefer to take them.

Bartosz

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Jonathan Cameron]

On Sun, 16 Feb 2025 16:55:04 +0100
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> On Sun, Feb 16, 2025 at 3:23 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Fri, 14 Feb 2025 15:37:48 +0100
> > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> >  
> > > On Fri, Feb 14, 2025 at 3:35 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:  
> > > >
> > > > On Fri, Feb 14, 2025 at 12:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:  
> > > > > On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:  
> > > > > > This series was inspired by some minor annoyance I have experienced a
> > > > > > few times in recent reviews.  
> > > >
> > > > ...
> > > >  
> > > > > [07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
> > > > >         commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
> > > > > [08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
> > > > >         commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
> > > > > [09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
> > > > >         commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
> > > > > [10/15] iio: resolver: ad2s1210: use bitmap_write
> > > > >         commit: a67e45055ea90048372066811da7c7fe2d91f9aa  
> > > >
> > > > FWIW, Jonathan usually takes care of patch queue on weekends.
> > > > But whatever, it's not my business after all :-)
> > > >  
> > >
> > > Too many conflicting suggestions. I just picked up all Acked patches. ¯\_(ツ)_/¯  
> >
> > Resolution of any issues 'should' be easy enough. Let's keep an eye on how
> > it goes as other series hit Linux next.  Might be a little work to be done there
> > and by Linus in next merge window.
> >
> > Jonathan
> >  
> 
> I'm totally fine with removing the iio commits from my queue if you
> prefer to take them.
> 
Hi Bartosz,

That's probably going to prove slightly less painful, so please do.
I'll merge in that immutable tag and pick them up once you've dropped them.

Jonathan

> Bartosz

Re: [PATCH v3 07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep

[Jonathan Cameron]

On Mon, 10 Feb 2025 16:33:33 -0600
David Lechner <dlechner@baylibre.com> wrote:

> Reduce verbosity by using gpiod_multi_set_value_cansleep() instead of
> gpiod_set_array_value().
> 
> These are not called in an atomic context, so changing to the cansleep
> variant is fine.
> 
> Also drop unnecessary braces while we are at it.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
I've applied this to the IIO tree now but won't push out as anything other than
testing until Bartosz has dropped it. (thanks for doing that btw!)

Due to code movement it ends up being all in ad7606.c

diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
index 1e8b03a518b8..87908cc51e48 100644
--- a/drivers/iio/adc/ad7606.c
+++ b/drivers/iio/adc/ad7606.c
@@ -828,8 +828,7 @@ static int ad7606_write_os_hw(struct iio_dev *indio_dev, int val)
 
        values[0] = val & GENMASK(2, 0);
 
-       gpiod_set_array_value(st->gpio_os->ndescs, st->gpio_os->desc,
-                             st->gpio_os->info, values);
+       gpiod_multi_set_value_cansleep(st->gpio_os, values);
 
        /* AD7616 requires a reset to update value */
        if (st->chip_info->os_req_reset)
@@ -1260,10 +1259,9 @@ static int ad7606b_sw_mode_setup(struct iio_dev *indio_dev)
         * in the device tree, then they need to be set to high,
         * otherwise, they must be hardwired to VDD
         */
-       if (st->gpio_os) {
-               gpiod_set_array_value(st->gpio_os->ndescs, st->gpio_os->desc,
-                                     st->gpio_os->info, os);
-       }
+       if (st->gpio_os)
+               gpiod_multi_set_value_cansleep(st->gpio_os, os);
+
        /* OS of 128 and 256 are available only in software mode */
        st->oversampling_avail = ad7606b_oversampling_avail;
        st->num_os_ratios = ARRAY_SIZE(ad7606b_oversampling_avail);


> ---
>  drivers/iio/adc/ad7606.c     | 3 +--
>  drivers/iio/adc/ad7606_spi.c | 7 +++----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c
> index d8e3c7a43678c57470a5118715637a68b39125c1..9a124139924e4a4fbbbd234a8514eb77024442b3 100644
> --- a/drivers/iio/adc/ad7606.c
> +++ b/drivers/iio/adc/ad7606.c
> @@ -818,8 +818,7 @@ static int ad7606_write_os_hw(struct iio_dev *indio_dev, int val)
>  
>  	values[0] = val & GENMASK(2, 0);
>  
> -	gpiod_set_array_value(st->gpio_os->ndescs, st->gpio_os->desc,
> -			      st->gpio_os->info, values);
> +	gpiod_multi_set_value_cansleep(st->gpio_os, values);
>  
>  	/* AD7616 requires a reset to update value */
>  	if (st->chip_info->os_req_reset)
> diff --git a/drivers/iio/adc/ad7606_spi.c b/drivers/iio/adc/ad7606_spi.c
> index e2c1475257065c98bf8e2512bda921d6d88a3002..091f31edb6604da3a8ec4d2d5328ac6550faa22c 100644
> --- a/drivers/iio/adc/ad7606_spi.c
> +++ b/drivers/iio/adc/ad7606_spi.c
> @@ -296,10 +296,9 @@ static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)
>  	 * in the device tree, then they need to be set to high,
>  	 * otherwise, they must be hardwired to VDD
>  	 */
> -	if (st->gpio_os) {
> -		gpiod_set_array_value(st->gpio_os->ndescs,
> -				      st->gpio_os->desc, st->gpio_os->info, os);
> -	}
> +	if (st->gpio_os)
> +		gpiod_multi_set_value_cansleep(st->gpio_os, os);
> +
>  	/* OS of 128 and 256 are available only in software mode */
>  	st->oversampling_avail = ad7606B_oversampling_avail;
>  	st->num_os_ratios = ARRAY_SIZE(ad7606B_oversampling_avail);
> 

Re: [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Jonathan Cameron]

On Mon, 10 Feb 2025 16:33:26 -0600
David Lechner <dlechner@baylibre.com> wrote:

> This series was inspired by some minor annoyance I have experienced a
> few times in recent reviews.
> 
> Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> having so many parameters. In most cases, we already have a struct
> gpio_descs that contains the first 3 parameters so we end up with 3 (or
> often even 6) pointer indirections at each call site. Also, people have
> a tendency to want to hard-code the first argument instead of using
> struct gpio_descs.ndescs, often without checking that ndescs >= the
> hard-coded value.
> 
> So I'm proposing that we add a gpiod_multi_set_value_cansleep()
> function that is a wrapper around gpiod_set_array_value_cansleep()
> that has struct gpio_descs as the first parameter to make it a bit
> easier to read the code and avoid the hard-coding temptation.
> 
> I've just done gpiod_multi_set_value_cansleep() for now since there
> were over 10 callers of this one. There aren't as many callers of
> the get and atomic variants, but we can add those too if this seems
> like a useful thing to do.
> 
> Maintainers, if you prefer to have this go through the gpio tree, please
> give your Acked-by:. Several maintainers have also requested an
> immutable branch, so I expect that will be made available. And if there
> is anything leftover after the next kernel release, I will resend it.
I've applied 7-10 to the IIO tree after merging the immutable tag with patch 1.

Jonathan

> 
> ---
> Changes in v3:
> - Added IS_ERR_OR_NULL() check to gpiod_multi_set_value_cansleep()
> - Added new patches to clean up accessing bitmap directly (ts-nbus, ad2s1210).
> - Added function prefix for max3191x.
> - Removed unnecessary braces in ad7606 patch.
> - Picked up additional trailers.
> - Link to v2: https://lore.kernel.org/r/20250206-gpio-set-array-helper-v2-0-1c5f048f79c3@baylibre.com
> 
> Changes in v2:
> - Renamed new function from gpiods_multi_set_value_cansleep() to
>   gpiod_multi_set_value_cansleep()
> - Fixed typo in name of replaced function in all commit messages.
> - Picked up trailers.
> - Link to v1: https://lore.kernel.org/r/20250131-gpio-set-array-helper-v1-0-991c8ccb4d6e@baylibre.com
> 
> ---
> David Lechner (15):
>       gpiolib: add gpiod_multi_set_value_cansleep()
>       auxdisplay: seg-led-gpio: use gpiod_multi_set_value_cansleep
>       bus: ts-nbus: validate ts,data-gpios array size
>       bus: ts-nbus: use gpiod_multi_set_value_cansleep
>       bus: ts-nbus: use bitmap_get_value8()
>       gpio: max3191x: use gpiod_multi_set_value_cansleep
>       iio: adc: ad7606: use gpiod_multi_set_value_cansleep
>       iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
>       iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
>       iio: resolver: ad2s1210: use bitmap_write
>       mmc: pwrseq_simple: use gpiod_multi_set_value_cansleep
>       mux: gpio: use gpiod_multi_set_value_cansleep
>       net: mdio: mux-gpio: use gpiod_multi_set_value_cansleep
>       phy: mapphone-mdm6600: use gpiod_multi_set_value_cansleep
>       ASoC: adau1701: use gpiod_multi_set_value_cansleep
> 
>  drivers/auxdisplay/seg-led-gpio.c           |  3 +--
>  drivers/bus/ts-nbus.c                       | 15 +++++++++------
>  drivers/gpio/gpio-max3191x.c                | 18 +++++++-----------
>  drivers/iio/adc/ad7606.c                    |  3 +--
>  drivers/iio/adc/ad7606_spi.c                |  7 +++----
>  drivers/iio/amplifiers/hmc425a.c            |  3 +--
>  drivers/iio/resolver/ad2s1210.c             | 13 +++++--------
>  drivers/mmc/core/pwrseq_simple.c            |  3 +--
>  drivers/mux/gpio.c                          |  4 +---
>  drivers/net/mdio/mdio-mux-gpio.c            |  3 +--
>  drivers/phy/motorola/phy-mapphone-mdm6600.c |  4 +---
>  include/linux/gpio/consumer.h               | 11 +++++++++++
>  sound/soc/codecs/adau1701.c                 |  4 +---
>  13 files changed, 43 insertions(+), 48 deletions(-)
> ---
> base-commit: df4b2bbff898227db0c14264ac7edd634e79f755
> change-id: 20250131-gpio-set-array-helper-bd4a328370d3
> 
> Best regards,

Re: (subset) [PATCH v3 00/15] gpiolib: add gpiod_multi_set_value_cansleep

[Bartosz Golaszewski]

On Mon, Feb 17, 2025 at 2:11 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 16 Feb 2025 16:55:04 +0100
> Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > On Sun, Feb 16, 2025 at 3:23 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > >
> > > On Fri, 14 Feb 2025 15:37:48 +0100
> > > Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > >
> > > > On Fri, Feb 14, 2025 at 3:35 PM Andy Shevchenko
> > > > <andy.shevchenko@gmail.com> wrote:
> > > > >
> > > > > On Fri, Feb 14, 2025 at 12:21 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > > > > > On Mon, 10 Feb 2025 16:33:26 -0600, David Lechner wrote:
> > > > > > > This series was inspired by some minor annoyance I have experienced a
> > > > > > > few times in recent reviews.
> > > > >
> > > > > ...
> > > > >
> > > > > > [07/15] iio: adc: ad7606: use gpiod_multi_set_value_cansleep
> > > > > >         commit: 8203bc81f025a3fb084357a3d8a6eb3053bc613a
> > > > > > [08/15] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
> > > > > >         commit: e18d359b0a132eb6619836d1bf701f5b3b53299b
> > > > > > [09/15] iio: resolver: ad2s1210: use gpiod_multi_set_value_cansleep
> > > > > >         commit: 7920df29f0dd3aae3acd8a7115d5a25414eed68f
> > > > > > [10/15] iio: resolver: ad2s1210: use bitmap_write
> > > > > >         commit: a67e45055ea90048372066811da7c7fe2d91f9aa
> > > > >
> > > > > FWIW, Jonathan usually takes care of patch queue on weekends.
> > > > > But whatever, it's not my business after all :-)
> > > > >
> > > >
> > > > Too many conflicting suggestions. I just picked up all Acked patches. ¯\_(ツ)_/¯
> > >
> > > Resolution of any issues 'should' be easy enough. Let's keep an eye on how
> > > it goes as other series hit Linux next.  Might be a little work to be done there
> > > and by Linus in next merge window.
> > >
> > > Jonathan
> > >
> >
> > I'm totally fine with removing the iio commits from my queue if you
> > prefer to take them.
> >
> Hi Bartosz,
>
> That's probably going to prove slightly less painful, so please do.
> I'll merge in that immutable tag and pick them up once you've dropped them.
>

Done, you can push your branch out and the next "next" should be ok now.

Bart

Re: [PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[Simon Horman]

On Mon, Feb 10, 2025 at 04:33:31PM -0600, David Lechner wrote:
> Use bitmap_get_value8() instead of accessing the bitmap directly.
> 
> Accessing the bitmap directly is not considered good practice. We now
> have a helper function that can be used instead, so let's use it.
> 
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
u> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/bus/ts-nbus.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
> index b4c9308caf0647a3261071d9527fffce77784af2..beac67f3b820377f8bb1fc4f4ee77e15ee240834 100644
> --- a/drivers/bus/ts-nbus.c
> +++ b/drivers/bus/ts-nbus.c
> @@ -10,6 +10,7 @@
>   * TS-4600 SoM.
>   */
>  
> +#include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
> @@ -107,7 +108,7 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
>  {
>  	DECLARE_BITMAP(values, 8);
>  
> -	values[0] = 0;
> +	bitmap_set_value8(values, byte, 0);

Hi David,

byte doesn't appear to exist in the scope of this function.

I tried this:

	bitmap_set_value8(values, 0, 8);

But when compiling with GCC 14.2.0 I see warnings that values
is used uninitialised - bitmap_set_value8() appears to rely on
it being so.

  CC      drivers/bus/ts-nbus.o
In file included from drivers/bus/ts-nbus.c:13:
In function ‘bitmap_write’,
    inlined from ‘ts_nbus_reset_bus’ at drivers/bus/ts-nbus.c:111:2:
./include/linux/bitmap.h:818:12: error: ‘values’ is used uninitialized [-Werror=uninitialized]
  818 |         map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start));
      |         ~~~^~~~~~~
In file included from ./include/linux/kasan-checks.h:5,
                 from ./include/asm-generic/rwonce.h:26,
                 from ./arch/x86/include/generated/asm/rwonce.h:1,
                 from ./include/linux/compiler.h:344,
                 from ./include/linux/build_bug.h:5,
                 from ./include/linux/bits.h:22,
                 from ./include/linux/bitops.h:6,
                 from ./include/linux/bitmap.h:8:
drivers/bus/ts-nbus.c: In function ‘ts_nbus_reset_bus’:
drivers/bus/ts-nbus.c:109:24: note: ‘values’ declared here
  109 |         DECLARE_BITMAP(values, 8);
      |                        ^~~~~~
./include/linux/types.h:11:23: note: in definition of macro ‘DECLARE_BITMAP’
   11 |         unsigned long name[BITS_TO_LONGS(bits)]
      |                       ^~~~


>  
>  	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
>  	gpiod_set_value_cansleep(ts_nbus->csn, 0);
> @@ -151,7 +152,7 @@ static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
>  {
>  	DECLARE_BITMAP(values, 8);
>  
> -	values[0] = byte;
> +	bitmap_set_value8(values, byte, 8);
>  
>  	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
>  }
> 
> -- 
> 2.43.0
> 

Re: [PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[Andy Shevchenko]

On Thu, Feb 20, 2025 at 12:17 PM Simon Horman <horms@kernel.org> wrote:
> On Mon, Feb 10, 2025 at 04:33:31PM -0600, David Lechner wrote:

...

> But when compiling with GCC 14.2.0 I see warnings that values
> is used uninitialised - bitmap_set_value8() appears to rely on
> it being so.

> In file included from drivers/bus/ts-nbus.c:13:
> In function ‘bitmap_write’,
>     inlined from ‘ts_nbus_reset_bus’ at drivers/bus/ts-nbus.c:111:2:
> ./include/linux/bitmap.h:818:12: error: ‘values’ is used uninitialized [-Werror=uninitialized]
>   818 |         map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start));
>       |         ~~~^~~~~~~

Heh, the compiler is dumb. Even if it's not initialised we do not care.

...

Wondering if the bitmap_write() will work better...

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[David Lechner]

On 2/20/25 6:06 AM, Andy Shevchenko wrote:
> On Thu, Feb 20, 2025 at 12:17 PM Simon Horman <horms@kernel.org> wrote:
>> On Mon, Feb 10, 2025 at 04:33:31PM -0600, David Lechner wrote:
> 
> ...
> 
>> But when compiling with GCC 14.2.0 I see warnings that values
>> is used uninitialised - bitmap_set_value8() appears to rely on
>> it being so.
> 
>> In file included from drivers/bus/ts-nbus.c:13:
>> In function ‘bitmap_write’,
>>     inlined from ‘ts_nbus_reset_bus’ at drivers/bus/ts-nbus.c:111:2:
>> ./include/linux/bitmap.h:818:12: error: ‘values’ is used uninitialized [-Werror=uninitialized]
>>   818 |         map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start));
>>       |         ~~~^~~~~~~
> 
> Heh, the compiler is dumb. Even if it's not initialised we do not care.
> 
> ...
> 
> Wondering if the bitmap_write() will work better...
> 

It is already using that:

#define bitmap_set_value8(map, value, start)		\
	bitmap_write(map, value, start, BITS_PER_BYTE)

Re: [PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[David Lechner]

On 2/20/25 4:17 AM, Simon Horman wrote:
> On Mon, Feb 10, 2025 at 04:33:31PM -0600, David Lechner wrote:
>> Use bitmap_get_value8() instead of accessing the bitmap directly.
>>
>> Accessing the bitmap directly is not considered good practice. We now
>> have a helper function that can be used instead, so let's use it.
>>
>> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> u> Signed-off-by: David Lechner <dlechner@baylibre.com>
>> ---
>>  drivers/bus/ts-nbus.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
>> index b4c9308caf0647a3261071d9527fffce77784af2..beac67f3b820377f8bb1fc4f4ee77e15ee240834 100644
>> --- a/drivers/bus/ts-nbus.c
>> +++ b/drivers/bus/ts-nbus.c
>> @@ -10,6 +10,7 @@
>>   * TS-4600 SoM.
>>   */
>>  
>> +#include <linux/bitmap.h>
>>  #include <linux/bitops.h>
>>  #include <linux/gpio/consumer.h>
>>  #include <linux/kernel.h>
>> @@ -107,7 +108,7 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
>>  {
>>  	DECLARE_BITMAP(values, 8);
>>  
>> -	values[0] = 0;
>> +	bitmap_set_value8(values, byte, 0);
> 
> Hi David,
> 
> byte doesn't appear to exist in the scope of this function.
> 
> I tried this:
> 
> 	bitmap_set_value8(values, 0, 8);
> 
> But when compiling with GCC 14.2.0 I see warnings that values
> is used uninitialised - bitmap_set_value8() appears to rely on
> it being so.

Ah yes, I see the problem (I don't think this driver compiles with
allmodconfig so the compiler didn't catch it for me).

> 
>   CC      drivers/bus/ts-nbus.o
> In file included from drivers/bus/ts-nbus.c:13:
> In function ‘bitmap_write’,
>     inlined from ‘ts_nbus_reset_bus’ at drivers/bus/ts-nbus.c:111:2:
> ./include/linux/bitmap.h:818:12: error: ‘values’ is used uninitialized [-Werror=uninitialized]
>   818 |         map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start));
>       |         ~~~^~~~~~~
> In file included from ./include/linux/kasan-checks.h:5,
>                  from ./include/asm-generic/rwonce.h:26,
>                  from ./arch/x86/include/generated/asm/rwonce.h:1,
>                  from ./include/linux/compiler.h:344,
>                  from ./include/linux/build_bug.h:5,
>                  from ./include/linux/bits.h:22,
>                  from ./include/linux/bitops.h:6,
>                  from ./include/linux/bitmap.h:8:
> drivers/bus/ts-nbus.c: In function ‘ts_nbus_reset_bus’:
> drivers/bus/ts-nbus.c:109:24: note: ‘values’ declared here
>   109 |         DECLARE_BITMAP(values, 8);
>       |                        ^~~~~~
> ./include/linux/types.h:11:23: note: in definition of macro ‘DECLARE_BITMAP’
>    11 |         unsigned long name[BITS_TO_LONGS(bits)]
>       |                       ^~~~
> 
> 
>>  
>>  	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
>>  	gpiod_set_value_cansleep(ts_nbus->csn, 0);
>> @@ -151,7 +152,7 @@ static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
>>  {
>>  	DECLARE_BITMAP(values, 8);

We can fix by zero-initialing the bitmap.

	DECLARE_BITMAP(values, 8) = { };

Would you like me to send a new version of the patch?

>>  
>> -	values[0] = byte;
>> +	bitmap_set_value8(values, byte, 8);
>>  
>>  	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
>>  }
>>
>> -- 
>> 2.43.0
>>

Re: [PATCH v3 10/15] iio: resolver: ad2s1210: use bitmap_write

[David Lechner]

On 2/10/25 4:33 PM, David Lechner wrote:
> Replace bitmap array access with bitmap_write.
> 
> Accessing the bitmap array directly is not recommended and now there is
> a helper function that can be used.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/resolver/ad2s1210.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> index 7f18df790157f1e411fb70de193a49f0677c999f..04879e6d538bce664469c5f6759d8b1cedea16e9 100644
> --- a/drivers/iio/resolver/ad2s1210.c
> +++ b/drivers/iio/resolver/ad2s1210.c
> @@ -46,6 +46,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/bitmap.h>
>  #include <linux/bits.h>
>  #include <linux/cleanup.h>
>  #include <linux/clk.h>
> @@ -180,7 +181,7 @@ static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
>  	if (!gpios)
>  		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
>  
> -	bitmap[0] = mode;
> +	bitmap_write(bitmap, mode, 0, 2);
>  
>  	return gpiod_multi_set_value_cansleep(gpios, bitmap);
>  }
> @@ -1470,7 +1471,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  			return dev_err_probe(dev, -EINVAL,
>  				      "requires exactly 2 resolution-gpios\n");
>  
> -		bitmap[0] = st->resolution;
> +		bitmap_write(bitmap, st->resolution, 0, 2);
>  
>  		ret = gpiod_multi_set_value_cansleep(resolution_gpios, bitmap);
>  		if (ret < 0)
> 

There is actually a bug here pointed out in a similar patch. bitmap_write()
only modifies the bitmap, so this introduces an unintialized use bug. [1]
Here, we only use the bits that we set, so runtime behavior would not actually
be buggy but still best to fully initialize the memory.

I'm a bit surprised that my local compiler and iio/testing both didn't catch that
since GCC 14 caught it in the other driver.

[1]: https://lore.kernel.org/linux-gpio/20250217132152.29d86d6c@jic23-huawei/T/#m3163d2c5db5b7376504d8ad6f23716f1119de761

The fix is simple, we can zero-initialize the bitmap.

diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
index 04879e6d538b..ab860cedecd1 100644
--- a/drivers/iio/resolver/ad2s1210.c
+++ b/drivers/iio/resolver/ad2s1210.c
@@ -176,7 +176,7 @@ struct ad2s1210_state {
 static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
 {
 	struct gpio_descs *gpios = st->mode_gpios;
-	DECLARE_BITMAP(bitmap, 2);
+	DECLARE_BITMAP(bitmap, 2) = { };
 
 	if (!gpios)
 		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
@@ -1427,7 +1427,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
 	struct device *dev = &st->sdev->dev;
 	struct gpio_descs *resolution_gpios;
 	struct gpio_desc *reset_gpio;
-	DECLARE_BITMAP(bitmap, 2);
+	DECLARE_BITMAP(bitmap, 2) = { };
 	int ret;
 
 	/* should not be sampling on startup */

Re: [PATCH v3 05/15] bus: ts-nbus: use bitmap_get_value8()

[Simon Horman]

On Thu, Feb 20, 2025 at 11:32:10AM -0600, David Lechner wrote:
> On 2/20/25 4:17 AM, Simon Horman wrote:
> > On Mon, Feb 10, 2025 at 04:33:31PM -0600, David Lechner wrote:
> >> Use bitmap_get_value8() instead of accessing the bitmap directly.
> >>
> >> Accessing the bitmap directly is not considered good practice. We now
> >> have a helper function that can be used instead, so let's use it.
> >>
> >> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > u> Signed-off-by: David Lechner <dlechner@baylibre.com>
> >> ---
> >>  drivers/bus/ts-nbus.c | 5 +++--
> >>  1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/bus/ts-nbus.c b/drivers/bus/ts-nbus.c
> >> index b4c9308caf0647a3261071d9527fffce77784af2..beac67f3b820377f8bb1fc4f4ee77e15ee240834 100644
> >> --- a/drivers/bus/ts-nbus.c
> >> +++ b/drivers/bus/ts-nbus.c
> >> @@ -10,6 +10,7 @@
> >>   * TS-4600 SoM.
> >>   */
> >>  
> >> +#include <linux/bitmap.h>
> >>  #include <linux/bitops.h>
> >>  #include <linux/gpio/consumer.h>
> >>  #include <linux/kernel.h>
> >> @@ -107,7 +108,7 @@ static void ts_nbus_reset_bus(struct ts_nbus *ts_nbus)
> >>  {
> >>  	DECLARE_BITMAP(values, 8);
> >>  
> >> -	values[0] = 0;
> >> +	bitmap_set_value8(values, byte, 0);
> > 
> > Hi David,
> > 
> > byte doesn't appear to exist in the scope of this function.
> > 
> > I tried this:
> > 
> > 	bitmap_set_value8(values, 0, 8);
> > 
> > But when compiling with GCC 14.2.0 I see warnings that values
> > is used uninitialised - bitmap_set_value8() appears to rely on
> > it being so.
> 
> Ah yes, I see the problem (I don't think this driver compiles with
> allmodconfig so the compiler didn't catch it for me).

Thanks, that would explain things.

FWIIW, I think you can exercise this with allmodconfig by simply running:

  make drivers/bus/ts-nbus.o

> 
> > 
> >   CC      drivers/bus/ts-nbus.o
> > In file included from drivers/bus/ts-nbus.c:13:
> > In function ‘bitmap_write’,
> >     inlined from ‘ts_nbus_reset_bus’ at drivers/bus/ts-nbus.c:111:2:
> > ./include/linux/bitmap.h:818:12: error: ‘values’ is used uninitialized [-Werror=uninitialized]
> >   818 |         map[index] &= (fit ? (~(mask << offset)) : ~BITMAP_FIRST_WORD_MASK(start));
> >       |         ~~~^~~~~~~
> > In file included from ./include/linux/kasan-checks.h:5,
> >                  from ./include/asm-generic/rwonce.h:26,
> >                  from ./arch/x86/include/generated/asm/rwonce.h:1,
> >                  from ./include/linux/compiler.h:344,
> >                  from ./include/linux/build_bug.h:5,
> >                  from ./include/linux/bits.h:22,
> >                  from ./include/linux/bitops.h:6,
> >                  from ./include/linux/bitmap.h:8:
> > drivers/bus/ts-nbus.c: In function ‘ts_nbus_reset_bus’:
> > drivers/bus/ts-nbus.c:109:24: note: ‘values’ declared here
> >   109 |         DECLARE_BITMAP(values, 8);
> >       |                        ^~~~~~
> > ./include/linux/types.h:11:23: note: in definition of macro ‘DECLARE_BITMAP’
> >    11 |         unsigned long name[BITS_TO_LONGS(bits)]
> >       |                       ^~~~
> > 
> > 
> >>  
> >>  	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
> >>  	gpiod_set_value_cansleep(ts_nbus->csn, 0);
> >> @@ -151,7 +152,7 @@ static void ts_nbus_write_byte(struct ts_nbus *ts_nbus, u8 byte)
> >>  {
> >>  	DECLARE_BITMAP(values, 8);
> 
> We can fix by zero-initialing the bitmap.
> 
> 	DECLARE_BITMAP(values, 8) = { };

Thanks, I confirmed that adding that to ts_nbus_reset_bus()
makes the compiler happy. And it seems sensible to me.

I guess that theoretically it should also be added to ts_nbus_write_byte(),
although GCC has nothing to say about that either way.

> Would you like me to send a new version of the patch?

It's not really my call. But I would expect that is a good next step.

> >> -	values[0] = byte;
> >> +	bitmap_set_value8(values, byte, 8);
> >>  
> >>  	gpiod_multi_set_value_cansleep(ts_nbus->data, values);
> >>  }

Re: [PATCH v3 10/15] iio: resolver: ad2s1210: use bitmap_write

[Jonathan Cameron]

On Thu, 20 Feb 2025 14:54:53 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 2/10/25 4:33 PM, David Lechner wrote:
> > Replace bitmap array access with bitmap_write.
> > 
> > Accessing the bitmap array directly is not recommended and now there is
> > a helper function that can be used.
> > 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> >  drivers/iio/resolver/ad2s1210.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> > index 7f18df790157f1e411fb70de193a49f0677c999f..04879e6d538bce664469c5f6759d8b1cedea16e9 100644
> > --- a/drivers/iio/resolver/ad2s1210.c
> > +++ b/drivers/iio/resolver/ad2s1210.c
> > @@ -46,6 +46,7 @@
> >   */
> >  
> >  #include <linux/bitfield.h>
> > +#include <linux/bitmap.h>
> >  #include <linux/bits.h>
> >  #include <linux/cleanup.h>
> >  #include <linux/clk.h>
> > @@ -180,7 +181,7 @@ static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
> >  	if (!gpios)
> >  		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
> >  
> > -	bitmap[0] = mode;
> > +	bitmap_write(bitmap, mode, 0, 2);
> >  
> >  	return gpiod_multi_set_value_cansleep(gpios, bitmap);
> >  }
> > @@ -1470,7 +1471,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
> >  			return dev_err_probe(dev, -EINVAL,
> >  				      "requires exactly 2 resolution-gpios\n");
> >  
> > -		bitmap[0] = st->resolution;
> > +		bitmap_write(bitmap, st->resolution, 0, 2);
> >  
> >  		ret = gpiod_multi_set_value_cansleep(resolution_gpios, bitmap);
> >  		if (ret < 0)
> >   
> 
> There is actually a bug here pointed out in a similar patch. bitmap_write()
> only modifies the bitmap, so this introduces an unintialized use bug. [1]
> Here, we only use the bits that we set, so runtime behavior would not actually
> be buggy but still best to fully initialize the memory.
> 
> I'm a bit surprised that my local compiler and iio/testing both didn't catch that
> since GCC 14 caught it in the other driver.
> 
> [1]: https://lore.kernel.org/linux-gpio/20250217132152.29d86d6c@jic23-huawei/T/#m3163d2c5db5b7376504d8ad6f23716f1119de761
> 
> The fix is simple, we can zero-initialize the bitmap.

Please send this as a fix patch on top as I'd rather not unwind my tree
for just this and the patch is already pushed out on what is mostly a non
rebasing branch.

> 
> diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> index 04879e6d538b..ab860cedecd1 100644
> --- a/drivers/iio/resolver/ad2s1210.c
> +++ b/drivers/iio/resolver/ad2s1210.c
> @@ -176,7 +176,7 @@ struct ad2s1210_state {
>  static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
>  {
>  	struct gpio_descs *gpios = st->mode_gpios;
> -	DECLARE_BITMAP(bitmap, 2);
> +	DECLARE_BITMAP(bitmap, 2) = { };
>  
>  	if (!gpios)
>  		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
> @@ -1427,7 +1427,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  	struct device *dev = &st->sdev->dev;
>  	struct gpio_descs *resolution_gpios;
>  	struct gpio_desc *reset_gpio;
> -	DECLARE_BITMAP(bitmap, 2);
> +	DECLARE_BITMAP(bitmap, 2) = { };
>  	int ret;
>  
>  	/* should not be sampling on startup */
> 

Re: [PATCH v3 10/15] iio: resolver: ad2s1210: use bitmap_write

[Jonathan Cameron]

On Thu, 20 Feb 2025 14:54:53 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 2/10/25 4:33 PM, David Lechner wrote:
> > Replace bitmap array access with bitmap_write.
> > 
> > Accessing the bitmap array directly is not recommended and now there is
> > a helper function that can be used.
> > 
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> >  drivers/iio/resolver/ad2s1210.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> > index 7f18df790157f1e411fb70de193a49f0677c999f..04879e6d538bce664469c5f6759d8b1cedea16e9 100644
> > --- a/drivers/iio/resolver/ad2s1210.c
> > +++ b/drivers/iio/resolver/ad2s1210.c
> > @@ -46,6 +46,7 @@
> >   */
> >  
> >  #include <linux/bitfield.h>
> > +#include <linux/bitmap.h>
> >  #include <linux/bits.h>
> >  #include <linux/cleanup.h>
> >  #include <linux/clk.h>
> > @@ -180,7 +181,7 @@ static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
> >  	if (!gpios)
> >  		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
> >  
> > -	bitmap[0] = mode;
> > +	bitmap_write(bitmap, mode, 0, 2);
> >  
> >  	return gpiod_multi_set_value_cansleep(gpios, bitmap);
> >  }
> > @@ -1470,7 +1471,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
> >  			return dev_err_probe(dev, -EINVAL,
> >  				      "requires exactly 2 resolution-gpios\n");
> >  
> > -		bitmap[0] = st->resolution;
> > +		bitmap_write(bitmap, st->resolution, 0, 2);
> >  
> >  		ret = gpiod_multi_set_value_cansleep(resolution_gpios, bitmap);
> >  		if (ret < 0)
> >   
> 
> There is actually a bug here pointed out in a similar patch. bitmap_write()
> only modifies the bitmap, so this introduces an unintialized use bug. [1]
> Here, we only use the bits that we set, so runtime behavior would not actually
> be buggy but still best to fully initialize the memory.
> 
> I'm a bit surprised that my local compiler and iio/testing both didn't catch that
> since GCC 14 caught it in the other driver.
> 
> [1]: https://lore.kernel.org/linux-gpio/20250217132152.29d86d6c@jic23-huawei/T/#m3163d2c5db5b7376504d8ad6f23716f1119de761
> 
> The fix is simple, we can zero-initialize the bitmap.
Ignore previous. I'm looking at wrong branch. I can tweak this just fine.
Done so and tree pushed out.

> 
> diff --git a/drivers/iio/resolver/ad2s1210.c b/drivers/iio/resolver/ad2s1210.c
> index 04879e6d538b..ab860cedecd1 100644
> --- a/drivers/iio/resolver/ad2s1210.c
> +++ b/drivers/iio/resolver/ad2s1210.c
> @@ -176,7 +176,7 @@ struct ad2s1210_state {
>  static int ad2s1210_set_mode(struct ad2s1210_state *st, enum ad2s1210_mode mode)
>  {
>  	struct gpio_descs *gpios = st->mode_gpios;
> -	DECLARE_BITMAP(bitmap, 2);
> +	DECLARE_BITMAP(bitmap, 2) = { };
>  
>  	if (!gpios)
>  		return mode == st->fixed_mode ? 0 : -EOPNOTSUPP;
> @@ -1427,7 +1427,7 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state *st)
>  	struct device *dev = &st->sdev->dev;
>  	struct gpio_descs *resolution_gpios;
>  	struct gpio_desc *reset_gpio;
> -	DECLARE_BITMAP(bitmap, 2);
> +	DECLARE_BITMAP(bitmap, 2) = { };
>  	int ret;
>  
>  	/* should not be sampling on startup */
>