Re: [PATCH v4 net-next 1/2] dt-bindings: phy: cadence-torrent: Update property values to support 3 clocks

[Gokul Praveen <g-praveen@ti.com>]

Hi Krzysztof,

On 02/07/26 11:53, Krzysztof Kozlowski wrote:
> On Wed, Jul 01, 2026 at 07:54:56PM +0530, Gokul Praveen wrote:
>> Update maxItems value of "clocks" property to 3 as description of
>> this parameter already indicates 3 clocks(refclk,pll1_refclk(optional)
>> and phy_en_refclk(optional)).
> But what if description is wrong? You need to provide rationale why you
> are doing it and you cannot use existing code alone as that rationale,
> because as you pointed out - existing code is not fully correct.
>
>> Update the maxItems and items value of "clock-names" property with multiple
>> combination of clock-names possible since pll1_refclk and phy_en_refclk are
>> optional clocks.
> Why? You need to describe why you are doing this, not what you are
> doing.
>
Would the below commit description be good enough ? Please feel free to 
give any suggestions on this:

''

dt-bindings: phy: cadence-torrent: Update property values to support 3 
clocks

     Increase the maxItems value of "clocks" property to 3 to support
     2 input clocks(refclk,pll1_refclk) and 1 output clock(phy_en_refclk).

     For multilink SERDES configurations where the links require 2 
different input clock speeds,
     2 different input reference clocks and 1 output clock is needed so 
as to cater to this requirement.

     For eg: Considering the USXGMII+SGMII multilink SERDES 
configuration usecase ,
     having only 1 input reference clock fails because USXGMII requires 
an input clock speed of 156.25 Mhz and
     SGMII protocol requires an input clock speed of 100 Mhz.

     Since one input reference clock(refclk) alone cannot cater to the 2 
different clock speed requirements
     of these protocols, the second input reference clock(pll1_refclk) 
has to be added.


     Signed-off-by: Gokul Praveen <g-praveen@ti.com>
''

>> Signed-off-by: Gokul Praveen <g-praveen@ti.com>
>> ---
>>   .../bindings/phy/phy-cadence-torrent.yaml        | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> index 9af39b33646a..96c664d50629 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> +++ b/Documentation/devicetree/bindings/phy/phy-cadence-torrent.yaml
>> @@ -34,7 +34,7 @@ properties:
>>   
>>     clocks:
>>       minItems: 1
>> -    maxItems: 2
>> +    maxItems: 3
>>       description:
>>         PHY input reference clocks - refclk (for PLL0) & pll1_refclk (for PLL1).
>>         pll1_refclk is optional and used for multi-protocol configurations requiring
>> @@ -45,9 +45,17 @@ properties:
>>   
>>     clock-names:
>>       minItems: 1
>> -    items:
>> -      - const: refclk
>> -      - enum: [ pll1_refclk, phy_en_refclk ]
>> +    maxItems: 3
> Drop
sure krzysztof.
>> +    oneOf:
>> +      - items:
>> +          - const: refclk
>> +      - items:
>> +          - const: refclk
>> +          - enum: [ pll1_refclk, phy_en_refclk ]
> Drop these, pointless. You were supposed to grow existing syntax.
>
Sure Krzysztof
>> +      - items:
>> +          - const: refclk
>> +          - const: pll1_refclk
> So here is the enum.
>
>> +          - const: phy_en_refclk
>>
>> And this stays.

SUre Krzysztof

>>
>> You make changes which do not make the binding better and are not
>> explained in commit msg. Focus on WHY you are doing things and also
>> explain WHY you did such complicated syntax (if you insist on rewriting
>> correct code into something odd we do not expect).
>>
>> Best regards,
>> Krzysztof

Best Regards

Gokul Praveen

>