Hi Krzysztof, > From: Krzysztof Kozlowski, Sent: Thursday, September 22, 2022 4:37 PM > > On 21/09/2022 10:47, Yoshihiro Shimoda wrote: > > Document Renesas Etherent Switch for R-Car S4-8 (r8a779f0). > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > --- > > .../bindings/net/renesas,etherswitch.yaml | 286 ++++++++++++++++++ > > 1 file changed, 286 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/renesas,etherswitch.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml > b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml > > new file mode 100644 > > index 000000000000..988d14f5c54e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/renesas,etherswitch.yaml > > Isn't dsa directory for this? As Andrew mentioned, this is not a DSA driver. > > @@ -0,0 +1,286 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/renesas,etherswitch.yaml# > > Filename: renesas,r8a779f0-ether-switch.yaml I'll rename this file. > > +$schema: <snip> > > + > > +title: Renesas Ethernet Switch > > + > > +maintainers: > > + - Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > + > > +properties: > > + compatible: > > + const: renesas,r8a779f0-ether-switch > > + > > + reg: > > + maxItems: 2 > > + > > + reg-names: > > + items: > > + - const: base > > + - const: secure_base > > + > > + interrupts: > > + maxItems: 47 > > + > > + interrupt-names: > > + items: > > + - const: mfwd_error > > + - const: race_error > > + - const: coma_error > > + - const: gwca0_error > > + - const: gwca1_error > > + - const: etha0_error > > + - const: etha1_error > > + - const: etha2_error > > + - const: gptp0_status > > + - const: gptp1_status > > + - const: mfwd_status > > + - const: race_status > > + - const: coma_status > > + - const: gwca0_status > > + - const: gwca1_status > > + - const: etha0_status > > + - const: etha1_status > > + - const: etha2_status > > + - const: rmac0_status > > + - const: rmac1_status > > + - const: rmac2_status > > + - const: gwca0_rxtx0 > > + - const: gwca0_rxtx1 > > + - const: gwca0_rxtx2 > > + - const: gwca0_rxtx3 > > + - const: gwca0_rxtx4 > > + - const: gwca0_rxtx5 > > + - const: gwca0_rxtx6 > > + - const: gwca0_rxtx7 > > + - const: gwca1_rxtx0 > > + - const: gwca1_rxtx1 > > + - const: gwca1_rxtx2 > > + - const: gwca1_rxtx3 > > + - const: gwca1_rxtx4 > > + - const: gwca1_rxtx5 > > + - const: gwca1_rxtx6 > > + - const: gwca1_rxtx7 > > + - const: gwca0_rxts0 > > + - const: gwca0_rxts1 > > + - const: gwca1_rxts0 > > + - const: gwca1_rxts1 > > + - const: rmac0_mdio > > + - const: rmac1_mdio > > + - const: rmac2_mdio > > + - const: rmac0_phy > > + - const: rmac1_phy > > + - const: rmac2_phy > > + > > + clocks: > > + maxItems: 2 > > + > > + clock-names: > > + items: > > + - const: fck > > + - const: tsn > > + > > + resets: > > + maxItems: 2 > > + > > + reset-names: > > + items: > > + - const: rswitch2 > > + - const: tsn > > + > > + iommus: > > + maxItems: 16 > > + > > + power-domains: > > + maxItems: 1 > > + > > + ethernet-ports: > > + type: object > > + > > + properties: > > + '#address-cells': > > + description: Port number of ETHA (TSNA). > > + const: 1 > > Blank line 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. > > + > > + 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) ----- > > + > > + mdio: > > + $ref: "/schemas/net/mdio.yaml#" > > No need for quotes. I got it. > Are you sure this is property of each port? I don't > know the net/ethernet bindings that good, so I need to ask sometimes > basic questions. Other bindings seem to do it differently a bit. Yes, each port has mdio bus. > > + unevaluatedProperties: false > > + > > + required: > > + - phy-handle > > + - phy-mode > > + - phys > > + - mdio > > + > > +required: > > + - compatible > > + - reg > > + - reg-names > > + - interrupts > > + - interrupt-names > > + - clocks > > + - clock-names > > + - resets > > + - power-domains > > + - ethernet-ports > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/clock/r8a779f0-cpg-mssr.h> > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + #include <dt-bindings/power/r8a779f0-sysc.h> > > + > > + ethernet@e6880000 { > > + compatible = "renesas,r8a779f0-ether-switch"; > > Wrong indentation. Use 4 spaces. I'll fix it. Best regards, Yoshihiro Shimoda > > + reg = <0xe6880000 0x20000>, <0xe68c0000 0x20000>; > > + reg-names = "base", "secure_base"; > > + interrupts = <GIC_SPI 256 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 257 IRQ_TYPE_LEVEL_HIGH>, > Best regards, > Krzysztof
