On Mon, Jul 8, 2024 at 11:03 AM Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote: > > > On Thu, Jul 04, 2024 at 10:08:10AM +0200, Lorenzo Bianconi wrote: > > > Introduce device-tree binding documentation for Airoha EN7581 ethernet > > > mac controller. > > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx> > > > --- > > > .../bindings/net/airoha,en7581-eth.yaml | 146 ++++++++++++++++++ > > > 1 file changed, 146 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml > > > new file mode 100644 > > > index 000000000000..f4b1f8afddd0 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml > > > @@ -0,0 +1,146 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Airoha EN7581 Frame Engine Ethernet controller > > > + > > > +allOf: > > > + - $ref: ethernet-controller.yaml# > > > > Again, to rephrase, what are you using from this binding? It does not > > make sense for the parent and child both to use it. > > Below I reported the ethernet dts node I am using (I have not posted the dts > changes yet): What happens when you remove this $ref? Nothing, because you use 0 properties from it. If none of the properties apply, then don't reference it. It is that simple. > > eth0: ethernet@1fb50000 { > compatible = "airoha,en7581-eth"; > reg = <0 0x1fb50000 0 0x2600>, > <0 0x1fb54000 0 0x2000>, > <0 0x1fb56000 0 0x2000>; > reg-names = "fe", "qdma0", "qdma1"; > > resets = <&scuclk EN7581_FE_RST>, > <&scuclk EN7581_FE_PDMA_RST>, > <&scuclk EN7581_FE_QDMA_RST>, > <&scuclk EN7581_XSI_MAC_RST>, > <&scuclk EN7581_DUAL_HSI0_MAC_RST>, > <&scuclk EN7581_DUAL_HSI1_MAC_RST>, > <&scuclk EN7581_HSI_MAC_RST>, > <&scuclk EN7581_XFP_MAC_RST>; > reset-names = "fe", "pdma", "qdma", "xsi-mac", > "hsi0-mac", "hsi1-mac", "hsi-mac", > "xfp-mac"; > > interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>; > > status = "disabled"; > > #address-cells = <1>; > #size-cells = <0>; > > gdm1: mac@1 { > compatible = "airoha,eth-mac"; > reg = <1>; > phy-mode = "internal"; > status = "disabled"; > > fixed-link { > speed = <1000>; > full-duplex; > pause; > }; > }; > }; > > I am using phy related binding for gdm1:mac@1 node. Right, so you should reference ethernet-controller.yaml for the mac node because you use properties from the schema. > gdm1 is the GMAC port used > as cpu port by the mt7530 dsa switch That has nothing to do with *this* binding... > > switch: switch@1fb58000 { > compatible = "airoha,en7581-switch"; > reg = <0 0x1fb58000 0 0x8000>; > resets = <&scuclk EN7581_GSW_RST>; > > interrupt-controller; > #interrupt-cells = <1>; > interrupt-parent = <&gic>; > interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>; > > status = "disabled"; > > #address-cells = <1>; > #size-cells = <1>; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > gsw_port1: port@1 { > reg = <1>; > label = "lan1"; > phy-mode = "internal"; > phy-handle = <&gsw_phy1>; > }; > > gsw_port2: port@2 { > reg = <2>; > label = "lan2"; > phy-mode = "internal"; > phy-handle = <&gsw_phy2>; > }; > > gsw_port3: port@3 { > reg = <3>; > label = "lan3"; > phy-mode = "internal"; > phy-handle = <&gsw_phy3>; > }; > > gsw_port4: port@4 { > reg = <4>; > label = "lan4"; > phy-mode = "internal"; > phy-handle = <&gsw_phy4>; > }; > > port@6 { > reg = <6>; > label = "cpu"; > ethernet = <&gdm1>; > phy-mode = "internal"; > > fixed-link { > speed = <1000>; > full-duplex; > pause; > }; > }; > }; > > mdio { > #address-cells = <1>; > #size-cells = <0>; > > gsw_phy1: ethernet-phy@1 { > compatible = "ethernet-phy-ieee802.3-c22"; > reg = <9>; > phy-mode = "internal"; > }; > > gsw_phy2: ethernet-phy@2 { > compatible = "ethernet-phy-ieee802.3-c22"; > reg = <10>; > phy-mode = "internal"; > }; > > gsw_phy3: ethernet-phy@3 { > compatible = "ethernet-phy-ieee802.3-c22"; > reg = <11>; > phy-mode = "internal"; > }; > > gsw_phy4: ethernet-phy@4 { > compatible = "ethernet-phy-ieee802.3-c22"; > reg = <12>; > phy-mode = "internal"; > }; > }; > }; None of this is relevant. > > > +patternProperties: > > > + "^mac@[1-4]$": > > > > 'ethernet' is the defined node name for users of > > ethernet-controller.yaml. > > Looking at the dts above, ethernet is already used by the parent node. So? Not really any reason a node named foo can't have a child named foo, too. An 'ethernet' node should implement an ethernet interface. It is the child nodes that implement the ethernet interface(s). Whether you use 'ethernet' on the parent or not, I don't care too much. > This approach has been already used here [0],[1],[2]. Is it fine to reuse it? That one appears to be wrong too with the parent referencing ethernet-controller.yaml. Rob > Regards, > Lorenzo > > [0] https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/mediatek/mt7622.dtsi#L964 > [1] https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts#L136 > [2] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/mediatek%2Cnet.yaml#L370
