When brcm_ahci_resume() succeeds with reset_control_reset(), any
subsequent failure in ahci_platform_enable_clks(),
ahci_platform_enable_regulators(), ahci_platform_enable_phys(),
or ahci_platform_resume_host() leaves the shared reset line's
triggered_count incremented by one. On the next attempt to reset
the hardware, atomic_inc_return() sees a count greater than one
and the reset is silently skipped, potentially causing data
corruption or device malfunction.

Add a reset_control_rearm() call in the common error path after
brcm_sata_phys_disable() and ahci_platform_disable_regulators()
to properly balance the triggered_count, matching what the probe
error path already does.

Fixes: c0cdf2ac4b5b ("ata: ahci_brcm: Fix AHCI resources management")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/ata/ahci_brcm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 29be74fedcf0..38c63d73d210 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -415,6 +415,7 @@ static int __maybe_unused brcm_ahci_resume(struct device *dev)
 out_disable_phys:
 	brcm_sata_phys_disable(priv);
 	ahci_platform_disable_regulators(hpriv);
+	reset_control_rearm(priv->rcdev_rescal);
 out_disable_clks:
 	ahci_platform_disable_clks(hpriv);
 	return ret;
-- 
2.34.1

On Wed, Jun 03, 2026 at 10:24:20AM +0000, Wentao Liang wrote:
> When brcm_ahci_resume() succeeds with reset_control_reset(), any
> subsequent failure in ahci_platform_enable_clks(),
> ahci_platform_enable_regulators(), ahci_platform_enable_phys(),
> or ahci_platform_resume_host() leaves the shared reset line's
> triggered_count incremented by one. On the next attempt to reset
> the hardware, atomic_inc_return() sees a count greater than one
> and the reset is silently skipped, potentially causing data
> corruption or device malfunction.
> 
> Add a reset_control_rearm() call in the common error path after
> brcm_sata_phys_disable() and ahci_platform_disable_regulators()
> to properly balance the triggered_count, matching what the probe
> error path already does.
> 
> Fixes: c0cdf2ac4b5b ("ata: ahci_brcm: Fix AHCI resources management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/ata/ahci_brcm.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
> index 29be74fedcf0..38c63d73d210 100644
> --- a/drivers/ata/ahci_brcm.c
> +++ b/drivers/ata/ahci_brcm.c
> @@ -415,6 +415,7 @@ static int __maybe_unused brcm_ahci_resume(struct device *dev)
>  out_disable_phys:
>  	brcm_sata_phys_disable(priv);
>  	ahci_platform_disable_regulators(hpriv);
> +	reset_control_rearm(priv->rcdev_rescal);
>  out_disable_clks:
>  	ahci_platform_disable_clks(hpriv);
>  	return ret;

When writing a fix that has a Fixes-tag, please CC the author of the offending
commit. Note that if you use ./scripts/get_maintainer.pn on a a patch, it will
automatically include the author of the offending commit in the list of people
to CC.


Now you are adding reset_control_rearm() before disabling clocks to the error
handling in brcm_ahci_resume(), however in the error handing in
brcm_ahci_probe(), we call reset_control_rearm() after disabling clocks.
Why should the error handling in brcm_ahci_resume() not match that of
brcm_ahci_probe() ?


I am confused as to why the error handling in brcm_ahci_probe() is calling both
reset_control_rearm() and reset_control_reset().

The documentation for reset_control, explicitly says not to do this:
https://github.com/torvalds/linux/blob/v7.1-rc6/drivers/reset/core.c#L365-L366

And in libahci_platform.c, we always do either:
return reset_control_rearm() or return reset_control_reset():
https://github.com/torvalds/linux/blob/v7.1-rc6/drivers/ata/libahci_platform.c#L188-L193

Too bad that this driver is not using the generic AHCI functions in
libahci_platform.c.


Kind regards,
Niklas

On Thu, Jun 04, 2026 at 12:03:53PM +0200, Niklas Cassel wrote:

(snip)

> The documentation for reset_control, explicitly says not to do this:
> https://github.com/torvalds/linux/blob/v7.1-rc6/drivers/reset/core.c#L365-L366
> 
> And in libahci_platform.c, we always do either:
> return reset_control_rearm() or return reset_control_reset():
> https://github.com/torvalds/linux/blob/v7.1-rc6/drivers/ata/libahci_platform.c#L188-L193

I realize that I am stupid...

The code is doing it on two different reset handles.

My comment that the cleanup in brcm_ahci_resume() should match that in
brcm_ahci_resume() probe still stands.

i.e. you should also add a call to:
reset_control_assert(priv->rcdev_ahci);

And you should do it after disabling clocks, just as brcm_ahci_resume().


Kind regards,
Niklas