Re: [PATCH v5 5/5] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Resource leak on devm_phy_get() error path in dwc3_qcom_probe().
- [High] Probe fails due to unhandled -ENOSYS when CONFIG_GENERIC_PHY is disabled.
- [Low] Comment describing DP/DM interrupt polarity contradicts new eUSB2 logic.
--

--- Patch [5]: [PATCH v5 5/5] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets ---
commit e7aac8b655de03b8e81bfa727922960500cb9ae3
Author: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>

usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets

This commit modifies the interrupt handling logic in the DWC3 Qualcomm
driver to support eUSB2 PHY targets. It queries the PHY type during
probe and configures remote wakeup interrupt polarities based on the
eUSB2 specification for different device speeds.

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index f43f73ac36ff1..4c559b97c6276 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
[ ... ]
> @@ -289,8 +298,10 @@ static void dwc3_qcom_disable_port_interrupts(struct dwc3_qcom_port *port)
>  	dwc3_qcom_disable_wakeup_irq(port->ss_phy_irq);
>  }
>  
> -static void dwc3_qcom_enable_port_interrupts(struct dwc3_qcom_port *port)
> +static void dwc3_qcom_enable_port_interrupts(struct dwc3_qcom *qcom, int port_index)
>  {
> +	struct dwc3_qcom_port *port = &qcom->ports[port_index];
> +
>  	dwc3_qcom_enable_wakeup_irq(port->qusb2_phy_irq, 0);
>  
>  	/*
> @@ -303,12 +314,20 @@ static void dwc3_qcom_enable_port_interrupts(struct dwc3_qcom_port *port)
>  	 */

[Severity: Low]
This isn't a bug, but does this comment block need to be updated?  The newly
added eUSB2 logic below configures these lines as rising edge, which
contradicts the behavior described in this comment.

>  
>  	if (port->usb2_speed == USB_SPEED_LOW) {
> -		dwc3_qcom_enable_wakeup_irq(port->dm_hs_phy_irq,
> -					    IRQ_TYPE_EDGE_FALLING);
> +		if (qcom->has_eusb2_phy)
> +			dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
> +						    IRQ_TYPE_EDGE_RISING);
[ ... ]
> @@ -683,6 +703,19 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	if (ignore_pipe_clk)
>  		dwc3_qcom_select_utmi_clk(qcom);
>  
> +	if (qcom->num_ports == 1)
> +		phy = devm_phy_get(dev, "usb2-phy");
> +	else
> +		phy = devm_phy_get(dev, "usb2-0");
> +
> +	if (IS_ERR(phy)) {
> +		if (PTR_ERR(phy) != -ENODEV)

[Severity: High]
If the kernel is compiled without CONFIG_GENERIC_PHY, devm_phy_get() will
return -ENOSYS. Since this check only ignores -ENODEV, will this cause the
probe to fail entirely for configurations without generic PHY support?

> +			return dev_err_probe(dev, PTR_ERR(phy),
> +					     "failed to get usb2 phy\n");

[Severity: High]
Does this return directly without jumping to the clk_disable error label?

Earlier in dwc3_qcom_probe(), clk_bulk_prepare_enable() is called.  If
devm_phy_get() fails with an error like -EPROBE_DEFER, will this leak the
clock prepare and enable counts?

> +		phy = NULL;
> +	}
> +	qcom->has_eusb2_phy = phy && (phy_get_type(phy) == PHY_TYPE_EUSB2);
> +
>  	qcom->mode = usb_get_dr_mode(dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-eusb2-int-handling-v5-v5-0-f32ea6c1a318@oss.qualcomm.com?part=5