Re: [PATCH] mmc: pwrseq_simple: fix probe failure when CONFIG_RESET_GPIO is enabled

[Philipp Zabel]

On Mi, 2026-03-04 at 08:41 -0800, Bartosz Golaszewski wrote:
> On Fri, 20 Feb 2026 20:47:25 +0100, Sergey Suloev
> <sergey.suloev@xxxxxxxxx> said:
> > The driver uses a heuristic — checking if exactly one entry exists in
> > 'reset-gpios' — to decide whether to attempt reset controller API usage
> > via devm_reset_control_get_optional_shared(). This is semantically
> > incorrect and causes a permanent probe failure when CONFIG_RESET_GPIO
> > is enabled.
> > 
> > When CONFIG_RESET_GPIO=y, __of_reset_control_get() intercepts the
> > 'reset-gpios' DT property and attempts to register the GPIO as a reset
> > controller via __reset_add_reset_gpio_device(). If the GPIO controller
> > is not yet fully initialized at probe time, this returns -ENOENT instead
> > of -EPROBE_DEFER, and the error path does not respect the 'optional'
> 
> If this is what's happening, then it is not the expected behavior[1].

Is this -ENOENT returned because of #gpio-cells != 2 [1]?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/reset/core.c#n883

It looks like it [2].

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/allwinner/sunxi-bananapi-m2-plus.dtsi#n103

> > flag, causing pwrseq_simple to fail permanently rather than defer.
> > This prevents mmc1 from probing on boards that use 'reset-gpios' for
> > WiFi power sequencing, such as BananaPi M2 Magic with AP6212/BCM43430
> > on Allwinner R16.
> > 
> > The correct indicator of reset controller intent is the explicit presence
> > of a 'resets' property in the device tree node, not the count of
> > 'reset-gpios' entries. A single 'reset-gpios' entry is just as likely
> > to be a plain GPIO as multiple entries.
> > 
> 
> Am I getting it right? You have a node that has both the "resets" property
> as well as "reset-gpios" with more than one entry?

According to the mmc-pwrseq-simple binding doc, a "resets" property is
not valid at all. Apparently, this is just a "reset-gpios" property
with a single GPIO with three cells.

> If that's the case then it should be fixed in reset core, not in a user
> because there'll surely be another one in no time.
> 
> > Replace the ngpio == 1 heuristic with an explicit check for the 'resets'
> > DT property, making the behavior unambiguous and immune to
> > CONFIG_RESET_GPIO interference.
> > 
> > Fixes: 73bf4b7381f7 ("mmc: pwrseq_simple: add support for one reset control")
> > Signed-off-by: Sergey Suloev <sergey.suloev@xxxxxxxxx>
> > ---
> >  drivers/mmc/core/pwrseq_simple.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c
> > index 4b47e6c3b04b..780c8818a273 100644
> > --- a/drivers/mmc/core/pwrseq_simple.c
> > +++ b/drivers/mmc/core/pwrseq_simple.c
> > @@ -124,7 +124,6 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
> >  {
> >  	struct mmc_pwrseq_simple *pwrseq;
> >  	struct device *dev = &pdev->dev;
> > -	int ngpio;
> > 
> >  	pwrseq = devm_kzalloc(dev, sizeof(*pwrseq), GFP_KERNEL);
> >  	if (!pwrseq)
> > @@ -134,8 +133,7 @@ static int mmc_pwrseq_simple_probe(struct platform_device *pdev)
> >  	if (IS_ERR(pwrseq->ext_clk) && PTR_ERR(pwrseq->ext_clk) != -ENOENT)
> >  		return dev_err_probe(dev, PTR_ERR(pwrseq->ext_clk), "external clock not ready\n");
> > 
> > -	ngpio = of_count_phandle_with_args(dev->of_node, "reset-gpios", "#gpio-cells");
> > -	if (ngpio == 1) {
> > +	if (device_property_present(dev, "resets")) {

This effectively turns the following into dead code because the binding
does not allow a "resets" property.

> >  		pwrseq->reset_ctrl = devm_reset_control_get_optional_shared(dev, NULL);
> >  		if (IS_ERR(pwrseq->reset_ctrl))
> >  			return dev_err_probe(dev, PTR_ERR(pwrseq->reset_ctrl),
> > --
> > 2.43.0
> > 
> > 
> > 
> 
> I think reset core should check if the client node has both "reset" and
> "reset-gpios".

Why check both? If the "resets" property exists, we use it [3].
"reset-gpios" is only checked as a fallback if the "resets" property is
not present [4].

[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/reset/core.c#n1019
[4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/reset/core.c#n1031

> If so - it should assume the client wants to resolve "resets"
> and only fall-back to "reset-gpios" if there's no "resets" and number of GPIOs
> is 1.

That is exactly what happens right now.

> Philipp, Krzysztof: what do you think?

I think this is a missing feature in reset-gpio.
This driver could fall back to to devm_gpiod_get_array() also if
devm_reset_control_get_optional_shared() returns -ENOENT instead of
erroring out (maybe -ENOTSUPP would be a better return code for [1]?).

regards
Philipp