Thread (15 messages) flat view 15 messages, 2 authors, 4d ago

Re: [PATCH v8 3/8] usb: misc: qcom_eud: add per-path High-Speed PHY control

From: sashiko-bot@kernel.org
Date: 2026-09-12 23:16:50

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Early return in disable_eud skips PHY disablement, leaking High-Speed PHY resources.

Pre-existing issues:
- [High] Calling readl_poll_timeout from a hardirq handler sleeps in atomic context.
--

commit 895071c1614ba92a198085df0882e77e4e133a74
Author: Elson Serrao [off-list ref]

usb: misc: qcom_eud: add per-path High-Speed PHY control

EUD hardware can support multiple High-Speed USB paths, each routed
through its own PHY. This patch restructures the driver to implement
per-path PHY management, ensuring that the PHY for the currently selected
path is properly initialized and powered.

Reviewed-by: Peter Chen <peter.chen@kernel.org>
Reviewed-by: Konrad Dybcio <redacted>
Signed-off-by: Elson Serrao <redacted>
quoted hunk ↗ jump to hunk
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 25a6a8f110ec8..868725ad35e09 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -75,6 +135,8 @@ static int disable_eud(struct eud_chip *priv)
 		return ret;
 
 	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
+	eud_phy_disable(priv);
+
 	return 0;
 }
[Severity: Medium]
Does this early return bypass the newly introduced PHY disablement on error?

If qcom_scm_io_writel() fails, the function returns early and skips
eud_phy_disable(priv). Will this leave the High-Speed PHY resources
initialized and powered on, leaking the hardware state?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
readl_poll_timeout() from a hardirq handler?

When the EUD_INT_SAFE_MODE hardware interrupt fires, the primary hardirq
handler directly calls pet_eud():

drivers/usb/misc/qcom_eud.c:handle_eud_irq() {
	...
	case EUD_INT_SAFE_MODE:
		pet_eud(chip);
		return IRQ_HANDLED;
}

Which then calls readl_poll_timeout() with a non-zero delay:

drivers/usb/misc/qcom_eud.c:pet_eud() {
	...
	ret = readl_poll_timeout(chip->base + EUD_REG_SW_ATTACH_DET,
				reg, (reg == 0), 1, 100);
	...
}

Since readl_poll_timeout() internally uses usleep_range() which sleeps,
can this cause a panic or deadlock by sleeping in an atomic context?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912225301.3396556-1-elson.serrao@oss.qualcomm.com?part=3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help