[PATCH v2 4/8] dt-bindings: net: renesas: Document Renesas Ethernet Switch

[Krzysztof Kozlowski]

On 26/09/2022 08:10, Yoshihiro Shimoda wrote:

> I'll add a blank line here.
> 
>>> +      '#size-cells':
>>> +        const: 0
>>> +
>>> +    additionalProperties: false
>>
>> Don't put it between properties. For nested object usually this is
>> before properties:
> 
> I'll drop it.

Don't drop, but instead put it before "properties" for this nested object.

> 
>>> +
>>> +    patternProperties:
>>> +      "^port@[0-9a-f]+$":
>>> +        type: object
>>> +
>>
>> Skip blank line.
> 
> I got it.
> 
>>> +        $ref: "/schemas/net/ethernet-controller.yaml#"
>>
>> No need for quotes.
> 
> I'll drop the quotes.
> 
>>> +        unevaluatedProperties: false
>>> +
>>> +        properties:
>>> +          reg:
>>> +            description:
>>> +              Port number of ETHA (TSNA).
>>> +
>>> +          phy-handle:
>>> +            description:
>>> +              Phandle of an Ethernet PHY.
>>
>> Why do you need to mention this property? Isn't it coming from
>> ethernet-controller.yaml?
> 
> Indeed. I'll drop the description.
> 
>>> +
>>> +          phy-mode:
>>> +            description:
>>> +              This specifies the interface used by the Ethernet PHY.
>>> +            enum:
>>> +              - mii
>>> +              - sgmii
>>> +              - usxgmii
>>> +
>>> +          phys:
>>> +            maxItems: 1
>>> +            description:
>>> +              Phandle of an Ethernet SERDES.
>>
>> This is getting confusing. You have now:
>> - phy-handle
>> - phy
>> - phy-device
>> - phys
>> in one schema... although lan966x serdes seems to do the same. :/
> 
> Yes... I found the following documents have "phy" and "phy-handle" by using
> git grep -l -w "phys" `git grep -l phy-handle Documentation/devicetree/bindings/`:
> Documentation/devicetree/bindings/net/cdns,macb.yaml
> Documentation/devicetree/bindings/net/cpsw.txt
> Documentation/devicetree/bindings/net/microchip,lan966x-switch.yaml
> Documentation/devicetree/bindings/net/microchip,sparx5-switch.yaml
> Documentation/devicetree/bindings/net/ti,cpsw-switch.yaml
> Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> Documentation/devicetree/bindings/phy/phy-bindings.txt
> 
> And I'm interesting that the phy-bindings.txt said the following:
> -----
> phys : the phandle for the PHY device (used by the PHY subsystem; not to be
>        confused with the Ethernet specific 'phy' and 'phy-handle' properties,
>        see Documentation/devicetree/bindings/net/ethernet.txt for these)
> -----

Indeed, seems ok.

> 

Best regards,
Krzysztof