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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing wakeup on disconnect for eUSB2 PHY targets in host mode.
--

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

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

This commit modifies the interrupt handling for eUSB2 PHY targets in host
mode. It updates the wakeup interrupt configuration based on device speed to
correctly detect remote wakeup signaling on the eD+ or eD- lines according
to the eUSB2 specification.

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index f43f73ac36ff1..65996d47da1f0 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c

[ ... ]

> @@ -272,15 +273,23 @@ static void dwc3_qcom_disable_wakeup_irq(int irq)
>  	disable_irq_nosync(irq);
>  }
>  
> -static void dwc3_qcom_disable_port_interrupts(struct dwc3_qcom_port *port)
> +static void dwc3_qcom_disable_port_interrupts(struct dwc3_qcom *qcom, int port_index)
>  {
> +	struct dwc3_qcom_port *port = &qcom->ports[port_index];
> +
>  	dwc3_qcom_disable_wakeup_irq(port->qusb2_phy_irq);
>  
>  	if (port->usb2_speed == USB_SPEED_LOW) {
> -		dwc3_qcom_disable_wakeup_irq(port->dm_hs_phy_irq);
> +		if (qcom->has_eusb2_phy)
> +			dwc3_qcom_disable_wakeup_irq(port->dp_hs_phy_irq);
> +		else
> +			dwc3_qcom_disable_wakeup_irq(port->dm_hs_phy_irq);
>  	} else if ((port->usb2_speed == USB_SPEED_HIGH) ||
>  			(port->usb2_speed == USB_SPEED_FULL)) {
> -		dwc3_qcom_disable_wakeup_irq(port->dp_hs_phy_irq);
> +		if (qcom->has_eusb2_phy)
> +			dwc3_qcom_disable_wakeup_irq(port->dm_hs_phy_irq);

[Severity: Medium]
Will this result in skipping the disconnect interrupt cleanup for eUSB2
PHY targets? It seems we only disable the single interrupt line used for
remote wakeup here, mirroring the omission of the disconnect interrupt in
the enable path.

> +		else
> +			dwc3_qcom_disable_wakeup_irq(port->dp_hs_phy_irq);
>  	} else {
>  		dwc3_qcom_disable_wakeup_irq(port->dp_hs_phy_irq);
>  		dwc3_qcom_disable_wakeup_irq(port->dm_hs_phy_irq);

[ ... ]

> @@ -303,12 +314,20 @@ static void dwc3_qcom_enable_port_interrupts(struct dwc3_qcom_port *port)
>  	 */
>  
>  	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);
> +		else
> +			dwc3_qcom_enable_wakeup_irq(port->dm_hs_phy_irq,
> +						    IRQ_TYPE_EDGE_FALLING);
>  	} else if ((port->usb2_speed == USB_SPEED_HIGH) ||
>  			(port->usb2_speed == USB_SPEED_FULL)) {
> -		dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
> -					    IRQ_TYPE_EDGE_FALLING);
> +		if (qcom->has_eusb2_phy)
> +			dwc3_qcom_enable_wakeup_irq(port->dm_hs_phy_irq,
> +						    IRQ_TYPE_EDGE_RISING);

[Severity: Medium]
Does this setup miss wakeup events when a device is disconnected?

The standard USB configuration uses a falling edge to detect both remote
wakeup and disconnect (SE0 state). By using only a rising edge interrupt
to detect remote wakeup (K-state), it seems we fail to configure a falling
edge on the other line to detect device disconnect.

If an eUSB2 device is disconnected while the host is suspended, it appears
the controller will not wake up, leaving the host out of sync with the
physical port state.

> +		else
> +			dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
> +						    IRQ_TYPE_EDGE_FALLING);
>  	} else {
>  		dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
>  					    IRQ_TYPE_EDGE_RISING);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-eusb2-interrupt-handling-v5-v4-0-d4f993925608@oss.qualcomm.com?part=2