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

[Bartosz Golaszewski]

On Mon, 8 Jun 2026 23:01:08 +0200, Daniel Drake <dan at 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 at 107c000000/gpio at 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 at 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 at 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