Re: [PATCH net-next 02/14] dt-bindings: net: mediatek,net: add mt7986-eth binding

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 7116 bytes --]

> On Fri, May 06, 2022 at 02:30:19PM +0200, Lorenzo Bianconi wrote:
> > Introduce dts bindings for mt7986 soc in mediatek,net.yaml.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  .../devicetree/bindings/net/mediatek,net.yaml | 133 +++++++++++++++++-
> >  1 file changed, 131 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/mediatek,net.yaml b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> > index 43cc4024ef98..da1294083eeb 100644
> > --- a/Documentation/devicetree/bindings/net/mediatek,net.yaml
> > +++ b/Documentation/devicetree/bindings/net/mediatek,net.yaml
> > @@ -21,6 +21,7 @@ properties:
> >        - mediatek,mt7623-eth
> >        - mediatek,mt7622-eth
> >        - mediatek,mt7629-eth
> > +      - mediatek,mt7986-eth
> >        - ralink,rt5350-eth
> >  
> >    reg:
> > @@ -28,7 +29,7 @@ properties:
> >  
> >    interrupts:
> >      minItems: 3
> > -    maxItems: 3
> > +    maxItems: 4
> 
> What's the new interrupt? This should describe what each entry is.
> 
> If the mt7986-eth must have all 4 interrupts, then the if/then needs a 
> 'minItems: 4'.

ack, I will fix it in v2.

> 
> >  
> >    power-domains:
> >      maxItems: 1
> > @@ -189,6 +190,43 @@ allOf:
> >            minItems: 2
> >            maxItems: 2
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: mediatek,mt7986-eth
> > +    then:
> > +      properties:
> > +        clocks:
> > +          minItems: 15
> > +          maxItems: 15
> > +
> > +        clock-names:
> > +          items:
> > +            - const: fe
> > +            - const: gp2
> > +            - const: gp1
> > +            - const: wocpu1
> > +            - const: wocpu0
> > +            - const: sgmii_tx250m
> > +            - const: sgmii_rx250m
> > +            - const: sgmii_cdr_ref
> > +            - const: sgmii_cdr_fb
> > +            - const: sgmii2_tx250m
> > +            - const: sgmii2_rx250m
> > +            - const: sgmii2_cdr_ref
> > +            - const: sgmii2_cdr_fb
> > +            - const: netsys0
> > +            - const: netsys1
> > +
> > +        mediatek,sgmiisys:
> > +          minItems: 2
> > +          maxItems: 2
> > +
> 
> > +        assigned-clocks: true
> > +
> > +        assigned-clock-parents: true
> 
> These are automatically allowed on any node with 'clocks' (and now 
> #clock-cells), so you can drop them.

ack, I will fix it in v2.

> 
> > +
> >  patternProperties:
> >    "^mac@[0-1]$":
> >      type: object
> > @@ -219,7 +257,6 @@ required:
> >    - interrupts
> >    - clocks
> >    - clock-names
> > -  - power-domains
> 
> Is that because this chip doesn't have power domains, or support for 
> them hasn't been added? In the latter case, then you should keep this. 

power domain are not supported in mt7986 since the soc is intendent mainly for
router soc where the device is connected to a power adapter and so the power
domain is always on.

Regards,
Lorenzo

> 
> >    - mediatek,ethsys
> >  
> >  unevaluatedProperties: false
> > @@ -295,3 +332,95 @@ examples:
> >          };
> >        };
> >      };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/clock/mt7622-clk.h>
> > +
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      eth: ethernet@15100000 {
> > +        #define CLK_ETH_FE_EN            0
> > +        #define CLK_ETH_WOCPU1_EN        3
> > +        #define CLK_ETH_WOCPU0_EN        4
> > +        #define CLK_TOP_NETSYS_SEL      43
> > +        #define CLK_TOP_NETSYS_500M_SEL 44
> > +        #define CLK_TOP_NETSYS_2X_SEL   46
> > +        #define CLK_TOP_SGM_325M_SEL    47
> > +        #define CLK_APMIXED_NET2PLL      1
> > +        #define CLK_APMIXED_SGMPLL       3
> > +
> > +        compatible = "mediatek,mt7986-eth";
> > +        reg = <0 0x15100000 0 0x80000>;
> > +        interrupts = <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>,
> > +                     <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>,
> > +                     <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>,
> > +                     <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>;
> > +        clocks = <&ethsys CLK_ETH_FE_EN>,
> > +                 <&ethsys CLK_ETH_GP2_EN>,
> > +                 <&ethsys CLK_ETH_GP1_EN>,
> > +                 <&ethsys CLK_ETH_WOCPU1_EN>,
> > +                 <&ethsys CLK_ETH_WOCPU0_EN>,
> > +                 <&sgmiisys0 CLK_SGMII_TX250M_EN>,
> > +                 <&sgmiisys0 CLK_SGMII_RX250M_EN>,
> > +                 <&sgmiisys0 CLK_SGMII_CDR_REF>,
> > +                 <&sgmiisys0 CLK_SGMII_CDR_FB>,
> > +                 <&sgmiisys1 CLK_SGMII_TX250M_EN>,
> > +                 <&sgmiisys1 CLK_SGMII_RX250M_EN>,
> > +                 <&sgmiisys1 CLK_SGMII_CDR_REF>,
> > +                 <&sgmiisys1 CLK_SGMII_CDR_FB>,
> > +                 <&topckgen CLK_TOP_NETSYS_SEL>,
> > +                 <&topckgen CLK_TOP_NETSYS_SEL>;
> > +        clock-names = "fe", "gp2", "gp1", "wocpu1", "wocpu0",
> > +                      "sgmii_tx250m", "sgmii_rx250m",
> > +                      "sgmii_cdr_ref", "sgmii_cdr_fb",
> > +                      "sgmii2_tx250m", "sgmii2_rx250m",
> > +                      "sgmii2_cdr_ref", "sgmii2_cdr_fb",
> > +                      "netsys0", "netsys1";
> > +        mediatek,ethsys = <&ethsys>;
> > +        mediatek,sgmiisys = <&sgmiisys0>, <&sgmiisys1>;
> > +        assigned-clocks = <&topckgen CLK_TOP_NETSYS_2X_SEL>,
> > +                          <&topckgen CLK_TOP_SGM_325M_SEL>;
> > +        assigned-clock-parents = <&apmixedsys CLK_APMIXED_NET2PLL>,
> > +                                 <&apmixedsys CLK_APMIXED_SGMPLL>;
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        mdio: mdio-bus {
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +
> > +          phy5: ethernet-phy@0 {
> > +            compatible = "ethernet-phy-id67c9.de0a";
> > +            phy-mode = "2500base-x";
> > +            reset-gpios = <&pio 6 1>;
> > +            reset-deassert-us = <20000>;
> > +            reg = <5>;
> > +          };
> > +
> > +          phy6: ethernet-phy@1 {
> > +            compatible = "ethernet-phy-id67c9.de0a";
> > +            phy-mode = "2500base-x";
> > +            reg = <6>;
> > +          };
> > +        };
> > +
> > +        mac0: mac@0 {
> > +          compatible = "mediatek,eth-mac";
> > +          phy-mode = "2500base-x";
> > +          phy-handle = <&phy5>;
> > +          reg = <0>;
> > +        };
> > +
> > +        mac1: mac@1 {
> > +          compatible = "mediatek,eth-mac";
> > +          phy-mode = "2500base-x";
> > +          phy-handle = <&phy6>;
> > +          reg = <1>;
> > +        };
> > +      };
> > +    };
> > -- 
> > 2.35.1
> > 
> > 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]