Re: [PATCH 8/9] dt-bindings: phy: sparx5: document lan969x in sparx5 dt-bindings

[Krzysztof Kozlowski]

On 09/09/2024 10:22, Daniel Machon wrote:
> Hi Krzysztof,
> 
> Thanks your comments.
> 
>> On 06/09/2024 14:52, Daniel Machon wrote:
>>> Document lan969x in the existing Sparx5 dt-bindings.
>>>
>>
>> Say something useful, not copy of subject.
>>
>>> Signed-off-by: Daniel Machon <daniel.machon@xxxxxxxxxxxxx>
>>
>> A nit, subject: drop second/last, redundant "dt-bindings". The
>> "dt-bindings" prefix is already stating that these are bindings.
>> See also:
>> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> 
> Duly noted.
> 
>>
>>
>>> Reviewed-by: Steen Hegelund <Steen.Hegelund@xxxxxxxxxxxxx>
>>> ---
>>>  .../bindings/phy/microchip,sparx5-serdes.yaml          | 18 ++++++++++++++++--
>>>  1 file changed, 16 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/microchip,sparx5-serdes.yaml b/Documentation/devicetree/bindings/phy/microchip,sparx5-serdes.yaml
>>> index bdbdb3bbddbe..1e07a311e8a5 100644
>>> --- a/Documentation/devicetree/bindings/phy/microchip,sparx5-serdes.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/microchip,sparx5-serdes.yaml
>>> @@ -8,6 +8,7 @@ title: Microchip Sparx5 Serdes controller
>>>
>>>  maintainers:
>>>    - Steen Hegelund <steen.hegelund@xxxxxxxxxxxxx>
>>> +  - Daniel Machon <daniel.machon@xxxxxxxxxxxxx>
>>>
>>>  description: |
>>>    The Sparx5 SERDES interfaces share the same basic functionality, but
>>> @@ -62,12 +63,17 @@ description: |
>>>    * 10.3125 Gbps (10GBASE-R/10GBASE-KR/USXGMII)
>>>    * 25.78125 Gbps (25GBASE-KR/25GBASE-CR/25GBASE-SR/25GBASE-LR/25GBASE-ER)
>>>
>>> +  lan969x has ten SERDES10G interfaces that share the same features, operating
>>> +  modes and data rates as the equivalent Sparx5 SERDES10G interfaces.
>>> +
>>>  properties:
>>>    $nodename:
>>>      pattern: "^serdes@[0-9a-f]+$"
>>>
>>>    compatible:
>>> -    const: microchip,sparx5-serdes
>>> +    enum:
>>> +      - microchip,sparx5-serdes
>>> +      - microchip,lan969x-serdes
>>
>> It seems there is no lan969x SoC/chip. Are you sure you are using
>> correct naming, matching what kernel is using? Maybe you just sent
>> whatever you had in downstream (hint: that's never a good idea).
> 
> You are right. There is no upstream support for lan969x SoC yet. The
> upstreaming of the lan969x SoC has just begun, and this series is part
> of that upstreaming effort. The lan969x switch driver (not submitted
> yet) will depend on this SERDES driver, however, their functionality is
> really independent of each other. That is why I am also upstreaming the
> SERDES- and switch driver series independent of each other.

That's not exactly my point. Becayse lan969x appears. I claim you use
incorrect name, so are you sure you do not use wildcards?
Best regards,
Krzysztof