Re: [PATCH v2] usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Wed, May 06, 2026, Krishna Kurapati wrote:
> eUSB2 targets handle wakeup interrupts differently depending on device
> speed when operating in host mode.
> 
> According to the eUSB2 specification, remote wakeup signaling in host
> mode is detected via different data-line assertions based on the
> connected device speed.
> 
> When a low-speed device is connected, the host repeater drives eD+ to
> logic '1' upon detecting a K-state on the USB lines during remote wakeup
> (eUSB2 specification, Section 5.5.14).
> 
> When a full-speed or high-speed device is connected, the host repeater
> drives eD- to logic '1' upon detecting a K-state on the USB line during
> remote wakeup (eUSB2 specification, Sections 5.5.15 and 5.5.18).
> 
> Since the eUSB2 PHY's "DP" and "DM" interrupt lines monitor the eD+ and
> eD- line states, configure the wakeup interrupts accordingly
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
> Changes in v2:
> - Updated commit message
> - added supported eUSB2 targets
> 
> Link to v1:
> https://urldefense.com/v3/__https://lore.kernel.org/all/20260502095616.666938-1-krishna.kurapati@oss.qualcomm.com/__;!!A4F2R9G_pg!eraeWXDT_mjR2i_3v-c7MelUNh1b4K59jTy1pNFh-YWpd9JQ3tUBDOC5L8JVTnPN7U_dLDtRB_qUKyyhoqjeEdZmipvmFXs1TO_61w$ 
> 
> This patch was tested on Glymur.
> 
>  drivers/usb/dwc3/dwc3-qcom.c | 99 ++++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index f43f73ac36ff..4fc0017c697c 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -60,6 +60,10 @@ static const u32 pwr_evnt_irq_stat_reg[DWC3_QCOM_MAX_PORTS] = {
>  	0x238,
>  };
>  
> +struct dwc3_qcom_platform_data {
> +	bool			uses_eusb2_phy;
> +};
> +
>  struct dwc3_qcom_port {
>  	int			qusb2_phy_irq;
>  	int			dp_hs_phy_irq;
> @@ -85,6 +89,7 @@ struct dwc3_qcom {
>  	struct icc_path		*icc_path_apps;
>  
>  	enum usb_role		current_role;
> +	bool			uses_eusb2_phy;
>  };
>  
>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> @@ -272,15 +277,21 @@ 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 i)
>  {

We should rename i to port_index or something more descriptive.

> +	struct dwc3_qcom_port *port = &qcom->ports[i];
> +
>  	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);
> +		dwc3_qcom_disable_wakeup_irq(qcom->uses_eusb2_phy ?
> +					     port->dp_hs_phy_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);
> +		dwc3_qcom_disable_wakeup_irq(qcom->uses_eusb2_phy ?
> +					     port->dm_hs_phy_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);
> @@ -289,8 +300,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 i)
>  {
> +	struct dwc3_qcom_port *port = &qcom->ports[i];
> +
>  	dwc3_qcom_enable_wakeup_irq(port->qusb2_phy_irq, 0);
>  
>  	/*
> @@ -303,11 +316,19 @@ 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,
> +		dwc3_qcom_enable_wakeup_irq(qcom->uses_eusb2_phy ?
> +					    port->dp_hs_phy_irq :
> +					    port->dm_hs_phy_irq,
> +					    qcom->uses_eusb2_phy ?
> +					    IRQ_TYPE_EDGE_RISING :
>  					    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,
> +		dwc3_qcom_enable_wakeup_irq(qcom->uses_eusb2_phy ?
> +					    port->dm_hs_phy_irq :
> +					    port->dp_hs_phy_irq,
> +					    qcom->uses_eusb2_phy ?
> +					    IRQ_TYPE_EDGE_RISING :
>  					    IRQ_TYPE_EDGE_FALLING);

I would just use if-else and some local variable here instead of the
ternary operator for better readability.

>  	} else {
>  		dwc3_qcom_enable_wakeup_irq(port->dp_hs_phy_irq,
> @@ -324,7 +345,7 @@ static void dwc3_qcom_disable_interrupts(struct dwc3_qcom *qcom)
>  	int i;
>  
>  	for (i = 0; i < qcom->num_ports; i++)
> -		dwc3_qcom_disable_port_interrupts(&qcom->ports[i]);
> +		dwc3_qcom_disable_port_interrupts(qcom, i);
>  }
>  
>  static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
> @@ -332,7 +353,7 @@ static void dwc3_qcom_enable_interrupts(struct dwc3_qcom *qcom)
>  	int i;
>  
>  	for (i = 0; i < qcom->num_ports; i++)
> -		dwc3_qcom_enable_port_interrupts(&qcom->ports[i]);
> +		dwc3_qcom_enable_port_interrupts(qcom, i);
>  }
>  
>  static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
> @@ -609,6 +630,7 @@ struct dwc3_glue_ops dwc3_qcom_glue_ops = {
>  
>  static int dwc3_qcom_probe(struct platform_device *pdev)
>  {
> +	const struct dwc3_qcom_platform_data *pdata;
>  	struct dwc3_probe_data	probe_data = {};
>  	struct device		*dev = &pdev->dev;
>  	struct dwc3_qcom	*qcom;
> @@ -624,6 +646,10 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  
>  	qcom->dev = &pdev->dev;
>  
> +	pdata = device_get_match_data(dev);
> +	if (pdata)
> +		qcom->uses_eusb2_phy = pdata->uses_eusb2_phy;
> +
>  	qcom->resets = devm_reset_control_array_get_optional_exclusive(dev);
>  	if (IS_ERR(qcom->resets)) {
>  		return dev_err_probe(&pdev->dev, PTR_ERR(qcom->resets),
> @@ -838,8 +864,63 @@ static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>  	.prepare = pm_sleep_ptr(dwc3_qcom_prepare),
>  };
>  
> +static const struct dwc3_qcom_platform_data dwc3_qcom_pdata = {
> +	.uses_eusb2_phy = false,
> +};

Do we need this pdata for non-eUSB2 targets?

> +
> +static const struct dwc3_qcom_platform_data dwc3_qcom_glymur_pdata = {
> +	.uses_eusb2_phy = true,
> +};
> +
>  static const struct of_device_id dwc3_qcom_of_match[] = {
> -	{ .compatible = "qcom,snps-dwc3" },
> +	{
> +		.compatible = "qcom,snps-dwc3",
> +		.data = &dwc3_qcom_pdata,
> +	},
> +	{
> +		.compatible = "qcom,eliza-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,glymur-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,glymur-dwc3-mp",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,glymur-dwc3-mp",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,kaanapali-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,milos-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,sm8550-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},

Duplicate copy-paste issue?

> +	{
> +		.compatible = "qcom,sm8650-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,sm8750-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,x1e80100-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,x1e80100-dwc3-mp",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, dwc3_qcom_of_match);
> -- 
> 2.34.1
> 

BR,
Thinh