Re: [PATCH 2/2] dt-bindings: net: dwmac: Convert stm32 dwmac to DT schema

[Christophe ROULLIER <christophe.roullier@st.com>]

Hi Rob, all,

On 11/14/19 7:22 PM, Rob Herring wrote:
> On Fri, Nov 08, 2019 at 11:35:26AM +0100, Christophe Roullier wrote:
>> Convert stm32 dwmac to DT schema.
> Lots of checkpatch errors with trailing WS.
ok, sorry I forgot to execute checkpatch before upstream.
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>>   .../devicetree/bindings/net/stm32-dwmac.txt   |  44 -----
>>   .../devicetree/bindings/net/stm32-dwmac.yaml  | 161 ++++++++++++++++++
>>   2 files changed, 161 insertions(+), 44 deletions(-)
>>   delete mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.txt
>>   create mode 100644 Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.txt b/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> deleted file mode 100644
>> index a90eef11dc46..000000000000
>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.txt
>> +++ /dev/null
>> @@ -1,44 +0,0 @@
>> -STMicroelectronics STM32 / MCU DWMAC glue layer controller
>> -
>> -This file documents platform glue layer for stmmac.
>> -Please see stmmac.txt for the other unchanged properties.
>> -
>> -The device node has following properties.
>> -
>> -Required properties:
>> -- compatible:  For MCU family should be "st,stm32-dwmac" to select glue, and
>> -	       "snps,dwmac-3.50a" to select IP version.
>> -	       For MPU family should be "st,stm32mp1-dwmac" to select
>> -	       glue, and "snps,dwmac-4.20a" to select IP version.
>> -- clocks: Must contain a phandle for each entry in clock-names.
>> -- clock-names: Should be "stmmaceth" for the host clock.
>> -	       Should be "mac-clk-tx" for the MAC TX clock.
>> -	       Should be "mac-clk-rx" for the MAC RX clock.
>> -	       For MPU family need to add also "ethstp" for power mode clock
>> -- interrupt-names: Should contain a list of interrupt names corresponding to
>> -           the interrupts in the interrupts property, if available.
>> -		   Should be "macirq" for the main MAC IRQ
>> -		   Should be "eth_wake_irq" for the IT which wake up system
>> -- st,syscon : Should be phandle/offset pair. The phandle to the syscon node which
>> -	       encompases the glue register, and the offset of the control register.
>> -
>> -Optional properties:
>> -- clock-names:     For MPU family "eth-ck" for PHY without quartz
>> -- st,eth-clk-sel (boolean) : set this property in RGMII PHY when you want to select RCC clock instead of ETH_CLK125.
>> -- st,eth-ref-clk-sel (boolean) :  set this property in RMII mode when you have PHY without crystal 50MHz and want to select RCC clock instead of ETH_REF_CLK.
>> -
>> -Example:
>> -
>> -	ethernet@40028000 {
>> -		compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
>> -		reg = <0x40028000 0x8000>;
>> -		reg-names = "stmmaceth";
>> -		interrupts = <0 61 0>, <0 62 0>;
>> -		interrupt-names = "macirq", "eth_wake_irq";
>> -		clock-names = "stmmaceth", "mac-clk-tx", "mac-clk-rx";
>> -		clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
>> -		st,syscon = <&syscfg 0x4>;
>> -		snps,pbl = <8>;
>> -		snps,mixed-burst;
>> -		dma-ranges;
>> -	};
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> new file mode 100644
>> index 000000000000..eb0fd831f59d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> @@ -0,0 +1,161 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +# Copyright 2019 BayLibre, SAS
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/net/stm32-dwmac.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: STMicroelectronics STM32 / MCU DWMAC glue layer controller
>> +
>> +maintainers:
>> +  - Alexandre Torgue <alexandre.torgue@st.com>
>> +  - Christophe Roullier <christophe.roullier@st.com>
>> +
>> +description:
>> +  This file documents platform glue layer for stmmac.
>> +
>> +# We need a select here so we don't match all nodes with 'snps,dwmac'
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +          - st,stm32-dwmac
>> +          - st,stm32mp1-dwmac
>> +  required:
>> +    - compatible
>> +
>> +allOf:
>> +  - $ref: "snps,dwmac.yaml#"
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32-dwmac
>> +              - st,stm32mp1-dwmac
>> +    then:
>> +      properties:
>> +       clocks:
>> +         minItems: 3
>> +         maxItems: 5
>> +         items:
>> +          - description: GMAC main clock
>> +          - description: MAC TX clock
>> +          - description: MAC RX clock
>> +          - description: For MPU family, used for power mode
> What does 'power mode' mean? IIRC, some DW MACs have a clock for WoL
> called LPI or something. Are you sure this is ST specific and not DW
> config or version specific?

Yes that right, it is clock used to manage WoL during suspend.

My problem it is may be to manage MCU and MPU family in same yaml file

Because in MCU family only 3 clocks used (GMAC main clock, MAC TX clock 
and MAC RX clock)

In MPU family it is same clocks + clock to manage WoL and clock to 
manage PHY without Cristal

>> +          - description: For MPU family, used for PHY without quartz
> It would be cleaner to define the clock always present and use a
> fixed-clock when you have an external quartz.

You are right, I will define clocks always present and manage them in my 
driver (enable or disable it in function of phy mode, if with or without 
quartz in PHY etc..)

So for MCU (st,stm32-dwmac) I need 3 clocks and for MPU 
(st,stm32mp1-dwmac) 5 clocks

How to manage this in yaml ? 2 differents files ?

>> +
>> +       clock-names:
>> +         minItems: 3
>> +         maxItems: 5
>> +         contains:
>> +          enum:
>> +            - stmmaceth
>> +            - mac-clk-tx
>> +            - mac-clk-rx
>> +            - ethstp
>> +            - eth-ck
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - st,stm32mp1-dwmac
>> +          - const: snps,dwmac-4.20a
>> +      - items:
>> +          - enum:
>> +              - st,stm32-dwmac
>> +          - const: snps,dwmac-4.10a
>> +      - items:
>> +          - enum:
>> +              - st,stm32-dwmac
>> +          - const: snps,dwmac-3.50a
>> +
>> +  st,syscon:
>> +    allOf:
>> +      - $ref: "/schemas/types.yaml#/definitions/phandle-array"
>> +    description:
>> +      Should be phandle/offset pair. The phandle to the syscon node which
>> +      encompases the glue register, and the offset of the control register
>> +
>> +  st,eth-clk-sel:
>> +    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:
>> +    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.
> Wrap lines.
ok
>> +    type: boolean
>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +  - clock-names
>> +  - st,syscon
>> +
>> +examples:
>> + - |
>> +    #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 1
>> +     ethernet0: ethernet@5800a000 {
>> +       compatible = "st,stm32mp1-dwmac", "snps,dwmac-4.20a";
>> +       reg = <0x5800a000 0x2000>;
>> +       reg-names = "stmmaceth";
>> +       interrupts = <&intc GIC_SPI 61 IRQ_TYPE_LEVEL_HIGH>;
>> +       interrupt-names = "macirq";
>> +       clock-names = "stmmaceth",
>> +                     "mac-clk-tx",
>> +                     "mac-clk-rx",
>> +                     "ethstp",
>> +                     "eth-ck";				
>> +       clocks = <&rcc ETHMAC>,
>> +                <&rcc ETHTX>,
>> +                <&rcc ETHRX>,
>> +                <&rcc ETHSTP>,
>> +                <&rcc ETHCK_K>;	
>> +       st,syscon = <&syscfg 0x4>;
>> +       snps,pbl = <2>;
>> +       snps,axi-config = <&stmmac_axi_config_0>;
>> +       snps,tso;
>> +       status = "disabled";
>> +       phy-mode = "rgmii";
>> +       };
>> +
>> +    //Example 1 (MCU example)
> 2 Example 1's?
>
exact, I will rename it

Thanks Rob

>> +     ethernet1: ethernet@40028000 {
>> +       compatible = "st,stm32-dwmac", "snps,dwmac-3.50a";
>> +       reg = <0x40028000 0x8000>;
>> +       reg-names = "stmmaceth";
>> +       interrupts = <0 61 0>, <0 62 0>;
>> +       interrupt-names = "macirq", "eth_wake_irq";
>> +       clock-names = "stmmaceth", "mac-clk-tx", "mac-clk-rx";
>> +       clocks = <&rcc 0 25>, <&rcc 0 26>, <&rcc 0 27>;
>> +       st,syscon = <&syscfg 0x4>;
>> +       snps,pbl = <8>;
>> +       snps,mixed-burst;
>> +       dma-ranges;
>> +       phy-mode = "mii";
>> +       };
>> +
>> +    //Example 2
>> +     ethernet2: ethernet@40027000 {
>> +       compatible = "st,stm32-dwmac", "snps,dwmac-4.10a";
>> +       reg = <0x40028000 0x8000>;
>> +       reg-names = "stmmaceth";
>> +       interrupts = <61>;
>> +       interrupt-names = "macirq";
>> +       clock-names = "stmmaceth", "mac-clk-tx", "mac-clk-rx";
>> +       clocks = <&rcc 62>, <&rcc 61>, <&rcc 60>;
>> +       st,syscon = <&syscfg 0x4>;
>> +       snps,pbl = <8>;
>> +       status = "disabled";
>> +       phy-mode = "mii";
>> +       };
>> +
>> +
>> -- 
>> 2.17.1
>>