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

[Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>]

On 5/6/2026 4:09 AM, Thinh Nguyen wrote:
> 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.

ACK.

> 
>> +	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.
> 

ACK. Will do it for both the enable and disable calls.

>>   	} 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?
> 

My bad. Yes.

>> +	{
>> +		.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