Re: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add ExynosAutov920 combo ssphy

[Krzysztof Kozlowski]

On 26/08/2025 08:37, Pritam Manohar Sutar wrote:
> Hi Krzysztof, 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
>> Sent: 24 August 2025 02:26 PM
>> To: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx>
>> Cc: vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; robh@xxxxxxxxxx;
>> krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx;
>> andre.draszik@xxxxxxxxxx; peter.griffin@xxxxxxxxxx; kauschluss@xxxxxxxxxxx;
>> ivo.ivanov.ivanov1@xxxxxxxxx; igor.belwon@xxxxxxxxxxxxxxxxxxxxxxxxxx;
>> johan@xxxxxxxxxx; m.szyprowski@xxxxxxxxxxx; s.nawrocki@xxxxxxxxxxx;
>> linux-phy@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung-
>> soc@xxxxxxxxxxxxxxx; rosa.pila@xxxxxxxxxxx; dev.tailor@xxxxxxxxxxx;
>> faraz.ata@xxxxxxxxxxx; muhammed.ali@xxxxxxxxxxx;
>> selvarasu.g@xxxxxxxxxxx
>> Subject: Re: [PATCH v7 5/6] dt-bindings: phy: samsung,usb3-drd-phy: add
>> ExynosAutov920 combo ssphy
>>
>> On Fri, Aug 22, 2025 at 03:08:44PM +0530, Pritam Manohar Sutar wrote:
>>> This phy supports USB3.1 SSP+(10Gbps) protocol and is backwards
>>> compatible to the USB3.0 SS(5Gbps). It requires two clocks, named
>>> "phy" and "ref". The required supplies for USB3.1 are named as
>>> vdd075_usb30(0.75v), vdd18_usb30(1.8v).
>>
>> Please do not describe the schema, but hardware. This sentence does not help
>> me in my question further.
> 
> This is a combo phy having Synopsys usb20 and usb30 phys (these 2 phys are totally different). 
> One PHY only supports usb2.0 and data rates whereas another one does usb3.1 ssp+ and usb3.1 ssp
> 	
> This patch only explains about usb30 (since these are two different phys) phy and omitted inclusion of usb20 reference (added separate patch for this patch no 3). 
> 	
> Hope this is clear.

No. That sentence still explains what schema is doing.

BTW, wrap your email correctly.

> 
>>
>>>
>>> Add schemas for combo ssphy found on this SoC.
>>>
>>> Signed-off-by: Pritam Manohar Sutar <pritam.sutar@xxxxxxxxxxx>
>>> ---
>>>  .../bindings/phy/samsung,usb3-drd-phy.yaml    | 23 +++++++++++++++++++
>>>  1 file changed, 23 insertions(+)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> index f0cfca5736b8..96e5bbb2e42c 100644
>>> --- a/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/samsung,usb3-drd-phy.yaml
>>> @@ -34,6 +34,7 @@ properties:
>>>        - samsung,exynos7870-usbdrd-phy
>>>        - samsung,exynos850-usbdrd-phy
>>>        - samsung,exynos990-usbdrd-phy
>>> +      - samsung,exynosautov920-usb31drd-combo-ssphy
>>>        - samsung,exynosautov920-usbdrd-combo-hsphy
>>>        - samsung,exynosautov920-usbdrd-phy
>>>
>>> @@ -118,6 +119,12 @@ properties:
>>>    vdd18-usb20-supply:
>>>      description: 1.8V power supply for the USB 2.0 phy.
>>>
>>> +  dvdd075-usb30-supply:
>>> +    description: 0.75V power supply for the USB 3.0 phy.
>>> +
>>> +  vdd18-usb30-supply:
>>> +    description: 1.8V power supply for the USB 3.0 phy.
>>> +
>>>  required:
>>>    - compatible
>>>    - clocks
>>> @@ -227,6 +234,7 @@ allOf:
>>>                - samsung,exynos7870-usbdrd-phy
>>>                - samsung,exynos850-usbdrd-phy
>>>                - samsung,exynos990-usbdrd-phy
>>> +              - samsung,exynosautov920-usb31drd-combo-ssphy
>>>                - samsung,exynosautov920-usbdrd-combo-hsphy
>>>                - samsung,exynosautov920-usbdrd-phy
>>>      then:
>>> @@ -262,6 +270,21 @@ allOf:
>>>        properties:
>>>          dvdd075-usb20-supply: false
>>>          vdd18-usb20-supply: false
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - samsung,exynosautov920-usb31drd-combo-ssphy
>>> +    then:
>>> +      required:
>>> +        - dvdd075-usb30-supply
>>> +        - vdd18-usb30-supply
>>
>> Why are you adding usb20 and usb30 suffixes to the supplies? These are
>> separate devices, so they do not have both variants at the same time.
> 
> This is a combo phy consisting of usb2 and usb3 phys combined. 
> To drive these separate phys, added suffixes for these supplies respectively.

But they are separate.

> 
> Moreover, gs101 is also using similar convention for its usb20 and dp supplies. 
> Added suffix for usb2 and usb3 as per our last communication https://lore.kernel.org/linux-phy/6e1c67d2-9bfa-442a-9d53-8c5970a2a9ef@xxxxxxxxxx/

Then please review patches on the list and help to improve them BEFORE
they got merged.

I questioned the suffix there, so I really do not understand why did you
added it.

> 
>>
>> From this device point of view, the supply is called dvdd075 or vdd18.
>> If you open device datasheet (not SoC datasheet), that's how it will be called,
>> most likely.
> 
> Yes, Agree. In device datasheet, suffixes are not mentioned, but in our board schematic it is mentioned. 
> Let me know your suggestion about adding suffixes?

I already said, multiple times on various discussions. You name these
based on how the inputs are called in this device.

Best regards,
Krzysztof