lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230922-surface-graduate-a269a700e5c5@spud>
Date: Fri, 22 Sep 2023 12:43:06 +0100
From: Conor Dooley <conor@...nel.org>
To: Christophe Roullier <christophe.roullier@...s.st.com>
Cc: "David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Maxime Coquelin <mcoquelin.stm32@...il.com>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Richard Cochran <richardcochran@...il.com>,
	Jose Abreu <joabreu@...opsys.com>,
	Liam Girdwood <lgirdwood@...il.com>,
	Mark Brown <broonie@...nel.org>, netdev@...r.kernel.org,
	devicetree@...r.kernel.org,
	linux-stm32@...md-mailman.stormreply.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/7] dt-bindings: net: add STM32MP13 compatible in
 documentation for stm32

Yo,

On Thu, Sep 21, 2023 at 05:06:16PM +0200, Christophe Roullier wrote:
> New STM32 SOC have 2 GMACs instances.
> GMAC IP version is SNPS 4.20.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@...s.st.com>
> ---
>  .../devicetree/bindings/net/stm32-dwmac.yaml  | 140 +++++++++++++++---
>  1 file changed, 118 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> index fc8c96b08d7d..75836916c38c 100644
> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> @@ -22,15 +22,17 @@ select:
>          enum:
>            - st,stm32-dwmac
>            - st,stm32mp1-dwmac
> +          - st,stm32mp13-dwmac
>    required:
>      - compatible
>  
> -allOf:
> -  - $ref: snps,dwmac.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> +      - items:
> +          - enum:
> +              - st,stm32mp13-dwmac
> +          - const: snps,dwmac-4.20a

The enum just below this is also for the 4.20a, no? Why not just put
this mp13 compatible into that enum?

>        - items:
>            - enum:
>                - st,stm32mp1-dwmac
> @@ -72,27 +74,69 @@ properties:
>          - eth-ck
>          - ptp_ref
>  
> -  st,syscon:

Please try to avoid defining properties inside if/then/else sections and
only move the variable bits if possible.

> -    $ref: /schemas/types.yaml#/definitions/phandle-array
> -    items:
> -      - items:
> -          - description: phandle to the syscon node which encompases the glue register
> -          - description: offset of the control register
> +  phy-supply:
> +    description: PHY regulator
> +
> +  st,ext-phyclk:
>      description:
> -      Should be phandle/offset pair. The phandle to the syscon node which
> -      encompases the glue register, and the offset of the control register
> +      set this property in RMII mode when you have PHY without crystal 50MHz and want to
> +      select RCC clock instead of ETH_REF_CLK. or in RGMII mode when you want to select
> +      RCC clock instead of ETH_CLK125.
> +    type: boolean
>  
>    st,eth-clk-sel:
> +    deprecated: true

Why have these been marked as deprecated? That doesn't appear to be
mention in the commit message & sounds like it should be a different
commit.

>      description:
>        set this property in RGMII PHY when you want to select RCC clock instead of ETH_CLK125.
>      type: boolean
>  
>    st,eth-ref-clk-sel:
> +    deprecated: true

Ditto.

>      description:
>        set this property in RMII mode when you have PHY without crystal 50MHz and want to
>        select RCC clock instead of ETH_REF_CLK.
>      type: boolean
>  
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp1-dwmac
> +              - st,stm32-dwmac
> +    then:
> +      properties:
> +        st,syscon:
> +          $ref: /schemas/types.yaml#/definitions/phandle-array
> +          items:
> +            - items:
> +                - description: phandle to the syscon node which encompases the glue register
> +                - description: offset of the control register
> +          description:
> +            Should be phandle/offset pair. The phandle to the syscon node which
> +            encompases the glue register, and the offset of the control register
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp13-dwmac

You've got 2 if/then sections containing tests for 3 compatibles. There
are only 2 compatibles total right now & 3 with the patch, so it looks
like you'd get away with if/then/else instead.

> +    then:
> +      properties:
> +        st,syscon:
> +          $ref: /schemas/types.yaml#/definitions/phandle-array
> +          items:
> +            - items:
> +                - description: phandle to the syscon node which encompases the glue register
> +                - description: offset of the control register
> +                - description: field to set mask in register
> +          description:
> +            Should be phandle/offset pair. The phandle to the syscon node which
> +            encompases the glue register, the offset of the control register and
> +            the mask to set bitfield in control register
> +
>  required:
>    - compatible
>    - clocks
> @@ -112,24 +156,36 @@ examples:
>             compatible = "st,stm32mp1-dwmac", "snps,dwmac-4.20a";

I don't understand why this existing example is changing.

Thanks,
Conor.

>             reg = <0x5800a000 0x2000>;
>             reg-names = "stmmaceth";
> -           interrupts = <GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
> -           interrupt-names = "macirq";
> +           interrupts-extended = <&intc GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>,
> +                                 <&exti 70 IRQ_TYPE_LEVEL_HIGH>;
> +           interrupt-names = "macirq",
> +                             "eth_wake_irq";
>             clock-names = "stmmaceth",
> -                     "mac-clk-tx",
> -                     "mac-clk-rx",
> -                     "ethstp",
> -                     "eth-ck";
> +                         "mac-clk-tx",
> +                         "mac-clk-rx",
> +                         "eth-ck",
> +                         "ptp_ref",
> +                         "ethstp";
>             clocks = <&rcc ETHMAC>,
> -                <&rcc ETHTX>,
> -                <&rcc ETHRX>,
> -                <&rcc ETHSTP>,
> -                <&rcc ETHCK_K>;
> +                    <&rcc ETHTX>,
> +                    <&rcc ETHRX>,
> +                    <&rcc ETHCK_K>,
> +                    <&rcc ETHPTP_K>,
> +                    <&rcc ETHSTP>;
>             st,syscon = <&syscfg 0x4>;
> +           snps,mixed-burst;
>             snps,pbl = <2>;
> +           snps,en-tx-lpi-clockgating;
>             snps,axi-config = <&stmmac_axi_config_0>;
>             snps,tso;
>             phy-mode = "rgmii";
> -       };
> +
> +           stmmac_axi_config_0: stmmac-axi-config {
> +                                snps,wr_osr_lmt = <0x7>;
> +                                snps,rd_osr_lmt = <0x7>;
> +                                snps,blen = <0 0 0 0 16 8 4>;
> +           };
> +     };
>  
>    - |
>      //Example 2 (MCU example)
> @@ -161,3 +217,43 @@ examples:
>             snps,pbl = <8>;
>             phy-mode = "mii";
>         };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/stm32mp1-clks.h>
> +    #include <dt-bindings/reset/stm32mp1-resets.h>
> +    #include <dt-bindings/mfd/stm32h7-rcc.h>
> +    //Example 4
> +     ethernet3: ethernet@...0a000 {
> +           compatible = "st,stm32mp13-dwmac", "snps,dwmac-4.20a";
> +           reg = <0x5800a000 0x2000>;
> +           reg-names = "stmmaceth";
> +           interrupts-extended = <&intc GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>,
> +                                 <&exti 68 IRQ_TYPE_LEVEL_HIGH>;
> +           interrupt-names = "macirq",
> +                             "eth_wake_irq";
> +           clock-names = "stmmaceth",
> +                         "mac-clk-tx",
> +                         "mac-clk-rx",
> +                         "eth-ck",
> +                         "ptp_ref",
> +                         "ethstp";
> +           clocks = <&rcc ETHMAC>,
> +                    <&rcc ETHTX>,
> +                    <&rcc ETHRX>,
> +                    <&rcc ETHCK_K>,
> +                    <&rcc ETHPTP_K>,
> +                    <&rcc ETHSTP>;
> +           st,syscon = <&syscfg 0x4 0xff0000>;
> +           snps,mixed-burst;
> +           snps,pbl = <2>;
> +           snps,axi-config = <&stmmac_axi_config_1>;
> +           snps,tso;
> +           phy-mode = "rmii";
> +
> +           stmmac_axi_config_1: stmmac-axi-config {
> +                                snps,wr_osr_lmt = <0x7>;
> +                                snps,rd_osr_lmt = <0x7>;
> +                                snps,blen = <0 0 0 0 16 8 4>;
> +           };
> +     };
> -- 
> 2.25.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ