[PATCH v5 net-next 1/2] dt-bindings: net: airoha: Add EN7581 ethernet controller
Rob Herring
robh at kernel.org
Mon Jul 8 13:19:57 PDT 2024
On Mon, Jul 8, 2024 at 11:03 AM Lorenzo Bianconi <lorenzo at kernel.org> 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 at kernel.org>
> > > ---
> > > .../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 at 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 at 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 at 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 at 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 at 1 {
> reg = <1>;
> label = "lan1";
> phy-mode = "internal";
> phy-handle = <&gsw_phy1>;
> };
>
> gsw_port2: port at 2 {
> reg = <2>;
> label = "lan2";
> phy-mode = "internal";
> phy-handle = <&gsw_phy2>;
> };
>
> gsw_port3: port at 3 {
> reg = <3>;
> label = "lan3";
> phy-mode = "internal";
> phy-handle = <&gsw_phy3>;
> };
>
> gsw_port4: port at 4 {
> reg = <4>;
> label = "lan4";
> phy-mode = "internal";
> phy-handle = <&gsw_phy4>;
> };
>
> port at 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 at 1 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <9>;
> phy-mode = "internal";
> };
>
> gsw_phy2: ethernet-phy at 2 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <10>;
> phy-mode = "internal";
> };
>
> gsw_phy3: ethernet-phy at 3 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <11>;
> phy-mode = "internal";
> };
>
> gsw_phy4: ethernet-phy at 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
More information about the linux-arm-kernel
mailing list