[PATCH v2 00/11] Series to deliver Ethernets for STM32MP13

[PATCH v2 00/11] Series to deliver Ethernets for STM32MP13

[Christophe Roullier]

STM32MP13 is STM32 SOC with 2 GMACs instances
This board have 2 RMII phy:
  -Ethernet1: RMII with crystal
  -Ethernet2: RMII without crystal
Rework dwmac glue to simplify management for next stm32
Add support for PHY regulator

V2: - Remark from Rob Herring (add Krzysztof's ack in patch 02/11, update in yaml)
      Remark from Serge Semin (upate commits msg)

Christophe Roullier (11):
  dt-bindings: net: add STM32MP13 compatible in documentation for stm32
  dt-bindings: net: add phy-supply property for stm32
  net: stmmac: dwmac-stm32: rework glue to simplify management
  net: stmmac: dwmac-stm32: add management of stm32mp13
  net: stmmac: dwmac-stm32: update config management for phy wo cristal
  net: stmmac: dwmac-stm32: clean the way to manage wol irqwake
  net: stmmac: dwmac-stm32: support the phy-supply regulator binding
  ARM: dts: stm32: add ethernet1 and ethernet2 support on stm32mp13
  ARM: dts: stm32: add ethernet1/2 RMII pins for STM32MP13F-DK board
  ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board
  ARM: multi_v7_defconfig: Add MCP23S08 pinctrl support

 .../devicetree/bindings/net/stm32-dwmac.yaml  |  79 +++++-
 arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi   |  71 ++++++
 arch/arm/boot/dts/st/stm32mp131.dtsi          |  31 +++
 arch/arm/boot/dts/st/stm32mp133.dtsi          |  30 +++
 arch/arm/boot/dts/st/stm32mp135f-dk.dts       |  48 ++++
 arch/arm/configs/multi_v7_defconfig           |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 235 ++++++++++++------
 7 files changed, 416 insertions(+), 79 deletions(-)

-- 
2.25.1

[PATCH v2 01/11] dt-bindings: net: add STM32MP13 compatible in documentation for stm32

[Christophe Roullier]

New STM32 SOC have 2 GMACs instances.
GMAC IP version is SNPS 4.20.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../devicetree/bindings/net/stm32-dwmac.yaml  | 76 ++++++++++++++++---
 1 file changed, 67 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
index f2714b5b6cf4..b901a432dfa9 100644
--- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
@@ -22,18 +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,stm32mp1-dwmac
+              - st,stm32mp13-dwmac
           - const: snps,dwmac-4.20a
       - items:
           - enum:
@@ -74,13 +73,16 @@ properties:
 
   st,syscon:
     $ref: /schemas/types.yaml#/definitions/phandle-array
+    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
     items:
-      - items:
+      - minItems: 2
+        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
+          - description: field to set mask in register
 
   st,eth-clk-sel:
     description:
@@ -105,12 +107,38 @@ required:
 
 unevaluatedProperties: false
 
+allOf:
+  - $ref: snps,dwmac.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,stm32mp1-dwmac
+              - st,stm32-dwmac
+    then:
+      properties:
+        st,syscon:
+          items:
+            maxItems: 2
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - st,stm32mp13-dwmac
+    then:
+      properties:
+        st,syscon:
+          items:
+            minItems: 3
+            maxItems: 3
+
 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";
@@ -165,3 +193,33 @@ examples:
            snps,pbl = <8>;
            phy-mode = "mii";
        };
+
+  - |
+     //Example 4
+     ethernet3: ethernet@5800a000 {
+           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";
+     };
-- 
2.25.1

Re: [PATCH v2 01/11] dt-bindings: net: add STM32MP13 compatible in documentation for stm32

[Marek Vasut]

On 4/26/24 2:56 PM, Christophe Roullier wrote:
> New STM32 SOC

Which New STM32 SoC is that ? Please add type/model/...

> have 2 GMACs instances.
> GMAC IP version is SNPS 4.20.

The commit message should describe what does this patch do, and why does 
it do so, something like:

"
Document support for STM32MP13xx. This SoC contains two instances of 
DWMAC 4.20 IP. Because this is <somehow special>, document <some 
difference>.
"

[...]

Re: [PATCH v2 01/11] dt-bindings: net: add STM32MP13 compatible in documentation for stm32

[Rob Herring]

On Fri, Apr 26, 2024 at 02:56:57PM +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@foss.st.com>
> ---
>  .../devicetree/bindings/net/stm32-dwmac.yaml  | 76 ++++++++++++++++---
>  1 file changed, 67 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> index f2714b5b6cf4..b901a432dfa9 100644
> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> @@ -22,18 +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,stm32mp1-dwmac
> +              - st,stm32mp13-dwmac
>            - const: snps,dwmac-4.20a
>        - items:
>            - enum:
> @@ -74,13 +73,16 @@ properties:
>  
>    st,syscon:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:

Why do you move the location of 'description'? It just increases the 
diff. My preference is description is either 1st or last and not in the 
middle.

> +      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
>      items:
> -      - items:
> +      - minItems: 2
> +        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
> +          - description: field to set mask in register
>  
>    st,eth-clk-sel:
>      description:
> @@ -105,12 +107,38 @@ required:
>  
>  unevaluatedProperties: false
>  
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp1-dwmac
> +              - st,stm32-dwmac
> +    then:
> +      properties:
> +        st,syscon:
> +          items:
> +            maxItems: 2
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - st,stm32mp13-dwmac
> +    then:
> +      properties:
> +        st,syscon:
> +          items:
> +            minItems: 3
> +            maxItems: 3

Just minItems is needed.

> +
>  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";
> @@ -165,3 +193,33 @@ examples:
>             snps,pbl = <8>;
>             phy-mode = "mii";
>         };
> +
> +  - |
> +     //Example 4

I don't think it is worth a whole new example just for 1 property 
difference.

> +     ethernet3: ethernet@5800a000 {
> +           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";
> +     };
> -- 
> 2.25.1
> 

[PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Christophe Roullier]

Phandle to a regulator that provides power to the PHY. This
regulator will be managed during the PHY power on/off sequence.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
index b901a432dfa9..7c3aa181abcb 100644
--- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
@@ -84,6 +84,9 @@ properties:
           - description: offset of the control register
           - description: field to set mask in register
 
+  phy-supply:
+    description: PHY regulator
+
   st,eth-clk-sel:
     description:
       set this property in RGMII PHY when you want to select RCC clock instead of ETH_CLK125.
-- 
2.25.1

Re: [PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Marek Vasut]

On 4/26/24 2:56 PM, Christophe Roullier wrote:
> Phandle to a regulator that provides power to the PHY. This
> regulator will be managed during the PHY power on/off sequence.
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>

Maybe this entire regulator business should be separate series from the 
MP13 DWMAC ethernet series ?

Re: [PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Christophe ROULLIER]

Hi,

On 4/26/24 16:47, Marek Vasut wrote:
> On 4/26/24 2:56 PM, Christophe Roullier wrote:
>> Phandle to a regulator that provides power to the PHY. This
>> regulator will be managed during the PHY power on/off sequence.
>>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>
> Maybe this entire regulator business should be separate series from 
> the MP13 DWMAC ethernet series ?
I prefer push it with MP13 Ethernet series if possible.

Re: [PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Marek Vasut]

On 5/13/24 1:45 PM, Christophe ROULLIER wrote:
> Hi,

Hi,

> On 4/26/24 16:47, Marek Vasut wrote:
>> On 4/26/24 2:56 PM, Christophe Roullier wrote:
>>> Phandle to a regulator that provides power to the PHY. This
>>> regulator will be managed during the PHY power on/off sequence.
>>>
>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>>
>> Maybe this entire regulator business should be separate series from 
>> the MP13 DWMAC ethernet series ?
> I prefer push it with MP13 Ethernet series if possible.

This is separate functionality, independent of the MP13 support and not 
required for the MP13 support, correct ?

If yes, move it into separate patch(set) to make both series easier to 
review.

Re: [PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Rob Herring]

On Fri, Apr 26, 2024 at 02:56:58PM +0200, Christophe Roullier wrote:
> Phandle to a regulator that provides power to the PHY. This
> regulator will be managed during the PHY power on/off sequence.
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> ---
>  Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> index b901a432dfa9..7c3aa181abcb 100644
> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
> @@ -84,6 +84,9 @@ properties:
>            - description: offset of the control register
>            - description: field to set mask in register
>  
> +  phy-supply:
> +    description: PHY regulator

This is for which PHY? The serdes phy or ethernet phy? This only makes 
sense here if the phy is part of the MAC. Otherwise, it belongs in the 
phy node.

Rob

Re: [PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Christophe ROULLIER]

Hi

On 4/26/24 17:30, Rob Herring wrote:
> On Fri, Apr 26, 2024 at 02:56:58PM +0200, Christophe Roullier wrote:
>> Phandle to a regulator that provides power to the PHY. This
>> regulator will be managed during the PHY power on/off sequence.
>>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>> ---
>>   Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> index b901a432dfa9..7c3aa181abcb 100644
>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> @@ -84,6 +84,9 @@ properties:
>>             - description: offset of the control register
>>             - description: field to set mask in register
>>   
>> +  phy-supply:
>> +    description: PHY regulator
> This is for which PHY? The serdes phy or ethernet phy? This only makes
> sense here if the phy is part of the MAC. Otherwise, it belongs in the
> phy node.
>
> Rob

You are right, normally it should be managed in Ethernet PHY (Realtek, 
Microchip etc...)

Lots of glue manage this like this. Does it forbidden now ? if yes need 
to update PHY driver to manage this property.

Re: [PATCH v2 02/11] dt-bindings: net: add phy-supply property for stm32

[Marek Vasut]

On 5/13/24 4:06 PM, Christophe ROULLIER wrote:
> Hi
> 
> On 4/26/24 17:30, Rob Herring wrote:
>> On Fri, Apr 26, 2024 at 02:56:58PM +0200, Christophe Roullier wrote:
>>> Phandle to a regulator that provides power to the PHY. This
>>> regulator will be managed during the PHY power on/off sequence.
>>>
>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>>> ---
>>>   Documentation/devicetree/bindings/net/stm32-dwmac.yaml | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml 
>>> b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>> index b901a432dfa9..7c3aa181abcb 100644
>>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>> @@ -84,6 +84,9 @@ properties:
>>>             - description: offset of the control register
>>>             - description: field to set mask in register
>>> +  phy-supply:
>>> +    description: PHY regulator
>> This is for which PHY? The serdes phy or ethernet phy? This only makes
>> sense here if the phy is part of the MAC. Otherwise, it belongs in the
>> phy node.
>>
>> Rob
> 
> You are right, normally it should be managed in Ethernet PHY (Realtek, 
> Microchip etc...)
> 
> Lots of glue manage this like this. Does it forbidden now ? if yes need 
> to update PHY driver to manage this property.

If the regulator is connected to the PHY, then the supply should be 
described in the PHY node and you wouldn't even need these PHY patches 
(also see my comment that you should split the PHY regulator part of 
this patchset into separate series).

[PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to simplify management

[Christophe Roullier]

Change glue to be more generic and manage easily next stm32 products.
The goal of this commit is to have one stm32mp1_set_mode function which
can manage different STM32 SOC. SOC can have different SYSCFG register
bitfields. so in pmcsetr we defined the bitfields corresponding to the SOC.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 76 +++++++++++++------
 1 file changed, 51 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index c92dfc4ecf57..68a02de25ac7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -23,10 +23,6 @@
 
 #define SYSCFG_MCU_ETH_MASK		BIT(23)
 #define SYSCFG_MP1_ETH_MASK		GENMASK(23, 16)
-#define SYSCFG_PMCCLRR_OFFSET		0x40
-
-#define SYSCFG_PMCR_ETH_CLK_SEL		BIT(16)
-#define SYSCFG_PMCR_ETH_REF_CLK_SEL	BIT(17)
 
 /* CLOCK feed to PHY*/
 #define ETH_CK_F_25M	25000000
@@ -46,9 +42,6 @@
  * RMII  |   1	 |   0	  |   0	   |  n/a  |
  *------------------------------------------
  */
-#define SYSCFG_PMCR_ETH_SEL_MII		BIT(20)
-#define SYSCFG_PMCR_ETH_SEL_RGMII	BIT(21)
-#define SYSCFG_PMCR_ETH_SEL_RMII	BIT(23)
 #define SYSCFG_PMCR_ETH_SEL_GMII	0
 #define SYSCFG_MCU_ETH_SEL_MII		0
 #define SYSCFG_MCU_ETH_SEL_RMII		1
@@ -90,19 +83,33 @@ struct stm32_dwmac {
 	int eth_ref_clk_sel_reg;
 	int irq_pwr_wakeup;
 	u32 mode_reg;		 /* MAC glue-logic mode register */
+	u32 mode_mask;
 	struct regmap *regmap;
 	u32 speed;
 	const struct stm32_ops *ops;
 	struct device *dev;
 };
 
+struct stm32_syscfg_pmcsetr {
+	u32 eth1_clk_sel;
+	u32 eth1_ref_clk_sel;
+	u32 eth1_selmii;
+	u32 eth1_sel_rgmii;
+	u32 eth1_sel_rmii;
+	u32 eth2_clk_sel;
+	u32 eth2_ref_clk_sel;
+	u32 eth2_sel_rgmii;
+	u32 eth2_sel_rmii;
+};
+
 struct stm32_ops {
 	int (*set_mode)(struct plat_stmmacenet_data *plat_dat);
 	int (*suspend)(struct stm32_dwmac *dwmac);
 	void (*resume)(struct stm32_dwmac *dwmac);
 	int (*parse_data)(struct stm32_dwmac *dwmac,
 			  struct device *dev);
-	u32 syscfg_eth_mask;
+	u32 syscfg_clr_off;
+	struct stm32_syscfg_pmcsetr pmcsetr;
 	bool clk_rx_enable_in_suspend;
 };
 
@@ -161,7 +168,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 {
 	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
 	u32 reg = dwmac->mode_reg, clk_rate;
-	int val;
+	int val = 0;
 
 	clk_rate = clk_get_rate(dwmac->clk_eth_ck);
 	dwmac->enable_eth_ck = false;
@@ -169,7 +176,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	case PHY_INTERFACE_MODE_MII:
 		if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
 			dwmac->enable_eth_ck = true;
-		val = SYSCFG_PMCR_ETH_SEL_MII;
+		val = dwmac->ops->pmcsetr.eth1_selmii;
 		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
 		break;
 	case PHY_INTERFACE_MODE_GMII:
@@ -177,16 +184,17 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 		if (clk_rate == ETH_CK_F_25M &&
 		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
 			dwmac->enable_eth_ck = true;
-			val |= SYSCFG_PMCR_ETH_CLK_SEL;
+			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
 		}
 		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
 		break;
 	case PHY_INTERFACE_MODE_RMII:
-		val = SYSCFG_PMCR_ETH_SEL_RMII;
+		val = dwmac->ops->pmcsetr.eth1_sel_rmii | dwmac->ops->pmcsetr.eth2_sel_rmii;
 		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) &&
 		    (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {
 			dwmac->enable_eth_ck = true;
-			val |= SYSCFG_PMCR_ETH_REF_CLK_SEL;
+			val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel;
+			val |= dwmac->ops->pmcsetr.eth2_ref_clk_sel;
 		}
 		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RMII\n");
 		break;
@@ -194,11 +202,12 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	case PHY_INTERFACE_MODE_RGMII_ID:
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
-		val = SYSCFG_PMCR_ETH_SEL_RGMII;
+		val = dwmac->ops->pmcsetr.eth1_sel_rgmii | dwmac->ops->pmcsetr.eth2_sel_rgmii;
 		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) &&
 		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
 			dwmac->enable_eth_ck = true;
-			val |= SYSCFG_PMCR_ETH_CLK_SEL;
+			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
+			val |= dwmac->ops->pmcsetr.eth2_clk_sel;
 		}
 		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RGMII\n");
 		break;
@@ -210,12 +219,12 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	}
 
 	/* Need to update PMCCLRR (clear register) */
-	regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
-		     dwmac->ops->syscfg_eth_mask);
+	regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off,
+		     dwmac->mode_mask);
 
 	/* Update PMCSETR (set register) */
 	return regmap_update_bits(dwmac->regmap, reg,
-				 dwmac->ops->syscfg_eth_mask, val);
+				 dwmac->mode_mask, val);
 }
 
 static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
@@ -241,7 +250,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
 	}
 
 	return regmap_update_bits(dwmac->regmap, reg,
-				 dwmac->ops->syscfg_eth_mask, val << 23);
+				 SYSCFG_MCU_ETH_MASK, val << 23);
 }
 
 static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, bool suspend)
@@ -286,10 +295,17 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
 		return PTR_ERR(dwmac->regmap);
 
 	err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
+	if (err) {
+		dev_err(dev, "Can't get sysconfig register offset (%d)\n", err);
+		return err;
+	}
+
+	dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
+	err = of_property_read_u32_index(np, "st,syscon", 2, &dwmac->mode_mask);
 	if (err)
-		dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
+		pr_debug("Warning sysconfig register mask not set\n");
 
-	return err;
+	return 0;
 }
 
 static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
@@ -478,8 +494,7 @@ static SIMPLE_DEV_PM_OPS(stm32_dwmac_pm_ops,
 	stm32_dwmac_suspend, stm32_dwmac_resume);
 
 static struct stm32_ops stm32mcu_dwmac_data = {
-	.set_mode = stm32mcu_set_mode,
-	.syscfg_eth_mask = SYSCFG_MCU_ETH_MASK
+	.set_mode = stm32mcu_set_mode
 };
 
 static struct stm32_ops stm32mp1_dwmac_data = {
@@ -487,8 +502,19 @@ static struct stm32_ops stm32mp1_dwmac_data = {
 	.suspend = stm32mp1_suspend,
 	.resume = stm32mp1_resume,
 	.parse_data = stm32mp1_parse_data,
-	.syscfg_eth_mask = SYSCFG_MP1_ETH_MASK,
-	.clk_rx_enable_in_suspend = true
+	.clk_rx_enable_in_suspend = true,
+	.syscfg_clr_off = 0x44,
+	.pmcsetr = {
+		.eth1_clk_sel		= BIT(16),
+		.eth1_ref_clk_sel	= BIT(17),
+		.eth1_selmii		= BIT(20),
+		.eth1_sel_rgmii		= BIT(21),
+		.eth1_sel_rmii		= BIT(23),
+		.eth2_clk_sel		= 0,
+		.eth2_ref_clk_sel	= 0,
+		.eth2_sel_rgmii		= 0,
+		.eth2_sel_rmii		= 0
+	}
 };
 
 static const struct of_device_id stm32_dwmac_match[] = {
-- 
2.25.1

Re: [PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to simplify management

[Marek Vasut]

On 4/26/24 2:56 PM, Christophe Roullier wrote:
> Change glue to be more generic and manage easily next stm32 products.
> The goal of this commit is to have one stm32mp1_set_mode function which
> can manage different STM32 SOC. SOC can have different SYSCFG register
> bitfields. so in pmcsetr we defined the bitfields corresponding to the SOC.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> ---
>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 76 +++++++++++++------
>   1 file changed, 51 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> index c92dfc4ecf57..68a02de25ac7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -23,10 +23,6 @@
>   
>   #define SYSCFG_MCU_ETH_MASK		BIT(23)
>   #define SYSCFG_MP1_ETH_MASK		GENMASK(23, 16)
> -#define SYSCFG_PMCCLRR_OFFSET		0x40
> -
> -#define SYSCFG_PMCR_ETH_CLK_SEL		BIT(16)
> -#define SYSCFG_PMCR_ETH_REF_CLK_SEL	BIT(17)
>   
>   /* CLOCK feed to PHY*/
>   #define ETH_CK_F_25M	25000000
> @@ -46,9 +42,6 @@
>    * RMII  |   1	 |   0	  |   0	   |  n/a  |
>    *------------------------------------------
>    */
> -#define SYSCFG_PMCR_ETH_SEL_MII		BIT(20)
> -#define SYSCFG_PMCR_ETH_SEL_RGMII	BIT(21)
> -#define SYSCFG_PMCR_ETH_SEL_RMII	BIT(23)
>   #define SYSCFG_PMCR_ETH_SEL_GMII	0
>   #define SYSCFG_MCU_ETH_SEL_MII		0
>   #define SYSCFG_MCU_ETH_SEL_RMII		1
> @@ -90,19 +83,33 @@ struct stm32_dwmac {
>   	int eth_ref_clk_sel_reg;
>   	int irq_pwr_wakeup;
>   	u32 mode_reg;		 /* MAC glue-logic mode register */
> +	u32 mode_mask;
>   	struct regmap *regmap;
>   	u32 speed;
>   	const struct stm32_ops *ops;
>   	struct device *dev;
>   };
>   
> +struct stm32_syscfg_pmcsetr {
> +	u32 eth1_clk_sel;
> +	u32 eth1_ref_clk_sel;
> +	u32 eth1_selmii;
> +	u32 eth1_sel_rgmii;
> +	u32 eth1_sel_rmii;
> +	u32 eth2_clk_sel;
> +	u32 eth2_ref_clk_sel;
> +	u32 eth2_sel_rgmii;
> +	u32 eth2_sel_rmii;
> +};

[...]

> @@ -487,8 +502,19 @@ static struct stm32_ops stm32mp1_dwmac_data = {
>   	.suspend = stm32mp1_suspend,
>   	.resume = stm32mp1_resume,
>   	.parse_data = stm32mp1_parse_data,
> -	.syscfg_eth_mask = SYSCFG_MP1_ETH_MASK,
> -	.clk_rx_enable_in_suspend = true
> +	.clk_rx_enable_in_suspend = true,
> +	.syscfg_clr_off = 0x44,
> +	.pmcsetr = {
> +		.eth1_clk_sel		= BIT(16),
> +		.eth1_ref_clk_sel	= BIT(17),
> +		.eth1_selmii		= BIT(20),
> +		.eth1_sel_rgmii		= BIT(21),
> +		.eth1_sel_rmii		= BIT(23),
> +		.eth2_clk_sel		= 0,
> +		.eth2_ref_clk_sel	= 0,
> +		.eth2_sel_rgmii		= 0,
> +		.eth2_sel_rmii		= 0
> +	}
>   };

Is this structure really necessary ?

It seems the MP15 single ethernet config bitfield is at offset 16.
MP13 has two bitfields, one at offset 16, the other at offset 24 .

All you need to do is figure out which of the two MACs you are 
configuring, and then shift the bitfield mask by 16 or 24, since the 
bits are at the same offset for both bitfields.

See the matching upstream U-Boot commit for how this shift can be done:
a440d19c6c91 ("net: dwc_eth_qos: Add DT parsing for STM32MP13xx platform")

Re: [PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to simplify management

[Christophe ROULLIER]

Hi

On 4/26/24 16:53, Marek Vasut wrote:
> On 4/26/24 2:56 PM, Christophe Roullier wrote:
>> Change glue to be more generic and manage easily next stm32 products.
>> The goal of this commit is to have one stm32mp1_set_mode function which
>> can manage different STM32 SOC. SOC can have different SYSCFG register
>> bitfields. so in pmcsetr we defined the bitfields corresponding to 
>> the SOC.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>> ---
>>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 76 +++++++++++++------
>>   1 file changed, 51 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c 
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> index c92dfc4ecf57..68a02de25ac7 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> @@ -23,10 +23,6 @@
>>     #define SYSCFG_MCU_ETH_MASK        BIT(23)
>>   #define SYSCFG_MP1_ETH_MASK        GENMASK(23, 16)
>> -#define SYSCFG_PMCCLRR_OFFSET        0x40
>> -
>> -#define SYSCFG_PMCR_ETH_CLK_SEL        BIT(16)
>> -#define SYSCFG_PMCR_ETH_REF_CLK_SEL    BIT(17)
>>     /* CLOCK feed to PHY*/
>>   #define ETH_CK_F_25M    25000000
>> @@ -46,9 +42,6 @@
>>    * RMII  |   1     |   0      |   0       |  n/a  |
>>    *------------------------------------------
>>    */
>> -#define SYSCFG_PMCR_ETH_SEL_MII        BIT(20)
>> -#define SYSCFG_PMCR_ETH_SEL_RGMII    BIT(21)
>> -#define SYSCFG_PMCR_ETH_SEL_RMII    BIT(23)
>>   #define SYSCFG_PMCR_ETH_SEL_GMII    0
>>   #define SYSCFG_MCU_ETH_SEL_MII        0
>>   #define SYSCFG_MCU_ETH_SEL_RMII        1
>> @@ -90,19 +83,33 @@ struct stm32_dwmac {
>>       int eth_ref_clk_sel_reg;
>>       int irq_pwr_wakeup;
>>       u32 mode_reg;         /* MAC glue-logic mode register */
>> +    u32 mode_mask;
>>       struct regmap *regmap;
>>       u32 speed;
>>       const struct stm32_ops *ops;
>>       struct device *dev;
>>   };
>>   +struct stm32_syscfg_pmcsetr {
>> +    u32 eth1_clk_sel;
>> +    u32 eth1_ref_clk_sel;
>> +    u32 eth1_selmii;
>> +    u32 eth1_sel_rgmii;
>> +    u32 eth1_sel_rmii;
>> +    u32 eth2_clk_sel;
>> +    u32 eth2_ref_clk_sel;
>> +    u32 eth2_sel_rgmii;
>> +    u32 eth2_sel_rmii;
>> +};
>
> [...]
>
>> @@ -487,8 +502,19 @@ static struct stm32_ops stm32mp1_dwmac_data = {
>>       .suspend = stm32mp1_suspend,
>>       .resume = stm32mp1_resume,
>>       .parse_data = stm32mp1_parse_data,
>> -    .syscfg_eth_mask = SYSCFG_MP1_ETH_MASK,
>> -    .clk_rx_enable_in_suspend = true
>> +    .clk_rx_enable_in_suspend = true,
>> +    .syscfg_clr_off = 0x44,
>> +    .pmcsetr = {
>> +        .eth1_clk_sel        = BIT(16),
>> +        .eth1_ref_clk_sel    = BIT(17),
>> +        .eth1_selmii        = BIT(20),
>> +        .eth1_sel_rgmii        = BIT(21),
>> +        .eth1_sel_rmii        = BIT(23),
>> +        .eth2_clk_sel        = 0,
>> +        .eth2_ref_clk_sel    = 0,
>> +        .eth2_sel_rgmii        = 0,
>> +        .eth2_sel_rmii        = 0
>> +    }
>>   };
>
> Is this structure really necessary ?
>
I prefer to keep this implementation for the moment, as it is working 
fine. Maybe at a later stage, I will send some optimizations.

> It seems the MP15 single ethernet config bitfield is at offset 16.
> MP13 has two bitfields, one at offset 16, the other at offset 24 .
>
> All you need to do is figure out which of the two MACs you are 
> configuring, and then shift the bitfield mask by 16 or 24, since the 
> bits are at the same offset for both bitfields.
>
> See the matching upstream U-Boot commit for how this shift can be done:
> a440d19c6c91 ("net: dwc_eth_qos: Add DT parsing for STM32MP13xx 
> platform")
>

Re: [PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to simplify management

[Marek Vasut]

On 5/13/24 2:48 PM, Christophe ROULLIER wrote:
> Hi
> 
> On 4/26/24 16:53, Marek Vasut wrote:
>> On 4/26/24 2:56 PM, Christophe Roullier wrote:
>>> Change glue to be more generic and manage easily next stm32 products.
>>> The goal of this commit is to have one stm32mp1_set_mode function which
>>> can manage different STM32 SOC. SOC can have different SYSCFG register
>>> bitfields. so in pmcsetr we defined the bitfields corresponding to 
>>> the SOC.
>>>
>>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>>> ---
>>>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 76 +++++++++++++------
>>>   1 file changed, 51 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c 
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>> index c92dfc4ecf57..68a02de25ac7 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>> @@ -23,10 +23,6 @@
>>>     #define SYSCFG_MCU_ETH_MASK        BIT(23)
>>>   #define SYSCFG_MP1_ETH_MASK        GENMASK(23, 16)
>>> -#define SYSCFG_PMCCLRR_OFFSET        0x40
>>> -
>>> -#define SYSCFG_PMCR_ETH_CLK_SEL        BIT(16)
>>> -#define SYSCFG_PMCR_ETH_REF_CLK_SEL    BIT(17)
>>>     /* CLOCK feed to PHY*/
>>>   #define ETH_CK_F_25M    25000000
>>> @@ -46,9 +42,6 @@
>>>    * RMII  |   1     |   0      |   0       |  n/a  |
>>>    *------------------------------------------
>>>    */
>>> -#define SYSCFG_PMCR_ETH_SEL_MII        BIT(20)
>>> -#define SYSCFG_PMCR_ETH_SEL_RGMII    BIT(21)
>>> -#define SYSCFG_PMCR_ETH_SEL_RMII    BIT(23)
>>>   #define SYSCFG_PMCR_ETH_SEL_GMII    0
>>>   #define SYSCFG_MCU_ETH_SEL_MII        0
>>>   #define SYSCFG_MCU_ETH_SEL_RMII        1
>>> @@ -90,19 +83,33 @@ struct stm32_dwmac {
>>>       int eth_ref_clk_sel_reg;
>>>       int irq_pwr_wakeup;
>>>       u32 mode_reg;         /* MAC glue-logic mode register */
>>> +    u32 mode_mask;
>>>       struct regmap *regmap;
>>>       u32 speed;
>>>       const struct stm32_ops *ops;
>>>       struct device *dev;
>>>   };
>>>   +struct stm32_syscfg_pmcsetr {
>>> +    u32 eth1_clk_sel;
>>> +    u32 eth1_ref_clk_sel;
>>> +    u32 eth1_selmii;
>>> +    u32 eth1_sel_rgmii;
>>> +    u32 eth1_sel_rmii;
>>> +    u32 eth2_clk_sel;
>>> +    u32 eth2_ref_clk_sel;
>>> +    u32 eth2_sel_rgmii;
>>> +    u32 eth2_sel_rmii;
>>> +};
>>
>> [...]
>>
>>> @@ -487,8 +502,19 @@ static struct stm32_ops stm32mp1_dwmac_data = {
>>>       .suspend = stm32mp1_suspend,
>>>       .resume = stm32mp1_resume,
>>>       .parse_data = stm32mp1_parse_data,
>>> -    .syscfg_eth_mask = SYSCFG_MP1_ETH_MASK,
>>> -    .clk_rx_enable_in_suspend = true
>>> +    .clk_rx_enable_in_suspend = true,
>>> +    .syscfg_clr_off = 0x44,
>>> +    .pmcsetr = {
>>> +        .eth1_clk_sel        = BIT(16),
>>> +        .eth1_ref_clk_sel    = BIT(17),
>>> +        .eth1_selmii        = BIT(20),
>>> +        .eth1_sel_rgmii        = BIT(21),
>>> +        .eth1_sel_rmii        = BIT(23),
>>> +        .eth2_clk_sel        = 0,
>>> +        .eth2_ref_clk_sel    = 0,
>>> +        .eth2_sel_rgmii        = 0,
>>> +        .eth2_sel_rmii        = 0
>>> +    }
>>>   };
>>
>> Is this structure really necessary ?
>>
> I prefer to keep this implementation for the moment, as it is working 
> fine. Maybe at a later stage, I will send some optimizations.

BIT() and left shift by 8 works all the same, without all this 
complexity and new structures, since all you really have on MP13 are two 
identical bitfields (one at offset 16, the other at offset 24) with the 
same bits in them, so why not make this simple ?

[PATCH v2 04/11] net: stmmac: dwmac-stm32: add management of stm32mp13

[Christophe Roullier]

Add Ethernet support for STM32MP13.
STM32MP13 is STM32 SOC with 2 GMACs instances.
GMAC IP version is SNPS 4.20.
GMAC IP configure with 1 RX and 1 TX queue.
DMA HW capability register supported
RX Checksum Offload Engine supported
TX Checksum insertion supported
Wake-Up On Lan supported
TSO supported

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 68a02de25ac7..7529a8d15492 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -517,9 +517,30 @@ static struct stm32_ops stm32mp1_dwmac_data = {
 	}
 };
 
+static struct stm32_ops stm32mp13_dwmac_data = {
+	.set_mode = stm32mp1_set_mode,
+	.suspend = stm32mp1_suspend,
+	.resume = stm32mp1_resume,
+	.parse_data = stm32mp1_parse_data,
+	.clk_rx_enable_in_suspend = true,
+	.syscfg_clr_off = 0x08,
+	.pmcsetr = {
+		.eth1_clk_sel		= BIT(16),
+		.eth1_ref_clk_sel	= BIT(17),
+		.eth1_selmii		= 0,
+		.eth1_sel_rgmii		= BIT(21),
+		.eth1_sel_rmii		= BIT(23),
+		.eth2_clk_sel		= BIT(24),
+		.eth2_ref_clk_sel	= BIT(25),
+		.eth2_sel_rgmii		= BIT(29),
+		.eth2_sel_rmii		= BIT(31)
+	}
+};
+
 static const struct of_device_id stm32_dwmac_match[] = {
 	{ .compatible = "st,stm32-dwmac", .data = &stm32mcu_dwmac_data},
 	{ .compatible = "st,stm32mp1-dwmac", .data = &stm32mp1_dwmac_data},
+	{ .compatible = "st,stm32mp13-dwmac", .data = &stm32mp13_dwmac_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
-- 
2.25.1

[PATCH v2 05/11] net: stmmac: dwmac-stm32: update config management for phy wo cristal

[Christophe Roullier]

Some cleaning because some Ethernet PHY configs do not need to add
st,ext-phyclk property.
Change print info message "No phy clock provided" only when debug.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++---------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 7529a8d15492..e648c4e790a7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -55,17 +55,17 @@
  *|         |        |      25MHz    |        50MHz       |                  |
  * ---------------------------------------------------------------------------
  *|  MII    |	 -   |     eth-ck    |	      n/a	  |	  n/a        |
- *|         |        | st,ext-phyclk |                    |		     |
+ *|         |        |	             |                    |		     |
  * ---------------------------------------------------------------------------
  *|  GMII   |	 -   |     eth-ck    |	      n/a	  |	  n/a        |
- *|         |        | st,ext-phyclk |                    |		     |
+ *|         |        |               |                    |		     |
  * ---------------------------------------------------------------------------
  *| RGMII   |	 -   |     eth-ck    |	      n/a	  |      eth-ck      |
- *|         |        | st,ext-phyclk |                    | st,eth-clk-sel or|
+ *|         |        |               |                    | st,eth-clk-sel or|
  *|         |        |               |                    | st,ext-phyclk    |
  * ---------------------------------------------------------------------------
  *| RMII    |	 -   |     eth-ck    |	    eth-ck        |	  n/a        |
- *|         |        | st,ext-phyclk | st,eth-ref-clk-sel |		     |
+ *|         |        |               | st,eth-ref-clk-sel |		     |
  *|         |        |               | or st,ext-phyclk   |		     |
  * ---------------------------------------------------------------------------
  *
@@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	dwmac->enable_eth_ck = false;
 	switch (plat_dat->mac_interface) {
 	case PHY_INTERFACE_MODE_MII:
-		if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
+		if (clk_rate == ETH_CK_F_25M)
 			dwmac->enable_eth_ck = true;
 		val = dwmac->ops->pmcsetr.eth1_selmii;
 		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
 		break;
 	case PHY_INTERFACE_MODE_GMII:
 		val = SYSCFG_PMCR_ETH_SEL_GMII;
-		if (clk_rate == ETH_CK_F_25M &&
-		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
+		if (clk_rate == ETH_CK_F_25M)
 			dwmac->enable_eth_ck = true;
-			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
-		}
 		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
 		break;
 	case PHY_INTERFACE_MODE_RMII:
 		val = dwmac->ops->pmcsetr.eth1_sel_rmii | dwmac->ops->pmcsetr.eth2_sel_rmii;
-		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) &&
+		if (clk_rate == ETH_CK_F_25M)
+			dwmac->enable_eth_ck = true;
+		if (clk_rate == ETH_CK_F_50M &&
 		    (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {
 			dwmac->enable_eth_ck = true;
 			val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel;
@@ -203,7 +202,9 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	case PHY_INTERFACE_MODE_RGMII_RXID:
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		val = dwmac->ops->pmcsetr.eth1_sel_rgmii | dwmac->ops->pmcsetr.eth2_sel_rgmii;
-		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) &&
+		if (clk_rate == ETH_CK_F_25M)
+			dwmac->enable_eth_ck = true;
+		if (clk_rate == ETH_CK_F_125M &&
 		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
 			dwmac->enable_eth_ck = true;
 			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
@@ -219,7 +220,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
 	}
 
 	/* Need to update PMCCLRR (clear register) */
-	regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off,
+	regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off,
 		     dwmac->mode_mask);
 
 	/* Update PMCSETR (set register) */
@@ -328,7 +329,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
 	/*  Get ETH_CLK clocks */
 	dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck");
 	if (IS_ERR(dwmac->clk_eth_ck)) {
-		dev_info(dev, "No phy clock provided...\n");
+		dev_dbg(dev, "No phy clock provided...\n");
 		dwmac->clk_eth_ck = NULL;
 	}
 
-- 
2.25.1

Re: [PATCH v2 05/11] net: stmmac: dwmac-stm32: update config management for phy wo cristal

[Marek Vasut]

On 4/26/24 2:57 PM, Christophe Roullier wrote:
> Some cleaning because some Ethernet PHY configs do not need to add
> st,ext-phyclk property.
> Change print info message "No phy clock provided" only when debug.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
> ---
>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++---------
>   1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> index 7529a8d15492..e648c4e790a7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
> @@ -55,17 +55,17 @@
>    *|         |        |      25MHz    |        50MHz       |                  |
>    * ---------------------------------------------------------------------------
>    *|  MII    |	 -   |     eth-ck    |	      n/a	  |	  n/a        |
> - *|         |        | st,ext-phyclk |                    |		     |
> + *|         |        |	             |                    |		     |
>    * ---------------------------------------------------------------------------
>    *|  GMII   |	 -   |     eth-ck    |	      n/a	  |	  n/a        |
> - *|         |        | st,ext-phyclk |                    |		     |
> + *|         |        |               |                    |		     |
>    * ---------------------------------------------------------------------------
>    *| RGMII   |	 -   |     eth-ck    |	      n/a	  |      eth-ck      |
> - *|         |        | st,ext-phyclk |                    | st,eth-clk-sel or|
> + *|         |        |               |                    | st,eth-clk-sel or|
>    *|         |        |               |                    | st,ext-phyclk    |
>    * ---------------------------------------------------------------------------
>    *| RMII    |	 -   |     eth-ck    |	    eth-ck        |	  n/a        |
> - *|         |        | st,ext-phyclk | st,eth-ref-clk-sel |		     |
> + *|         |        |               | st,eth-ref-clk-sel |		     |
>    *|         |        |               | or st,ext-phyclk   |		     |
>    * ---------------------------------------------------------------------------
>    *
> @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>   	dwmac->enable_eth_ck = false;
>   	switch (plat_dat->mac_interface) {
>   	case PHY_INTERFACE_MODE_MII:
> -		if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
> +		if (clk_rate == ETH_CK_F_25M)

I see two problems here.

First, according to the table above, in MII mode, clk_rate cannot be 
anything else but 25 MHz, so the (clk_rate == ETH_CK_F_25M) condition is 
always true. Why not drop that condition ?

The "dwmac->ext_phyclk" means "Ethernet PHY have no crystal", which 
means the clock are provided by the STM32 RCC clock IP instead, which 
means if the dwmac->ext_phyclk is true, dwmac->enable_eth_ck should be 
set to true, because dwmac->enable_eth_ck controls the enablement of 
these STM32 clock IP generated clock.

Second, as far as I understand it, there is no way to operate this IP 
with external clock in MII mode, so this section should always be only:

dwmac->enable_eth_ck = true;

>   			dwmac->enable_eth_ck = true;
>   		val = dwmac->ops->pmcsetr.eth1_selmii;
>   		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
>   		break;
>   	case PHY_INTERFACE_MODE_GMII:
>   		val = SYSCFG_PMCR_ETH_SEL_GMII;
> -		if (clk_rate == ETH_CK_F_25M &&
> -		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
> +		if (clk_rate == ETH_CK_F_25M)
>   			dwmac->enable_eth_ck = true;
> -			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
> -		}
>   		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
>   		break;
>   	case PHY_INTERFACE_MODE_RMII:
>   		val = dwmac->ops->pmcsetr.eth1_sel_rmii | dwmac->ops->pmcsetr.eth2_sel_rmii;
> -		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) &&
> +		if (clk_rate == ETH_CK_F_25M)
> +			dwmac->enable_eth_ck = true;
> +		if (clk_rate == ETH_CK_F_50M &&
>   		    (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {

This doesn't seem to be equivalent change to the previous code . Here, 
if the clock frequency is 25 MHz, the clock are unconditionally enabled. 
Before, the code enabled the clock only if clock frequency was 25 MHz 
AND one of the "dwmac->eth_ref_clk_sel_reg" or "dwmac->ext_phyclk" was 
set (i.e. clock provided by SoC RCC clock IP).

I think it might make this code easier if you drop all of the frequency 
test conditionals, which aren't really all that useful, and only enable 
the clock if either dwmac->ext_phyclk / dwmac->eth_clk_sel_reg / 
dwmac->eth_ref_clk_sel_reg is set , because effectively what this entire 
convoluted code is implementing is "if (clock supplied by clock IP i.e. 
RCC) enable the clock()" *, right ?

* And it is also toggling the right clock mux bit in PMCSETR.

So, for MII this would be plain:
dwmac->enable_eth_ck = true;

For GMII/RGMII this would be:
if (dwmac->ext_phyclk || dwmac->eth_clk_sel_reg)
   dwmac->enable_eth_ck = true;

For RMII this would be:
if (dwmac->ext_phyclk || dwmac->eth_ref_clk_sel_reg)
   dwmac->enable_eth_ck = true;

Maybe the clock frequency validation can be retained, but done separately?

>   			dwmac->enable_eth_ck = true;
>   			val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel;
> @@ -203,7 +202,9 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>   	case PHY_INTERFACE_MODE_RGMII_RXID:
>   	case PHY_INTERFACE_MODE_RGMII_TXID:
>   		val = dwmac->ops->pmcsetr.eth1_sel_rgmii | dwmac->ops->pmcsetr.eth2_sel_rgmii;
> -		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) &&
> +		if (clk_rate == ETH_CK_F_25M)
> +			dwmac->enable_eth_ck = true;
> +		if (clk_rate == ETH_CK_F_125M &&
>   		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
>   			dwmac->enable_eth_ck = true;
>   			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
> @@ -219,7 +220,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>   	}
>   
>   	/* Need to update PMCCLRR (clear register) */
> -	regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off,
> +	regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off,
>   		     dwmac->mode_mask);
>   
>   	/* Update PMCSETR (set register) */
> @@ -328,7 +329,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
>   	/*  Get ETH_CLK clocks */
>   	dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck");
>   	if (IS_ERR(dwmac->clk_eth_ck)) {
> -		dev_info(dev, "No phy clock provided...\n");
> +		dev_dbg(dev, "No phy clock provided...\n");
>   		dwmac->clk_eth_ck = NULL;
>   	}
>   

Re: [PATCH v2 05/11] net: stmmac: dwmac-stm32: update config management for phy wo cristal

[Christophe ROULLIER]

On 4/26/24 17:37, Marek Vasut wrote:
> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>> Some cleaning because some Ethernet PHY configs do not need to add
>> st,ext-phyclk property.
>> Change print info message "No phy clock provided" only when debug.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>> ---
>>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++---------
>>   1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c 
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> index 7529a8d15492..e648c4e790a7 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> @@ -55,17 +55,17 @@
>>    *|         |        |      25MHz    |        50MHz 
>> |                  |
>>    * 
>> ---------------------------------------------------------------------------
>>    *|  MII    |     -   |     eth-ck    |          n/a |      
>> n/a        |
>> - *|         |        | st,ext-phyclk | |             |
>> + *|         |        |                 | |             |
>>    * 
>> ---------------------------------------------------------------------------
>>    *|  GMII   |     -   |     eth-ck    |          n/a |      
>> n/a        |
>> - *|         |        | st,ext-phyclk | |             |
>> + *|         |        |               | |             |
>>    * 
>> ---------------------------------------------------------------------------
>>    *| RGMII   |     -   |     eth-ck    |          n/a |      
>> eth-ck      |
>> - *|         |        | st,ext-phyclk |                    | 
>> st,eth-clk-sel or|
>> + *|         |        |               |                    | 
>> st,eth-clk-sel or|
>>    *|         |        |               |                    | 
>> st,ext-phyclk    |
>>    * 
>> ---------------------------------------------------------------------------
>>    *| RMII    |     -   |     eth-ck    |        eth-ck |      
>> n/a        |
>> - *|         |        | st,ext-phyclk | st,eth-ref-clk-sel 
>> |             |
>> + *|         |        |               | st,eth-ref-clk-sel 
>> |             |
>>    *|         |        |               | or st,ext-phyclk 
>> |             |
>>    * 
>> ---------------------------------------------------------------------------
>>    *
>> @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct 
>> plat_stmmacenet_data *plat_dat)
>>       dwmac->enable_eth_ck = false;
>>       switch (plat_dat->mac_interface) {
>>       case PHY_INTERFACE_MODE_MII:
>> -        if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
>> +        if (clk_rate == ETH_CK_F_25M)
>
> I see two problems here.
>
> First, according to the table above, in MII mode, clk_rate cannot be 
> anything else but 25 MHz, so the (clk_rate == ETH_CK_F_25M) condition 
> is always true. Why not drop that condition ?
Not agree, there is also "Normal" case MII (MII with quartz/cristal) 
(first column in the table above), so need to keep this test to check 
clk_rate 25MHz.
>
> The "dwmac->ext_phyclk" means "Ethernet PHY have no crystal", which 
> means the clock are provided by the STM32 RCC clock IP instead, which 
> means if the dwmac->ext_phyclk is true, dwmac->enable_eth_ck should be 
> set to true, because dwmac->enable_eth_ck controls the enablement of 
> these STM32 clock IP generated clock.
Right
>
> Second, as far as I understand it, there is no way to operate this IP 
> with external clock in MII mode, so this section should always be only:
>
> dwmac->enable_eth_ck = true;
Not for case "Normal" MII :-)
>
>>               dwmac->enable_eth_ck = true;
>>           val = dwmac->ops->pmcsetr.eth1_selmii;
>>           pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
>>           break;
>>       case PHY_INTERFACE_MODE_GMII:
>>           val = SYSCFG_PMCR_ETH_SEL_GMII;
>> -        if (clk_rate == ETH_CK_F_25M &&
>> -            (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
>> +        if (clk_rate == ETH_CK_F_25M)
>>               dwmac->enable_eth_ck = true;
>> -            val |= dwmac->ops->pmcsetr.eth1_clk_sel;
>> -        }
>>           pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
>>           break;
>>       case PHY_INTERFACE_MODE_RMII:
>>           val = dwmac->ops->pmcsetr.eth1_sel_rmii | 
>> dwmac->ops->pmcsetr.eth2_sel_rmii;
>> -        if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) &&
>> +        if (clk_rate == ETH_CK_F_25M)
>> +            dwmac->enable_eth_ck = true;
>> +        if (clk_rate == ETH_CK_F_50M &&
>>               (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {
>
> This doesn't seem to be equivalent change to the previous code . Here, 
> if the clock frequency is 25 MHz, the clock are unconditionally 
> enabled. Before, the code enabled the clock only if clock frequency 
> was 25 MHz AND one of the "dwmac->eth_ref_clk_sel_reg" or 
> "dwmac->ext_phyclk" was set (i.e. clock provided by SoC RCC clock IP).

You are right, but in STM32MP15/MP13 reference manual it is write that 
we need to update SYSCFG (SYSCFG_PMCSETR) register only in "Ethernet 
50MHz RMII clock selection":

Bit 17 ETH_REF_CLK_SEL: Ethernet 50MHz RMII clock selection.

     Set by software.

       0: Writing '0' has no effect, reading '0' means External clock is 
used. Need selection of AFMux. Could be used with all PHY

       1: Writing '1' set this bit, reading '1' means Internal clock 
ETH_CLK1 from RCC is used regardless AFMux. Could be used only with RMII PHY

>
> I think it might make this code easier if you drop all of the 
> frequency test conditionals, which aren't really all that useful, and 
> only enable the clock if either dwmac->ext_phyclk / 
> dwmac->eth_clk_sel_reg / dwmac->eth_ref_clk_sel_reg is set , because 
> effectively what this entire convoluted code is implementing is "if 
> (clock supplied by clock IP i.e. RCC) enable the clock()" *, right ?
>
> * And it is also toggling the right clock mux bit in PMCSETR.
>
> So, for MII this would be plain:
> dwmac->enable_eth_ck = true;
>
> For GMII/RGMII this would be:
> if (dwmac->ext_phyclk || dwmac->eth_clk_sel_reg)
>   dwmac->enable_eth_ck = true;
>
> For RMII this would be:
> if (dwmac->ext_phyclk || dwmac->eth_ref_clk_sel_reg)
>   dwmac->enable_eth_ck = true;
>
> Maybe the clock frequency validation can be retained, but done 
> separately?
As explained previously, need to keep check of clock frequency in this test.
>
>>               dwmac->enable_eth_ck = true;
>>               val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel;
>> @@ -203,7 +202,9 @@ static int stm32mp1_set_mode(struct 
>> plat_stmmacenet_data *plat_dat)
>>       case PHY_INTERFACE_MODE_RGMII_RXID:
>>       case PHY_INTERFACE_MODE_RGMII_TXID:
>>           val = dwmac->ops->pmcsetr.eth1_sel_rgmii | 
>> dwmac->ops->pmcsetr.eth2_sel_rgmii;
>> -        if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) &&
>> +        if (clk_rate == ETH_CK_F_25M)
>> +            dwmac->enable_eth_ck = true;
>> +        if (clk_rate == ETH_CK_F_125M &&
>>               (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
>>               dwmac->enable_eth_ck = true;
>>               val |= dwmac->ops->pmcsetr.eth1_clk_sel;
>> @@ -219,7 +220,7 @@ static int stm32mp1_set_mode(struct 
>> plat_stmmacenet_data *plat_dat)
>>       }
>>         /* Need to update PMCCLRR (clear register) */
>> -    regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off,
>> +    regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off,
>>                dwmac->mode_mask);
>>         /* Update PMCSETR (set register) */
>> @@ -328,7 +329,7 @@ static int stm32mp1_parse_data(struct stm32_dwmac 
>> *dwmac,
>>       /*  Get ETH_CLK clocks */
>>       dwmac->clk_eth_ck = devm_clk_get(dev, "eth-ck");
>>       if (IS_ERR(dwmac->clk_eth_ck)) {
>> -        dev_info(dev, "No phy clock provided...\n");
>> +        dev_dbg(dev, "No phy clock provided...\n");
>>           dwmac->clk_eth_ck = NULL;
>>       }

Re: [PATCH v2 05/11] net: stmmac: dwmac-stm32: update config management for phy wo cristal

[Marek Vasut]

On 5/13/24 5:11 PM, Christophe ROULLIER wrote:
> 
> On 4/26/24 17:37, Marek Vasut wrote:
>> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>>> Some cleaning because some Ethernet PHY configs do not need to add
>>> st,ext-phyclk property.
>>> Change print info message "No phy clock provided" only when debug.
>>>
>>> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
>>> ---
>>>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 27 ++++++++++---------
>>>   1 file changed, 14 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c 
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>> index 7529a8d15492..e648c4e790a7 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>>> @@ -55,17 +55,17 @@
>>>    *|         |        |      25MHz    |        50MHz 
>>> |                  |
>>>    * 
>>> ---------------------------------------------------------------------------
>>>    *|  MII    |     -   |     eth-ck    |          n/a | n/a        |
>>> - *|         |        | st,ext-phyclk | |             |
>>> + *|         |        |                 | |             |
>>>    * 
>>> ---------------------------------------------------------------------------
>>>    *|  GMII   |     -   |     eth-ck    |          n/a | n/a        |
>>> - *|         |        | st,ext-phyclk | |             |
>>> + *|         |        |               | |             |
>>>    * 
>>> ---------------------------------------------------------------------------
>>>    *| RGMII   |     -   |     eth-ck    |          n/a | eth-ck      |
>>> - *|         |        | st,ext-phyclk |                    | 
>>> st,eth-clk-sel or|
>>> + *|         |        |               |                    | 
>>> st,eth-clk-sel or|
>>>    *|         |        |               |                    | 
>>> st,ext-phyclk    |
>>>    * 
>>> ---------------------------------------------------------------------------
>>>    *| RMII    |     -   |     eth-ck    |        eth-ck | n/a        |
>>> - *|         |        | st,ext-phyclk | st,eth-ref-clk-sel 
>>> |             |
>>> + *|         |        |               | st,eth-ref-clk-sel 
>>> |             |
>>>    *|         |        |               | or st,ext-phyclk 
>>> |             |
>>>    * 
>>> ---------------------------------------------------------------------------
>>>    *
>>> @@ -174,23 +174,22 @@ static int stm32mp1_set_mode(struct 
>>> plat_stmmacenet_data *plat_dat)
>>>       dwmac->enable_eth_ck = false;
>>>       switch (plat_dat->mac_interface) {
>>>       case PHY_INTERFACE_MODE_MII:
>>> -        if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
>>> +        if (clk_rate == ETH_CK_F_25M)
>>
>> I see two problems here.
>>
>> First, according to the table above, in MII mode, clk_rate cannot be 
>> anything else but 25 MHz, so the (clk_rate == ETH_CK_F_25M) condition 
>> is always true. Why not drop that condition ?
> Not agree, there is also "Normal" case MII (MII with quartz/cristal) 
> (first column in the table above), so need to keep this test to check 
> clk_rate 25MHz.

What other rate is supported in the MII mode ? Isn't the rate always 25 
MHz for MII , no matter whether it is generated by RCC or external Xtal ?

>> The "dwmac->ext_phyclk" means "Ethernet PHY have no crystal", which 
>> means the clock are provided by the STM32 RCC clock IP instead, which 
>> means if the dwmac->ext_phyclk is true, dwmac->enable_eth_ck should be 
>> set to true, because dwmac->enable_eth_ck controls the enablement of 
>> these STM32 clock IP generated clock.
> Right
>>
>> Second, as far as I understand it, there is no way to operate this IP 
>> with external clock in MII mode, so this section should always be only:
>>
>> dwmac->enable_eth_ck = true;
> Not for case "Normal" MII :-)

What happens if that external clock source is not an xtal, but an 
oscillator with its own driver described in DT, and enabled e.g. using 
GPIO (that's compatible "gpio-gate-clock" for that oscillator) . Then 
you do need to enable those clock even in the "normal" (with external 
clock source instead of RCC clock source) MII case.

>>>               dwmac->enable_eth_ck = true;
>>>           val = dwmac->ops->pmcsetr.eth1_selmii;
>>>           pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
>>>           break;
>>>       case PHY_INTERFACE_MODE_GMII:
>>>           val = SYSCFG_PMCR_ETH_SEL_GMII;
>>> -        if (clk_rate == ETH_CK_F_25M &&
>>> -            (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
>>> +        if (clk_rate == ETH_CK_F_25M)
>>>               dwmac->enable_eth_ck = true;
>>> -            val |= dwmac->ops->pmcsetr.eth1_clk_sel;
>>> -        }
>>>           pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
>>>           break;
>>>       case PHY_INTERFACE_MODE_RMII:
>>>           val = dwmac->ops->pmcsetr.eth1_sel_rmii | 
>>> dwmac->ops->pmcsetr.eth2_sel_rmii;
>>> -        if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) &&
>>> +        if (clk_rate == ETH_CK_F_25M)
>>> +            dwmac->enable_eth_ck = true;
>>> +        if (clk_rate == ETH_CK_F_50M &&
>>>               (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {
>>
>> This doesn't seem to be equivalent change to the previous code . Here, 
>> if the clock frequency is 25 MHz, the clock are unconditionally 
>> enabled. Before, the code enabled the clock only if clock frequency 
>> was 25 MHz AND one of the "dwmac->eth_ref_clk_sel_reg" or 
>> "dwmac->ext_phyclk" was set (i.e. clock provided by SoC RCC clock IP).
> 
> You are right, but in STM32MP15/MP13 reference manual it is write that 
> we need to update SYSCFG (SYSCFG_PMCSETR) register only in "Ethernet 
> 50MHz RMII clock selection":
> 
> Bit 17 ETH_REF_CLK_SEL: Ethernet 50MHz RMII clock selection.
> 
>      Set by software.
> 
>        0: Writing '0' has no effect, reading '0' means External clock is 
> used. Need selection of AFMux. Could be used with all PHY
> 
>        1: Writing '1' set this bit, reading '1' means Internal clock 
> ETH_CLK1 from RCC is used regardless AFMux. Could be used only with RMII 
> PHY

Look at this:
"
RM0436 Rev 6 Reset and clock control (RCC)
Clock distribution for Ethernet (ETH)
Figure 83. Peripheral clock distribution for Ethernet
Page 575
"
See the mux at bottom left side in the PKCS group.

I suspect that no matter whether the clock are 25 MHz or 50 MHz, they 
enter this mux, the mux selects the clock source from either RCC or from 
external oscillator, and therefore the original code was correct.

>> I think it might make this code easier if you drop all of the 
>> frequency test conditionals, which aren't really all that useful, and 
>> only enable the clock if either dwmac->ext_phyclk / 
>> dwmac->eth_clk_sel_reg / dwmac->eth_ref_clk_sel_reg is set , because 
>> effectively what this entire convoluted code is implementing is "if 
>> (clock supplied by clock IP i.e. RCC) enable the clock()" *, right ?
>>
>> * And it is also toggling the right clock mux bit in PMCSETR.
>>
>> So, for MII this would be plain:
>> dwmac->enable_eth_ck = true;
>>
>> For GMII/RGMII this would be:
>> if (dwmac->ext_phyclk || dwmac->eth_clk_sel_reg)
>>   dwmac->enable_eth_ck = true;
>>
>> For RMII this would be:
>> if (dwmac->ext_phyclk || dwmac->eth_ref_clk_sel_reg)
>>   dwmac->enable_eth_ck = true;
>>
>> Maybe the clock frequency validation can be retained, but done 
>> separately?
> As explained previously, need to keep check of clock frequency in this 
> test.

I sent 5 patches which split this code up, you were on CC:

[net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32: Separate out external 
clock rate validation

Maybe you can apply those, fix them up as needed, and then add this 
series on top ? I think it would simplify this code a lot, since that 
series splits up the clock rate validation / PMCR configuration / 
external-internal clock selection into separate steps. What do you think ?

[PATCH v2 06/11] net: stmmac: dwmac-stm32: clean the way to manage wol irqwake

[Christophe Roullier]

On STM32 platforms it is no longer needed to use a dedicated wakeup to
wake up system from CStop. This patch removes the dedicated wake up usage
and clean the way to register the wake up irq.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 66 +++++++++----------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index e648c4e790a7..5d8f4eac4bc0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -81,7 +81,6 @@ struct stm32_dwmac {
 	int enable_eth_ck;
 	int eth_clk_sel_reg;
 	int eth_ref_clk_sel_reg;
-	int irq_pwr_wakeup;
 	u32 mode_reg;		 /* MAC glue-logic mode register */
 	u32 mode_mask;
 	struct regmap *regmap;
@@ -312,9 +311,7 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
 static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
 			       struct device *dev)
 {
-	struct platform_device *pdev = to_platform_device(dev);
 	struct device_node *np = dev->of_node;
-	int err = 0;
 
 	/* Ethernet PHY have no crystal */
 	dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk");
@@ -346,29 +343,24 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
 	if (IS_ERR(dwmac->syscfg_clk))
 		dwmac->syscfg_clk = NULL;
 
-	/* Get IRQ information early to have an ability to ask for deferred
-	 * probe if needed before we went too far with resource allocation.
-	 */
-	dwmac->irq_pwr_wakeup = platform_get_irq_byname_optional(pdev,
-							"stm32_pwr_wakeup");
-	if (dwmac->irq_pwr_wakeup == -EPROBE_DEFER)
-		return -EPROBE_DEFER;
-
-	if (!dwmac->clk_eth_ck && dwmac->irq_pwr_wakeup >= 0) {
-		err = device_init_wakeup(&pdev->dev, true);
-		if (err) {
-			dev_err(&pdev->dev, "Failed to init wake up irq\n");
-			return err;
-		}
-		err = dev_pm_set_dedicated_wake_irq(&pdev->dev,
-						    dwmac->irq_pwr_wakeup);
-		if (err) {
-			dev_err(&pdev->dev, "Failed to set wake up irq\n");
-			device_init_wakeup(&pdev->dev, false);
-		}
-		device_set_wakeup_enable(&pdev->dev, false);
+	return 0;
+}
+
+static int stm32_dwmac_wake_init(struct device *dev,
+				 struct stmmac_resources *stmmac_res)
+{
+	int err;
+
+	device_set_wakeup_capable(dev, true);
+
+	err = dev_pm_set_wake_irq(dev, stmmac_res->wol_irq);
+	if (err) {
+		dev_err(dev, "Failed to set wake up irq\n");
+		device_set_wakeup_capable(dev, false);
+		return err;
 	}
-	return err;
+
+	return 0;
 }
 
 static int stm32_dwmac_probe(struct platform_device *pdev)
@@ -406,11 +398,17 @@ static int stm32_dwmac_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (stmmac_res.wol_irq && !dwmac->clk_eth_ck) {
+		ret = stm32_dwmac_wake_init(&pdev->dev, &stmmac_res);
+		if (ret)
+			goto err_wake_init_disable;
+	}
+
 	plat_dat->bsp_priv = dwmac;
 
 	ret = stm32_dwmac_init(plat_dat, false);
 	if (ret)
-		return ret;
+		goto err_wake_init_disable;
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
 	if (ret)
@@ -420,7 +418,11 @@ static int stm32_dwmac_probe(struct platform_device *pdev)
 
 err_clk_disable:
 	stm32_dwmac_clk_disable(dwmac, false);
-
+err_wake_init_disable:
+	if (stmmac_res.wol_irq && !dwmac->clk_eth_ck) {
+		dev_pm_clear_wake_irq(&pdev->dev);
+		device_set_wakeup_capable(&pdev->dev, false);
+	}
 	return ret;
 }
 
@@ -428,16 +430,12 @@ static void stm32_dwmac_remove(struct platform_device *pdev)
 {
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct stmmac_priv *priv = netdev_priv(ndev);
-	struct stm32_dwmac *dwmac = priv->plat->bsp_priv;
 
 	stmmac_dvr_remove(&pdev->dev);
+	stm32_dwmac_clk_disable(priv->plat->bsp_priv, false);
 
-	stm32_dwmac_clk_disable(dwmac, false);
-
-	if (dwmac->irq_pwr_wakeup >= 0) {
-		dev_pm_clear_wake_irq(&pdev->dev);
-		device_init_wakeup(&pdev->dev, false);
-	}
+	dev_pm_clear_wake_irq(&pdev->dev);
+	device_init_wakeup(&pdev->dev, false);
 }
 
 static int stm32mp1_suspend(struct stm32_dwmac *dwmac)
-- 
2.25.1

Re: [PATCH v2 06/11] net: stmmac: dwmac-stm32: clean the way to manage wol irqwake

[Marek Vasut]

On 4/26/24 2:57 PM, Christophe Roullier wrote:
> On STM32 platforms it is no longer needed to use a dedicated wakeup to
> wake up system from CStop.

This really needs more clarification.

Why was the code needed before ? Maybe because it was used by some of 
the older STM32F4/F7/H7 SoCs ? Is it still needed by those SoCs ? Will 
this patch break those older SoCs ?

> This patch removes the dedicated wake up usage
> and clean the way to register the wake up irq.

[...]

Re: [PATCH v2 06/11] net: stmmac: dwmac-stm32: clean the way to manage wol irqwake

[Christophe ROULLIER]

On 4/26/24 17:40, Marek Vasut wrote:
> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>> On STM32 platforms it is no longer needed to use a dedicated wakeup to
>> wake up system from CStop.
>
> This really needs more clarification.
>
> Why was the code needed before ? Maybe because it was used by some of 
> the older STM32F4/F7/H7 SoCs ? Is it still needed by those SoCs ? Will 
> this patch break those older SoCs ?
Yes you are right, if power mode use in STM32F4/F7/H7 SoC, issue with 
this patch, I will abandon it.
>
>> This patch removes the dedicated wake up usage
>> and clean the way to register the wake up irq.
>
> [...]

[PATCH v2 07/11] net: stmmac: dwmac-stm32: support the phy-supply regulator binding

[Christophe Roullier]

Configure the phy regulator if defined by the "phy-supply" DT phandle.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 51 ++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index 5d8f4eac4bc0..964173d08026 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -14,6 +14,7 @@
 #include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/pm_wakeirq.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
@@ -84,6 +85,7 @@ struct stm32_dwmac {
 	u32 mode_reg;		 /* MAC glue-logic mode register */
 	u32 mode_mask;
 	struct regmap *regmap;
+	struct regulator *regulator;
 	u32 speed;
 	const struct stm32_ops *ops;
 	struct device *dev;
@@ -305,6 +307,16 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
 	if (err)
 		pr_debug("Warning sysconfig register mask not set\n");
 
+	dwmac->regulator = devm_regulator_get_optional(dev, "phy");
+	if (IS_ERR(dwmac->regulator)) {
+		if (PTR_ERR(dwmac->regulator) == -EPROBE_DEFER) {
+			dev_dbg(dev, "phy regulator is not available yet, deferred probing\n");
+			return -EPROBE_DEFER;
+		}
+		dev_dbg(dev, "no regulator found\n");
+		dwmac->regulator = NULL;
+	}
+
 	return 0;
 }
 
@@ -363,6 +375,27 @@ static int stm32_dwmac_wake_init(struct device *dev,
 	return 0;
 }
 
+static int phy_power_on(struct stm32_dwmac *bsp_priv, bool enable)
+{
+	int ret;
+	struct device *dev = bsp_priv->dev;
+
+	if (!bsp_priv->regulator)
+		return 0;
+
+	if (enable) {
+		ret = regulator_enable(bsp_priv->regulator);
+		if (ret)
+			dev_err(dev, "fail to enable phy-supply\n");
+	} else {
+		ret = regulator_disable(bsp_priv->regulator);
+		if (ret)
+			dev_err(dev, "fail to disable phy-supply\n");
+	}
+
+	return ret;
+}
+
 static int stm32_dwmac_probe(struct platform_device *pdev)
 {
 	struct plat_stmmacenet_data *plat_dat;
@@ -410,12 +443,18 @@ static int stm32_dwmac_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_wake_init_disable;
 
-	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	ret = phy_power_on(plat_dat->bsp_priv, true);
 	if (ret)
 		goto err_clk_disable;
 
+	ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (ret)
+		goto err_gmac_powerdown;
+
 	return 0;
 
+err_gmac_powerdown:
+	phy_power_on(plat_dat->bsp_priv, false);
 err_clk_disable:
 	stm32_dwmac_clk_disable(dwmac, false);
 err_wake_init_disable:
@@ -436,6 +475,8 @@ static void stm32_dwmac_remove(struct platform_device *pdev)
 
 	dev_pm_clear_wake_irq(&pdev->dev);
 	device_init_wakeup(&pdev->dev, false);
+
+	phy_power_on(priv->plat->bsp_priv, false);
 }
 
 static int stm32mp1_suspend(struct stm32_dwmac *dwmac)
@@ -466,6 +507,10 @@ static int stm32_dwmac_suspend(struct device *dev)
 	if (dwmac->ops->suspend)
 		ret = dwmac->ops->suspend(dwmac);
 
+	/* Keep the PHY up if we use Wake-on-Lan. */
+	if (!device_may_wakeup(dwmac->dev))
+		phy_power_on(dwmac, false);
+
 	return ret;
 }
 
@@ -479,6 +524,10 @@ static int stm32_dwmac_resume(struct device *dev)
 	if (dwmac->ops->resume)
 		dwmac->ops->resume(dwmac);
 
+	/* The PHY was up for Wake-on-Lan. */
+	if (!device_may_wakeup(dwmac->dev))
+		phy_power_on(dwmac, true);
+
 	ret = stm32_dwmac_init(priv->plat, true);
 	if (ret)
 		return ret;
-- 
2.25.1

Re: [PATCH v2 07/11] net: stmmac: dwmac-stm32: support the phy-supply regulator binding

[Marek Vasut]

On 4/26/24 2:57 PM, Christophe Roullier wrote:
> Configure the phy regulator if defined by the "phy-supply" DT phandle.
> 
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>

Maybe this should be separate from this series ?

[PATCH v2 08/11] ARM: dts: stm32: add ethernet1 and ethernet2 support on stm32mp13

[Christophe Roullier]

Both instances ethernet based on GMAC SNPS IP on stm32mp13.
GMAC IP version is SNPS 4.20.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp131.dtsi | 31 ++++++++++++++++++++++++++++
 arch/arm/boot/dts/st/stm32mp133.dtsi | 30 +++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp131.dtsi b/arch/arm/boot/dts/st/stm32mp131.dtsi
index ecfa120827ba..00d258d2032e 100644
--- a/arch/arm/boot/dts/st/stm32mp131.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp131.dtsi
@@ -854,6 +854,37 @@ crc1: crc@58009000 {
 			status = "disabled";
 		};
 
+		ethernet1: ethernet@5800a000 {
+			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 1>;
+			interrupt-names = "macirq", "eth_wake_irq";
+			clock-names = "stmmaceth",
+				      "mac-clk-tx",
+				      "mac-clk-rx",
+				      "ethstp",
+				      "eth-ck";
+			clocks = <&rcc ETH1MAC>,
+				 <&rcc ETH1TX>,
+				 <&rcc ETH1RX>,
+				 <&rcc ETH1STP>,
+				 <&rcc ETH1CK_K>;
+			st,syscon = <&syscfg 0x4 0xff0000>;
+			snps,mixed-burst;
+			snps,pbl = <2>;
+			snps,axi-config = <&stmmac_axi_config_1>;
+			snps,tso;
+			status = "disabled";
+
+			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>;
+			};
+		};
+
 		usbh_ohci: usb@5800c000 {
 			compatible = "generic-ohci";
 			reg = <0x5800c000 0x1000>;
diff --git a/arch/arm/boot/dts/st/stm32mp133.dtsi b/arch/arm/boot/dts/st/stm32mp133.dtsi
index 3e394c8e58b9..2796d15276df 100644
--- a/arch/arm/boot/dts/st/stm32mp133.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp133.dtsi
@@ -68,4 +68,34 @@ channel@18 {
 			};
 		};
 	};
+
+	ethernet2: ethernet@5800e000 {
+		compatible = "st,stm32mp13-dwmac", "snps,dwmac-4.20a";
+		reg = <0x5800e000 0x2000>;
+		reg-names = "stmmaceth";
+		interrupts-extended = <&intc GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "macirq";
+		clock-names = "stmmaceth",
+			      "mac-clk-tx",
+			      "mac-clk-rx",
+			      "ethstp",
+			      "eth-ck";
+		clocks = <&rcc ETH2MAC>,
+			 <&rcc ETH2TX>,
+			 <&rcc ETH2RX>,
+			 <&rcc ETH2STP>,
+			 <&rcc ETH2CK_K>;
+		st,syscon = <&syscfg 0x4 0xff000000>;
+		snps,mixed-burst;
+		snps,pbl = <2>;
+		snps,axi-config = <&stmmac_axi_config_2>;
+		snps,tso;
+		status = "disabled";
+
+		stmmac_axi_config_2: 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

[PATCH v2 09/11] ARM: dts: stm32: add ethernet1/2 RMII pins for STM32MP13F-DK board

[Christophe Roullier]

Those pins are used for Ethernet 1 and 2 on STM32MP13F-DK board.
ethernet1: RMII with crystal.
ethernet2: RMII without crystal.
Add analog gpio pin configuration ("sleep") to manage power mode on
stm32mp13.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi | 71 +++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi b/arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi
index 32c5d8a1e06a..7f72c62da0a6 100644
--- a/arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi
+++ b/arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi
@@ -13,6 +13,77 @@ pins {
 		};
 	};
 
+	eth1_rmii_pins_a: eth1-rmii-0 {
+		pins1 {
+			pinmux = <STM32_PINMUX('G', 13, AF11)>, /* ETH_RMII_TXD0 */
+				 <STM32_PINMUX('G', 14, AF11)>, /* ETH_RMII_TXD1 */
+				 <STM32_PINMUX('B', 11, AF11)>, /* ETH_RMII_TX_EN */
+				 <STM32_PINMUX('A', 1, AF11)>, /* ETH_RMII_REF_CLK */
+				 <STM32_PINMUX('A', 2, AF11)>, /* ETH_MDIO */
+				 <STM32_PINMUX('G', 2, AF11)>; /* ETH_MDC */
+			bias-disable;
+			drive-push-pull;
+			slew-rate = <1>;
+		};
+
+		pins2 {
+			pinmux = <STM32_PINMUX('C', 4, AF11)>, /* ETH_RMII_RXD0 */
+				 <STM32_PINMUX('C', 5, AF11)>, /* ETH_RMII_RXD1 */
+				 <STM32_PINMUX('C', 1, AF10)>; /* ETH_RMII_CRS_DV */
+			bias-disable;
+		};
+
+	};
+
+	eth1_rmii_sleep_pins_a: eth1-rmii-sleep-0 {
+		pins1 {
+			pinmux = <STM32_PINMUX('G', 13, ANALOG)>, /* ETH_RMII_TXD0 */
+				 <STM32_PINMUX('G', 14, ANALOG)>, /* ETH_RMII_TXD1 */
+				 <STM32_PINMUX('B', 11, ANALOG)>, /* ETH_RMII_TX_EN */
+				 <STM32_PINMUX('A', 1, ANALOG)>, /* ETH_RMII_REF_CLK */
+				 <STM32_PINMUX('A', 2, ANALOG)>, /* ETH_MDIO */
+				 <STM32_PINMUX('G', 2, ANALOG)>, /* ETH_MDC */
+				 <STM32_PINMUX('C', 4, ANALOG)>, /* ETH_RMII_RXD0 */
+				 <STM32_PINMUX('C', 5, ANALOG)>, /* ETH_RMII_RXD1 */
+				 <STM32_PINMUX('C', 1, ANALOG)>; /* ETH_RMII_CRS_DV */
+		};
+	};
+
+	eth2_rmii_pins_a: eth2-rmii-0 {
+		pins1 {
+			pinmux = <STM32_PINMUX('F', 7, AF11)>, /* ETH_RMII_TXD0 */
+				 <STM32_PINMUX('G', 11, AF10)>, /* ETH_RMII_TXD1 */
+				 <STM32_PINMUX('G', 8, AF13)>, /* ETH_RMII_ETHCK */
+				 <STM32_PINMUX('F', 6, AF11)>, /* ETH_RMII_TX_EN */
+				 <STM32_PINMUX('B', 2, AF11)>, /* ETH_MDIO */
+				 <STM32_PINMUX('G', 5, AF10)>; /* ETH_MDC */
+			bias-disable;
+			drive-push-pull;
+			slew-rate = <1>;
+		};
+
+		pins2 {
+			pinmux = <STM32_PINMUX('F', 4, AF11)>, /* ETH_RMII_RXD0 */
+				 <STM32_PINMUX('E', 2, AF10)>, /* ETH_RMII_RXD1 */
+				 <STM32_PINMUX('A', 12, AF11)>; /* ETH_RMII_CRS_DV */
+			bias-disable;
+		};
+	};
+
+	eth2_rmii_sleep_pins_a: eth2-rmii-sleep-0 {
+		pins1 {
+			pinmux = <STM32_PINMUX('F', 7, ANALOG)>, /* ETH_RMII_TXD0 */
+				 <STM32_PINMUX('G', 11, ANALOG)>, /* ETH_RMII_TXD1 */
+				 <STM32_PINMUX('G', 8, ANALOG)>, /* ETH_RMII_ETHCK */
+				 <STM32_PINMUX('F', 6, ANALOG)>, /* ETH_RMII_TX_EN */
+				 <STM32_PINMUX('B', 2, ANALOG)>, /* ETH_MDIO */
+				 <STM32_PINMUX('G', 5, ANALOG)>, /* ETH_MDC */
+				 <STM32_PINMUX('F', 4, ANALOG)>, /* ETH_RMII_RXD0 */
+				 <STM32_PINMUX('E', 2, ANALOG)>, /* ETH_RMII_RXD1 */
+				 <STM32_PINMUX('A', 12, ANALOG)>; /* ETH_RMII_CRS_DV */
+		};
+	};
+
 	i2c1_pins_a: i2c1-0 {
 		pins {
 			pinmux = <STM32_PINMUX('D', 12, AF5)>, /* I2C1_SCL */
-- 
2.25.1

[PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Christophe Roullier]

Add dual Ethernet:
-Ethernet1: RMII with crystal
-Ethernet2: RMII without crystal
PHYs used are SMSC (LAN8742A)

With Ethernet1, we can performed WoL from PHY instead of GMAC point
of view.
(in this case IRQ for WoL is managed as wakeup pin and configured
in OS secure).

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 arch/arm/boot/dts/st/stm32mp135f-dk.dts | 48 +++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
index 567e53ad285f..3b8eb0ab9ab9 100644
--- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
+++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
@@ -19,6 +19,8 @@ / {
 	compatible = "st,stm32mp135f-dk", "st,stm32mp135";
 
 	aliases {
+		ethernet0 = &ethernet1;
+		ethernet1 = &ethernet2;
 		serial0 = &uart4;
 		serial1 = &usart1;
 		serial2 = &uart8;
@@ -141,6 +143,52 @@ &cryp {
 	status = "okay";
 };
 
+&ethernet1 {
+	status = "okay";
+	pinctrl-0 = <&eth1_rmii_pins_a>;
+	pinctrl-1 = <&eth1_rmii_sleep_pins_a>;
+	pinctrl-names = "default", "sleep";
+	phy-mode = "rmii";
+	max-speed = <100>;
+	phy-handle = <&phy0_eth1>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+
+		phy0_eth1: ethernet-phy@0 {
+			compatible = "ethernet-phy-id0007.c131";
+			reset-gpios =  <&mcp23017 9 GPIO_ACTIVE_LOW>;
+			reg = <0>;
+			wakeup-source;
+		};
+	};
+};
+
+&ethernet2 {
+	status = "okay";
+	pinctrl-0 = <&eth2_rmii_pins_a>;
+	pinctrl-1 = <&eth2_rmii_sleep_pins_a>;
+	pinctrl-names = "default", "sleep";
+	phy-mode = "rmii";
+	max-speed = <100>;
+	phy-handle = <&phy0_eth2>;
+	st,ext-phyclk;
+	phy-supply = <&scmi_v3v3_sw>;
+
+	mdio {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "snps,dwmac-mdio";
+		phy0_eth2: ethernet-phy@0 {
+			compatible = "ethernet-phy-id0007.c131";
+			reset-gpios = <&mcp23017 10 GPIO_ACTIVE_LOW>;
+			reg = <0>;
+		};
+	};
+};
+
 &i2c1 {
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&i2c1_pins_a>;
-- 
2.25.1

Re: [PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Marek Vasut]

On 4/26/24 2:57 PM, Christophe Roullier wrote:
> Add dual Ethernet:
> -Ethernet1: RMII with crystal
> -Ethernet2: RMII without crystal
> PHYs used are SMSC (LAN8742A)
> 
> With Ethernet1, we can performed WoL from PHY instead of GMAC point
> of view.
> (in this case IRQ for WoL is managed as wakeup pin and configured
> in OS secure).

How does the Linux PHY driver process such a PHY IRQ ?

Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?

> diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
> index 567e53ad285f..3b8eb0ab9ab9 100644
> --- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
> +++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
> @@ -19,6 +19,8 @@ / {
>   	compatible = "st,stm32mp135f-dk", "st,stm32mp135";
>   
>   	aliases {
> +		ethernet0 = &ethernet1;
> +		ethernet1 = &ethernet2;
>   		serial0 = &uart4;
>   		serial1 = &usart1;
>   		serial2 = &uart8;
> @@ -141,6 +143,52 @@ &cryp {
>   	status = "okay";
>   };
>   
> +&ethernet1 {
> +	status = "okay";
> +	pinctrl-0 = <&eth1_rmii_pins_a>;
> +	pinctrl-1 = <&eth1_rmii_sleep_pins_a>;
> +	pinctrl-names = "default", "sleep";
> +	phy-mode = "rmii";
> +	max-speed = <100>;
> +	phy-handle = <&phy0_eth1>;

Keep the list sorted please (is the max-speed even needed? if not, drop it)

> +	mdio {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "snps,dwmac-mdio";
> +
> +		phy0_eth1: ethernet-phy@0 {
> +			compatible = "ethernet-phy-id0007.c131";
> +			reset-gpios =  <&mcp23017 9 GPIO_ACTIVE_LOW>;
> +			reg = <0>;
> +			wakeup-source;
> +		};
> +	};
> +};
> +
> +&ethernet2 {
> +	status = "okay";
> +	pinctrl-0 = <&eth2_rmii_pins_a>;
> +	pinctrl-1 = <&eth2_rmii_sleep_pins_a>;
> +	pinctrl-names = "default", "sleep";
> +	phy-mode = "rmii";
> +	max-speed = <100>;
> +	phy-handle = <&phy0_eth2>;
> +	st,ext-phyclk;
> +	phy-supply = <&scmi_v3v3_sw>;

Sort please

[...]

Re: [PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Alexandre TORGUE]

Hi Marek

On 4/26/24 17:44, Marek Vasut wrote:
> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>> Add dual Ethernet:
>> -Ethernet1: RMII with crystal
>> -Ethernet2: RMII without crystal
>> PHYs used are SMSC (LAN8742A)
>>
>> With Ethernet1, we can performed WoL from PHY instead of GMAC point
>> of view.
>> (in this case IRQ for WoL is managed as wakeup pin and configured
>> in OS secure).
> 
> How does the Linux PHY driver process such a PHY IRQ ?
> 
> Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?

In this case, we want to have an example to wakeup the system from 
Standby low power mode (VDDCPU and VDD_CORE off) thanks to a magic 
packet detected by the PHY. The PHY then assert his interrupt output signal.
On MP13 DK platform, this PHY signal is connected to a specific GPIO
aka "Wakeup pins" (only 6 wakeup pins an MP13). Those specific GPIOs are 
handled by the PWR peripheral which is controlled by the secure OS.

On WoL packet, the Secure OS catches the PHY interrupt and uses 
asynchronous notification mechanism to warn Linux (on our platform we 
use a PPI). On Linux side, Optee core driver creates an irq 
domain/irqchip triggered on the asynchronous notification. Each device 
which use a wakeup pin need then to request an IRQ on this "Optee irq 
domain".

This OPTEE irq domain will be pushed soon.

cheers
Alex

> 
>> diff --git a/arch/arm/boot/dts/st/stm32mp135f-dk.dts 
>> b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
>> index 567e53ad285f..3b8eb0ab9ab9 100644
>> --- a/arch/arm/boot/dts/st/stm32mp135f-dk.dts
>> +++ b/arch/arm/boot/dts/st/stm32mp135f-dk.dts
>> @@ -19,6 +19,8 @@ / {
>>       compatible = "st,stm32mp135f-dk", "st,stm32mp135";
>>       aliases {
>> +        ethernet0 = &ethernet1;
>> +        ethernet1 = &ethernet2;
>>           serial0 = &uart4;
>>           serial1 = &usart1;
>>           serial2 = &uart8;
>> @@ -141,6 +143,52 @@ &cryp {
>>       status = "okay";
>>   };
>> +&ethernet1 {
>> +    status = "okay";
>> +    pinctrl-0 = <&eth1_rmii_pins_a>;
>> +    pinctrl-1 = <&eth1_rmii_sleep_pins_a>;
>> +    pinctrl-names = "default", "sleep";
>> +    phy-mode = "rmii";
>> +    max-speed = <100>;
>> +    phy-handle = <&phy0_eth1>;
> 
> Keep the list sorted please (is the max-speed even needed? if not, drop it)
> 
>> +    mdio {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        compatible = "snps,dwmac-mdio";
>> +
>> +        phy0_eth1: ethernet-phy@0 {
>> +            compatible = "ethernet-phy-id0007.c131";
>> +            reset-gpios =  <&mcp23017 9 GPIO_ACTIVE_LOW>;
>> +            reg = <0>;
>> +            wakeup-source;
>> +        };
>> +    };
>> +};
>> +
>> +&ethernet2 {
>> +    status = "okay";
>> +    pinctrl-0 = <&eth2_rmii_pins_a>;
>> +    pinctrl-1 = <&eth2_rmii_sleep_pins_a>;
>> +    pinctrl-names = "default", "sleep";
>> +    phy-mode = "rmii";
>> +    max-speed = <100>;
>> +    phy-handle = <&phy0_eth2>;
>> +    st,ext-phyclk;
>> +    phy-supply = <&scmi_v3v3_sw>;
> 
> Sort please
> 
> [...]

Re: [PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Marek Vasut]

On 5/13/24 6:01 PM, Alexandre TORGUE wrote:
> Hi Marek

Hi,

> On 4/26/24 17:44, Marek Vasut wrote:
>> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>>> Add dual Ethernet:
>>> -Ethernet1: RMII with crystal
>>> -Ethernet2: RMII without crystal
>>> PHYs used are SMSC (LAN8742A)
>>>
>>> With Ethernet1, we can performed WoL from PHY instead of GMAC point
>>> of view.
>>> (in this case IRQ for WoL is managed as wakeup pin and configured
>>> in OS secure).
>>
>> How does the Linux PHY driver process such a PHY IRQ ?
>>
>> Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?
> 
> In this case, we want to have an example to wakeup the system from 
> Standby low power mode (VDDCPU and VDD_CORE off) thanks to a magic 
> packet detected by the PHY. The PHY then assert his interrupt output 
> signal.
> On MP13 DK platform, this PHY signal is connected to a specific GPIO
> aka "Wakeup pins" (only 6 wakeup pins an MP13). Those specific GPIOs are 
> handled by the PWR peripheral which is controlled by the secure OS.

What does configure the PHY for this wakeup mode ?

> On WoL packet, the Secure OS catches the PHY interrupt and uses 
> asynchronous notification mechanism to warn Linux (on our platform we 
> use a PPI). On Linux side, Optee core driver creates an irq 
> domain/irqchip triggered on the asynchronous notification. Each device 
> which use a wakeup pin need then to request an IRQ on this "Optee irq 
> domain".
> 
> This OPTEE irq domain will be pushed soon.

I suspect it might make sense to add this WoL part separately from the 
actual ethernet DT nodes, so ethernet could land and the WoL 
functionality can be added when it is ready ?

Re: [PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Alexandre TORGUE]

Hi

On 5/16/24 02:23, Marek Vasut wrote:
> On 5/13/24 6:01 PM, Alexandre TORGUE wrote:
>> Hi Marek
> 
> Hi,
> 
>> On 4/26/24 17:44, Marek Vasut wrote:
>>> On 4/26/24 2:57 PM, Christophe Roullier wrote:
>>>> Add dual Ethernet:
>>>> -Ethernet1: RMII with crystal
>>>> -Ethernet2: RMII without crystal
>>>> PHYs used are SMSC (LAN8742A)
>>>>
>>>> With Ethernet1, we can performed WoL from PHY instead of GMAC point
>>>> of view.
>>>> (in this case IRQ for WoL is managed as wakeup pin and configured
>>>> in OS secure).
>>>
>>> How does the Linux PHY driver process such a PHY IRQ ?
>>>
>>> Or is Linux unaware of the PHY IRQ ? Doesn't that cause issues ?
>>
>> In this case, we want to have an example to wakeup the system from 
>> Standby low power mode (VDDCPU and VDD_CORE off) thanks to a magic 
>> packet detected by the PHY. The PHY then assert his interrupt output 
>> signal.
>> On MP13 DK platform, this PHY signal is connected to a specific GPIO
>> aka "Wakeup pins" (only 6 wakeup pins an MP13). Those specific GPIOs 
>> are handled by the PWR peripheral which is controlled by the secure OS.
> 
> What does configure the PHY for this wakeup mode ?

Linux device tree.

> 
>> On WoL packet, the Secure OS catches the PHY interrupt and uses 
>> asynchronous notification mechanism to warn Linux (on our platform we 
>> use a PPI). On Linux side, Optee core driver creates an irq 
>> domain/irqchip triggered on the asynchronous notification. Each device 
>> which use a wakeup pin need then to request an IRQ on this "Optee irq 
>> domain".
>>
>> This OPTEE irq domain will be pushed soon.
> 
> I suspect it might make sense to add this WoL part separately from the 
> actual ethernet DT nodes, so ethernet could land and the WoL 
> functionality can be added when it is ready ?

If at the end we want to have this Wol from PHY then I agree we need to 
wait. We could push a WoL from MAC for this node before optee driver 
patches merge but not sure it makes sens.

Alex

Re: [PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Andrew Lunn]

> > I suspect it might make sense to add this WoL part separately from the
> > actual ethernet DT nodes, so ethernet could land and the WoL
> > functionality can be added when it is ready ?
> 
> If at the end we want to have this Wol from PHY then I agree we need to
> wait. We could push a WoL from MAC for this node before optee driver patches
> merge but not sure it makes sens.

In general, it is better if the PHY does WoL, since the MAC can then
be powered down. MAC WoL should only be used when the PHY does not
support the requested WoL configuration, but the MAC can. And
sometimes you need to spread it over both the PHY and the MAC.

	Andrew

Re: [PATCH v2 10/11] ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board

[Alexandre TORGUE]

On 5/16/24 14:22, Andrew Lunn wrote:
>>> I suspect it might make sense to add this WoL part separately from the
>>> actual ethernet DT nodes, so ethernet could land and the WoL
>>> functionality can be added when it is ready ?
>>
>> If at the end we want to have this Wol from PHY then I agree we need to
>> wait. We could push a WoL from MAC for this node before optee driver patches
>> merge but not sure it makes sens.
> 
> In general, it is better if the PHY does WoL, since the MAC can then
> be powered down. MAC WoL should only be used when the PHY does not
> support the requested WoL configuration, but the MAC can. And
> sometimes you need to spread it over both the PHY and the MAC.
>

thanks Andrew. So lets wait the optee driver missing part.

alex


> 	Andrew

[PATCH v2 11/11] ARM: multi_v7_defconfig: Add MCP23S08 pinctrl support

[Christophe Roullier]

Need to enable MCP23S08 I/O expanders to manage Ethernet phy
reset in STM32MP135F-DK board
STMMAC driver defer is not silent, need to put this config in
built-in to avoid huge of Ethernet messages

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index b2955dcb5a53..0abbe00372df 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -469,6 +469,7 @@ CONFIG_SPI_XILINX=y
 CONFIG_SPI_SPIDEV=y
 CONFIG_SPMI=y
 CONFIG_PINCTRL_AS3722=y
+CONFIG_PINCTRL_MCP23S08=y
 CONFIG_PINCTRL_MICROCHIP_SGPIO=y
 CONFIG_PINCTRL_OCELOT=y
 CONFIG_PINCTRL_PALMAS=y
-- 
2.25.1

Re: [PATCH v2 00/11] Series to deliver Ethernets for STM32MP13

[Rob Herring]

On Fri, 26 Apr 2024 14:56:56 +0200, Christophe Roullier wrote:
> STM32MP13 is STM32 SOC with 2 GMACs instances
> This board have 2 RMII phy:
>   -Ethernet1: RMII with crystal
>   -Ethernet2: RMII without crystal
> Rework dwmac glue to simplify management for next stm32
> Add support for PHY regulator
> 
> V2: - Remark from Rob Herring (add Krzysztof's ack in patch 02/11, update in yaml)
>       Remark from Serge Semin (upate commits msg)
> 
> Christophe Roullier (11):
>   dt-bindings: net: add STM32MP13 compatible in documentation for stm32
>   dt-bindings: net: add phy-supply property for stm32
>   net: stmmac: dwmac-stm32: rework glue to simplify management
>   net: stmmac: dwmac-stm32: add management of stm32mp13
>   net: stmmac: dwmac-stm32: update config management for phy wo cristal
>   net: stmmac: dwmac-stm32: clean the way to manage wol irqwake
>   net: stmmac: dwmac-stm32: support the phy-supply regulator binding
>   ARM: dts: stm32: add ethernet1 and ethernet2 support on stm32mp13
>   ARM: dts: stm32: add ethernet1/2 RMII pins for STM32MP13F-DK board
>   ARM: dts: stm32: add ethernet1 and ethernet2 for STM32MP135F-DK board
>   ARM: multi_v7_defconfig: Add MCP23S08 pinctrl support
> 
>  .../devicetree/bindings/net/stm32-dwmac.yaml  |  79 +++++-
>  arch/arm/boot/dts/st/stm32mp13-pinctrl.dtsi   |  71 ++++++
>  arch/arm/boot/dts/st/stm32mp131.dtsi          |  31 +++
>  arch/arm/boot/dts/st/stm32mp133.dtsi          |  30 +++
>  arch/arm/boot/dts/st/stm32mp135f-dk.dts       |  48 ++++
>  arch/arm/configs/multi_v7_defconfig           |   1 +
>  .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 235 ++++++++++++------
>  7 files changed, 416 insertions(+), 79 deletions(-)
> 
> --
> 2.25.1
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


New warnings running 'make CHECK_DTBS=y st/stm32mp135f-dk.dtb' for 20240426125707.585269-1-christophe.roullier@foss.st.com:

arch/arm/boot/dts/st/stm32mp135f-dk.dtb: ethernet@5800e000: Unevaluated properties are not allowed ('st,ext-phyclk' was unexpected)
	from schema $id: http://devicetree.org/schemas/net/stm32-dwmac.yaml#