Thread (22 messages) flat view 22 messages, 6 authors, 5d ago

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

From: sashiko-bot@kernel.org
Date: 2026-09-08 10:00:36

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help