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

[Krzysztof Kozlowski <krzk@kernel.org>]

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