Re: [PATCH] ata: ahci_brcm: fix reset refcount leak in brcm_ahci_resume()

[Niklas Cassel <cassel@kernel.org>]

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