[PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks

Bartosz Golaszewski posted 12 patches 4 months ago
[PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Bartosz Golaszewski 4 months ago
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

struct gpio_chip now has callbacks for setting line values that return
an integer, allowing to indicate failures. Convert the driver to using
them.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpio-mmio.c | 53 ++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 4841e4ebe7a67d0f954e9a6f995ec5758f124edd..9169eccadb238efe944d494054b1e009f16eee7f 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -211,11 +211,12 @@ static int bgpio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask,
 	return 0;
 }
 
-static void bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
+static int bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
 {
+	return 0;
 }
 
-static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+static int bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 {
 	unsigned long mask = bgpio_line2mask(gc, gpio);
 	unsigned long flags;
@@ -230,10 +231,12 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	gc->write_reg(gc->reg_dat, gc->bgpio_data);
 
 	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
+
+	return 0;
 }
 
-static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
-				 int val)
+static int bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
+				int val)
 {
 	unsigned long mask = bgpio_line2mask(gc, gpio);
 
@@ -241,9 +244,11 @@ static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
 		gc->write_reg(gc->reg_set, mask);
 	else
 		gc->write_reg(gc->reg_clr, mask);
+
+	return 0;
 }
 
-static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
+static int bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
 {
 	unsigned long mask = bgpio_line2mask(gc, gpio);
 	unsigned long flags;
@@ -258,6 +263,8 @@ static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
 	gc->write_reg(gc->reg_set, gc->bgpio_data);
 
 	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
+
+	return 0;
 }
 
 static void bgpio_multiple_get_masks(struct gpio_chip *gc,
@@ -298,21 +305,25 @@ static void bgpio_set_multiple_single_reg(struct gpio_chip *gc,
 	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
 }
 
-static void bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+static int bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
 			       unsigned long *bits)
 {
 	bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_dat);
+
+	return 0;
 }
 
-static void bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
-				   unsigned long *bits)
+static int bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
+				  unsigned long *bits)
 {
 	bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_set);
+
+	return 0;
 }
 
-static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
-					  unsigned long *mask,
-					  unsigned long *bits)
+static int bgpio_set_multiple_with_clear(struct gpio_chip *gc,
+					 unsigned long *mask,
+					 unsigned long *bits)
 {
 	unsigned long set_mask, clear_mask;
 
@@ -322,6 +333,8 @@ static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
 		gc->write_reg(gc->reg_set, set_mask);
 	if (clear_mask)
 		gc->write_reg(gc->reg_clr, clear_mask);
+
+	return 0;
 }
 
 static int bgpio_dir_return(struct gpio_chip *gc, unsigned int gpio, bool dir_out)
@@ -510,18 +523,18 @@ static int bgpio_setup_io(struct gpio_chip *gc,
 	if (set && clr) {
 		gc->reg_set = set;
 		gc->reg_clr = clr;
-		gc->set = bgpio_set_with_clear;
-		gc->set_multiple = bgpio_set_multiple_with_clear;
+		gc->set_rv = bgpio_set_with_clear;
+		gc->set_multiple_rv = bgpio_set_multiple_with_clear;
 	} else if (set && !clr) {
 		gc->reg_set = set;
-		gc->set = bgpio_set_set;
-		gc->set_multiple = bgpio_set_multiple_set;
+		gc->set_rv = bgpio_set_set;
+		gc->set_multiple_rv = bgpio_set_multiple_set;
 	} else if (flags & BGPIOF_NO_OUTPUT) {
-		gc->set = bgpio_set_none;
-		gc->set_multiple = NULL;
+		gc->set_rv = bgpio_set_none;
+		gc->set_multiple_rv = NULL;
 	} else {
-		gc->set = bgpio_set;
-		gc->set_multiple = bgpio_set_multiple;
+		gc->set_rv = bgpio_set;
+		gc->set_multiple_rv = bgpio_set_multiple;
 	}
 
 	if (!(flags & BGPIOF_UNREADABLE_REG_SET) &&
@@ -654,7 +667,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
 	}
 
 	gc->bgpio_data = gc->read_reg(gc->reg_dat);
-	if (gc->set == bgpio_set_set &&
+	if (gc->set_rv == bgpio_set_set &&
 			!(flags & BGPIOF_UNREADABLE_REG_SET))
 		gc->bgpio_data = gc->read_reg(gc->reg_set);
 

-- 
2.48.1
Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Marek Szyprowski 3 months, 3 weeks ago
On 10.06.2025 14:33, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>   drivers/gpio/gpio-mmio.c | 53 ++++++++++++++++++++++++++++++------------------
>   1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
> index 4841e4ebe7a67d0f954e9a6f995ec5758f124edd..9169eccadb238efe944d494054b1e009f16eee7f 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -211,11 +211,12 @@ static int bgpio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask,
>   	return 0;
>   }
>   
> -static void bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
>   {
> +	return 0;
>   }
>   
> -static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   {
>   	unsigned long mask = bgpio_line2mask(gc, gpio);
>   	unsigned long flags;
> @@ -230,10 +231,12 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   	gc->write_reg(gc->reg_dat, gc->bgpio_data);
>   
>   	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> +
> +	return 0;
>   }
>   
> -static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> -				 int val)
> +static int bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> +				int val)
>   {
>   	unsigned long mask = bgpio_line2mask(gc, gpio);
>   
> @@ -241,9 +244,11 @@ static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
>   		gc->write_reg(gc->reg_set, mask);
>   	else
>   		gc->write_reg(gc->reg_clr, mask);
> +
> +	return 0;
>   }
>   
> -static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   {
>   	unsigned long mask = bgpio_line2mask(gc, gpio);
>   	unsigned long flags;
> @@ -258,6 +263,8 @@ static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
>   	gc->write_reg(gc->reg_set, gc->bgpio_data);
>   
>   	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> +
> +	return 0;
>   }
>   
>   static void bgpio_multiple_get_masks(struct gpio_chip *gc,
> @@ -298,21 +305,25 @@ static void bgpio_set_multiple_single_reg(struct gpio_chip *gc,
>   	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>   }
>   
> -static void bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
> +static int bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
>   			       unsigned long *bits)
>   {
>   	bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_dat);
> +
> +	return 0;
>   }
>   
> -static void bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
> -				   unsigned long *bits)
> +static int bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
> +				  unsigned long *bits)
>   {
>   	bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_set);
> +
> +	return 0;
>   }
>   
> -static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> -					  unsigned long *mask,
> -					  unsigned long *bits)
> +static int bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> +					 unsigned long *mask,
> +					 unsigned long *bits)
>   {
>   	unsigned long set_mask, clear_mask;
>   
> @@ -322,6 +333,8 @@ static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
>   		gc->write_reg(gc->reg_set, set_mask);
>   	if (clear_mask)
>   		gc->write_reg(gc->reg_clr, clear_mask);
> +
> +	return 0;
>   }
>   
>   static int bgpio_dir_return(struct gpio_chip *gc, unsigned int gpio, bool dir_out)
> @@ -510,18 +523,18 @@ static int bgpio_setup_io(struct gpio_chip *gc,
>   	if (set && clr) {
>   		gc->reg_set = set;
>   		gc->reg_clr = clr;
> -		gc->set = bgpio_set_with_clear;
> -		gc->set_multiple = bgpio_set_multiple_with_clear;
> +		gc->set_rv = bgpio_set_with_clear;
> +		gc->set_multiple_rv = bgpio_set_multiple_with_clear;
>   	} else if (set && !clr) {
>   		gc->reg_set = set;
> -		gc->set = bgpio_set_set;
> -		gc->set_multiple = bgpio_set_multiple_set;
> +		gc->set_rv = bgpio_set_set;
> +		gc->set_multiple_rv = bgpio_set_multiple_set;
>   	} else if (flags & BGPIOF_NO_OUTPUT) {
> -		gc->set = bgpio_set_none;
> -		gc->set_multiple = NULL;
> +		gc->set_rv = bgpio_set_none;
> +		gc->set_multiple_rv = NULL;
>   	} else {
> -		gc->set = bgpio_set;
> -		gc->set_multiple = bgpio_set_multiple;
> +		gc->set_rv = bgpio_set;
> +		gc->set_multiple_rv = bgpio_set_multiple;
>   	}
>   
>   	if (!(flags & BGPIOF_UNREADABLE_REG_SET) &&
> @@ -654,7 +667,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
>   	}
>   
>   	gc->bgpio_data = gc->read_reg(gc->reg_dat);
> -	if (gc->set == bgpio_set_set &&
> +	if (gc->set_rv == bgpio_set_set &&
>   			!(flags & BGPIOF_UNREADABLE_REG_SET))
>   		gc->bgpio_data = gc->read_reg(gc->reg_set);
>   


A few more changes are needed to avoid NULL pointer dereference 
(observed on RasbperrryPi5), because this driver calls ->set method 
internally:

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 9169eccadb23..57622f45d33e 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -362,7 +362,7 @@ static int bgpio_dir_out_err(struct gpio_chip *gc, 
unsigned int gpio,
  static int bgpio_simple_dir_out(struct gpio_chip *gc, unsigned int gpio,
                                 int val)
  {
-       gc->set(gc, gpio, val);
+       gc->set_rv(gc, gpio, val);

         return bgpio_dir_return(gc, gpio, true);
  }
@@ -427,14 +427,14 @@ static int bgpio_dir_out_dir_first(struct 
gpio_chip *gc, unsigned int gpio,
                                    int val)
  {
         bgpio_dir_out(gc, gpio, val);
-       gc->set(gc, gpio, val);
+       gc->set_rv(gc, gpio, val);
         return bgpio_dir_return(gc, gpio, true);
  }

  static int bgpio_dir_out_val_first(struct gpio_chip *gc, unsigned int 
gpio,
                                    int val)
  {
-       gc->set(gc, gpio, val);
+       gc->set_rv(gc, gpio, val);
         bgpio_dir_out(gc, gpio, val);
         return bgpio_dir_return(gc, gpio, true);
  }

Do You want a formal patch with the above changes, or will You just 
amend them to the updated patch?

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Bartosz Golaszewski 3 months, 3 weeks ago
On Wed, Jun 18, 2025 at 6:43 PM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> A few more changes are needed to avoid NULL pointer dereference
> (observed on RasbperrryPi5), because this driver calls ->set method
> internally:
>
> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
> index 9169eccadb23..57622f45d33e 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -362,7 +362,7 @@ static int bgpio_dir_out_err(struct gpio_chip *gc,
> unsigned int gpio,
>   static int bgpio_simple_dir_out(struct gpio_chip *gc, unsigned int gpio,
>                                  int val)
>   {
> -       gc->set(gc, gpio, val);
> +       gc->set_rv(gc, gpio, val);
>
>          return bgpio_dir_return(gc, gpio, true);
>   }
> @@ -427,14 +427,14 @@ static int bgpio_dir_out_dir_first(struct
> gpio_chip *gc, unsigned int gpio,
>                                     int val)
>   {
>          bgpio_dir_out(gc, gpio, val);
> -       gc->set(gc, gpio, val);
> +       gc->set_rv(gc, gpio, val);
>          return bgpio_dir_return(gc, gpio, true);
>   }
>
>   static int bgpio_dir_out_val_first(struct gpio_chip *gc, unsigned int
> gpio,
>                                     int val)
>   {
> -       gc->set(gc, gpio, val);
> +       gc->set_rv(gc, gpio, val);
>          bgpio_dir_out(gc, gpio, val);
>          return bgpio_dir_return(gc, gpio, true);
>   }
>
> Do You want a formal patch with the above changes, or will You just
> amend them to the updated patch?
>

Thanks, a patch[1] is already up for review. Please give it a try and
leave your Tested-by: if you can.

Bartosz

[1] https://lore.kernel.org/all/20250618-gpio-mmio-fix-setter-v1-2-2578ffb77019@linaro.org/
Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Mark Brown 3 months, 3 weeks ago
On Tue, Jun 10, 2025 at 02:33:11PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.

I'm seeing boot failures on a UDOOq (an i.MX6 based board) in -next
today which bisect to this patch (in -next as b908d35d0003cc7).  We get
a NULL pointer dereference during boot while probing the poweroff driver
for the system:

[    0.443319] Unable to handle kernel NULL pointer dereference at virtual address 00000000 when execute
[    0.443330] [00000000] *pgd=00000000
[    0.443347] Internal error: Oops: 80000005 [#2] SMP ARM

...

[    2.522761]  bgpio_dir_out_val_first from gpiod_direction_output_raw_commit+0x194/0x390
[    2.533330]  gpiod_direction_output_raw_commit from gpiod_find_and_request+0x134/0x434
[    2.541276]  gpiod_find_and_request from gpiod_get_index+0x58/0x70
[    2.547482]  gpiod_get_index from devm_gpiod_get_index+0x10/0x78
[    2.553516]  devm_gpiod_get_index from gpio_poweroff_probe+0xe8/0x174
[    2.559990]  gpio_poweroff_probe from platform_probe+0x5c/0xb4

Full boot log:

    https://lava.sirena.org.uk/scheduler/job/1485011#L752

bisect log:

# bad: [6e5ab6fee68df8c40b338baeae6e269fa25a7e25] Add linux-next specific files for 20250618
# good: [94466168a05ed9aa92afdd54efcd89b3b227650d] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
# good: [3e1c01d06e1f52f78fe00ef26a9cf80dbb0a3115] regulator: rpi-panel-v2: Add shutdown hook
# good: [d9f38d9824bfb1b046d2e720349d2f45959ab184] ASoC: tegra: AHUB: Remove unneeded semicolon
# good: [dce4bc30f42d313b4dc5832316196411b7f07ad0] spi: spi-fsl-dspi: Revert unintended dependency change in config SPI_FSL_DSPI
# good: [47972c1c3315672352f25c68f91dd88543541947] ASoC: Intel: Replace deprecated strcpy() with strscpy()
# good: [5eb8a0d7733d4cd32a776acf1d1aa1c7c01c8a14] ASoC: hdmi-codec: use SND_JACK_AVOUT as jack status
# good: [bb8d8ba4715cb8f997d63d90ba935f6073595df5] ASoC: mediatek: mt8183-afe-pcm: use local `dev` pointer in driver callbacks
# good: [8a5a5cecb79058b608e5562d8998123a3adb313c] ASoC: tas2781: Move the "include linux/debugfs.h" into tas2781.h
# good: [a4eb71ff98c4792f441f108910bd829da7a04092] regulator: rpi-panel-v2: Fix missing OF dependency
# good: [6cafcc53eb5fffd9b9bdfde700bb9bad21e98ed3] spi: spi-mt65xx: Add support for MT6991 Dimensity 9400 SPI IPM
# good: [7e10d7242ea8a5947878880b912ffa5806520705] ASoC: ops: dynamically allocate struct snd_ctl_elem_value
# good: [d6fa0ca959db8efd4462d7beef4bdc5568640fd0] regulator: rpi-panel-v2: Add missing GPIOLIB dependency
# good: [d49305862fdc4d9ff1b1093b4ed7d8e0cb9971b4] regulator: rpi-panel-v2: Add regulator for 7" Raspberry Pi 720x1280
# good: [6ba68e5aa9d5d15c8877a655db279fcfc0b38b04] ASoC: renesas: msiof: Convert to <linux/spi/sh_msiof.h>
# good: [1f5cdb6ab45e1c06ae0953609acbb52f8946b3e8] ASoC: codecs: Add support for Richtek RTQ9124
# good: [c459262159f39e6e6336797feb975799344b749b] spi: spi-pci1xxxx: Add support for 25MHz Clock frequency in C0
# good: [548d770c330cd1027549947a6ea899c56b5bc4e4] regulator: pca9450: Add support for mode operations
# good: [03b778d1994827ea5cc971dbdfbb457bbb7bfa5d] ASOC: rockchip: Use helper function devm_clk_get_enabled()
# good: [267be32b0a7b70cc777f8a46f0904c92c0521d89] ASoC: remove component->id
# good: [111a2c8ab462d77d1519b71b46f13ae1b46920b4] ASoC: imx-card: Use helper function for_each_child_of_node_scoped()
# good: [f6f914893d478b7ba08e5c375de1ced16deb5e92] ASoC: dt-bindings: tas57xx: add tas5753 compatibility
# good: [9a30e332c36c52e92e5316b4a012d45284dedeb5] spi: spi-fsl-dspi: Enable support for S32G platforms
# good: [c95e925daa434ee1a40a86aec6476ce588e4bd77] ASoC: Intel: avs: Add rt5640 machine board
# good: [c8c4694ede7ed42d8d4db0e8927dea9839a3e248] regmap: kunit: Constify regmap_range_cfg array
# good: [e6e8897995a9e6028563ce36c27877e5478c8571] ASoC: qcom: sm8250: Add Fairphone 5 soundcard compatible
# good: [b9ecde0bcf6a99a3ff08496d4ba90a385ebbfd68] ASoC: codecs: wcd939x: Add VDD_PX supply
# good: [ac209bde018fd320b79976657a44c23113181af6] ASoC: tas2781: Drop the unnecessary symbol imply
# good: [ece5d881004f041c2e1493436409dbcbea3ad5f8] ASoC: codecs: wcd939x: Drop unused 'struct wcd939x_priv' fields
# good: [7e17e80c3a7eb2734795f66ba946f933412d597f] Merge branch 'for-6.14/stack-order' into for-next
git bisect start '6e5ab6fee68df8c40b338baeae6e269fa25a7e25' '94466168a05ed9aa92afdd54efcd89b3b227650d' '3e1c01d06e1f52f78fe00ef26a9cf80dbb0a3115' 'd9f38d9824bfb1b046d2e720349d2f45959ab184' 'dce4bc30f42d313b4dc5832316196411b7f07ad0' '47972c1c3315672352f25c68f91dd88543541947' '5eb8a0d7733d4cd32a776acf1d1aa1c7c01c8a14' 'bb8d8ba4715cb8f997d63d90ba935f6073595df5' '8a5a5cecb79058b608e5562d8998123a3adb313c' 'a4eb71ff98c4792f441f108910bd829da7a04092' '6cafcc53eb5fffd9b9bdfde700bb9bad21e98ed3' '7e10d7242ea8a5947878880b912ffa5806520705' 'd6fa0ca959db8efd4462d7beef4bdc5568640fd0' 'd49305862fdc4d9ff1b1093b4ed7d8e0cb9971b4' '6ba68e5aa9d5d15c8877a655db279fcfc0b38b04' '1f5cdb6ab45e1c06ae0953609acbb52f8946b3e8' 'c459262159f39e6e6336797feb975799344b749b' '548d770c330cd1027549947a6ea899c56b5bc4e4' '03b778d1994827ea5cc971dbdfbb457bbb7bfa5d' '267be32b0a7b70cc777f8a46f0904c92c0521d89' '111a2c8ab462d77d1519b71b46f13ae1b46920b4' 'f6f914893d478b7ba08e5c375de1ced16deb5e92' '9a30e332c36c52e92e5316b4a012d45284dedeb5' 'c95e925daa434ee1a40a86aec6476ce588e4bd77' 'c8c4694ede7ed42d8d4db0e8927dea9839a3e248' 'e6e8897995a9e6028563ce36c27877e5478c8571' 'b9ecde0bcf6a99a3ff08496d4ba90a385ebbfd68' 'ac209bde018fd320b79976657a44c23113181af6' 'ece5d881004f041c2e1493436409dbcbea3ad5f8' '7e17e80c3a7eb2734795f66ba946f933412d597f'
# test job: [3e1c01d06e1f52f78fe00ef26a9cf80dbb0a3115] https://lava.sirena.org.uk/scheduler/job/1481662
# test job: [d9f38d9824bfb1b046d2e720349d2f45959ab184] https://lava.sirena.org.uk/scheduler/job/1481666
# test job: [dce4bc30f42d313b4dc5832316196411b7f07ad0] https://lava.sirena.org.uk/scheduler/job/1479434
# test job: [47972c1c3315672352f25c68f91dd88543541947] https://lava.sirena.org.uk/scheduler/job/1479486
# test job: [5eb8a0d7733d4cd32a776acf1d1aa1c7c01c8a14] https://lava.sirena.org.uk/scheduler/job/1474632
# test job: [bb8d8ba4715cb8f997d63d90ba935f6073595df5] https://lava.sirena.org.uk/scheduler/job/1472186
# test job: [8a5a5cecb79058b608e5562d8998123a3adb313c] https://lava.sirena.org.uk/scheduler/job/1471943
# test job: [a4eb71ff98c4792f441f108910bd829da7a04092] https://lava.sirena.org.uk/scheduler/job/1469028
# test job: [6cafcc53eb5fffd9b9bdfde700bb9bad21e98ed3] https://lava.sirena.org.uk/scheduler/job/1469031
# test job: [7e10d7242ea8a5947878880b912ffa5806520705] https://lava.sirena.org.uk/scheduler/job/1466093
# test job: [d6fa0ca959db8efd4462d7beef4bdc5568640fd0] https://lava.sirena.org.uk/scheduler/job/1464729
# test job: [d49305862fdc4d9ff1b1093b4ed7d8e0cb9971b4] https://lava.sirena.org.uk/scheduler/job/1463110
# test job: [6ba68e5aa9d5d15c8877a655db279fcfc0b38b04] https://lava.sirena.org.uk/scheduler/job/1463434
# test job: [1f5cdb6ab45e1c06ae0953609acbb52f8946b3e8] https://lava.sirena.org.uk/scheduler/job/1463022
# test job: [c459262159f39e6e6336797feb975799344b749b] https://lava.sirena.org.uk/scheduler/job/1461222
# test job: [548d770c330cd1027549947a6ea899c56b5bc4e4] https://lava.sirena.org.uk/scheduler/job/1460610
# test job: [03b778d1994827ea5cc971dbdfbb457bbb7bfa5d] https://lava.sirena.org.uk/scheduler/job/1461219
# test job: [267be32b0a7b70cc777f8a46f0904c92c0521d89] https://lava.sirena.org.uk/scheduler/job/1461158
# test job: [111a2c8ab462d77d1519b71b46f13ae1b46920b4] https://lava.sirena.org.uk/scheduler/job/1459889
# test job: [f6f914893d478b7ba08e5c375de1ced16deb5e92] https://lava.sirena.org.uk/scheduler/job/1460442
# test job: [9a30e332c36c52e92e5316b4a012d45284dedeb5] https://lava.sirena.org.uk/scheduler/job/1460919
# test job: [c95e925daa434ee1a40a86aec6476ce588e4bd77] https://lava.sirena.org.uk/scheduler/job/1461120
# test job: [c8c4694ede7ed42d8d4db0e8927dea9839a3e248] https://lava.sirena.org.uk/scheduler/job/1460230
# test job: [e6e8897995a9e6028563ce36c27877e5478c8571] https://lava.sirena.org.uk/scheduler/job/1460605
# test job: [b9ecde0bcf6a99a3ff08496d4ba90a385ebbfd68] https://lava.sirena.org.uk/scheduler/job/1460221
# test job: [ac209bde018fd320b79976657a44c23113181af6] https://lava.sirena.org.uk/scheduler/job/1460665
# test job: [ece5d881004f041c2e1493436409dbcbea3ad5f8] https://lava.sirena.org.uk/scheduler/job/1461108
# test job: [7e17e80c3a7eb2734795f66ba946f933412d597f] https://lava.sirena.org.uk/scheduler/job/1127031
# test job: [6e5ab6fee68df8c40b338baeae6e269fa25a7e25] https://lava.sirena.org.uk/scheduler/job/1485029
# bad: [6e5ab6fee68df8c40b338baeae6e269fa25a7e25] Add linux-next specific files for 20250618
git bisect bad 6e5ab6fee68df8c40b338baeae6e269fa25a7e25
# test job: [5f792b60601e6bf7a9fb037edfc565c3aae4c12d] https://lava.sirena.org.uk/scheduler/job/1485099
# good: [5f792b60601e6bf7a9fb037edfc565c3aae4c12d] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
git bisect good 5f792b60601e6bf7a9fb037edfc565c3aae4c12d
# test job: [4753f57fb1c41066519ab573a011801bf0553cbc] https://lava.sirena.org.uk/scheduler/job/1485186
# good: [4753f57fb1c41066519ab573a011801bf0553cbc] Merge branch 'drm-xe-next' of https://gitlab.freedesktop.org/drm/xe/kernel
git bisect good 4753f57fb1c41066519ab573a011801bf0553cbc
# test job: [b8dcd86b8eadb53eb153ba69434d93f954f4e2d8] https://lava.sirena.org.uk/scheduler/job/1485312
# good: [b8dcd86b8eadb53eb153ba69434d93f954f4e2d8] Merge branch 'driver-core-next' of git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
git bisect good b8dcd86b8eadb53eb153ba69434d93f954f4e2d8
# test job: [86f5e3b87e543714db4274eaaac4fe877e1943b3] https://lava.sirena.org.uk/scheduler/job/1485356
# good: [86f5e3b87e543714db4274eaaac4fe877e1943b3] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux.git
git bisect good 86f5e3b87e543714db4274eaaac4fe877e1943b3
# test job: [6f73eec838ac00ec82b6808bf458cfead784cf21] https://lava.sirena.org.uk/scheduler/job/1485541
# bad: [6f73eec838ac00ec82b6808bf458cfead784cf21] Merge branch 'mhi-next' of git://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git
git bisect bad 6f73eec838ac00ec82b6808bf458cfead784cf21
# test job: [3cbc2e700ace888f02a6059c9f865ef1a0743c29] https://lava.sirena.org.uk/scheduler/job/1485816
# bad: [3cbc2e700ace888f02a6059c9f865ef1a0743c29] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/pinctrl/intel.git
git bisect bad 3cbc2e700ace888f02a6059c9f865ef1a0743c29
# test job: [aaec273c7b511a7826df09123a1fd6e4896c1bfd] https://lava.sirena.org.uk/scheduler/job/1486062
# bad: [aaec273c7b511a7826df09123a1fd6e4896c1bfd] gpio: nomadik: use new GPIO line value setter callbacks
git bisect bad aaec273c7b511a7826df09123a1fd6e4896c1bfd
# test job: [7b2c2f1eb3914f5214a5b2ae966d7d7bb0057582] https://lava.sirena.org.uk/scheduler/job/1486129
# good: [7b2c2f1eb3914f5214a5b2ae966d7d7bb0057582] gpio: Use dev_fwnode() where applicable across drivers
git bisect good 7b2c2f1eb3914f5214a5b2ae966d7d7bb0057582
# test job: [d27746181905c256eced857f4b2c051ac44b0b45] https://lava.sirena.org.uk/scheduler/job/1486292
# bad: [d27746181905c256eced857f4b2c051ac44b0b45] gpio: mm-lantiq: use new GPIO line value setter callbacks
git bisect bad d27746181905c256eced857f4b2c051ac44b0b45
# test job: [367864935785382bab95f5e5a691535d28f5a21b] https://lava.sirena.org.uk/scheduler/job/1486339
# good: [367864935785382bab95f5e5a691535d28f5a21b] gpio: raspberrypi-exp: use new GPIO line value setter callbacks
git bisect good 367864935785382bab95f5e5a691535d28f5a21b
# test job: [b908d35d0003cc75d4ebf7c24a61b07d34e7f5dc] https://lava.sirena.org.uk/scheduler/job/1486428
# bad: [b908d35d0003cc75d4ebf7c24a61b07d34e7f5dc] gpio: mmio: use new GPIO line value setter callbacks
git bisect bad b908d35d0003cc75d4ebf7c24a61b07d34e7f5dc
# test job: [d03b53c9139352b744ed007bf562bd35517bacff] https://lava.sirena.org.uk/scheduler/job/1486540
# good: [d03b53c9139352b744ed007bf562bd35517bacff] dt-bindings: gpio: gpio-xilinx: Mark clocks as required property
git bisect good d03b53c9139352b744ed007bf562bd35517bacff
# first bad commit: [b908d35d0003cc75d4ebf7c24a61b07d34e7f5dc] gpio: mmio: use new GPIO line value setter callbacks
Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Bartosz Golaszewski 3 months, 3 weeks ago
On Wed, Jun 18, 2025 at 6:21 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Jun 10, 2025 at 02:33:11PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > struct gpio_chip now has callbacks for setting line values that return
> > an integer, allowing to indicate failures. Convert the driver to using
> > them.
>
> I'm seeing boot failures on a UDOOq (an i.MX6 based board) in -next
> today which bisect to this patch (in -next as b908d35d0003cc7).  We get
> a NULL pointer dereference during boot while probing the poweroff driver
> for the system:
>
> [    0.443319] Unable to handle kernel NULL pointer dereference at virtual address 00000000 when execute
> [    0.443330] [00000000] *pgd=00000000
> [    0.443347] Internal error: Oops: 80000005 [#2] SMP ARM
>
> ...
>
> [    2.522761]  bgpio_dir_out_val_first from gpiod_direction_output_raw_commit+0x194/0x390
> [    2.533330]  gpiod_direction_output_raw_commit from gpiod_find_and_request+0x134/0x434
> [    2.541276]  gpiod_find_and_request from gpiod_get_index+0x58/0x70
> [    2.547482]  gpiod_get_index from devm_gpiod_get_index+0x10/0x78
> [    2.553516]  devm_gpiod_get_index from gpio_poweroff_probe+0xe8/0x174
> [    2.559990]  gpio_poweroff_probe from platform_probe+0x5c/0xb4
>

Thanks, a patch[1] is already up for review. Please give it a try and
leave your Tested-by: if you can.

Bartosz

[1] https://lore.kernel.org/all/20250618-gpio-mmio-fix-setter-v1-2-2578ffb77019@linaro.org/
Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Klara Modin 3 months, 3 weeks ago
Hi,

On 2025-06-10 14:33:11 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  drivers/gpio/gpio-mmio.c | 53 ++++++++++++++++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
> index 4841e4ebe7a67d0f954e9a6f995ec5758f124edd..9169eccadb238efe944d494054b1e009f16eee7f 100644
> --- a/drivers/gpio/gpio-mmio.c
> +++ b/drivers/gpio/gpio-mmio.c
> @@ -211,11 +211,12 @@ static int bgpio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask,
>  	return 0;
>  }
>  
> -static void bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
>  {
> +	return 0;
>  }
>  
> -static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>  {
>  	unsigned long mask = bgpio_line2mask(gc, gpio);
>  	unsigned long flags;
> @@ -230,10 +231,12 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
>  	gc->write_reg(gc->reg_dat, gc->bgpio_data);
>  
>  	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> +
> +	return 0;
>  }
>  
> -static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> -				 int val)
> +static int bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> +				int val)
>  {
>  	unsigned long mask = bgpio_line2mask(gc, gpio);
>  
> @@ -241,9 +244,11 @@ static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
>  		gc->write_reg(gc->reg_set, mask);
>  	else
>  		gc->write_reg(gc->reg_clr, mask);
> +
> +	return 0;
>  }
>  
> -static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
> +static int bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
>  {
>  	unsigned long mask = bgpio_line2mask(gc, gpio);
>  	unsigned long flags;
> @@ -258,6 +263,8 @@ static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
>  	gc->write_reg(gc->reg_set, gc->bgpio_data);
>  
>  	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> +
> +	return 0;
>  }
>  
>  static void bgpio_multiple_get_masks(struct gpio_chip *gc,
> @@ -298,21 +305,25 @@ static void bgpio_set_multiple_single_reg(struct gpio_chip *gc,
>  	raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
>  }
>  
> -static void bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
> +static int bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
>  			       unsigned long *bits)
>  {
>  	bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_dat);
> +
> +	return 0;
>  }
>  
> -static void bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
> -				   unsigned long *bits)
> +static int bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
> +				  unsigned long *bits)
>  {
>  	bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_set);
> +
> +	return 0;
>  }
>  
> -static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> -					  unsigned long *mask,
> -					  unsigned long *bits)
> +static int bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> +					 unsigned long *mask,
> +					 unsigned long *bits)
>  {
>  	unsigned long set_mask, clear_mask;
>  
> @@ -322,6 +333,8 @@ static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
>  		gc->write_reg(gc->reg_set, set_mask);
>  	if (clear_mask)
>  		gc->write_reg(gc->reg_clr, clear_mask);
> +
> +	return 0;
>  }
>  
>  static int bgpio_dir_return(struct gpio_chip *gc, unsigned int gpio, bool dir_out)
> @@ -510,18 +523,18 @@ static int bgpio_setup_io(struct gpio_chip *gc,
>  	if (set && clr) {
>  		gc->reg_set = set;
>  		gc->reg_clr = clr;
> -		gc->set = bgpio_set_with_clear;
> -		gc->set_multiple = bgpio_set_multiple_with_clear;
> +		gc->set_rv = bgpio_set_with_clear;
> +		gc->set_multiple_rv = bgpio_set_multiple_with_clear;
>  	} else if (set && !clr) {
>  		gc->reg_set = set;
> -		gc->set = bgpio_set_set;
> -		gc->set_multiple = bgpio_set_multiple_set;
> +		gc->set_rv = bgpio_set_set;
> +		gc->set_multiple_rv = bgpio_set_multiple_set;
>  	} else if (flags & BGPIOF_NO_OUTPUT) {
> -		gc->set = bgpio_set_none;
> -		gc->set_multiple = NULL;
> +		gc->set_rv = bgpio_set_none;
> +		gc->set_multiple_rv = NULL;
>  	} else {
> -		gc->set = bgpio_set;
> -		gc->set_multiple = bgpio_set_multiple;
> +		gc->set_rv = bgpio_set;
> +		gc->set_multiple_rv = bgpio_set_multiple;
>  	}
>  
>  	if (!(flags & BGPIOF_UNREADABLE_REG_SET) &&
> @@ -654,7 +667,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
>  	}
>  
>  	gc->bgpio_data = gc->read_reg(gc->reg_dat);
> -	if (gc->set == bgpio_set_set &&
> +	if (gc->set_rv == bgpio_set_set &&
>  			!(flags & BGPIOF_UNREADABLE_REG_SET))
>  		gc->bgpio_data = gc->read_reg(gc->reg_set);
>  
> 
> -- 
> 2.48.1
> 

Isn't this missing to convert gc->set() to gc-set_rv() in several
places?

Without the attached diff I get a null pointer reference on e.g. the
spacemit k1 driver.

Regards,
Klara Modin

--

diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
index 9169eccadb23..57622f45d33e 100644
--- a/drivers/gpio/gpio-mmio.c
+++ b/drivers/gpio/gpio-mmio.c
@@ -362,7 +362,7 @@ static int bgpio_dir_out_err(struct gpio_chip *gc, unsigned int gpio,
 static int bgpio_simple_dir_out(struct gpio_chip *gc, unsigned int gpio,
 				int val)
 {
-	gc->set(gc, gpio, val);
+	gc->set_rv(gc, gpio, val);
 
 	return bgpio_dir_return(gc, gpio, true);
 }
@@ -427,14 +427,14 @@ static int bgpio_dir_out_dir_first(struct gpio_chip *gc, unsigned int gpio,
 				   int val)
 {
 	bgpio_dir_out(gc, gpio, val);
-	gc->set(gc, gpio, val);
+	gc->set_rv(gc, gpio, val);
 	return bgpio_dir_return(gc, gpio, true);
 }
 
 static int bgpio_dir_out_val_first(struct gpio_chip *gc, unsigned int gpio,
 				   int val)
 {
-	gc->set(gc, gpio, val);
+	gc->set_rv(gc, gpio, val);
 	bgpio_dir_out(gc, gpio, val);
 	return bgpio_dir_return(gc, gpio, true);
 }
Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Bartosz Golaszewski 3 months, 3 weeks ago
On Wed, Jun 18, 2025 at 1:59 PM Klara Modin <klarasmodin@gmail.com> wrote:
>
> Hi,
>
> On 2025-06-10 14:33:11 +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > struct gpio_chip now has callbacks for setting line values that return
> > an integer, allowing to indicate failures. Convert the driver to using
> > them.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  drivers/gpio/gpio-mmio.c | 53 ++++++++++++++++++++++++++++++------------------
> >  1 file changed, 33 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-mmio.c b/drivers/gpio/gpio-mmio.c
> > index 4841e4ebe7a67d0f954e9a6f995ec5758f124edd..9169eccadb238efe944d494054b1e009f16eee7f 100644
> > --- a/drivers/gpio/gpio-mmio.c
> > +++ b/drivers/gpio/gpio-mmio.c
> > @@ -211,11 +211,12 @@ static int bgpio_get_multiple_be(struct gpio_chip *gc, unsigned long *mask,
> >       return 0;
> >  }
> >
> > -static void bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
> > +static int bgpio_set_none(struct gpio_chip *gc, unsigned int gpio, int val)
> >  {
> > +     return 0;
> >  }
> >
> > -static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> > +static int bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >  {
> >       unsigned long mask = bgpio_line2mask(gc, gpio);
> >       unsigned long flags;
> > @@ -230,10 +231,12 @@ static void bgpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >       gc->write_reg(gc->reg_dat, gc->bgpio_data);
> >
> >       raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > +
> > +     return 0;
> >  }
> >
> > -static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> > -                              int val)
> > +static int bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> > +                             int val)
> >  {
> >       unsigned long mask = bgpio_line2mask(gc, gpio);
> >
> > @@ -241,9 +244,11 @@ static void bgpio_set_with_clear(struct gpio_chip *gc, unsigned int gpio,
> >               gc->write_reg(gc->reg_set, mask);
> >       else
> >               gc->write_reg(gc->reg_clr, mask);
> > +
> > +     return 0;
> >  }
> >
> > -static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
> > +static int bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >  {
> >       unsigned long mask = bgpio_line2mask(gc, gpio);
> >       unsigned long flags;
> > @@ -258,6 +263,8 @@ static void bgpio_set_set(struct gpio_chip *gc, unsigned int gpio, int val)
> >       gc->write_reg(gc->reg_set, gc->bgpio_data);
> >
> >       raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> > +
> > +     return 0;
> >  }
> >
> >  static void bgpio_multiple_get_masks(struct gpio_chip *gc,
> > @@ -298,21 +305,25 @@ static void bgpio_set_multiple_single_reg(struct gpio_chip *gc,
> >       raw_spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> >  }
> >
> > -static void bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
> > +static int bgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask,
> >                              unsigned long *bits)
> >  {
> >       bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_dat);
> > +
> > +     return 0;
> >  }
> >
> > -static void bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
> > -                                unsigned long *bits)
> > +static int bgpio_set_multiple_set(struct gpio_chip *gc, unsigned long *mask,
> > +                               unsigned long *bits)
> >  {
> >       bgpio_set_multiple_single_reg(gc, mask, bits, gc->reg_set);
> > +
> > +     return 0;
> >  }
> >
> > -static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> > -                                       unsigned long *mask,
> > -                                       unsigned long *bits)
> > +static int bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> > +                                      unsigned long *mask,
> > +                                      unsigned long *bits)
> >  {
> >       unsigned long set_mask, clear_mask;
> >
> > @@ -322,6 +333,8 @@ static void bgpio_set_multiple_with_clear(struct gpio_chip *gc,
> >               gc->write_reg(gc->reg_set, set_mask);
> >       if (clear_mask)
> >               gc->write_reg(gc->reg_clr, clear_mask);
> > +
> > +     return 0;
> >  }
> >
> >  static int bgpio_dir_return(struct gpio_chip *gc, unsigned int gpio, bool dir_out)
> > @@ -510,18 +523,18 @@ static int bgpio_setup_io(struct gpio_chip *gc,
> >       if (set && clr) {
> >               gc->reg_set = set;
> >               gc->reg_clr = clr;
> > -             gc->set = bgpio_set_with_clear;
> > -             gc->set_multiple = bgpio_set_multiple_with_clear;
> > +             gc->set_rv = bgpio_set_with_clear;
> > +             gc->set_multiple_rv = bgpio_set_multiple_with_clear;
> >       } else if (set && !clr) {
> >               gc->reg_set = set;
> > -             gc->set = bgpio_set_set;
> > -             gc->set_multiple = bgpio_set_multiple_set;
> > +             gc->set_rv = bgpio_set_set;
> > +             gc->set_multiple_rv = bgpio_set_multiple_set;
> >       } else if (flags & BGPIOF_NO_OUTPUT) {
> > -             gc->set = bgpio_set_none;
> > -             gc->set_multiple = NULL;
> > +             gc->set_rv = bgpio_set_none;
> > +             gc->set_multiple_rv = NULL;
> >       } else {
> > -             gc->set = bgpio_set;
> > -             gc->set_multiple = bgpio_set_multiple;
> > +             gc->set_rv = bgpio_set;
> > +             gc->set_multiple_rv = bgpio_set_multiple;
> >       }
> >
> >       if (!(flags & BGPIOF_UNREADABLE_REG_SET) &&
> > @@ -654,7 +667,7 @@ int bgpio_init(struct gpio_chip *gc, struct device *dev,
> >       }
> >
> >       gc->bgpio_data = gc->read_reg(gc->reg_dat);
> > -     if (gc->set == bgpio_set_set &&
> > +     if (gc->set_rv == bgpio_set_set &&
> >                       !(flags & BGPIOF_UNREADABLE_REG_SET))
> >               gc->bgpio_data = gc->read_reg(gc->reg_set);
> >
> >
> > --
> > 2.48.1
> >
>
> Isn't this missing to convert gc->set() to gc-set_rv() in several
> places?
>
> Without the attached diff I get a null pointer reference on e.g. the
> spacemit k1 driver.
>

Ah, yes, sorry for this and thanks for the catch. I will send a follow-up.

Bartosz
Re: [PATCH 01/12] gpio: mmio: use new GPIO line value setter callbacks
Posted by Linus Walleij 4 months ago
On Tue, Jun 10, 2025 at 2:33 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> struct gpio_chip now has callbacks for setting line values that return
> an integer, allowing to indicate failures. Convert the driver to using
> them.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

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

Yours,
Linus Walleij