Re: [PATCH v5 3/5] phy: snps-eusb2: Set phy type to EUSB2

Re: [PATCH v5 3/5] phy: snps-eusb2: Set phy type to EUSB2

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

Hi,

On Thu, Jul 23, 2026 at 04:09:06PM +0530, Krishna Kurapati wrote:
> USB controllers like DWC3 are agnostic of the kind of phy (USB2/eUSB2)
> connected to it. Set the phy type to EUSB2 to let the controller know
> the phy type when queried during its probe.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/phy/phy-snps-eusb2.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> index f90bf7e95463..e139b17bcda5 100644
> --- a/drivers/phy/phy-snps-eusb2.c
> +++ b/drivers/phy/phy-snps-eusb2.c
> @@ -13,6 +13,8 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/reset.h>
>  
> +#include <dt-bindings/phy/phy.h>
> +
>  #define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
>  #define USB_PHY_RST_MASK		GENMASK(1, 0)
>  #define UTMI_PORT_RST_MASK		GENMASK(5, 4)
> @@ -597,6 +599,8 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>  		return PTR_ERR(generic_phy);
>  	}
>  
> +	phy_set_type(generic_phy, PHY_TYPE_EUSB2);

Isn't there a potential race condition if it's handled in this way?

1. phy-snps-eusb2 probe: PHY is registered
2. dwc3 probes
3. phy-snps-eusb2 probe: PHY type is set

Greetings,

-- Sebastian

> +
>  	dev_set_drvdata(dev, phy);
>  	phy_set_drvdata(generic_phy, phy);
>  
> 
> -- 
> 2.34.1
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 3/5] phy: snps-eusb2: Set phy type to EUSB2

[Krishna Kurapati]

On 7/25/2026 3:21 AM, Sebastian Reichel wrote:
> Hi,
> 
> On Thu, Jul 23, 2026 at 04:09:06PM +0530, Krishna Kurapati wrote:
>> USB controllers like DWC3 are agnostic of the kind of phy (USB2/eUSB2)
>> connected to it. Set the phy type to EUSB2 to let the controller know
>> the phy type when queried during its probe.
>>
>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>> ---
>>   drivers/phy/phy-snps-eusb2.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
>> index f90bf7e95463..e139b17bcda5 100644
>> --- a/drivers/phy/phy-snps-eusb2.c
>> +++ b/drivers/phy/phy-snps-eusb2.c
>> @@ -13,6 +13,8 @@
>>   #include <linux/regulator/consumer.h>
>>   #include <linux/reset.h>
>>   
>> +#include <dt-bindings/phy/phy.h>
>> +
>>   #define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
>>   #define USB_PHY_RST_MASK		GENMASK(1, 0)
>>   #define UTMI_PORT_RST_MASK		GENMASK(5, 4)
>> @@ -597,6 +599,8 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
>>   		return PTR_ERR(generic_phy);
>>   	}
>>   
>> +	phy_set_type(generic_phy, PHY_TYPE_EUSB2);
> 
> Isn't there a potential race condition if it's handled in this way?
> 
> 1. phy-snps-eusb2 probe: PHY is registered
> 2. dwc3 probes
> 3. phy-snps-eusb2 probe: PHY type is set
> 

How about we do this get_type call after dwc3_probe call during 
dwc3_qcom_probe. That way we can ensure phy is up for sure since core 
init has to happen as part of dwc3_probe.

Regards,
Krishna,

> Greetings,
> 
> -- Sebastian
> 
>> +
>>   	dev_set_drvdata(dev, phy);
>>   	phy_set_drvdata(generic_phy, phy);
>>   
>>
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH v5 3/5] phy: snps-eusb2: Set phy type to EUSB2

[Thinh Nguyen]

On Sun, Jul 26, 2026, Krishna Kurapati wrote:
> 
> 
> On 7/25/2026 3:21 AM, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Thu, Jul 23, 2026 at 04:09:06PM +0530, Krishna Kurapati wrote:
> > > USB controllers like DWC3 are agnostic of the kind of phy (USB2/eUSB2)
> > > connected to it. Set the phy type to EUSB2 to let the controller know
> > > the phy type when queried during its probe.
> > > 
> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > >   drivers/phy/phy-snps-eusb2.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> > > index f90bf7e95463..e139b17bcda5 100644
> > > --- a/drivers/phy/phy-snps-eusb2.c
> > > +++ b/drivers/phy/phy-snps-eusb2.c
> > > @@ -13,6 +13,8 @@
> > >   #include <linux/regulator/consumer.h>
> > >   #include <linux/reset.h>
> > > +#include <dt-bindings/phy/phy.h>
> > > +
> > >   #define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
> > >   #define USB_PHY_RST_MASK		GENMASK(1, 0)
> > >   #define UTMI_PORT_RST_MASK		GENMASK(5, 4)
> > > @@ -597,6 +599,8 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
> > >   		return PTR_ERR(generic_phy);
> > >   	}
> > > +	phy_set_type(generic_phy, PHY_TYPE_EUSB2);
> > 
> > Isn't there a potential race condition if it's handled in this way?
> > 
> > 1. phy-snps-eusb2 probe: PHY is registered
> > 2. dwc3 probes
> > 3. phy-snps-eusb2 probe: PHY type is set
> > 
> 
> How about we do this get_type call after dwc3_probe call during
> dwc3_qcom_probe. That way we can ensure phy is up for sure since core init
> has to happen as part of dwc3_probe.
> 

There should be no race here. The phy_set_type() is called before the
phy is registered. dwc3-qcom would get -EPROBE_DEFER if it probes before
the phy is registered.

BR,
Thinh

Re: [PATCH v5 3/5] phy: snps-eusb2: Set phy type to EUSB2

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

Hi,

On Tue, Aug 04, 2026 at 10:28:53PM +0000, Thinh Nguyen wrote:
> On Sun, Jul 26, 2026, Krishna Kurapati wrote:
> > On 7/25/2026 3:21 AM, Sebastian Reichel wrote:
> > > On Thu, Jul 23, 2026 at 04:09:06PM +0530, Krishna Kurapati wrote:
> > > > USB controllers like DWC3 are agnostic of the kind of phy (USB2/eUSB2)
> > > > connected to it. Set the phy type to EUSB2 to let the controller know
> > > > the phy type when queried during its probe.
> > > > 
> > > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > > ---
> > > >   drivers/phy/phy-snps-eusb2.c | 4 ++++
> > > >   1 file changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/phy/phy-snps-eusb2.c b/drivers/phy/phy-snps-eusb2.c
> > > > index f90bf7e95463..e139b17bcda5 100644
> > > > --- a/drivers/phy/phy-snps-eusb2.c
> > > > +++ b/drivers/phy/phy-snps-eusb2.c
> > > > @@ -13,6 +13,8 @@
> > > >   #include <linux/regulator/consumer.h>
> > > >   #include <linux/reset.h>
> > > > +#include <dt-bindings/phy/phy.h>
> > > > +
> > > >   #define EXYNOS_USB_PHY_HS_PHY_CTRL_RST	(0x0)
> > > >   #define USB_PHY_RST_MASK		GENMASK(1, 0)
> > > >   #define UTMI_PORT_RST_MASK		GENMASK(5, 4)
> > > > @@ -597,6 +599,8 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev)
> > > >   		return PTR_ERR(generic_phy);
> > > >   	}
> > > > +	phy_set_type(generic_phy, PHY_TYPE_EUSB2);
> > > 
> > > Isn't there a potential race condition if it's handled in this way?
> > > 
> > > 1. phy-snps-eusb2 probe: PHY is registered
> > > 2. dwc3 probes
> > > 3. phy-snps-eusb2 probe: PHY type is set
> > > 
> > 
> > How about we do this get_type call after dwc3_probe call during
> > dwc3_qcom_probe. That way we can ensure phy is up for sure since core init
> > has to happen as part of dwc3_probe.
> 
> There should be no race here. The phy_set_type() is called before the
> phy is registered. dwc3-qcom would get -EPROBE_DEFER if it probes before
> the phy is registered.

Indeed, it was me being stupid. I blame the heat :)

Greetings,

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v5 2/5] include: linux: phy: Add phy attribute "type" and associated helpers

[Dmitry Baryshkov]

On Thu, Jul 23, 2026 at 04:09:05PM +0530, Krishna Kurapati wrote:
> In cases like USB High-speed phys which can be either USB2 or EUSB2, it is
> required to know the type of phy (rather than the operating mode).

Why?

> Add
> support to cache the PHY_TYPE and add the following helpers:
> 
> - phy_set_type() for the phy drivers (like m31_eusb2 or snps-eusb2) to
> declare what type of PHY they are (in this case PHY_TYPE_EUSB2).

PHY type is usually coveyed through the PHY cells in DT. Why do you need
to set it from the driver?

> 
> - phy_get_type() for the consumers (like USB controllers) to query the
> type of phy connected to them.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  include/linux/phy/phy.h | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 2/5] include: linux: phy: Add phy attribute "type" and associated helpers

[Thinh Nguyen]

On Sun, Jul 26, 2026, Dmitry Baryshkov wrote:
> On Thu, Jul 23, 2026 at 04:09:05PM +0530, Krishna Kurapati wrote:
> > In cases like USB High-speed phys which can be either USB2 or EUSB2, it is
> > required to know the type of phy (rather than the operating mode).
> 
> Why?

Perhaps Krishna should expand on this in the commit message. See patch
5/5 for the reason.

> 
> > Add
> > support to cache the PHY_TYPE and add the following helpers:
> > 
> > - phy_set_type() for the phy drivers (like m31_eusb2 or snps-eusb2) to
> > declare what type of PHY they are (in this case PHY_TYPE_EUSB2).
> 
> PHY type is usually coveyed through the PHY cells in DT. Why do you need
> to set it from the driver?

PHY_TYPE_* via phy cells is typically used for combo PHYs for multiple
protocols where cell arg selects the lane. In this case, the eusb2 phy
is a single protocol phy with #phy-cells = <0>. Adding a cell arg just
for the type would require updating all the eusb2 phy bindings and every
dts file referencing it. Introducing a new interface and setting it from
the phy driver is IMHO a cleaner approach.

BR,
Thinh

> 
> > 
> > - phy_get_type() for the consumers (like USB controllers) to query the
> > type of phy connected to them.
> > 
> > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > ---
> >  include/linux/phy/phy.h | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> 
> -- 
> With best wishes
> Dmitry

Re: [PATCH v5 1/5] dt-bindings: phy: Add PHY_TYPE_EUSB2 definition

[Dmitry Baryshkov]

On Thu, Jul 23, 2026 at 04:09:04PM +0530, Krishna Kurapati wrote:
> Add definition for EUSB2 phy type.

Why?

> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  include/dt-bindings/phy/phy.h | 1 +
>  1 file changed, 1 insertion(+)
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 1/5] dt-bindings: phy: Add PHY_TYPE_EUSB2 definition

[Krzysztof Kozlowski]

On Thu, Jul 23, 2026 at 04:09:04PM +0530, Krishna Kurapati wrote:
> Add definition for EUSB2 phy type.

And what is EUSB2 and how does it differ from USB2 which is already
there? I see that USB org defined a standard for such phy, but a very
quick googling suggests difference is only in the voltage, so not a
different type. At the end, this is still a USB2 phy, no?

There might be more differences and that's why you have entire commit
msg instead of repeating the subject and diff.

Basically in all three places - subject, commit msg and diff - you say
the same obvious thing.

Best regards,
Krzysztof

Re: [PATCH v5 4/5] phy: qcom: m31-eusb2: Set phy type to EUSB2

[Thinh Nguyen]

On Thu, Jul 23, 2026, Krishna Kurapati wrote:
> USB controllers like DWC3 are agnostic of the kind of phy (USB2/eUSB2)
> connected to it. Set the phy type to EUSB2 to let the controller know
> the phy type when queried during its probe.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-m31-eusb2.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> index 0bec8657149c..158c3a55ba56 100644
> --- a/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-m31-eusb2.c
> @@ -16,6 +16,8 @@
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  
> +#include <dt-bindings/phy/phy.h>
> +

Same here. This is already included in <linux/phy/phy.h>

BR,
Thinh

>  #include <linux/regulator/consumer.h>
>  
>  #define USB_PHY_UTMI_CTRL0		(0x3c)
> @@ -277,6 +279,8 @@ static int m31eusb2_phy_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(phy->phy),
>  				     "failed to create phy\n");
>  
> +	phy_set_type(phy->phy, PHY_TYPE_EUSB2);
> +
>  	ret = devm_regulator_bulk_get_const(dev, M31_EUSB_NUM_VREGS,
>  					    m31_eusb_phy_vregs, &phy->vregs);
>  	if (ret)
> 
> -- 
> 2.34.1
> 

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

[Thinh Nguyen]

On Thu, Jul 23, 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, query the phy type and configure the wakeup interrupts
> accordingly.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  drivers/usb/dwc3/dwc3-qcom.c | 53 +++++++++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index f43f73ac36ff..4c559b97c627 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -85,6 +85,7 @@ struct dwc3_qcom {
>  	struct icc_path		*icc_path_apps;
>  
>  	enum usb_role		current_role;
> +	bool			has_eusb2_phy;
>  };
>  
>  #define to_dwc3_qcom(d) container_of((d), struct dwc3_qcom, dwc)
> @@ -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);
> +		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);
> @@ -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)
>  	 */
>  
>  	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);
> +		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);
> @@ -324,7 +343,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 +351,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)
> @@ -617,6 +636,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
>  	int			ret;
>  	bool			ignore_pipe_clk;
>  	bool			wakeup_source;
> +	struct phy		*phy;
>  
>  	qcom = devm_kzalloc(&pdev->dev, sizeof(*qcom), GFP_KERNEL);
>  	if (!qcom)
> @@ -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");

Looks like you're getting the first phy? If so, can we just use
devm_of_phy_get_by_index(dev, dev->of_node, 0) instead of hard coding
the names?

Thanks,
Thinh

> +
> +	if (IS_ERR(phy)) {
> +		if (PTR_ERR(phy) != -ENODEV)
> +			return dev_err_probe(dev, PTR_ERR(phy),
> +					     "failed to get usb2 phy\n");
> +		phy = NULL;
> +	}
> +	qcom->has_eusb2_phy = phy && (phy_get_type(phy) == PHY_TYPE_EUSB2);
> +
>  	qcom->mode = usb_get_dr_mode(dev);
>  
>  	if (qcom->mode == USB_DR_MODE_HOST) {
> 
> -- 
> 2.34.1
>