[PATCH v4 0/2] Modify interrupt handling for eUSB2 Phy targets

[PATCH v4 0/2] Modify interrupt handling for eUSB2 Phy targets

[Krishna Kurapati]

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

Add new property to dt-binding indicating that the USB controller on SoC
has eUSB2 Phys or not.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
Changes in v4:
- Moved to using binding approach as suggested by Thinh

Link to v3:
https://lore.kernel.org/all/20260511094422.3561509-1-krishna.kurapati@oss.qualcomm.com/

Changes in v3:
- Removed multiple glymur-dwc3-mp pdata entries
- Replaced use of ternary operators with if-else

Link to v2:
https://lore.kernel.org/all/20260505194242.1947891-1-krishna.kurapati@oss.qualcomm.com/

Changes in v2:
- Updated commit message
- added supported eUSB2 targets

Link to v1:
https://lore.kernel.org/all/20260502095616.666938-1-krishna.kurapati@oss.qualcomm.com/

---
Krishna Kurapati (2):
      dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy
      usb: dwc3: qcom: Modify interrupt handling for eUSB2 Phy targets

 .../devicetree/bindings/usb/qcom,snps-dwc3.yaml    |  7 ++++
 drivers/usb/dwc3/dwc3-qcom.c                       | 40 ++++++++++++++++------
 2 files changed, 37 insertions(+), 10 deletions(-)
---
base-commit: 62481c92444386d46b3c4a9c3dbcd4917f07d138
change-id: 20260709-eusb2-interrupt-handling-v5-da49ab5d5a84

Best regards,
--  
Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>

[PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[Krishna Kurapati]

Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
or not. This is used to modify dp/dm interrupts during host mode suspend.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
index 8201656b41ed..0c6a839be82f 100644
--- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
@@ -152,6 +152,13 @@ properties:
       HS/FS/LS modes are supported.
     type: boolean
 
+  qcom,has-eusb2-phy:
+    description:
+      If present, it indicates that the controller is working with eUSB2
+      phy. This property is used to modify dp/dm interrupt configurations
+      while entering suspend.
+    type: boolean
+
   wakeup-source: true
 
 required:

-- 
2.34.1

Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `qcom,has-eusb2-phy` property introduces a redundant boolean flag that duplicates existing hardware topology information, and its description incorrectly encodes software behavior.
- [Medium] The driver implementation for the newly added device tree property is missing from the patch series, despite claims in the commit message.
--

--- Patch [1]: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy ---
commit 9b14e7faf16aedf430d06c05d4c7f31252c58f4c
Author: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>

dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
or not. This is used to modify dp/dm interrupts during host mode suspend.

> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> index 8201656b41ed7..0c6a839be82fd 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> @@ -152,6 +152,13 @@ properties:
>        HS/FS/LS modes are supported.
>      type: boolean
>  
> +  qcom,has-eusb2-phy:
> +    description:
> +      If present, it indicates that the controller is working with eUSB2
> +      phy. This property is used to modify dp/dm interrupt configurations
> +      while entering suspend.
> +    type: boolean

[Severity: Medium]
Is this new property strictly necessary to describe the hardware topology?

The hardware topology appears to already be fully described. The standard
phys property points to a PHY node which has its own specific compatible
string. Could the driver deduce the PHY type by querying the linked PHY node
or by using the controller's SoC-specific compatible string, rather than
relying on a custom boolean flag?

Also, the description explicitly mentions software policy ("used to modify
dp/dm interrupt configurations while entering suspend"). Should this be
updated to describe the hardware rather than how the OS driver behaves?

[Severity: Medium]
The commit message mentions that this property "is used to modify dp/dm
interrupts during host mode suspend", but the driver implementation for this
feature seems to be missing from the patch.

Should the driver changes in drivers/usb/ that actually parse and utilize
this property be included in the same series to allow for a complete review?

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

Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[Dmitry Baryshkov]

On Thu, Jul 09, 2026 at 02:22:36PM +0530, Krishna Kurapati wrote:
> Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
> or not. This is used to modify dp/dm interrupts during host mode suspend.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> index 8201656b41ed..0c6a839be82f 100644
> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> @@ -152,6 +152,13 @@ properties:
>        HS/FS/LS modes are supported.
>      type: boolean
>  
> +  qcom,has-eusb2-phy:
> +    description:
> +      If present, it indicates that the controller is working with eUSB2
> +      phy. This property is used to modify dp/dm interrupt configurations
> +      while entering suspend.
> +    type: boolean

I still have exactly the same concern. This information is already
present in the DT.

> +
>    wakeup-source: true
>  
>  required:
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[Thinh Nguyen]

On Thu, Jul 09, 2026, Dmitry Baryshkov wrote:
> On Thu, Jul 09, 2026 at 02:22:36PM +0530, Krishna Kurapati wrote:
> > Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
> > or not. This is used to modify dp/dm interrupts during host mode suspend.
> > 
> > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > ---
> >  Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> > index 8201656b41ed..0c6a839be82f 100644
> > --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> > +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> > @@ -152,6 +152,13 @@ properties:
> >        HS/FS/LS modes are supported.
> >      type: boolean
> >  
> > +  qcom,has-eusb2-phy:
> > +    description:
> > +      If present, it indicates that the controller is working with eUSB2
> > +      phy. This property is used to modify dp/dm interrupt configurations
> > +      while entering suspend.
> > +    type: boolean
> 
> I still have exactly the same concern. This information is already
> present in the DT.
> 

Could you clarify what you mean by "this information is already present
in the DT"? Are you suggesting we use the dwc3 node's compatible string
to infer the phy type?

The dwc3 glue compatible describes the controller, not the phy. They are
separate IP blocks. A board variant could reuse the same dwc3 compatible
with a different phy attached, making that inference silently wrong.

The phy type is described somewhere else in the DT (in the phy node
itself), referenced via the phy's phandle on the dwc3 core child node.
We could walk that phandle and check whether the phy node's compatible
contains "eusb2". However, that relies on a naming convention that isn't
enforced anywhere. All current eusb2 phy compatibles happen to follow it
(qcom,sm8550-snps-eusb2-phy, qcom,sm8750-m31-eusb2-phy,
samsung,exynos2200-eusb2-phy, etc.), but a future phy driver could break
that assumption silently. The alternative is maintaining an explicit
list of eusb2 compatibles inside dwc3-qcom.c just moves the maintenance
burden into the wrong place.

The "qcom,has-eusb2-phy" on the dwc3 node is explicit. I think it's the
right approach here. If you have a better suggestion, please share with
us.

Thanks,
Thinh

Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[Krzysztof Kozlowski]

On Fri, Jul 10, 2026 at 01:52:45AM +0000, Thinh Nguyen wrote:
> On Thu, Jul 09, 2026, Dmitry Baryshkov wrote:
> > On Thu, Jul 09, 2026 at 02:22:36PM +0530, Krishna Kurapati wrote:
> > > Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
> > > or not. This is used to modify dp/dm interrupts during host mode suspend.
> > > 
> > > Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> > > ---
> > >  Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> > > index 8201656b41ed..0c6a839be82f 100644
> > > --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> > > @@ -152,6 +152,13 @@ properties:
> > >        HS/FS/LS modes are supported.
> > >      type: boolean
> > >  
> > > +  qcom,has-eusb2-phy:
> > > +    description:
> > > +      If present, it indicates that the controller is working with eUSB2
> > > +      phy. This property is used to modify dp/dm interrupt configurations
> > > +      while entering suspend.
> > > +    type: boolean
> > 
> > I still have exactly the same concern. This information is already
> > present in the DT.
> > 
> 
> Could you clarify what you mean by "this information is already present
> in the DT"? Are you suggesting we use the dwc3 node's compatible string
> to infer the phy type?
> 
> The dwc3 glue compatible describes the controller, not the phy. They are
> separate IP blocks. A board variant could reuse the same dwc3 compatible
> with a different phy attached, making that inference silently wrong.
> 
> The phy type is described somewhere else in the DT (in the phy node
> itself), referenced via the phy's phandle on the dwc3 core child node.
> We could walk that phandle and check whether the phy node's compatible
> contains "eusb2". However, that relies on a naming convention that isn't
> enforced anywhere. All current eusb2 phy compatibles happen to follow it
> (qcom,sm8550-snps-eusb2-phy, qcom,sm8750-m31-eusb2-phy,
> samsung,exynos2200-eusb2-phy, etc.), but a future phy driver could break

You have a fixed/finite list of phys which needs something special, so
that information as Dmitry said is in DT already.

Anyway, the commit msg and property field have insufficient description
of actual hardware problem being represented by this property, so it
looks way too much as SW policy.

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[Krzysztof Kozlowski]

On 10/07/2026 09:16, Krzysztof Kozlowski wrote:
> On Fri, Jul 10, 2026 at 01:52:45AM +0000, Thinh Nguyen wrote:
>> On Thu, Jul 09, 2026, Dmitry Baryshkov wrote:
>>> On Thu, Jul 09, 2026 at 02:22:36PM +0530, Krishna Kurapati wrote:
>>>> Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
>>>> or not. This is used to modify dp/dm interrupts during host mode suspend.
>>>>
>>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>>>> index 8201656b41ed..0c6a839be82f 100644
>>>> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
>>>> @@ -152,6 +152,13 @@ properties:
>>>>        HS/FS/LS modes are supported.
>>>>      type: boolean
>>>>  
>>>> +  qcom,has-eusb2-phy:
>>>> +    description:
>>>> +      If present, it indicates that the controller is working with eUSB2
>>>> +      phy. This property is used to modify dp/dm interrupt configurations
>>>> +      while entering suspend.
>>>> +    type: boolean
>>>
>>> I still have exactly the same concern. This information is already
>>> present in the DT.
>>>
>>
>> Could you clarify what you mean by "this information is already present
>> in the DT"? Are you suggesting we use the dwc3 node's compatible string
>> to infer the phy type?
>>
>> The dwc3 glue compatible describes the controller, not the phy. They are
>> separate IP blocks. A board variant could reuse the same dwc3 compatible
>> with a different phy attached, making that inference silently wrong.
>>
>> The phy type is described somewhere else in the DT (in the phy node
>> itself), referenced via the phy's phandle on the dwc3 core child node.
>> We could walk that phandle and check whether the phy node's compatible
>> contains "eusb2". However, that relies on a naming convention that isn't
>> enforced anywhere. All current eusb2 phy compatibles happen to follow it
>> (qcom,sm8550-snps-eusb2-phy, qcom,sm8750-m31-eusb2-phy,
>> samsung,exynos2200-eusb2-phy, etc.), but a future phy driver could break
> 
> You have a fixed/finite list of phys which needs something special, so
> that information as Dmitry said is in DT already.
> 
> Anyway, the commit msg and property field have insufficient description
> of actual hardware problem being represented by this property, so it
> looks way too much as SW policy.


And Dmitry already said this at v3.

And Sashiko gave the same review:
"[Medium] The `qcom,has-eusb2-phy` property introduces a redundant
boolean flag that duplicates existing hardware topology information, and
its description incorrectly encodes software behavior."


Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy

[Thinh Nguyen]

On Fri, Jul 10, 2026, Krzysztof Kozlowski wrote:
> On 10/07/2026 09:16, Krzysztof Kozlowski wrote:
> > On Fri, Jul 10, 2026 at 01:52:45AM +0000, Thinh Nguyen wrote:
> >> On Thu, Jul 09, 2026, Dmitry Baryshkov wrote:
> >>> On Thu, Jul 09, 2026 at 02:22:36PM +0530, Krishna Kurapati wrote:
> >>>> Add property "qcom,has-eusb2-phy" to indicate whether the SoC has eUSB2 phy
> >>>> or not. This is used to modify dp/dm interrupts during host mode suspend.
> >>>>
> >>>> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml | 7 +++++++
> >>>>  1 file changed, 7 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> >>>> index 8201656b41ed..0c6a839be82f 100644
> >>>> --- a/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> >>>> +++ b/Documentation/devicetree/bindings/usb/qcom,snps-dwc3.yaml
> >>>> @@ -152,6 +152,13 @@ properties:
> >>>>        HS/FS/LS modes are supported.
> >>>>      type: boolean
> >>>>  
> >>>> +  qcom,has-eusb2-phy:
> >>>> +    description:
> >>>> +      If present, it indicates that the controller is working with eUSB2
> >>>> +      phy. This property is used to modify dp/dm interrupt configurations
> >>>> +      while entering suspend.
> >>>> +    type: boolean
> >>>
> >>> I still have exactly the same concern. This information is already
> >>> present in the DT.
> >>>
> >>
> >> Could you clarify what you mean by "this information is already present
> >> in the DT"? Are you suggesting we use the dwc3 node's compatible string
> >> to infer the phy type?
> >>
> >> The dwc3 glue compatible describes the controller, not the phy. They are
> >> separate IP blocks. A board variant could reuse the same dwc3 compatible
> >> with a different phy attached, making that inference silently wrong.
> >>
> >> The phy type is described somewhere else in the DT (in the phy node
> >> itself), referenced via the phy's phandle on the dwc3 core child node.
> >> We could walk that phandle and check whether the phy node's compatible
> >> contains "eusb2". However, that relies on a naming convention that isn't
> >> enforced anywhere. All current eusb2 phy compatibles happen to follow it
> >> (qcom,sm8550-snps-eusb2-phy, qcom,sm8750-m31-eusb2-phy,
> >> samsung,exynos2200-eusb2-phy, etc.), but a future phy driver could break
> > 
> > You have a fixed/finite list of phys which needs something special, so
> > that information as Dmitry said is in DT already.

Are you referring to a fixed list of eusb2 phy compatibles, or a fixed
list of dwc3 node compatibles?

Either way, the list is not fixed. New eusb2 SoC variants are added
regularly, and there are already 12 eusb2 phy compatible strings in the
DT bindings but only 3 in the PHY driver of_match tables, they don't
land at the same time. As for what information is already in DT, I've
addressed this above and in my earlier response.

> > 
> > Anyway, the commit msg and property field have insufficient description
> > of actual hardware problem being represented by this property, so it
> > looks way too much as SW policy.

That's fair feedback to raise with Krishna.

> 
> 
> And Dmitry already said this at v3.

And I've already responded to his comment in v3:
https://lore.kernel.org/linux-usb/ahjYwJtiMsm0BcCh@vbox/

> 
> And Sashiko gave the same review:
> "[Medium] The `qcom,has-eusb2-phy` property introduces a redundant
> boolean flag that duplicates existing hardware topology information, and
> its description incorrectly encodes software behavior."
> 

Are we relying on AI for expert reviews now? If so, here's what Claude
says:
    The DWC3 glue compatible is not a proxy for PHY type. It identifies
    the controller IP, while the PHY is an entirely separate block
    connected via phandle. With a single fallback compatible
    (qcom,snps-dwc3) covering 50+ SoC variants, there is no reliable
    mapping from DWC3 compatible to PHY type — the same string can
    appear on platforms with different PHYs.

    Inferring from the PHY node's compatible string is equally
    problematic. The "eusb2" substring appears consistently today, but
    this is coincidence, not a binding contract. Nothing in the DT
    schema enforces it. An allowlist in dwc3-qcom.c would need updating
    every time a new eUSB2 PHY is introduced — and as the gap between DT
    bindings and PHY driver of_match tables shows, those additions don't
    always land together.

    A boolean property on the DWC3 node is unambiguous,
    schema-validated, and places the decision with the DT author who has
    full knowledge of the platform's PHY topology.

BR,
Thinh

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

[Krishna Kurapati]

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>
---
 drivers/usb/dwc3/dwc3-qcom.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index f43f73ac36ff..65996d47da1f 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)
@@ -683,6 +702,7 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (ignore_pipe_clk)
 		dwc3_qcom_select_utmi_clk(qcom);
 
+	qcom->has_eusb2_phy = device_property_read_bool(dev, "qcom,has-eusb2-phy");
 	qcom->mode = usb_get_dr_mode(dev);
 
 	if (qcom->mode == USB_DR_MODE_HOST) {

-- 
2.34.1

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

[sashiko-bot]

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