Re: [PATCH v2 4/9] usb: chipidea: usb2: add optional reset support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double clock disable and hardware reset ordering issue in driver teardown.
--

commit 13027fbf78c576199d06490bac312974df846e8a
Author: Junzhong Pan <panjunzhong@linux.spacemit.com>

usb: chipidea: usb2: add optional reset support

This commit adds optional reset support for the USB2 controller on the 
SpacemiT K1 SoC to maintain hardware compatibility.

> diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
> index 6fae2d4a053a1..f81673edca3c1 100644
> --- a/drivers/usb/chipidea/ci_hdrc_usb2.c
> +++ b/drivers/usb/chipidea/ci_hdrc_usb2.c

[ ... ]

> @@ -75,6 +77,11 @@ static int ci_hdrc_usb2_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(priv->clk),
>  				     "failed to get or enable the clock\n");
>  
> +	priv->reset = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);

[Severity: High]
Could adding this devm-managed reset trigger a hardware hang during driver
unbind?

Since ci_hdrc_usb2_remove() manually disables the clock:

ci_hdrc_usb2_remove() {
        ...
        clk_disable_unprepare(priv->clk);
}

Devres actions execute in reverse order (LIFO) after remove() completes, so
the hardware reset will be asserted automatically by devres after the clock
is already manually disabled. Asserting a reset synchronously without an
active clock can hang the bus or lock up the SoC on many platforms.

Additionally, since the clock was previously updated to be acquired using
devm_clk_get_optional_enabled(), the manual clk_disable_unprepare() in
remove() results in a double-disable of the clock when the device is unbound,
triggering clock framework warnings.

Does this require cleaning up the manual teardown in ci_hdrc_usb2_remove()
to ensure proper ordering?

> +	if (IS_ERR(priv->reset))
> +		return dev_err_probe(dev, PTR_ERR(priv->reset),
> +				     "failed to get or deassert the reset control\n");
> +
>  	ci_pdata->name = dev_name(dev);
>  
>  	priv->ci_pdev = ci_hdrc_add_device(dev, pdev->resource,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-spacemit-k1-usb2-v2-0-f07342ffa051@linux.spacemit.com?part=4