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@xxxxxxxxxxxxxxxx>
> >>>> ---
> >>>>  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