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

[Neil Armstrong]

Hi,

On 5/2/26 11:56, Krishna Kurapati wrote:
> Modify interrupt handling for EUSB2 Phy targets. Enable DP Interrupt
> when an Low speed device is connnected and enable DM interrupt when
> a High Speed/ Full Speed device is connected.

Could you explain _why_ and not the content of the patch ?

> Signed-off-by: Krishna Kurapati <krishna.kurapati@xxxxxxxxxxxxxxxx>
> ---
> Tested remote wakeupon Glymur device by button press from a headset
> connected to both Type-C and Type-A ports.
>
>   drivers/usb/dwc3/dwc3-qcom.c | 63 ++++++++++++++++++++++++++++++------
>   1 file changed, 54 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index f43f73ac36ff..5956821eab45 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)
>   {
> +	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);
>   	} 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,27 @@ 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,
> +};
> +
> +static const struct dwc3_qcom_platform_data dwc3_qcom_glymur_pdata = {

SM8550 was the first QCom upstream SoC to use eUSB, should it be covered as well like SM8650, X1, ... ?

Thanks,
Neil

> +	.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,glymur-dwc3",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
> +	{
> +		.compatible = "qcom,glymur-dwc3-mp",
> +		.data = &dwc3_qcom_glymur_pdata,
> +	},
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, dwc3_qcom_of_match);