Re: [PATCH v8 5/5] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets
From: sashiko-bot@kernel.org
Date: 2026-09-13 14:53:19
Also in:
linux-devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] Probe fails unconditionally if CONFIG_GENERIC_PHY is disabled because the error check misses -ENOSYS. Pre-existing issues: - [High] Missing lock and lifecycle protection when accessing xHCI drvdata leads to NULL pointer dereference during concurrent driver unbind. -- commit fda37af83fa24f9ca3c0472a4d6f4d45d01b3ce6 Author: Krishna Kurapati [off-list ref] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets This patch modifies the interrupt handling for eUSB2 PHY targets in host mode to configure wakeup interrupts differently based on the connected device speed. The driver now queries the PHY type and configures DP/DM edge interrupts accordingly.
quoted hunk ↗ jump to hunk
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index ac68b4218b56b..e6b6beb38fcba 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -683,6 +705,17 @@ static int dwc3_qcom_probe(struct platform_device *pdev) if (ignore_pipe_clk) dwc3_qcom_select_utmi_clk(qcom); + phy = devm_of_phy_get_by_index(dev, dev->of_node, 0); + if (IS_ERR(phy)) { + if (PTR_ERR(phy) != -ENODEV) {
[Severity: High] Will this unconditionally fail to probe when CONFIG_GENERIC_PHY is disabled? If the kernel is built without generic PHY support, devm_of_phy_get_by_index() returns ERR_PTR(-ENOSYS). Since this check only allows -ENODEV as a benign error, an -ENOSYS return will cause the driver to invoke dev_err_probe() and abort initialization. Should this check also handle -ENOSYS to allow graceful fallback when the PHY subsystem is disabled, similar to how the DWC3 core handles it? (e.g., if (PTR_ERR(phy) != -ENODEV && PTR_ERR(phy) != -ENOSYS))
+ ret = dev_err_probe(dev, PTR_ERR(phy), + "failed to get usb2 phy\n"); + goto clk_disable; + } + phy = NULL; + } + qcom->has_eusb2_phy = (phy_get_type(phy) == PHY_TYPE_EUSB2); + qcom->mode = usb_get_dr_mode(dev);
While reviewing the driver context, I noticed a pre-existing issue in the
surrounding codebase:
drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_read_usb2_speed() {
...
/*
* FIXME: Fix this layering violation.
*/
hcd = platform_get_drvdata(dwc->xhci);
#ifdef CONFIG_USB
udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
...
}
[Severity: High]
This isn't a bug introduced by this patch, but does this missing lifecycle
protection for the xHCI drvdata lead to a NULL pointer dereference?
If the child xhci-plat driver fails to probe, or if a user unbinds it via sysfs
(e.g., echo xhci-hcd > /sys/bus/platform/drivers/xhci-hcd/unbind), the driver
core calls dev_set_drvdata(dev, NULL). This releases the PM usage count and
causes the parent dwc3-qcom controller to enter runtime suspend.
When dwc3_qcom_runtime_suspend() then calls dwc3_qcom_read_usb2_speed(), it
blindly retrieves the now-NULL drvdata via platform_get_drvdata() and
immediately dereferences it when accessing hcd->self.root_hub.
Could this race condition trigger a kernel panic during a concurrent driver
unbind?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260913-eusb2-interrupt-v8-v8-0-0f505b9c394b@oss.qualcomm.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy