Re: [PATCH 2/6] usb: chipidea: usb2: add optional reset support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Teardown order inversion between manual clock disable and devm-managed reset assertion.
--

--- Patch [2]: [PATCH 2/6] usb: chipidea: usb2: add optional reset support ---
commit 5129acb0e56d5e6166124800f0bd1996cf88389b
Author: Junzhong Pan <[email protected]>

usb: chipidea: usb2: add optional reset support

This commit adds optional reset control support for the USB2 controller
found on the SpacemiT K1 SoC.

> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 8ffa1e95d8e86..00d575cd29557 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
[ ... ]
> @@ -80,6 +82,12 @@ static int ci_hdrc_usb2_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	priv->reset = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
> +	if (IS_ERR(priv->reset)) {
> +		ret = PTR_ERR(priv->reset);
> +		goto clk_err;
> +	}
> +

[Severity: High]
Does this create a teardown order inversion later in ci_hdrc_usb2_probe()
or ci_hdrc_usb2_remove()?

Because the reset is acquired using
devm_reset_control_get_optional_exclusive_deasserted(), the devres cleanup
will automatically assert the reset after probe failure or device removal.

However, looking at the rest of ci_hdrc_usb2_probe(), if ci_hdrc_add_device()
fails, the code jumps to clk_err and manually calls clk_disable_unprepare()
before the devres cleanup runs:

ci_hdrc_usb2_probe() {
	...
	priv->ci_pdev = ci_hdrc_add_device(...);
	if (IS_ERR(priv->ci_pdev)) {
		...
		goto clk_err;
	}
	...
clk_err:
	clk_disable_unprepare(priv->clk);
	return ret;
}

This means the reset will be asserted after the clock is disabled. On many
architectures, asserting a peripheral reset while its clock is disabled can
cause bus lockups or system hangs.

This same inversion appears to affect device unbind as well, since
ci_hdrc_usb2_remove() manually disables the clock before devres runs.

Would using devres for the clock enablement ensure standard LIFO teardown
ordering?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260812-b4-spacemit-k1-usb2-v1-0-f28cf29e522b@linux.spacemit.com?part=2