[PATCH v2] gpiolib: handle gpio-hogs only once

[PATCH v2] gpiolib: handle gpio-hogs only once

[Daniel Drake]

Commit d1d564ec49929 ("gpio: move hogs into GPIO core") introduced a
behaviour change that breaks boot on Raspberry Pi 5 when using the
firmware-supplied device tree:

  gpiochip_add_data_with_key: GPIOs 544..575
    (/soc@107c000000/gpio@7d517c00) failed to register, -22
  brcmstb-gpio 107d517c00.gpio: Could not add gpiochip for bank 1
  brcmstb-gpio 107d517c00.gpio: probe with driver brcmstb-gpio failed
    with error -22

gpio-brcmstb registers two gpio_chips against the device tree
node gpio@7d517c00, one for each bank. The firmware-supplied DT includes
a gpio-hog on RP1 RUN, and this gpio-hog is attempted to be applied to
*both* gpio_chips. This succeeds against bank 0 (which hosts the GPIO)
and fails for bank 1 (which does not).

In the previous implementation, failures to apply gpio-hogs were
quietly ignored. In the new code, the error code propagates and causes
probe to fail.

Closely approximate the previous behaviour by using the OF_POPULATED flag
to ensure that each gpio-hog is processed only once. The flag was
previously being set before the gpio-hogs were processed, so as part
of this change, the flag now gets set only after the gpio-hog is actioned.
The handling of gpio-hogs on a DT node with multiple gpio_chips remains a
bit incomplete/unclear, but this at least retains the ability to apply
hogs to the first gpio_chip per node.

Signed-off-by: Daniel Drake <dan@reactivated.net>
---
 drivers/gpio/gpiolib-of.c | 5 -----
 drivers/gpio/gpiolib.c    | 8 ++++++++
 2 files changed, 8 insertions(+), 5 deletions(-)

v2: move OF_POPULATED flag setting to happen after the gpio-hog has
been applied (otherwise all hogs were considered already-applied
and never applied at all, oops!)

This bug is only exposed by the RPi5 firmware-provided DT that has the
gpio-hog. The DT shipped in the mainline kernel does not have the hog
here. I'm not sure to what extent Linux cares about supporting the
downstream firmware DT.

I'm also happy to consider other approaches. This multi-gpiochip setup is
a bit weird and gpio-brcmstb could perhaps be converted to register only a
single gpio_chip covering all banks. I verified that the other drivers
that obviously follow this same multiple-gpiochip pattern
(pinctrl-amlogic-a4, pinctrl-st and pinctrl-stm32) do not seem to be used by
any board DTs that include gpio-hogs.

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 2c923d17541f..813dbcb91f6f 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -1066,11 +1066,6 @@ int of_gpiochip_add(struct gpio_chip *chip)
 
 	of_node_get(np);
 
-	for_each_available_child_of_node_scoped(np, child) {
-		if (of_property_read_bool(child, "gpio-hog"))
-			of_node_set_flag(child, OF_POPULATED);
-	}
-
 	return ret;
 }
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 1e6dce430dca..b02d711289d0 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1031,9 +1031,17 @@ static int gpiochip_hog_lines(struct gpio_chip *gc)
 		if (!fwnode_property_present(fwnode, "gpio-hog"))
 			continue;
 
+		/* The hog may have been handled by another gpio_chip on the same fwnode */
+		if (is_of_node(fwnode) &&
+		    of_node_check_flag(to_of_node(fwnode), OF_POPULATED))
+			continue;
+
 		ret = gpiochip_add_hog(gc, fwnode);
 		if (ret)
 			return ret;
+
+		if (is_of_node(fwnode))
+			of_node_set_flag(to_of_node(fwnode), OF_POPULATED);
 	}
 
 	return 0;
-- 
2.54.0

Re: [PATCH v2] gpiolib: handle gpio-hogs only once

[Bartosz Golaszewski]

On Mon, 8 Jun 2026 23:01:08 +0200, Daniel Drake <dan@reactivated.net> said:
> Commit d1d564ec49929 ("gpio: move hogs into GPIO core") introduced a
> behaviour change that breaks boot on Raspberry Pi 5 when using the
> firmware-supplied device tree:
>
>   gpiochip_add_data_with_key: GPIOs 544..575
>     (/soc@107c000000/gpio@7d517c00) failed to register, -22
>   brcmstb-gpio 107d517c00.gpio: Could not add gpiochip for bank 1
>   brcmstb-gpio 107d517c00.gpio: probe with driver brcmstb-gpio failed
>     with error -22
>
> gpio-brcmstb registers two gpio_chips against the device tree
> node gpio@7d517c00, one for each bank. The firmware-supplied DT includes
> a gpio-hog on RP1 RUN, and this gpio-hog is attempted to be applied to
> *both* gpio_chips. This succeeds against bank 0 (which hosts the GPIO)
> and fails for bank 1 (which does not).
>
> In the previous implementation, failures to apply gpio-hogs were
> quietly ignored. In the new code, the error code propagates and causes
> probe to fail.
>
> Closely approximate the previous behaviour by using the OF_POPULATED flag
> to ensure that each gpio-hog is processed only once. The flag was
> previously being set before the gpio-hogs were processed, so as part
> of this change, the flag now gets set only after the gpio-hog is actioned.
> The handling of gpio-hogs on a DT node with multiple gpio_chips remains a
> bit incomplete/unclear, but this at least retains the ability to apply
> hogs to the first gpio_chip per node.
>
> Signed-off-by: Daniel Drake <dan@reactivated.net>

This needs a Fixes tag.

> ---
>  drivers/gpio/gpiolib-of.c | 5 -----
>  drivers/gpio/gpiolib.c    | 8 ++++++++
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> v2: move OF_POPULATED flag setting to happen after the gpio-hog has
> been applied (otherwise all hogs were considered already-applied
> and never applied at all, oops!)
>
> This bug is only exposed by the RPi5 firmware-provided DT that has the
> gpio-hog. The DT shipped in the mainline kernel does not have the hog
> here. I'm not sure to what extent Linux cares about supporting the
> downstream firmware DT.
>

We care about not breaking working setups, I think this should be fixed.

> I'm also happy to consider other approaches. This multi-gpiochip setup is
> a bit weird and gpio-brcmstb could perhaps be converted to register only a
> single gpio_chip covering all banks. I verified that the other drivers
> that obviously follow this same multiple-gpiochip pattern
> (pinctrl-amlogic-a4, pinctrl-st and pinctrl-stm32) do not seem to be used by
> any board DTs that include gpio-hogs.
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 2c923d17541f..813dbcb91f6f 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -1066,11 +1066,6 @@ int of_gpiochip_add(struct gpio_chip *chip)
>
>  	of_node_get(np);
>
> -	for_each_available_child_of_node_scoped(np, child) {
> -		if (of_property_read_bool(child, "gpio-hog"))
> -			of_node_set_flag(child, OF_POPULATED);
> -	}
> -
>  	return ret;
>  }
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 1e6dce430dca..b02d711289d0 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1031,9 +1031,17 @@ static int gpiochip_hog_lines(struct gpio_chip *gc)
>  		if (!fwnode_property_present(fwnode, "gpio-hog"))
>  			continue;
>
> +		/* The hog may have been handled by another gpio_chip on the same fwnode */
> +		if (is_of_node(fwnode) &&
> +		    of_node_check_flag(to_of_node(fwnode), OF_POPULATED))
> +			continue;
> +
>  		ret = gpiochip_add_hog(gc, fwnode);
>  		if (ret)
>  			return ret;
> +
> +		if (is_of_node(fwnode))
> +			of_node_set_flag(to_of_node(fwnode), OF_POPULATED);

Sashiko correctly points out that on errors, the state will be corrupted. We
could maybe move the clearing of the flag to gpiochip_free_hogs() and track its
state when processing fwnodes in order not to clear it incorrectly?

>  	}
>
>  	return 0;
> --
> 2.54.0
>
>

Thanks,
Bartosz

Re: [PATCH v2] gpiolib: handle gpio-hogs only once

[Daniel Drake]

On 09/06/2026 13:39, Bartosz Golaszewski wrote:
> On Mon, 8 Jun 2026 23:01:08 +0200, Daniel Drake <dan@reactivated.net> said:
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 1e6dce430dca..b02d711289d0 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -1031,9 +1031,17 @@ static int gpiochip_hog_lines(struct gpio_chip *gc)
>>   		if (!fwnode_property_present(fwnode, "gpio-hog"))
>>   			continue;
>>
>> +		/* The hog may have been handled by another gpio_chip on the same fwnode */
>> +		if (is_of_node(fwnode) &&
>> +		    of_node_check_flag(to_of_node(fwnode), OF_POPULATED))
>> +			continue;
>> +
>>   		ret = gpiochip_add_hog(gc, fwnode);
>>   		if (ret)
>>   			return ret;
>> +
>> +		if (is_of_node(fwnode))
>> +			of_node_set_flag(to_of_node(fwnode), OF_POPULATED);
> 
> Sashiko correctly points out that on errors, the state will be corrupted. We
> could maybe move the clearing of the flag to gpiochip_free_hogs() and track its
> state when processing fwnodes in order not to clear it incorrectly?

I guess you are referring to:

> Does setting OF_POPULATED here cause state corruption if a secondary chip on a
> shared node fails to probe?
> When multiple gpio_chip instances share a device node, the first chip processes
> its hogs and sets OF_POPULATED. If a subsequent chip fails probe (for example,
> returning -EPROBE_DEFER), its cleanup path calls of_gpiochip_remove() which
> clears the flag for all hogs.
> If the flag is unconditionally cleared, will the deferred chip attempt to
> process the first chip's hogs on retry, fail due to a mismatch, and
> permanently abort probe?


I don't think this is actually an issue. If we have two gpio_chips 
sharing a device node, a first one with a hog that probes fine and a 
subsequent one that fails during probe, both of the gpio_chips will 
brought down and the flag is cleared. If it was a EPROBE_DEFER case 
which is then retried later, the first chip's hogs will be set up a 2nd 
time when the probe is retried.

It is true that the teardown of the 2nd gpio_chip would erase the 
OF_POPULATED flag of a gpio-hog node that it does not "own", but the 
first gpio_chip would also be torn down at the same time (and 
OF_POPULATED unset a 2nd time). This is not ideal, but harmless as far 
as I can see.

I don't quite follow the suggestion for doing the clearing better in 
gpiochip_free_hogs(). It would be neat if we could go from a hogged 
gpio_desc back to the fwnode, so that we could only unset OF_POPULATED 
on the fwnode at the point when we are really removing the hog, but I 
don't see a way to derive the gpio-hog fwnode from gpio_desc. Also, this 
would be complicated because one gpio-hog node can hog multiple gpios.

Let me know if I'm missing anything or if you have any preference to 
handle this differently!

Thanks
Daniel

Re: [PATCH v2] gpiolib: handle gpio-hogs only once

[Bartosz Golaszewski]

On Tue, 9 Jun 2026 22:48:03 +0200, Daniel Drake <dan@reactivated.net> said:
> On 09/06/2026 13:39, Bartosz Golaszewski wrote:
>> On Mon, 8 Jun 2026 23:01:08 +0200, Daniel Drake <dan@reactivated.net> said:
>>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>>> index 1e6dce430dca..b02d711289d0 100644
>>> --- a/drivers/gpio/gpiolib.c
>>> +++ b/drivers/gpio/gpiolib.c
>>> @@ -1031,9 +1031,17 @@ static int gpiochip_hog_lines(struct gpio_chip *gc)
>>>   		if (!fwnode_property_present(fwnode, "gpio-hog"))
>>>   			continue;
>>>
>>> +		/* The hog may have been handled by another gpio_chip on the same fwnode */
>>> +		if (is_of_node(fwnode) &&
>>> +		    of_node_check_flag(to_of_node(fwnode), OF_POPULATED))
>>> +			continue;
>>> +
>>>   		ret = gpiochip_add_hog(gc, fwnode);
>>>   		if (ret)
>>>   			return ret;
>>> +
>>> +		if (is_of_node(fwnode))
>>> +			of_node_set_flag(to_of_node(fwnode), OF_POPULATED);
>>
>> Sashiko correctly points out that on errors, the state will be corrupted. We
>> could maybe move the clearing of the flag to gpiochip_free_hogs() and track its
>> state when processing fwnodes in order not to clear it incorrectly?
>
> I guess you are referring to:
>
>> Does setting OF_POPULATED here cause state corruption if a secondary chip on a
>> shared node fails to probe?
>> When multiple gpio_chip instances share a device node, the first chip processes
>> its hogs and sets OF_POPULATED. If a subsequent chip fails probe (for example,
>> returning -EPROBE_DEFER), its cleanup path calls of_gpiochip_remove() which
>> clears the flag for all hogs.
>> If the flag is unconditionally cleared, will the deferred chip attempt to
>> process the first chip's hogs on retry, fail due to a mismatch, and
>> permanently abort probe?
>
>
> I don't think this is actually an issue. If we have two gpio_chips
> sharing a device node, a first one with a hog that probes fine and a
> subsequent one that fails during probe, both of the gpio_chips will
> brought down and the flag is cleared. If it was a EPROBE_DEFER case
> which is then retried later, the first chip's hogs will be set up a 2nd
> time when the probe is retried.
>
> It is true that the teardown of the 2nd gpio_chip would erase the
> OF_POPULATED flag of a gpio-hog node that it does not "own", but the
> first gpio_chip would also be torn down at the same time (and
> OF_POPULATED unset a 2nd time). This is not ideal, but harmless as far
> as I can see.
>
> I don't quite follow the suggestion for doing the clearing better in
> gpiochip_free_hogs(). It would be neat if we could go from a hogged
> gpio_desc back to the fwnode, so that we could only unset OF_POPULATED
> on the fwnode at the point when we are really removing the hog, but I
> don't see a way to derive the gpio-hog fwnode from gpio_desc. Also, this
> would be complicated because one gpio-hog node can hog multiple gpios.
>
> Let me know if I'm missing anything or if you have any preference to
> handle this differently!
>
> Thanks
> Daniel
>
>

Ok, thanks for the clarification. Let's give this a chance then.

Bart

Re: [PATCH v2] gpiolib: handle gpio-hogs only once

[Bartosz Golaszewski]

On Mon, 08 Jun 2026 22:01:08 +0100, Daniel Drake wrote:
> Commit d1d564ec49929 ("gpio: move hogs into GPIO core") introduced a
> behaviour change that breaks boot on Raspberry Pi 5 when using the
> firmware-supplied device tree:
> 
>   gpiochip_add_data_with_key: GPIOs 544..575
>     (/soc@107c000000/gpio@7d517c00) failed to register, -22
>   brcmstb-gpio 107d517c00.gpio: Could not add gpiochip for bank 1
>   brcmstb-gpio 107d517c00.gpio: probe with driver brcmstb-gpio failed
>     with error -22
> 
> [...]

Applied, thanks!

[1/1] gpiolib: handle gpio-hogs only once
      https://git.kernel.org/brgl/c/a23226b7c1f69eafd9ced4e037fb51c9758c0501

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>