Re: [PATCH v8 5/5] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets - sashiko-bot

From: sashiko-bot@kernel.org
To: "Krishna Kurapati" <krishna.kurapati@oss.qualcomm.com>
Cc: linux-phy@lists.infradead.org, olteanv@gmail.com,
	robh@kernel.org, devicetree@vger.kernel.org,
	neil.armstrong@linaro.org, conor+dt@kernel.org, vkoul@kernel.org
Subject: Re: [PATCH v8 5/5] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets
Date: Sun, 13 Sep 2026 14:53:15 +0000	[thread overview]
Message-ID: <20260913145315.BD2A51F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260913-eusb2-interrupt-v8-v8-5-0f505b9c394b@oss.qualcomm.com>

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 <krishna.kurapati@oss.qualcomm.com>

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.

> 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
[ ... ]
> @@ -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