[PATCH 0/3] Add GBETH glue layer driver for Renesas RZ/V2H(P) SoC

[PATCH 0/3] Add GBETH glue layer driver for Renesas RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series adds support for the GBETH (Gigabit Ethernet) interface
on the Renesas RZ/V2H(P) SoC. The changes include updating the device tree
bindings, documenting the GBETH bindings, and adding the DWMAC glue layer
for the Renesas GBETH.
 
Cheers,
Prabhakar

Lad Prabhakar (3):
  dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and
    'interrupt-names'
  dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC
  net: stmmac: Add DWMAC glue layer for Renesas GBETH

 .../bindings/net/renesas,r9a09g057-gbeth.yaml | 212 ++++++++++++++++++
 .../devicetree/bindings/net/snps,dwmac.yaml   |   7 +-
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../stmicro/stmmac/dwmac-renesas-gbeth.c      | 118 ++++++++++
 5 files changed, 347 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/renesas,r9a09g057-gbeth.yaml
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c

-- 
2.43.0

[PATCH 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names'

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Increase the `maxItems` value for the `interrupts` and `interrupt-names`
properties to accommodate the Renesas RZ/V2H(P) SoC, which features the
`snps,dwmac-5.20` IP with 11 interrupts.

Also add `additionalItems: true` to allow specifying extra interrupts
beyond the predefined ones. Update the `interrupt-names` property to
allow specifying extra `interrupt-names`.

Also refactor the optional `interrupt-names` property by consolidating
repeated enums into a single enum list.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 91e75eb3f329..85d499bd49b5 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -110,6 +110,8 @@ properties:
 
   interrupts:
     minItems: 1
+    maxItems: 11
+    additionalItems: true
     items:
       - description: Combined signal for various interrupt events
       - description: The interrupt to manage the remote wake-up packet detection
@@ -118,11 +120,11 @@ properties:
 
   interrupt-names:
     minItems: 1
+    maxItems: 11
+    additionalItems: true
     items:
       - const: macirq
       - enum: [eth_wake_irq, eth_lpi, sfty]
-      - enum: [eth_wake_irq, eth_lpi, sfty]
-      - enum: [eth_wake_irq, eth_lpi, sfty]
 
   clocks:
     minItems: 1
-- 
2.43.0

Re: [PATCH 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names'

[Rob Herring]

On Sun, Mar 02, 2025 at 06:18:06PM +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Increase the `maxItems` value for the `interrupts` and `interrupt-names`
> properties to accommodate the Renesas RZ/V2H(P) SoC, which features the
> `snps,dwmac-5.20` IP with 11 interrupts.
> 
> Also add `additionalItems: true` to allow specifying extra interrupts
> beyond the predefined ones. Update the `interrupt-names` property to
> allow specifying extra `interrupt-names`.
> 
> Also refactor the optional `interrupt-names` property by consolidating
> repeated enums into a single enum list.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 91e75eb3f329..85d499bd49b5 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -110,6 +110,8 @@ properties:
>  
>    interrupts:
>      minItems: 1
> +    maxItems: 11
> +    additionalItems: true

This just increased the number if interrupts for anyone that allowed 4 
interrupts. Rockchip and Intel are 2 though it is not clear if they 
support more than 1. So maybe not a new problem, but you are making it 
worse. Any of the ones in the 'select' in this file are affected as 
well.

>      items:
>        - description: Combined signal for various interrupt events
>        - description: The interrupt to manage the remote wake-up packet detection
> @@ -118,11 +120,11 @@ properties:
>  
>    interrupt-names:
>      minItems: 1
> +    maxItems: 11
> +    additionalItems: true
>      items:
>        - const: macirq
>        - enum: [eth_wake_irq, eth_lpi, sfty]
> -      - enum: [eth_wake_irq, eth_lpi, sfty]
> -      - enum: [eth_wake_irq, eth_lpi, sfty]
>  
>    clocks:
>      minItems: 1
> -- 
> 2.43.0
> 

Re: [PATCH 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names'

[Lad, Prabhakar]

Hi Rob,

Thank you for the review.

On Mon, Mar 3, 2025 at 3:26 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Mar 02, 2025 at 06:18:06PM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Increase the `maxItems` value for the `interrupts` and `interrupt-names`
> > properties to accommodate the Renesas RZ/V2H(P) SoC, which features the
> > `snps,dwmac-5.20` IP with 11 interrupts.
> >
> > Also add `additionalItems: true` to allow specifying extra interrupts
> > beyond the predefined ones. Update the `interrupt-names` property to
> > allow specifying extra `interrupt-names`.
> >
> > Also refactor the optional `interrupt-names` property by consolidating
> > repeated enums into a single enum list.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 91e75eb3f329..85d499bd49b5 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -110,6 +110,8 @@ properties:
> >
> >    interrupts:
> >      minItems: 1
> > +    maxItems: 11
> > +    additionalItems: true
>
> This just increased the number if interrupts for anyone that allowed 4
> interrupts. Rockchip and Intel are 2 though it is not clear if they
> support more than 1. So maybe not a new problem, but you are making it
> worse. Any of the ones in the 'select' in this file are affected as
> well.
>
How do you want me to handle this case:
1] Update vendors binding
2] Duplicate snps,dwmac.yaml in vendors binding.

Cheers,
Prabhakar

Re: [PATCH 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names'

[Andrew Lunn]

> How do you want me to handle this case:
> 1] Update vendors binding
> 2] Duplicate snps,dwmac.yaml in vendors binding.

Not the second. The stmmac driver is a mess because vendors do their
own thing rather than try to be the same as all other
vendors. Duplicating it will just make it worse.

	Andrew

Re: [PATCH 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names'

[Lad, Prabhakar]

Hi Andrew,

On Mon, Mar 3, 2025 at 4:34 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > How do you want me to handle this case:
> > 1] Update vendors binding
> > 2] Duplicate snps,dwmac.yaml in vendors binding.
>
> Not the second. The stmmac driver is a mess because vendors do their
> own thing rather than try to be the same as all other
> vendors. Duplicating it will just make it worse.
>
Thanks, I will update the vendor's binding to add the constraints.

Cheers,
Prabhakar

[PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Document the device tree bindings for the GBETH glue layer for the Renesas
RZ/V2H(P) SoC, which integrates the `snps,dwmac-5.20` IP.

The generic compatible string 'renesas,rzv2h-gbeth' is added, as this
module is identical on both the RZ/V2H(P) and RZ/G3E SoCs.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 .../bindings/net/renesas,r9a09g057-gbeth.yaml | 212 ++++++++++++++++++
 .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
 2 files changed, 213 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/renesas,r9a09g057-gbeth.yaml

diff --git a/Documentation/devicetree/bindings/net/renesas,r9a09g057-gbeth.yaml b/Documentation/devicetree/bindings/net/renesas,r9a09g057-gbeth.yaml
new file mode 100644
index 000000000000..dc23e7f401cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/renesas,r9a09g057-gbeth.yaml
@@ -0,0 +1,212 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/renesas,r9a09g057-gbeth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GBETH glue layer for Renesas RZ/V2H(P) (and similar SoCs)
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - renesas,r9a09g057-gbeth
+          - renesas,rzv2h-gbeth
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a09g057-gbeth # RZ/V2H(P)
+      - const: renesas,rzv2h-gbeth
+      - const: snps,dwmac-5.20
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: CSR clock
+      - description: AXI system clock
+      - description: PTP clock
+      - description: TX clock
+      - description: RX clock
+      - description: TX clock phase-shifted by 180 degrees
+      - description: RX clock phase-shifted by 180 degrees
+
+  clock-names:
+    items:
+      - const: stmmaceth
+      - const: pclk
+      - const: ptp_ref
+      - const: tx
+      - const: rx
+      - const: tx-180
+      - const: rx-180
+
+  interrupts:
+    items:
+      - description: Subsystem interrupt
+      - description: The interrupt to manage the remote wake-up packet detection
+      - description: The interrupt that occurs when Tx/Rx enters/exits the LPI state
+      - description: Per-channel transmission-0 completion interrupt
+      - description: Per-channel transmission-1 completion interrupt
+      - description: Per-channel transmission-2 completion interrupt
+      - description: Per-channel transmission-3 completion interrupt
+      - description: Per-channel receive-0 completion interrupt
+      - description: Per-channel receive-1 completion interrupt
+      - description: Per-channel receive-2 completion interrupt
+      - description: Per-channel receive-3 completion interrupt
+
+  interrupt-names:
+    items:
+      - const: macirq
+      - const: eth_wake_irq
+      - const: eth_lpi
+      - const: tx0
+      - const: tx1
+      - const: tx2
+      - const: tx3
+      - const: rx0
+      - const: rx1
+      - const: rx2
+      - const: rx3
+
+  resets:
+    items:
+      - description: AXI power-on system reset
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interrupt-names
+  - resets
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/renesas-cpg-mssr.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ethernet@15c30000 {
+        compatible = "renesas,r9a09g057-gbeth", "renesas,rzv2h-gbeth", "snps,dwmac-5.20";
+        reg = <0x15c30000 0x10000>;
+        clocks =  <&cpg CPG_MOD 0xbd>, <&cpg CPG_MOD 0xbc>,
+                  <&ptp_clock>, <&cpg CPG_MOD 0xb8>,
+                  <&cpg CPG_MOD 0xb9>, <&cpg CPG_MOD 0xba>,
+                  <&cpg CPG_MOD 0xbb>;
+        clock-names = "stmmaceth", "pclk", "ptp_ref",
+                      "tx", "rx", "tx-180", "rx-180";
+        resets = <&cpg 0xb0>;
+        interrupts = <GIC_SPI 765 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 767 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 766 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 768 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 770 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 771 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 772 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 773 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 774 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 745 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi",
+                          "tx0", "tx1", "tx2", "tx3",
+                          "rx0", "rx1", "rx2", "rx3";
+        phy-mode = "rgmii-id";
+        snps,multicast-filter-bins = <256>;
+        snps,perfect-filter-entries = <128>;
+        rx-fifo-depth = <8192>;
+        tx-fifo-depth = <8192>;
+        snps,fixed-burst;
+        snps,force_thresh_dma_mode;
+        snps,axi-config = <&stmmac_axi_setup>;
+        snps,mtl-rx-config = <&mtl_rx_setup>;
+        snps,mtl-tx-config = <&mtl_tx_setup>;
+        snps,en-tx-lpi-clockgating;
+        snps,txpbl = <32>;
+        snps,rxpbl = <32>;
+        phy-handle = <&phy0>;
+
+        stmmac_axi_setup: stmmac-axi-config {
+            snps,lpi_en;
+            snps,wr_osr_lmt = <0xf>;
+            snps,rd_osr_lmt = <0xf>;
+            snps,blen = <16 8 4 0 0 0 0>;
+        };
+
+        mtl_rx_setup: rx-queues-config {
+            snps,rx-queues-to-use = <4>;
+            snps,rx-sched-sp;
+
+            queue0 {
+                snps,dcb-algorithm;
+                snps,priority = <0x1>;
+                snps,map-to-dma-channel = <0>;
+            };
+
+            queue1 {
+                snps,dcb-algorithm;
+                snps,priority = <0x2>;
+                snps,map-to-dma-channel = <1>;
+            };
+
+            queue2 {
+                snps,dcb-algorithm;
+                snps,priority = <0x4>;
+                snps,map-to-dma-channel = <2>;
+            };
+
+            queue3 {
+                snps,dcb-algorithm;
+                snps,priority = <0x8>;
+                snps,map-to-dma-channel = <3>;
+            };
+        };
+
+        mtl_tx_setup: tx-queues-config {
+            snps,tx-queues-to-use = <4>;
+
+            queue0 {
+                snps,dcb-algorithm;
+                snps,priority = <0x1>;
+            };
+
+            queue1 {
+                snps,dcb-algorithm;
+                snps,priority = <0x2>;
+            };
+
+            queue2 {
+                snps,dcb-algorithm;
+                snps,priority = <0x4>;
+            };
+
+            queue3 {
+                snps,dcb-algorithm;
+                snps,priority = <0x1>;
+            };
+        };
+
+        mdio {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "snps,dwmac-mdio";
+
+            phy0: ethernet-phy@0 {
+                reg = <0>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 85d499bd49b5..b5ae3da327bd 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -74,6 +74,7 @@ properties:
         - qcom,sm8150-ethqos
         - renesas,r9a06g032-gmac
         - renesas,rzn1-gmac
+        - renesas,rzv2h-gbeth
         - rockchip,px30-gmac
         - rockchip,rk3128-gmac
         - rockchip,rk3228-gmac
-- 
2.43.0

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Andrew Lunn]

> +  interrupts:
> +    items:
> +      - description: Subsystem interrupt
> +      - description: The interrupt to manage the remote wake-up packet detection
> +      - description: The interrupt that occurs when Tx/Rx enters/exits the LPI state
> +      - description: Per-channel transmission-0 completion interrupt
> +      - description: Per-channel transmission-1 completion interrupt
> +      - description: Per-channel transmission-2 completion interrupt
> +      - description: Per-channel transmission-3 completion interrupt
> +      - description: Per-channel receive-0 completion interrupt
> +      - description: Per-channel receive-1 completion interrupt
> +      - description: Per-channel receive-2 completion interrupt
> +      - description: Per-channel receive-3 completion interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: macirq
> +      - const: eth_wake_irq
> +      - const: eth_lpi
> +      - const: tx0
> +      - const: tx1
> +      - const: tx2
> +      - const: tx3
> +      - const: rx0
> +      - const: rx1
> +      - const: rx2
> +      - const: rx3

There has already been a discussion about trying to make the clock
names more uniform. But what about interrupts? Which of these are in
the IP databook? What names does the databook use for these
interrupts?

	Andrew

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 08:10:26PM +0100, Andrew Lunn wrote:
> > +  interrupts:
> > +    items:
> > +      - description: Subsystem interrupt
> > +      - description: The interrupt to manage the remote wake-up packet detection
> > +      - description: The interrupt that occurs when Tx/Rx enters/exits the LPI state
> > +      - description: Per-channel transmission-0 completion interrupt
> > +      - description: Per-channel transmission-1 completion interrupt
> > +      - description: Per-channel transmission-2 completion interrupt
> > +      - description: Per-channel transmission-3 completion interrupt
> > +      - description: Per-channel receive-0 completion interrupt
> > +      - description: Per-channel receive-1 completion interrupt
> > +      - description: Per-channel receive-2 completion interrupt
> > +      - description: Per-channel receive-3 completion interrupt
> > +
> > +  interrupt-names:
> > +    items:
> > +      - const: macirq
> > +      - const: eth_wake_irq
> > +      - const: eth_lpi
> > +      - const: tx0
> > +      - const: tx1
> > +      - const: tx2
> > +      - const: tx3
> > +      - const: rx0
> > +      - const: rx1
> > +      - const: rx2
> > +      - const: rx3
> 
> There has already been a discussion about trying to make the clock
> names more uniform. But what about interrupts? Which of these are in
> the IP databook? What names does the databook use for these
> interrupts?

From a quick look, I haven't found anything that suggests the above
is possible, but it clearly is... so I'll look more tomorrow.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Andrew Lunn]

On Sun, Mar 02, 2025 at 07:28:09PM +0000, Russell King (Oracle) wrote:
> On Sun, Mar 02, 2025 at 08:10:26PM +0100, Andrew Lunn wrote:
> > > +  interrupts:
> > > +    items:
> > > +      - description: Subsystem interrupt
> > > +      - description: The interrupt to manage the remote wake-up packet detection
> > > +      - description: The interrupt that occurs when Tx/Rx enters/exits the LPI state
> > > +      - description: Per-channel transmission-0 completion interrupt
> > > +      - description: Per-channel transmission-1 completion interrupt
> > > +      - description: Per-channel transmission-2 completion interrupt
> > > +      - description: Per-channel transmission-3 completion interrupt
> > > +      - description: Per-channel receive-0 completion interrupt
> > > +      - description: Per-channel receive-1 completion interrupt
> > > +      - description: Per-channel receive-2 completion interrupt
> > > +      - description: Per-channel receive-3 completion interrupt
> > > +
> > > +  interrupt-names:
> > > +    items:
> > > +      - const: macirq
> > > +      - const: eth_wake_irq
> > > +      - const: eth_lpi
> > > +      - const: tx0
> > > +      - const: tx1
> > > +      - const: tx2
> > > +      - const: tx3
> > > +      - const: rx0
> > > +      - const: rx1
> > > +      - const: rx2
> > > +      - const: rx3
> > 
> > There has already been a discussion about trying to make the clock
> > names more uniform. But what about interrupts? Which of these are in
> > the IP databook? What names does the databook use for these
> > interrupts?
> 
> >From a quick look, I haven't found anything that suggests the above
> is possible, but it clearly is... so I'll look more tomorrow.

stmmac_platform.c:	stmmac_res->irq = platform_get_irq_byname(pdev, "macirq");
stmmac_platform.c:		platform_get_irq_byname_optional(pdev, "eth_wake_irq");
stmmac_platform.c:		platform_get_irq_byname_optional(pdev, "eth_lpi");
stmmac_platform.c:		platform_get_irq_byname_optional(pdev, "sfty");

So it looks like these are already in common code. So there should be
no need to name them in individual bindings, they can be named in the
common binding, and the vendor binding then just needs to indicate
they are required, or not.

What i have not yet figure out is how the names tx0, .. tx3 in this
binding are used. There is no code added in this patchset. Yet
loongson_dwmac_msi_config() and the intel stmmac_config_multi_msi()
are the only ones setting res->tx_irq[], neither of which are using
DT? I must be missing something somewhere.

    Andrew

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Rob Herring]

On Sun, Mar 02, 2025 at 10:33:56PM +0100, Andrew Lunn wrote:
> On Sun, Mar 02, 2025 at 07:28:09PM +0000, Russell King (Oracle) wrote:
> > On Sun, Mar 02, 2025 at 08:10:26PM +0100, Andrew Lunn wrote:
> > > > +  interrupts:
> > > > +    items:
> > > > +      - description: Subsystem interrupt
> > > > +      - description: The interrupt to manage the remote wake-up packet detection
> > > > +      - description: The interrupt that occurs when Tx/Rx enters/exits the LPI state
> > > > +      - description: Per-channel transmission-0 completion interrupt
> > > > +      - description: Per-channel transmission-1 completion interrupt
> > > > +      - description: Per-channel transmission-2 completion interrupt
> > > > +      - description: Per-channel transmission-3 completion interrupt
> > > > +      - description: Per-channel receive-0 completion interrupt
> > > > +      - description: Per-channel receive-1 completion interrupt
> > > > +      - description: Per-channel receive-2 completion interrupt
> > > > +      - description: Per-channel receive-3 completion interrupt
> > > > +
> > > > +  interrupt-names:
> > > > +    items:
> > > > +      - const: macirq
> > > > +      - const: eth_wake_irq
> > > > +      - const: eth_lpi
> > > > +      - const: tx0
> > > > +      - const: tx1
> > > > +      - const: tx2
> > > > +      - const: tx3
> > > > +      - const: rx0
> > > > +      - const: rx1
> > > > +      - const: rx2
> > > > +      - const: rx3
> > > 
> > > There has already been a discussion about trying to make the clock
> > > names more uniform. But what about interrupts? Which of these are in
> > > the IP databook? What names does the databook use for these
> > > interrupts?
> > 
> > >From a quick look, I haven't found anything that suggests the above
> > is possible, but it clearly is... so I'll look more tomorrow.
> 
> stmmac_platform.c:	stmmac_res->irq = platform_get_irq_byname(pdev, "macirq");
> stmmac_platform.c:		platform_get_irq_byname_optional(pdev, "eth_wake_irq");
> stmmac_platform.c:		platform_get_irq_byname_optional(pdev, "eth_lpi");
> stmmac_platform.c:		platform_get_irq_byname_optional(pdev, "sfty");
> 
> So it looks like these are already in common code. So there should be
> no need to name them in individual bindings, they can be named in the
> common binding, and the vendor binding then just needs to indicate
> they are required, or not.

The vendor bindings need to define the order. And to define if they are 
required, you have to list the names again...

Rob

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Andrew Lunn]

> +  clock-names:
> +    items:
> +      - const: stmmaceth
> +      - const: pclk
> +      - const: ptp_ref
> +      - const: tx
> +      - const: rx
> +      - const: tx-180
> +      - const: rx-180

As Russell said in an older thread, tx and tx-180 are effectively the
same clock, but with an inverter added. You should be able to arrange
the clock tree that if you enable tx, it also enables tx-180 as a
parent/sibling relationship.

	       Andrew

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Lad, Prabhakar]

Hi Andrew,

On Sun, Mar 2, 2025 at 7:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +  clock-names:
> > +    items:
> > +      - const: stmmaceth
> > +      - const: pclk
> > +      - const: ptp_ref
> > +      - const: tx
> > +      - const: rx
> > +      - const: tx-180
> > +      - const: rx-180
>
> As Russell said in an older thread, tx and tx-180 are effectively the
> same clock, but with an inverter added. You should be able to arrange
> the clock tree that if you enable tx, it also enables tx-180 as a
> parent/sibling relationship.
>
I can certainly do that, but not sure in the DT we will be describing
the HW correctly then. I'll have to hide *-180  clocks In the DT and
handle and turning on/off these clocks in the clock driver.

Currently
eth0: ethernet@15c30000 {
             compatible = "renesas,r9a09g057-gbeth", "renesas,rzv2h-gbeth",
                               "snps,dwmac-5.20";
             reg = <0 0x15c30000 0 0x10000>;
             interrupts = <GIC_SPI 765 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 767 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 766 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 768 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 769 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 770 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 771 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 772 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 773 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 774 IRQ_TYPE_LEVEL_HIGH>,
                               <GIC_SPI 745 IRQ_TYPE_LEVEL_HIGH>;
             interrupt-names = "macirq", "eth_wake_irq", "eth_lpi",
                                         "tx0", "tx1", "tx2", "tx3",
                                         "rx0", "rx1", "rx2", "rx3";
             clocks =  <&cpg CPG_MOD 0xbd>,
                            <&cpg CPG_MOD 0xbc>,
                            <&cpg CPG_CORE R9A09G057_GBETH_0_CLK_PTP_REF_I>,
                            <&cpg CPG_MOD 0xb8>,
                            <&cpg CPG_MOD 0xb9>,
                            <&cpg CPG_MOD 0xba>,
                            <&cpg CPG_MOD 0xbb>;
             clock-names = "stmmaceth", "pclk", "ptp_ref",
                                "tx", "rx", "tx-180", "rx-180";
             resets = <&cpg 0xb0>;
             .....
};

Cheers,
Prabhakar

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 08:41:39PM +0000, Lad, Prabhakar wrote:
> Hi Andrew,
> 
> On Sun, Mar 2, 2025 at 7:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > +  clock-names:
> > > +    items:
> > > +      - const: stmmaceth
> > > +      - const: pclk
> > > +      - const: ptp_ref
> > > +      - const: tx
> > > +      - const: rx
> > > +      - const: tx-180
> > > +      - const: rx-180
> >
> > As Russell said in an older thread, tx and tx-180 are effectively the
> > same clock, but with an inverter added. You should be able to arrange
> > the clock tree that if you enable tx, it also enables tx-180 as a
> > parent/sibling relationship.
> >
> I can certainly do that, but not sure in the DT we will be describing
> the HW correctly then. I'll have to hide *-180  clocks In the DT and
> handle and turning on/off these clocks in the clock driver.
...
>              clocks =  <&cpg CPG_MOD 0xbd>,
>                             <&cpg CPG_MOD 0xbc>,
>                             <&cpg CPG_CORE R9A09G057_GBETH_0_CLK_PTP_REF_I>,
>                             <&cpg CPG_MOD 0xb8>,
>                             <&cpg CPG_MOD 0xb9>,
>                             <&cpg CPG_MOD 0xba>,
>                             <&cpg CPG_MOD 0xbb>;

Your SoC designer really implemented the 0° and 180° as two separate
independently controllable clocks?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Lad, Prabhakar]

Hi Russell,

On Sun, Mar 2, 2025 at 9:01 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Sun, Mar 02, 2025 at 08:41:39PM +0000, Lad, Prabhakar wrote:
> > Hi Andrew,
> >
> > On Sun, Mar 2, 2025 at 7:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: stmmaceth
> > > > +      - const: pclk
> > > > +      - const: ptp_ref
> > > > +      - const: tx
> > > > +      - const: rx
> > > > +      - const: tx-180
> > > > +      - const: rx-180
> > >
> > > As Russell said in an older thread, tx and tx-180 are effectively the
> > > same clock, but with an inverter added. You should be able to arrange
> > > the clock tree that if you enable tx, it also enables tx-180 as a
> > > parent/sibling relationship.
> > >
> > I can certainly do that, but not sure in the DT we will be describing
> > the HW correctly then. I'll have to hide *-180  clocks In the DT and
> > handle and turning on/off these clocks in the clock driver.
> ...
> >              clocks =  <&cpg CPG_MOD 0xbd>,
> >                             <&cpg CPG_MOD 0xbc>,
> >                             <&cpg CPG_CORE R9A09G057_GBETH_0_CLK_PTP_REF_I>,
> >                             <&cpg CPG_MOD 0xb8>,
> >                             <&cpg CPG_MOD 0xb9>,
> >                             <&cpg CPG_MOD 0xba>,
> >                             <&cpg CPG_MOD 0xbb>;
>
> Your SoC designer really implemented the 0° and 180° as two separate
> independently controllable clocks?
>
Yes there are separate bits to turn ON/OFF the 0° and 180° clocks.

Cheers,
Prabhakar

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Andrew Lunn]

> > > I can certainly do that, but not sure in the DT we will be describing
> > > the HW correctly then. I'll have to hide *-180  clocks In the DT and
> > > handle and turning on/off these clocks in the clock driver.
> > ...
> > >              clocks =  <&cpg CPG_MOD 0xbd>,
> > >                             <&cpg CPG_MOD 0xbc>,
> > >                             <&cpg CPG_CORE R9A09G057_GBETH_0_CLK_PTP_REF_I>,
> > >                             <&cpg CPG_MOD 0xb8>,
> > >                             <&cpg CPG_MOD 0xb9>,
> > >                             <&cpg CPG_MOD 0xba>,
> > >                             <&cpg CPG_MOD 0xbb>;
> >
> > Your SoC designer really implemented the 0° and 180° as two separate
> > independently controllable clocks?
> >
> Yes there are separate bits to turn ON/OFF the 0° and 180° clocks.

Do you know what the clock tree actually looks like? I can think of
two different ways this could be implemented:

----+----------on/off---
    |
    +----not---on/off---

or

-------on/off-+------------------
              |
	      +---not---on/off---

In the first, the clocks are siblings. In the second there is
parent/child relationship.

	Andrew

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Lad, Prabhakar]

On Sun, Mar 2, 2025 at 9:39 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > > > I can certainly do that, but not sure in the DT we will be describing
> > > > the HW correctly then. I'll have to hide *-180  clocks In the DT and
> > > > handle and turning on/off these clocks in the clock driver.
> > > ...
> > > >              clocks =  <&cpg CPG_MOD 0xbd>,
> > > >                             <&cpg CPG_MOD 0xbc>,
> > > >                             <&cpg CPG_CORE R9A09G057_GBETH_0_CLK_PTP_REF_I>,
> > > >                             <&cpg CPG_MOD 0xb8>,
> > > >                             <&cpg CPG_MOD 0xb9>,
> > > >                             <&cpg CPG_MOD 0xba>,
> > > >                             <&cpg CPG_MOD 0xbb>;
> > >
> > > Your SoC designer really implemented the 0° and 180° as two separate
> > > independently controllable clocks?
> > >
> > Yes there are separate bits to turn ON/OFF the 0° and 180° clocks.
>
> Do you know what the clock tree actually looks like? I can think of
> two different ways this could be implemented:
>
> ----+----------on/off---
>     |
>     +----not---on/off---
>
> or
>
> -------on/off-+------------------
>               |
>               +---not---on/off---
>
> In the first, the clocks are siblings. In the second there is
> parent/child relationship.
>
It's the first case in this SoC.

Cheers,
Prabhakar

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 09:43:47PM +0000, Lad, Prabhakar wrote:
> On Sun, Mar 2, 2025 at 9:39 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > > Your SoC designer really implemented the 0° and 180° as two separate
> > > > independently controllable clocks?
> > > >
> > > Yes there are separate bits to turn ON/OFF the 0° and 180° clocks.
> >
> > Do you know what the clock tree actually looks like? I can think of
> > two different ways this could be implemented:
> >
> > ----+----------on/off---
> >     |
> >     +----not---on/off---
> >
> > or
> >
> > -------on/off-+------------------
> >               |
> >               +---not---on/off---
> >
> > In the first, the clocks are siblings. In the second there is
> > parent/child relationship.
> >
> It's the first case in this SoC.

Umm, okay. I'll just pick my jaw up off the floor. :D

Given that, then yes, go with your existing clock binding, because
that's the most sensible.

However, what would be useful for future maintenance is to put some
commentry at the top of the new glue file describing this (pictorially)
so that when someone looks at this later we know why it is this way.
It'll be useful information if someone else does the same because then
we can say "hey, we already have a binding for this situation!"

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 09:49:55PM +0000, Russell King (Oracle) wrote:
> On Sun, Mar 02, 2025 at 09:43:47PM +0000, Lad, Prabhakar wrote:
> > On Sun, Mar 2, 2025 at 9:39 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > > > Your SoC designer really implemented the 0° and 180° as two separate
> > > > > independently controllable clocks?
> > > > >
> > > > Yes there are separate bits to turn ON/OFF the 0° and 180° clocks.
> > >
> > > Do you know what the clock tree actually looks like? I can think of
> > > two different ways this could be implemented:
> > >
> > > ----+----------on/off---
> > >     |
> > >     +----not---on/off---
> > >
> > > or
> > >
> > > -------on/off-+------------------
> > >               |
> > >               +---not---on/off---
> > >
> > > In the first, the clocks are siblings. In the second there is
> > > parent/child relationship.
> > >
> > It's the first case in this SoC.
> 
> Umm, okay. I'll just pick my jaw up off the floor. :D
> 
> Given that, then yes, go with your existing clock binding, because
> that's the most sensible.
> 
> However, what would be useful for future maintenance is to put some
> commentry at the top of the new glue file describing this (pictorially)
> so that when someone looks at this later we know why it is this way.
> It'll be useful information if someone else does the same because then
> we can say "hey, we already have a binding for this situation!"

Additionally, it would probably be useful to include it in the dt
binding commit description because that will probably assist the
review of that patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC

[Lad, Prabhakar]

On Sun, Mar 2, 2025 at 9:51 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Sun, Mar 02, 2025 at 09:49:55PM +0000, Russell King (Oracle) wrote:
> > On Sun, Mar 02, 2025 at 09:43:47PM +0000, Lad, Prabhakar wrote:
> > > On Sun, Mar 2, 2025 at 9:39 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > > > > > Your SoC designer really implemented the 0° and 180° as two separate
> > > > > > independently controllable clocks?
> > > > > >
> > > > > Yes there are separate bits to turn ON/OFF the 0° and 180° clocks.
> > > >
> > > > Do you know what the clock tree actually looks like? I can think of
> > > > two different ways this could be implemented:
> > > >
> > > > ----+----------on/off---
> > > >     |
> > > >     +----not---on/off---
> > > >
> > > > or
> > > >
> > > > -------on/off-+------------------
> > > >               |
> > > >               +---not---on/off---
> > > >
> > > > In the first, the clocks are siblings. In the second there is
> > > > parent/child relationship.
> > > >
> > > It's the first case in this SoC.
> >
> > Umm, okay. I'll just pick my jaw up off the floor. :D
> >
> > Given that, then yes, go with your existing clock binding, because
> > that's the most sensible.
> >
> > However, what would be useful for future maintenance is to put some
> > commentry at the top of the new glue file describing this (pictorially)
> > so that when someone looks at this later we know why it is this way.
> > It'll be useful information if someone else does the same because then
> > we can say "hey, we already have a binding for this situation!"
>
> Additionally, it would probably be useful to include it in the dt
> binding commit description because that will probably assist the
> review of that patch.
>
Sure will do that.

Cheers,
Prabhakar

[PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Renesas RZ/V2H(P) SoC is equipped with Synopsys DesignWare Ethernet
Quality-of-Service IP block version 5.20. This commit adds DWMAC glue
layer for the Renesas GBETH found on the RZ/V2H(P) SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  11 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../stmicro/stmmac/dwmac-renesas-gbeth.c      | 118 ++++++++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 4cc85a36a1ab..6e52a27f01b5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -131,6 +131,17 @@ config DWMAC_QCOM_ETHQOS
 	  This selects the Qualcomm ETHQOS glue layer support for the
 	  stmmac device driver.
 
+config DWMAC_RENESAS_GBETH
+	tristate "Renesas RZ/V2H(P) GBETH support"
+	default ARCH_RENESAS
+	depends on OF && (ARCH_RENESAS || COMPILE_TEST)
+	help
+	  Support for Gigabit Ethernet Interface (GBETH) on Renesas
+	  RZ/V2H(P) SoCs.
+
+	  This selects the Renesas RZ/V2H(P) Soc specific glue layer support
+	  for the stmmac device driver.
+
 config DWMAC_ROCKCHIP
 	tristate "Rockchip dwmac support"
 	default ARCH_ROCKCHIP
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index b26f0e79c2b3..91bf57fa3de4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_DWMAC_LPC18XX)	+= dwmac-lpc18xx.o
 obj-$(CONFIG_DWMAC_MEDIATEK)	+= dwmac-mediatek.o
 obj-$(CONFIG_DWMAC_MESON)	+= dwmac-meson.o dwmac-meson8b.o
 obj-$(CONFIG_DWMAC_QCOM_ETHQOS)	+= dwmac-qcom-ethqos.o
+obj-$(CONFIG_DWMAC_RENESAS_GBETH) += dwmac-renesas-gbeth.o
 obj-$(CONFIG_DWMAC_ROCKCHIP)	+= dwmac-rk.o
 obj-$(CONFIG_DWMAC_RZN1)	+= dwmac-rzn1.o
 obj-$(CONFIG_DWMAC_S32)		+= dwmac-s32.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
new file mode 100644
index 000000000000..f4488dc51b27
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * dwmac-renesas-gbeth.c - DWMAC Specific Glue layer for Renesas GBETH
+ *
+ * Copyright (C) 2025 Renesas Electronics Corporation
+ */
+
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include "dwmac4.h"
+#include "stmmac_platform.h"
+
+struct renesas_gbeth {
+	struct device *dev;
+	void __iomem *regs;
+	unsigned int num_clks;
+	struct clk_bulk_data *clks;
+};
+
+static const char *const renesas_gbeth_clks[] __initconst = {
+	"rx", "rx-180", "tx-180",
+};
+
+static int renesas_gbeth_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct device *dev = &pdev->dev;
+	struct renesas_gbeth *gbeth;
+	struct reset_control *rstc;
+	unsigned int i;
+	int err;
+
+	err = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (err)
+		return dev_err_probe(dev, err,
+				     "failed to get resources\n");
+
+	plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat))
+		return dev_err_probe(dev, PTR_ERR(plat_dat),
+				     "dt configuration failed\n");
+
+	gbeth = devm_kzalloc(dev, sizeof(*gbeth), GFP_KERNEL);
+	if (!gbeth)
+		return -ENOMEM;
+
+	plat_dat->clk_tx_i = devm_clk_get_enabled(dev, "tx");
+	if (IS_ERR(plat_dat->clk_tx_i))
+		return dev_err_probe(dev, PTR_ERR(plat_dat->clk_tx_i),
+				     "error getting tx clock\n");
+
+	gbeth->num_clks = ARRAY_SIZE(renesas_gbeth_clks);
+	gbeth->clks = devm_kcalloc(dev, gbeth->num_clks,
+				   sizeof(*gbeth->clks), GFP_KERNEL);
+	if (!gbeth->clks)
+		return -ENOMEM;
+
+	for (i = 0; i <  gbeth->num_clks; i++)
+		gbeth->clks[i].id = renesas_gbeth_clks[i];
+
+	err = devm_clk_bulk_get(dev, gbeth->num_clks, gbeth->clks);
+	if (err < 0)
+		return err;
+
+	err = clk_bulk_prepare_enable(gbeth->num_clks, gbeth->clks);
+	if (err)
+		return err;
+
+	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
+	if (IS_ERR(rstc))
+		return PTR_ERR(rstc);
+
+	gbeth->dev = dev;
+	gbeth->regs = stmmac_res.addr;
+	plat_dat->bsp_priv = gbeth;
+	plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
+	plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
+			   STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |
+			   STMMAC_FLAG_RX_CLK_RUNS_IN_LPI |
+			   STMMAC_FLAG_SPH_DISABLE;
+
+	return stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
+}
+
+static void renesas_gbeth_remove(struct platform_device *pdev)
+{
+	struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
+
+	stmmac_dvr_remove(&pdev->dev);
+
+	clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
+}
+
+static const struct of_device_id renesas_gbeth_match[] = {
+	{ .compatible = "renesas,rzv2h-gbeth", },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, renesas_gbeth_match);
+
+static struct platform_driver renesas_gbeth_driver = {
+	.probe  = renesas_gbeth_probe,
+	.remove = renesas_gbeth_remove,
+	.driver = {
+		.name		= "renesas-gbeth",
+		.pm		= &stmmac_pltfr_pm_ops,
+		.of_match_table	= renesas_gbeth_match,
+	},
+};
+module_platform_driver(renesas_gbeth_driver);
+
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Renesas GBETH DWMAC Specific Glue layer");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 06:18:08PM +0000, Prabhakar wrote:
> +	gbeth->dev = dev;
> +	gbeth->regs = stmmac_res.addr;
> +	plat_dat->bsp_priv = gbeth;
> +	plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;

Thanks for using that!

> +	plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> +			   STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |

I would like to know what value tx_clk_stop is in
stmmac_mac_enable_tx_lpi() for your setup. Ideally, stmmac should
use the capability report from the PHY to decide whether the
transmit clock can be gated, but sadly we haven't had any support
in phylib/phylink for that until recently, and I haven't modified
stmmac to allow use of that. However, it would be good to gain
knowledge in this area.

> +			   STMMAC_FLAG_RX_CLK_RUNS_IN_LPI |

What is the reason for setting this flag? If it's because of suspend/
resume failures, does my "net: stmmac: fix resume failures due to
RX clock" series solve this for you without requiring this flag?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 07:33:10PM +0000, Russell King (Oracle) wrote:
> On Sun, Mar 02, 2025 at 06:18:08PM +0000, Prabhakar wrote:
> > +	gbeth->dev = dev;
> > +	gbeth->regs = stmmac_res.addr;
> > +	plat_dat->bsp_priv = gbeth;
> > +	plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
> 
> Thanks for using that!
> 
> > +	plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> > +			   STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |
> 
> I would like to know what value tx_clk_stop is in
> stmmac_mac_enable_tx_lpi() for your setup. Ideally, stmmac should
> use the capability report from the PHY to decide whether the
> transmit clock can be gated, but sadly we haven't had any support
> in phylib/phylink for that until recently, and I haven't modified
> stmmac to allow use of that. However, it would be good to gain
> knowledge in this area.
> 
> > +			   STMMAC_FLAG_RX_CLK_RUNS_IN_LPI |
> 
> What is the reason for setting this flag? If it's because of suspend/
> resume failures, does my "net: stmmac: fix resume failures due to
> RX clock" series solve this for you without requiring this flag?

https://lore.kernel.org/r/Z8B4tVd4nLUKXdQ4@shell.armlinux.org.uk

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Russell,

On Sun, Mar 2, 2025 at 7:33 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Sun, Mar 02, 2025 at 06:18:08PM +0000, Prabhakar wrote:
> > +     gbeth->dev = dev;
> > +     gbeth->regs = stmmac_res.addr;
> > +     plat_dat->bsp_priv = gbeth;
> > +     plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
>
> Thanks for using that!
>
Yep, it shortens the glue driver further.

> > +     plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> > +                        STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |
>
> I would like to know what value tx_clk_stop is in
> stmmac_mac_enable_tx_lpi() for your setup. Ideally, stmmac should
> use the capability report from the PHY to decide whether the
> transmit clock can be gated, but sadly we haven't had any support
> in phylib/phylink for that until recently, and I haven't modified
> stmmac to allow use of that. However, it would be good to gain
> knowledge in this area.
>
tx_clk_stop =1,

root@rzv2h-evk-alpha:~# ifconfig eth0 up
[  587.830436] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-0
[  587.838636] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-1
[  587.846792] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-2
[  587.854734] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-3
[  587.926860] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
[  587.949380] dwmac4: Master AXI performs fixed burst length
[  587.954910] renesas-gbeth 15c30000.ethernet eth0: No Safety
Features support found
[  587.962556] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
Advanced Timestamp supported
[  587.971420] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
[  587.978004] renesas-gbeth 15c30000.ethernet eth0: configuring for
phy/rgmii-id link mode
root@rzv2h-evk-alpha:~# [  591.070448] renesas-gbeth 15c30000.ethernet
eth0: tx_clk_stop=1
[  591.076590] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx

With the below diff:

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index aec230353ac4..68f1954e6eea 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1100,6 +1100,7 @@ static int stmmac_mac_enable_tx_lpi(struct
phylink_config *config, u32 timer,
        struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
        int ret;

+       netdev_err(priv->dev, "tx_clk_stop=%d\n", tx_clk_stop);
        priv->tx_lpi_timer = timer;
        priv->eee_active = true;

> > +                        STMMAC_FLAG_RX_CLK_RUNS_IN_LPI |
>
> What is the reason for setting this flag? If it's because of suspend/
> resume failures, does my "net: stmmac: fix resume failures due to
> RX clock" series solve this for you without requiring this flag?
>
Ive set this flag based on the configuration supported by this IP.
Unfortunately the platform which I am working on doesn't support s2r
yet so I cannot test suspend/resume path yet. But I do see an issue
when I unload and load just the glue module the DMA reset fails.

Cheers,
Prabhakar

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> Hi Russell,
> > What is the reason for setting this flag? If it's because of suspend/
> > resume failures, does my "net: stmmac: fix resume failures due to
> > RX clock" series solve this for you without requiring this flag?
> >
> Ive set this flag based on the configuration supported by this IP.
> Unfortunately the platform which I am working on doesn't support s2r
> yet so I cannot test suspend/resume path yet. But I do see an issue
> when I unload and load just the glue module the DMA reset fails.

Thanks for that feedback - that's a scenario I hadn't considered.

I was trying to avoid having to disable LPI RX clock-stop on suspend by
ensuring that it was enabled at resume time. I think that's valid, but
you've brought up another similar scenario:

- device is brought up, configures RX clock stop
- links with media, negotiates EEE
- driver is unloaded, link doesn't go down, but due to no traffic goes
  into idle, so RX clock is stopped
- driver reloaded, RX clock still stopped, reset fails

I would like to solve that so we can get the power savings from
stopping the clock, but still have reset work when necessary.

I'm guessing that the "DMA reset fails" refers to this path:

stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
stmmac_init_dma_engine() -> stmmac_reset() ?

In other words, when the device is being brought back up
adminsitratively?

What happens if you (replace $if):

# ip li set dev $if down
# ip li set dev $if up

Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Russell,

On Sun, Mar 2, 2025 at 9:44 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> > Hi Russell,
> > > What is the reason for setting this flag? If it's because of suspend/
> > > resume failures, does my "net: stmmac: fix resume failures due to
> > > RX clock" series solve this for you without requiring this flag?
> > >
> > Ive set this flag based on the configuration supported by this IP.
> > Unfortunately the platform which I am working on doesn't support s2r
> > yet so I cannot test suspend/resume path yet. But I do see an issue
> > when I unload and load just the glue module the DMA reset fails.
>
> Thanks for that feedback - that's a scenario I hadn't considered.
>
> I was trying to avoid having to disable LPI RX clock-stop on suspend by
> ensuring that it was enabled at resume time. I think that's valid, but
> you've brought up another similar scenario:
>
> - device is brought up, configures RX clock stop
> - links with media, negotiates EEE
> - driver is unloaded, link doesn't go down, but due to no traffic goes
>   into idle, so RX clock is stopped
> - driver reloaded, RX clock still stopped, reset fails
>
> I would like to solve that so we can get the power savings from
> stopping the clock, but still have reset work when necessary.
>
I would be happy to test the patches ;)

> I'm guessing that the "DMA reset fails" refers to this path:
>
> stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
> stmmac_init_dma_engine() -> stmmac_reset() ?
>
Yes.

> In other words, when the device is being brought back up
> adminsitratively?
>
> What happens if you (replace $if):
>
> # ip li set dev $if down
> # ip li set dev $if up
>
> Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?
>
Logs without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
--------------------------------------------------------------
root@rzv2h-evk-alpha:~# ip li set dev eth1 down
[   33.606549] renesas-gbeth 15c40000.ethernet eth1: Link is Down
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# ip li set dev eth0 down
[   37.356992] renesas-gbeth 15c30000.ethernet eth0: Link is Down
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# ip li set dev eth1 up
[   43.974803] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-0
[   43.983189] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-1
[   43.991155] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-2
[   43.999128] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-3
[   44.072079] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
[   44.094605] dwmac4: Master AXI performs fixed burst length
[   44.100138] renesas-gbeth 15c40000.ethernet eth1: No Safety
Features support found
[   44.107748] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
Advanced Timestamp supported
[   44.116725] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
[   44.123352] renesas-gbeth 15c40000.ethernet eth1: configuring for
phy/rgmii-id link mode
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# ip li set dev eth1[   47.207761] renesas-gbeth
15c40000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
^C
root@rzv2h-evk-alpha:~# ^C
root@rzv2h-evk-alpha:~# ip li set dev eth0 up
[   55.636722] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-0
[   55.645139] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-1
[   55.653111] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-2
[   55.661073] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-3
[   55.732087] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
[   55.754612] dwmac4: Master AXI performs fixed burst length
[   55.760143] renesas-gbeth 15c30000.ethernet eth0: No Safety
Features support found
[   55.767740] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
Advanced Timestamp supported
[   55.776705] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
[   55.783333] renesas-gbeth 15c30000.ethernet eth0: configuring for
phy/rgmii-id link mode
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# [   58.855844] renesas-gbeth 15c30000.ethernet
eth0: tx_clk_stop=1
[   58.861989] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx

root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~#

Logs with STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
--------------------------------------------------------------
root@rzv2h-evk-alpha:~# ip li set dev eth1 down
[   30.053790] renesas-gbeth 15c40000.ethernet eth1: Link is Down
root@rzv2h-evk-alpha:~# ip li set dev eth0 down
[   35.366935] renesas-gbeth 15c30000.ethernet eth0: Link is Down
root@rzv2h-evk-alpha:~# ip li set dev eth1 up
[   40.448563] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-0
[   40.456725] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-1
[   40.464893] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-2
[   40.472840] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-3
[   40.543895] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
[   40.566419] dwmac4: Master AXI performs fixed burst length
[   40.571949] renesas-gbeth 15c40000.ethernet eth1: No Safety
Features support found
[   40.579550] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
Advanced Timestamp supported
[   40.588505] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
[   40.595135] renesas-gbeth 15c40000.ethernet eth1: configuring for
phy/rgmii-id link mode
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# [   43.687551] renesas-gbeth 15c40000.ethernet
eth1: Link is Up - 1Gbps/Full - flow control off

root@rzv2h-evk-alpha:~# ip li set dev eth0 up
[   49.644479] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-0
[   49.652719] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-1
[   49.660681] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-2
[   49.669059] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-3
[   49.740011] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
[   49.762518] dwmac4: Master AXI performs fixed burst length
[   49.768057] renesas-gbeth 15c30000.ethernet eth0: No Safety
Features support found
[   49.775655] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
Advanced Timestamp supported
[   49.784609] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
[   49.791236] renesas-gbeth 15c30000.ethernet eth0: configuring for
phy/rgmii-id link mode
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# [   52.871635] renesas-gbeth 15c30000.ethernet
eth0: tx_clk_stop=1
[   52.877777] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx


Cheers,
Prabhakar

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Sun, Mar 02, 2025 at 10:02:15PM +0000, Lad, Prabhakar wrote:
> Hi Russell,
> 
> On Sun, Mar 2, 2025 at 9:44 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> > > Hi Russell,
> > > > What is the reason for setting this flag? If it's because of suspend/
> > > > resume failures, does my "net: stmmac: fix resume failures due to
> > > > RX clock" series solve this for you without requiring this flag?
> > > >
> > > Ive set this flag based on the configuration supported by this IP.
> > > Unfortunately the platform which I am working on doesn't support s2r
> > > yet so I cannot test suspend/resume path yet. But I do see an issue
> > > when I unload and load just the glue module the DMA reset fails.
> >
> > Thanks for that feedback - that's a scenario I hadn't considered.
> >
> > I was trying to avoid having to disable LPI RX clock-stop on suspend by
> > ensuring that it was enabled at resume time. I think that's valid, but
> > you've brought up another similar scenario:
> >
> > - device is brought up, configures RX clock stop
> > - links with media, negotiates EEE
> > - driver is unloaded, link doesn't go down, but due to no traffic goes
> >   into idle, so RX clock is stopped
> > - driver reloaded, RX clock still stopped, reset fails
> >
> > I would like to solve that so we can get the power savings from
> > stopping the clock, but still have reset work when necessary.
> >
> I would be happy to test the patches ;)
> 
> > I'm guessing that the "DMA reset fails" refers to this path:
> >
> > stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
> > stmmac_init_dma_engine() -> stmmac_reset() ?
> >
> Yes.
> 
> > In other words, when the device is being brought back up
> > adminsitratively?
> >
> > What happens if you (replace $if):
> >
> > # ip li set dev $if down
> > # ip li set dev $if up
> >
> > Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?
> >
> Logs without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> --------------------------------------------------------------
> root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> [   33.606549] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> [   37.356992] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> [   43.974803] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-0
> [   43.983189] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-1
> [   43.991155] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-2
> [   43.999128] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-3
> [   44.072079] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> [   44.094605] dwmac4: Master AXI performs fixed burst length
> [   44.100138] renesas-gbeth 15c40000.ethernet eth1: No Safety
> Features support found
> [   44.107748] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> Advanced Timestamp supported
> [   44.116725] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> [   44.123352] renesas-gbeth 15c40000.ethernet eth1: configuring for
> phy/rgmii-id link mode
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~# ip li set dev eth1[   47.207761] renesas-gbeth
> 15c40000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
> ^C
> root@rzv2h-evk-alpha:~# ^C
> root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> [   55.636722] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-0
> [   55.645139] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-1
> [   55.653111] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-2
> [   55.661073] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-3
> [   55.732087] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> [   55.754612] dwmac4: Master AXI performs fixed burst length
> [   55.760143] renesas-gbeth 15c30000.ethernet eth0: No Safety
> Features support found
> [   55.767740] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> Advanced Timestamp supported
> [   55.776705] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> [   55.783333] renesas-gbeth 15c30000.ethernet eth0: configuring for
> phy/rgmii-id link mode
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~# [   58.855844] renesas-gbeth 15c30000.ethernet
> eth0: tx_clk_stop=1
> [   58.861989] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control rx/tx
> 
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~#
> 
> Logs with STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> --------------------------------------------------------------
> root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> [   30.053790] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> [   35.366935] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> [   40.448563] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-0
> [   40.456725] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-1
> [   40.464893] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-2
> [   40.472840] renesas-gbeth 15c40000.ethernet eth1: Register
> MEM_TYPE_PAGE_POOL RxQ-3
> [   40.543895] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> [   40.566419] dwmac4: Master AXI performs fixed burst length
> [   40.571949] renesas-gbeth 15c40000.ethernet eth1: No Safety
> Features support found
> [   40.579550] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> Advanced Timestamp supported
> [   40.588505] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> [   40.595135] renesas-gbeth 15c40000.ethernet eth1: configuring for
> phy/rgmii-id link mode
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~# [   43.687551] renesas-gbeth 15c40000.ethernet
> eth1: Link is Up - 1Gbps/Full - flow control off
> 
> root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> [   49.644479] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-0
> [   49.652719] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-1
> [   49.660681] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-2
> [   49.669059] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-3
> [   49.740011] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> [   49.762518] dwmac4: Master AXI performs fixed burst length
> [   49.768057] renesas-gbeth 15c30000.ethernet eth0: No Safety
> Features support found
> [   49.775655] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> Advanced Timestamp supported
> [   49.784609] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> [   49.791236] renesas-gbeth 15c30000.ethernet eth0: configuring for
> phy/rgmii-id link mode
> root@rzv2h-evk-alpha:~#
> root@rzv2h-evk-alpha:~# [   52.871635] renesas-gbeth 15c30000.ethernet
> eth0: tx_clk_stop=1
> [   52.877777] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control rx/tx

I would like to get to the bottom of why this fails for module removal/
insertion, but not for admistratively down/upping the interface.

Removal of your module will unregister the netdev, and part of that
work will bring the netdev administratively down. When re-inserting
the module, that will trigger various userspace events, and it will
be userspace bringing the network interface(s) back up. This should
be no different from administratively down/upping the interface but
it seems you get different behaviour.

I'd like to understand why that is, because at the moment I'm wondering
whether my patches that address the suspend/resume need further work
before I send them - but in order to assess that, I need to work out
why your issue only seems to occur in the module removal/insertion
and not down/up as well as I'd expect.

Please could you investigate this?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Russell,

On Mon, Mar 3, 2025 at 11:19 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Sun, Mar 02, 2025 at 10:02:15PM +0000, Lad, Prabhakar wrote:
> > Hi Russell,
> >
> > On Sun, Mar 2, 2025 at 9:44 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> > > > Hi Russell,
> > > > > What is the reason for setting this flag? If it's because of suspend/
> > > > > resume failures, does my "net: stmmac: fix resume failures due to
> > > > > RX clock" series solve this for you without requiring this flag?
> > > > >
> > > > Ive set this flag based on the configuration supported by this IP.
> > > > Unfortunately the platform which I am working on doesn't support s2r
> > > > yet so I cannot test suspend/resume path yet. But I do see an issue
> > > > when I unload and load just the glue module the DMA reset fails.
> > >
> > > Thanks for that feedback - that's a scenario I hadn't considered.
> > >
> > > I was trying to avoid having to disable LPI RX clock-stop on suspend by
> > > ensuring that it was enabled at resume time. I think that's valid, but
> > > you've brought up another similar scenario:
> > >
> > > - device is brought up, configures RX clock stop
> > > - links with media, negotiates EEE
> > > - driver is unloaded, link doesn't go down, but due to no traffic goes
> > >   into idle, so RX clock is stopped
> > > - driver reloaded, RX clock still stopped, reset fails
> > >
> > > I would like to solve that so we can get the power savings from
> > > stopping the clock, but still have reset work when necessary.
> > >
> > I would be happy to test the patches ;)
> >
> > > I'm guessing that the "DMA reset fails" refers to this path:
> > >
> > > stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
> > > stmmac_init_dma_engine() -> stmmac_reset() ?
> > >
> > Yes.
> >
> > > In other words, when the device is being brought back up
> > > adminsitratively?
> > >
> > > What happens if you (replace $if):
> > >
> > > # ip li set dev $if down
> > > # ip li set dev $if up
> > >
> > > Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?
> > >
> > Logs without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > --------------------------------------------------------------
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > [   33.606549] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > [   37.356992] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > [   43.974803] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   43.983189] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   43.991155] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   43.999128] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   44.072079] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   44.094605] dwmac4: Master AXI performs fixed burst length
> > [   44.100138] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > Features support found
> > [   44.107748] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   44.116725] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > [   44.123352] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# ip li set dev eth1[   47.207761] renesas-gbeth
> > 15c40000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
> > ^C
> > root@rzv2h-evk-alpha:~# ^C
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> > [   55.636722] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   55.645139] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   55.653111] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   55.661073] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   55.732087] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   55.754612] dwmac4: Master AXI performs fixed burst length
> > [   55.760143] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > Features support found
> > [   55.767740] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   55.776705] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > [   55.783333] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# [   58.855844] renesas-gbeth 15c30000.ethernet
> > eth0: tx_clk_stop=1
> > [   58.861989] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > 1Gbps/Full - flow control rx/tx
> >
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~#
> >
> > Logs with STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > --------------------------------------------------------------
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > [   30.053790] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > [   35.366935] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > [   40.448563] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   40.456725] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   40.464893] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   40.472840] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   40.543895] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   40.566419] dwmac4: Master AXI performs fixed burst length
> > [   40.571949] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > Features support found
> > [   40.579550] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   40.588505] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > [   40.595135] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# [   43.687551] renesas-gbeth 15c40000.ethernet
> > eth1: Link is Up - 1Gbps/Full - flow control off
> >
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> > [   49.644479] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   49.652719] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   49.660681] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   49.669059] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   49.740011] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   49.762518] dwmac4: Master AXI performs fixed burst length
> > [   49.768057] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > Features support found
> > [   49.775655] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   49.784609] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > [   49.791236] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# [   52.871635] renesas-gbeth 15c30000.ethernet
> > eth0: tx_clk_stop=1
> > [   52.877777] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > 1Gbps/Full - flow control rx/tx
>
> I would like to get to the bottom of why this fails for module removal/
> insertion, but not for admistratively down/upping the interface.
>
> Removal of your module will unregister the netdev, and part of that
> work will bring the netdev administratively down. When re-inserting
> the module, that will trigger various userspace events, and it will
> be userspace bringing the network interface(s) back up. This should
> be no different from administratively down/upping the interface but
> it seems you get different behaviour.
>
> I'd like to understand why that is, because at the moment I'm wondering
> whether my patches that address the suspend/resume need further work
> before I send them - but in order to assess that, I need to work out
> why your issue only seems to occur in the module removal/insertion
> and not down/up as well as I'd expect.
>
> Please could you investigate this?
>
Sure I will look into this. Just wanted to check on your platform does
unload/load work OK? Also do you know any specific reason why DMA
reset could be failing so that I can look at it closer.

Cheers,
Prabhakar

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Mon, Mar 03, 2025 at 04:04:55PM +0000, Lad, Prabhakar wrote:
> Hi Russell,
> 
> On Mon, Mar 3, 2025 at 11:19 AM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> > I would like to get to the bottom of why this fails for module removal/
> > insertion, but not for admistratively down/upping the interface.
> >
> > Removal of your module will unregister the netdev, and part of that
> > work will bring the netdev administratively down. When re-inserting
> > the module, that will trigger various userspace events, and it will
> > be userspace bringing the network interface(s) back up. This should
> > be no different from administratively down/upping the interface but
> > it seems you get different behaviour.
> >
> > I'd like to understand why that is, because at the moment I'm wondering
> > whether my patches that address the suspend/resume need further work
> > before I send them - but in order to assess that, I need to work out
> > why your issue only seems to occur in the module removal/insertion
> > and not down/up as well as I'd expect.
> >
> > Please could you investigate this?
> >
> Sure I will look into this. Just wanted to check on your platform does
> unload/load work OK? Also do you know any specific reason why DMA
> reset could be failing so that I can look at it closer.

It may be surprising, but I do not have stmmac hardware (although
there is some I might be able to use, it's rather complicated so I
haven't investigated that.) However, there's a lot of past history
here, because stmmac has been painful for me as phylink maintainer.
Consequently, I'm now taking a more active role in this driver,
cleaning it up and fixing some of the stuff it's got wrong.

That said, NVidia are in the process of arranging hardware for me.

You are not the first to encounter reset failures, and this has always
come down to clocks that aren't running.

The DWMAC core is documented as requiring *all* clocks for each part of
the core to be running in order for software reset to complete. If any
clock is stopped, then reset will fail. That includes the clk_rx_i /
clk_rx_180_i signals that come from the ethernet PHY's receive clock.

However, PHYs that have negotiated EEE are permitted to stop their
receive clock, which can be enabled by an appropriate control bit.
phy_eee_rx_clock_stop() manipulates that bit. stmmac has in most
cases permitted the PHY to stop its receive clock.

NVidia have been a recent victim of this - it is desirable to allow
receive clock stop, but there hasn't been the APIs in the kernel
to allow MAC drivers to re-enable the clock when they need it.

Up until now, I had thought this was just a suspend/resume issue
(which is NVidia's reported case). Your testing suggests that it is
more widespread than that.

While I've been waiting to hear from you, I've prepared some patches
that change the solution that I proposed for NVidia (currently on top
of that patch set).

However, before I proceed with them, I need you to get to the bottom
of why:

# ip li set dev $if down
# ip li set dev $if up

doesn't trigger it, but removing and re-inserting the module does.

I'd suggest looking at things such as:
- does the media link actually go down in one case but not the other
  (I don't mean does the kernel report the link went down - I mean
  did the remote end see the link go down, or is it still up, and
  thus *may* be in EEE low-power idle mode.)

- printing the statis from stmmac_host_irq_status() so we can see
  when the DWMAC tx/rx paths enters and exits LPI mode while the
  driver is active. (could be quite noisy).

- verify that .ndo_stop does get called when removing your module
  (it should, it's a core net function.)

- print the value of the LPI control/status register at various
  points that may be relevant (e.g. before the reset function is
  called.) bits 9 and 8 indicate receive and transmit LPI status.

I'm sure there's other things, but the above is just off the top of my
head.

Thanks for anything you can do to locate this.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Russell,

On Mon, Mar 3, 2025 at 4:32 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Mar 03, 2025 at 04:04:55PM +0000, Lad, Prabhakar wrote:
> > Hi Russell,
> >
> > On Mon, Mar 3, 2025 at 11:19 AM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > > I would like to get to the bottom of why this fails for module removal/
> > > insertion, but not for admistratively down/upping the interface.
> > >
> > > Removal of your module will unregister the netdev, and part of that
> > > work will bring the netdev administratively down. When re-inserting
> > > the module, that will trigger various userspace events, and it will
> > > be userspace bringing the network interface(s) back up. This should
> > > be no different from administratively down/upping the interface but
> > > it seems you get different behaviour.
> > >
> > > I'd like to understand why that is, because at the moment I'm wondering
> > > whether my patches that address the suspend/resume need further work
> > > before I send them - but in order to assess that, I need to work out
> > > why your issue only seems to occur in the module removal/insertion
> > > and not down/up as well as I'd expect.
> > >
> > > Please could you investigate this?
> > >
> > Sure I will look into this. Just wanted to check on your platform does
> > unload/load work OK? Also do you know any specific reason why DMA
> > reset could be failing so that I can look at it closer.
>
> It may be surprising, but I do not have stmmac hardware (although
> there is some I might be able to use, it's rather complicated so I
> haven't investigated that.) However, there's a lot of past history
> here, because stmmac has been painful for me as phylink maintainer.
> Consequently, I'm now taking a more active role in this driver,
> cleaning it up and fixing some of the stuff it's got wrong.
>
> That said, NVidia are in the process of arranging hardware for me.
>
> You are not the first to encounter reset failures, and this has always
> come down to clocks that aren't running.
>
> The DWMAC core is documented as requiring *all* clocks for each part of
> the core to be running in order for software reset to complete. If any
> clock is stopped, then reset will fail. That includes the clk_rx_i /
> clk_rx_180_i signals that come from the ethernet PHY's receive clock.
>
I did investigate on these lines:

1] With my current patch series I have the below in remove callback

+static void renesas_gbeth_remove(struct platform_device *pdev)
+{
+       struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
+
+       stmmac_dvr_remove(&pdev->dev);
+
+       clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
+}

After dumping the CLK registers I found out that the Rx and Rx-180 CLK
never got turned OFF after unbind.

2] I replaced the remove callback with below ie first turn OFF
Tx-180/Rx/Rx-180 clocks

+static void renesas_gbeth_remove(struct platform_device *pdev)
+{
+       struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
+
+       clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
+
+      stmmac_dvr_remove(&pdev->dev);
+}

After dumping the CLK registers I confirmed all the clocks were OFF
(CSR/PCLK/Tx/Tx-180/Rx/Rx-180) after unbind operation. Now when I do a
bind operation Rx clock fails to enable, which is probably because the
PHY is not providing any clock.

> However, PHYs that have negotiated EEE are permitted to stop their
> receive clock, which can be enabled by an appropriate control bit.
> phy_eee_rx_clock_stop() manipulates that bit. stmmac has in most
> cases permitted the PHY to stop its receive clock.
>
You mean phy_eee_rx_clock_stop() is the one which tells the PHY to
disable the Rx clocks? Actually I tried the below hunk with this as
well the Rx clock fails to be turned ON after unbind/bind operation.

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 0ba434104f5b..e16f4a6f5715 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -1756,6 +1756,7 @@ EXPORT_SYMBOL_GPL(phy_eee_tx_clock_stop_capable);
  */
 int phy_eee_rx_clock_stop(struct phy_device *phydev, bool clk_stop_enable)
 {
+       return 0;
        int ret;

        /* Configure the PHY to stop receiving xMII



> NVidia have been a recent victim of this - it is desirable to allow
> receive clock stop, but there hasn't been the APIs in the kernel
> to allow MAC drivers to re-enable the clock when they need it.
>
> Up until now, I had thought this was just a suspend/resume issue
> (which is NVidia's reported case). Your testing suggests that it is
> more widespread than that.
>
> While I've been waiting to hear from you, I've prepared some patches
> that change the solution that I proposed for NVidia (currently on top
> of that patch set).
>
I tried your latest patches [0], this didnt resolve the issue.
(Actually there was a build problem with the patch I fixed it below
hunk)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 9a62808cf935..21cdea4aec9a 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -2609,7 +2609,7 @@ void phylink_rx_clk_stop_block(struct phylink *pl)
         * function has been called and clock-stop was previously enabled.
         */
        if (pl->mac_rx_clk_stop_blocked++ == 0 &&
-           pl->mac_supports_eee_ops && pl->phydev)
+           pl->mac_supports_eee_ops && pl->phydev &&
            pl->config->eee_rx_clk_stop_enable)
                phy_eee_rx_clock_stop(pl->phydev, false);
 }


[0] https://lore.kernel.org/all/Z8bbnSG67rqTj0pH@shell.armlinux.org.uk/

> However, before I proceed with them, I need you to get to the bottom
> of why:
>
> # ip li set dev $if down
> # ip li set dev $if up
>
> doesn't trigger it, but removing and re-inserting the module does.
>
Doing the above does not turn OFF/ON all the clocks. Looking at the
dump from the CLK driver on my platform only stmmaceth and pclk are
the clocks which get toggled and rest remain ON. Note Im not sure if
the PHY is disabling the Rx clocks when I run ip li set dev $if down I
cannot scope that pin on the board.

Please let me know if you have any pointers for me to look further
into this issue.

Cheers,
Prabhakar

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Wed, Mar 05, 2025 at 09:26:43PM +0000, Lad, Prabhakar wrote:
> I did investigate on these lines:
> 
> 1] With my current patch series I have the below in remove callback
> 
> +static void renesas_gbeth_remove(struct platform_device *pdev)
> +{
> +       struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
> +
> +       stmmac_dvr_remove(&pdev->dev);
> +
> +       clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
> +}
> 
> After dumping the CLK registers I found out that the Rx and Rx-180 CLK
> never got turned OFF after unbind.

I think that's where further investigation needs to happen. This
suggests there's more enables than disables for these clocks, but
there's nothing that I can see in your submitted driver that would
account for that behaviour.

> 2] I replaced the remove callback with below ie first turn OFF
> Tx-180/Rx/Rx-180 clocks
> 
> +static void renesas_gbeth_remove(struct platform_device *pdev)
> +{
> +       struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
> +
> +       clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
> +
> +      stmmac_dvr_remove(&pdev->dev);
> +}
> 
> After dumping the CLK registers I confirmed all the clocks were OFF
> (CSR/PCLK/Tx/Tx-180/Rx/Rx-180) after unbind operation. Now when I do a
> bind operation Rx clock fails to enable, which is probably because the
> PHY is not providing any clock.

However, disabling clocks _before_ unregistering the net device is a
bad thing to do! The netdev could still be in use.

You can add:

	if (ndev->phydev)
		phy_eee_rx_clock_stop(ndev->phydev, false);

just before unregister_netdev() in stmmac_dvr_remove() only as a way
to test out that idea.

Do the clock registers you refer to only update when the relevant
clocks are actually running?

> > However, PHYs that have negotiated EEE are permitted to stop their
> > receive clock, which can be enabled by an appropriate control bit.
> > phy_eee_rx_clock_stop() manipulates that bit. stmmac has in most
> > cases permitted the PHY to stop its receive clock.
> >
> You mean phy_eee_rx_clock_stop() is the one which tells the PHY to
> disable the Rx clocks? Actually I tried the below hunk with this as
> well the Rx clock fails to be turned ON after unbind/bind operation.

phy_eee_rx_clock_stop() doesn't turn the clock on/off per-se, it
controls the bit which gives the PHY permission to disable the clock
when the media is in EEE low-power mode. Note that 802.3 does not
give a default setting for this bit, so:

> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 0ba434104f5b..e16f4a6f5715 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -1756,6 +1756,7 @@ EXPORT_SYMBOL_GPL(phy_eee_tx_clock_stop_capable);
>   */
>  int phy_eee_rx_clock_stop(struct phy_device *phydev, bool clk_stop_enable)
>  {
> +       return 0;
>         int ret;
> 
>         /* Configure the PHY to stop receiving xMII

May not be wise, and if you want to ensure that the PHY does not stop
the clock, then forcing clk_stop_enable to zero would be better.

> > NVidia have been a recent victim of this - it is desirable to allow
> > receive clock stop, but there hasn't been the APIs in the kernel
> > to allow MAC drivers to re-enable the clock when they need it.
> >
> > Up until now, I had thought this was just a suspend/resume issue
> > (which is NVidia's reported case). Your testing suggests that it is
> > more widespread than that.
> >
> > While I've been waiting to hear from you, I've prepared some patches
> > that change the solution that I proposed for NVidia (currently on top
> > of that patch set).
>
> I tried your latest patches [0], this didnt resolve the issue.

I assume without the modification to phy_eee_rx_clock_stop() above -
thanks. If so, then your issue is not EEE related.

> [0] https://lore.kernel.org/all/Z8bbnSG67rqTj0pH@shell.armlinux.org.uk/

Wasn't quite the latest, but still had the same build bug (thanks for
reporting, now fixed.) Latest is equivalent so no need to re-test.

> > However, before I proceed with them, I need you to get to the bottom
> > of why:
> >
> > # ip li set dev $if down
> > # ip li set dev $if up
> >
> > doesn't trigger it, but removing and re-inserting the module does.
> >
> Doing the above does not turn OFF/ON all the clocks. Looking at the
> dump from the CLK driver on my platform only stmmaceth and pclk are
> the clocks which get toggled and rest remain ON. Note Im not sure if
> the PHY is disabling the Rx clocks when I run ip li set dev $if down I
> cannot scope that pin on the board.
> 
> Please let me know if you have any pointers for me to look further
> into this issue.

Given the behaviour that you're reporting from your clock layer, I'm
wondering if the problem is actually there... it seems weird that clocks
aren't being turned off and on when they should.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Russell,

On Thu, Mar 6, 2025 at 12:31 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Wed, Mar 05, 2025 at 09:26:43PM +0000, Lad, Prabhakar wrote:
> > I did investigate on these lines:
> >
> > 1] With my current patch series I have the below in remove callback
> >
> > +static void renesas_gbeth_remove(struct platform_device *pdev)
> > +{
> > +       struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
> > +
> > +       stmmac_dvr_remove(&pdev->dev);
> > +
> > +       clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
> > +}
> >
> > After dumping the CLK registers I found out that the Rx and Rx-180 CLK
> > never got turned OFF after unbind.
>
> I think that's where further investigation needs to happen. This
> suggests there's more enables than disables for these clocks, but
> there's nothing that I can see in your submitted driver that would
> account for that behaviour.
>
> > 2] I replaced the remove callback with below ie first turn OFF
> > Tx-180/Rx/Rx-180 clocks
> >
> > +static void renesas_gbeth_remove(struct platform_device *pdev)
> > +{
> > +       struct renesas_gbeth *gbeth = get_stmmac_bsp_priv(&pdev->dev);
> > +
> > +       clk_bulk_disable_unprepare(gbeth->num_clks, gbeth->clks);
> > +
> > +      stmmac_dvr_remove(&pdev->dev);
> > +}
> >
> > After dumping the CLK registers I confirmed all the clocks were OFF
> > (CSR/PCLK/Tx/Tx-180/Rx/Rx-180) after unbind operation. Now when I do a
> > bind operation Rx clock fails to enable, which is probably because the
> > PHY is not providing any clock.
>
> However, disabling clocks _before_ unregistering the net device is a
> bad thing to do! The netdev could still be in use.
>
> You can add:
>
>         if (ndev->phydev)
>                 phy_eee_rx_clock_stop(ndev->phydev, false);
>
> just before unregister_netdev() in stmmac_dvr_remove() only as a way
> to test out that idea.
>
I tried the above and I can still see the PHY providing the Rx clocks.

> Do the clock registers you refer to only update when the relevant
> clocks are actually running?
>
> > > However, PHYs that have negotiated EEE are permitted to stop their
> > > receive clock, which can be enabled by an appropriate control bit.
> > > phy_eee_rx_clock_stop() manipulates that bit. stmmac has in most
> > > cases permitted the PHY to stop its receive clock.
> > >
> > You mean phy_eee_rx_clock_stop() is the one which tells the PHY to
> > disable the Rx clocks? Actually I tried the below hunk with this as
> > well the Rx clock fails to be turned ON after unbind/bind operation.
>
> phy_eee_rx_clock_stop() doesn't turn the clock on/off per-se, it
> controls the bit which gives the PHY permission to disable the clock
> when the media is in EEE low-power mode. Note that 802.3 does not
> give a default setting for this bit, so:
>
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 0ba434104f5b..e16f4a6f5715 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -1756,6 +1756,7 @@ EXPORT_SYMBOL_GPL(phy_eee_tx_clock_stop_capable);
> >   */
> >  int phy_eee_rx_clock_stop(struct phy_device *phydev, bool clk_stop_enable)
> >  {
> > +       return 0;
> >         int ret;
> >
> >         /* Configure the PHY to stop receiving xMII
>
> May not be wise, and if you want to ensure that the PHY does not stop
> the clock, then forcing clk_stop_enable to zero would be better.
>
> > > NVidia have been a recent victim of this - it is desirable to allow
> > > receive clock stop, but there hasn't been the APIs in the kernel
> > > to allow MAC drivers to re-enable the clock when they need it.
> > >
> > > Up until now, I had thought this was just a suspend/resume issue
> > > (which is NVidia's reported case). Your testing suggests that it is
> > > more widespread than that.
> > >
> > > While I've been waiting to hear from you, I've prepared some patches
> > > that change the solution that I proposed for NVidia (currently on top
> > > of that patch set).
> >
> > I tried your latest patches [0], this didnt resolve the issue.
>
> I assume without the modification to phy_eee_rx_clock_stop() above -
> thanks. If so, then your issue is not EEE related.
>
> > [0] https://lore.kernel.org/all/Z8bbnSG67rqTj0pH@shell.armlinux.org.uk/
>
> Wasn't quite the latest, but still had the same build bug (thanks for
> reporting, now fixed.) Latest is equivalent so no need to re-test.
>
Maybe looking at the behaviour on my SoC, we should request to disable
the PHY clocks upon module removal see below...

> > > However, before I proceed with them, I need you to get to the bottom
> > > of why:
> > >
> > > # ip li set dev $if down
> > > # ip li set dev $if up
> > >
> > > doesn't trigger it, but removing and re-inserting the module does.
> > >
> > Doing the above does not turn OFF/ON all the clocks. Looking at the
> > dump from the CLK driver on my platform only stmmaceth and pclk are
> > the clocks which get toggled and rest remain ON. Note Im not sure if
> > the PHY is disabling the Rx clocks when I run ip li set dev $if down I
> > cannot scope that pin on the board.
> >
> > Please let me know if you have any pointers for me to look further
> > into this issue.
>
> Given the behaviour that you're reporting from your clock layer, I'm
> wondering if the problem is actually there... it seems weird that clocks
> aren't being turned off and on when they should.
>
You were right, the problem was indeed in the clock layer. On the SoC
we have two registers one for clock gating (turn ON/OFF) and other for
monitoring the clock (ie CLOK_ON and CLK_MON registers). When we turn
ON the clock the CLK_ON bit gets set and to make sure it's turned ON
the corresponding CLK_MON bit is checked to ensure it's ON. When a
request is made to turn ON the clock first we check the CLK_MON bit
and if it's being set we return early as the clock was ON. This worked
OK up until now where the clocks used were internally generated.

In the case of RGMII interface where the Rx/Rx-180 clock was coming
from an PHY on an external pin the above didn't work as expected. When
we issued an unbind request on the glue driver all the clocks were
gated to OFF state i.e CLK_ON bits were set to '0'. Now when the bind
operation was requested  the clocks were requested to be turned ON, ie
when CLK_MON bits for RX/Rx-180 reported to be '1'  that is because
PHY was providing the clock and due to which the CLK_ON bit was unset
(and not gated to ON state)  due to which the DMA reset operation
failed in our case.

After fixing the clock driver, to ignore CLK_MON bits for clocks
coming from external pins and just relying on CLK_ON bits I am seeing
no more DMA reset timeouts.

Below are the logs, for unbind/bind operation working for both
eth0/eth1 interfaces.
-------------------------------------------------------------------------------------------------------------
root@rzv2h-evk-alpha:~# ethtool --show-eee eth0
EEE Settings for eth0:
        EEE status: enabled - active
        Tx LPI: 1000000 (us)
        Supported EEE link modes:  100baseT/Full
                                   1000baseT/Full
        Advertised EEE link modes:  100baseT/Full
                                    1000baseT/Full
        Link partner advertised EEE link modes:  100baseT/Full
                                                 1000baseT/Full
root@rzv2h-evk-alpha:~# ethtool --show-eee eth1
EEE Settings for eth1:
        EEE status: enabled - inactive
        Tx LPI: 1000000 (us)
        Supported EEE link modes:  100baseT/Full
                                   1000baseT/Full
        Advertised EEE link modes:  100baseT/Full
                                    1000baseT/Full
        Link partner advertised EEE link modes:  Not reported
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# cd /sys/bus/platform/drivers/renesas-gbeth/ &&
echo 15c30000.ethernet > unbind && cd -
[  440.623862] renesas-gbeth 15c30000.ethernet eth0:
stmmac_dvr_remove: removing driver
[  440.632176] renesas-gbeth 15c30000.ethernet eth0: Link is Down
/home/root
root@rzv2h-evk-alpha:~# cd /sys/bus/platform/drivers/renesas-gbeth/ &&
echo 15c30000.ethernet > bind && cd -
[  448.384065] renesas-gbeth 15c30000.ethernet: IRQ sfty not found
[  448.391491] renesas-gbeth 15c30000.ethernet: User ID: 0x1, Synopsys ID: 0x52
[  448.398735] renesas-gbeth 15c30000.ethernet:         DWMAC4/5
[  448.404007] renesas-gbeth 15c30000.ethernet: DMA HW capability
register supported
[  448.411633] renesas-gbeth 15c30000.ethernet: RX Checksum Offload
Engine supported
[  448.419245] renesas-gbeth 15c30000.ethernet: Wake-Up On Lan supported
[  448.425916] renesas-gbeth 15c30000.ethernet: Enable RX Mitigation
via HW Watchdog Timer
[  448.433977] renesas-gbeth 15c30000.ethernet: Enabled L3L4 Flow TC (entries=8)
[  448.441167] renesas-gbeth 15c30000.ethernet: Enabled RFS Flow TC (entries=10)
[  448.448352] renesas-gbeth 15c30000.ethernet: Using 32/32 bits DMA
host/device width
/home/root
[  448.468325] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-0
[  448.476762] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-1
[  448.484756] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-2
[  448.492705] renesas-gbeth 15c30000.ethernet eth0: Register
MEM_TYPE_PAGE_POOL RxQ-3
root@rzv2h-evk-alpha:~# [  448.564797] renesas-gbeth 15c30000.ethernet
eth0: PHY [stmmac-0:00] driver [Microchip KSZ9131 Gigabit PHY]
(irq=POLL)
[  448.587333] dwmac4: Master AXI performs fixed burst length
[  448.592868] renesas-gbeth 15c30000.ethernet eth0: No Safety
Features support found
[  448.600466] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
Advanced Timestamp supported
[  448.609409] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
[  448.616637] renesas-gbeth 15c30000.ethernet eth0: configuring for
phy/rgmii-id link mode
[  451.704179] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
1Gbps/Full - flow control rx/tx

root@rzv2h-evk-alpha:~# cd /sys/bus/platform/drivers/renesas-gbeth/ &&
echo 15c40000.ethernet > unbind && cd -
[  486.269883] renesas-gbeth 15c40000.ethernet eth1:
stmmac_dvr_remove: removing driver
[  486.278374] renesas-gbeth 15c40000.ethernet eth1: Link is Down
[  486.323261] audit: type=1334 audit(1741436947.780:15): prog-id=13 op=LOAD
[  486.330102] audit: type=1334 audit(1741436947.788:16): prog-id=14 op=LOAD
/home/root
root@rzv2h-evk-alpha:~#
root@rzv2h-evk-alpha:~# cd /sys/bus/platform/drivers/renesas-gbeth/ &&
echo 15c40000.ethernet > bind && cd -
[  491.951054] renesas-gbeth 15c40000.ethernet: IRQ sfty not found
[  491.958576] renesas-gbeth 15c40000.ethernet: User ID: 0x0, Synopsys ID: 0x52
[  491.965819] renesas-gbeth 15c40000.ethernet:         DWMAC4/5
[  491.971087] renesas-gbeth 15c40000.ethernet: DMA HW capability
register supported
[  491.978705] renesas-gbeth 15c40000.ethernet: RX Checksum Offload
Engine supported
[  491.986423] renesas-gbeth 15c40000.ethernet: Wake-Up On Lan supported
[  491.992927] renesas-gbeth 15c40000.ethernet: Enable RX Mitigation
via HW Watchdog Timer
[  492.001011] renesas-gbeth 15c40000.ethernet: device MAC address
de:08:73:a2:96:42
[  492.008547] renesas-gbeth 15c40000.ethernet: Enabled L3L4 Flow TC (entries=8)
[  492.015793] renesas-gbeth 15c40000.ethernet: Enabled RFS Flow TC (entries=10)
[  492.022941] renesas-gbeth 15c40000.ethernet: Using 32/32 bits DMA
host/device width
/home/root
root@rzv2h-evk-alpha:~# [  492.045756] renesas-gbeth 15c40000.ethernet
eth1: Register MEM_TYPE_PAGE_POOL RxQ-0
[  492.055375] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-1
[  492.063568] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-2
[  492.071575] renesas-gbeth 15c40000.ethernet eth1: Register
MEM_TYPE_PAGE_POOL RxQ-3
[  492.123684] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
[  492.146301] dwmac4: Master AXI performs fixed burst length
[  492.151840] renesas-gbeth 15c40000.ethernet eth1: No Safety
Features support found
[  492.159438] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
Advanced Timestamp supported
[  492.168266] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
[  492.175237] renesas-gbeth 15c40000.ethernet eth1: configuring for
phy/rgmii-id link mode

root@rzv2h-evk-alpha:~# [  495.255185] renesas-gbeth 15c40000.ethernet
eth1: Link is Up - 1Gbps/Full - flow control off
[  527.352545] audit: type=1334 audit(1741436988.809:17): prog-id=14 op=UNLOAD
[  527.359538] audit: type=1334 audit(1741436988.809:18): prog-id=13 op=UNLOAD

root@rzv2h-evk-alpha:~#

Cheers,
Prabhakar

RE: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Biju Das]

Hi Russel king,

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 03 March 2025 11:19
> Subject: Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
> 
> On Sun, Mar 02, 2025 at 10:02:15PM +0000, Lad, Prabhakar wrote:
> > Hi Russell,
> >
> > On Sun, Mar 2, 2025 at 9:44 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> > > > Hi Russell,
> > > > > What is the reason for setting this flag? If it's because of
> > > > > suspend/ resume failures, does my "net: stmmac: fix resume
> > > > > failures due to RX clock" series solve this for you without requiring this flag?
> > > > >
> > > > Ive set this flag based on the configuration supported by this IP.
> > > > Unfortunately the platform which I am working on doesn't support
> > > > s2r yet so I cannot test suspend/resume path yet. But I do see an
> > > > issue when I unload and load just the glue module the DMA reset fails.
> > >
> > > Thanks for that feedback - that's a scenario I hadn't considered.
> > >
> > > I was trying to avoid having to disable LPI RX clock-stop on suspend
> > > by ensuring that it was enabled at resume time. I think that's
> > > valid, but you've brought up another similar scenario:
> > >
> > > - device is brought up, configures RX clock stop
> > > - links with media, negotiates EEE
> > > - driver is unloaded, link doesn't go down, but due to no traffic goes
> > >   into idle, so RX clock is stopped
> > > - driver reloaded, RX clock still stopped, reset fails
> > >
> > > I would like to solve that so we can get the power savings from
> > > stopping the clock, but still have reset work when necessary.
> > >
> > I would be happy to test the patches ;)
> >
> > > I'm guessing that the "DMA reset fails" refers to this path:
> > >
> > > stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
> > > stmmac_init_dma_engine() -> stmmac_reset() ?
> > >
> > Yes.
> >
> > > In other words, when the device is being brought back up
> > > adminsitratively?
> > >
> > > What happens if you (replace $if):
> > >
> > > # ip li set dev $if down
> > > # ip li set dev $if up
> > >
> > > Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?
> > >
> > Logs without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > --------------------------------------------------------------
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > [   33.606549] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > [   37.356992] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > [   43.974803] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   43.983189] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   43.991155] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   43.999128] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   44.072079] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   44.094605] dwmac4: Master AXI performs fixed burst length
> > [   44.100138] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > Features support found
> > [   44.107748] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   44.116725] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > [   44.123352] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# ip li set dev eth1[   47.207761] renesas-gbeth
> > 15c40000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off ^C
> > root@rzv2h-evk-alpha:~# ^C root@rzv2h-evk-alpha:~# ip li set dev eth0
> > up
> > [   55.636722] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   55.645139] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   55.653111] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   55.661073] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   55.732087] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   55.754612] dwmac4: Master AXI performs fixed burst length
> > [   55.760143] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > Features support found
> > [   55.767740] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   55.776705] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > [   55.783333] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# [   58.855844] renesas-gbeth 15c30000.ethernet
> > eth0: tx_clk_stop=1
> > [   58.861989] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > 1Gbps/Full - flow control rx/tx
> >
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~#
> >
> > Logs with STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > --------------------------------------------------------------
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > [   30.053790] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > [   35.366935] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > [   40.448563] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   40.456725] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   40.464893] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   40.472840] renesas-gbeth 15c40000.ethernet eth1: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   40.543895] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   40.566419] dwmac4: Master AXI performs fixed burst length
> > [   40.571949] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > Features support found
> > [   40.579550] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   40.588505] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > [   40.595135] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# [   43.687551] renesas-gbeth 15c40000.ethernet
> > eth1: Link is Up - 1Gbps/Full - flow control off
> >
> > root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> > [   49.644479] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [   49.652719] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [   49.660681] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [   49.669059] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [   49.740011] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [   49.762518] dwmac4: Master AXI performs fixed burst length
> > [   49.768057] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > Features support found
> > [   49.775655] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > Advanced Timestamp supported
> > [   49.784609] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > [   49.791236] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~#
> > root@rzv2h-evk-alpha:~# [   52.871635] renesas-gbeth 15c30000.ethernet
> > eth0: tx_clk_stop=1
> > [   52.877777] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > 1Gbps/Full - flow control rx/tx
> 
down/upping the interface but it seems you get different behaviour.
> 
> I'd like to understand why that is, because at the moment I'm wondering whether my patches that
> address the suspend/resume need further work before I send them - but in order to assess that, I need
> to work out why your issue only seems to occur in the module removal/insertion and not down/up as well
> as I'd expect.

FYI, With linux next, On RZ/G3E SoC which has similar IP as RZ/V2H, ethernet works during suspend entry/exit
Even though STR is not fully functional yet.

I have integrated KEY_SLEEP button on RZ/G3E. On pressing the button, I can see the ethernet link goes down during
suspend entry and link up happens during suspend exit.

I will test your suspend/resume patch soon with net-next.

Cheers,
Biju 

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Tue, Mar 04, 2025 at 06:58:44AM +0000, Biju Das wrote:
> Hi Russel king,
> 
> > -----Original Message-----
> > From: Russell King <linux@armlinux.org.uk>
> > Sent: 03 March 2025 11:19
> > Subject: Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
> > 
> > On Sun, Mar 02, 2025 at 10:02:15PM +0000, Lad, Prabhakar wrote:
> > > Hi Russell,
> > >
> > > On Sun, Mar 2, 2025 at 9:44 PM Russell King (Oracle)
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> > > > > Hi Russell,
> > > > > > What is the reason for setting this flag? If it's because of
> > > > > > suspend/ resume failures, does my "net: stmmac: fix resume
> > > > > > failures due to RX clock" series solve this for you without requiring this flag?
> > > > > >
> > > > > Ive set this flag based on the configuration supported by this IP.
> > > > > Unfortunately the platform which I am working on doesn't support
> > > > > s2r yet so I cannot test suspend/resume path yet. But I do see an
> > > > > issue when I unload and load just the glue module the DMA reset fails.
> > > >
> > > > Thanks for that feedback - that's a scenario I hadn't considered.
> > > >
> > > > I was trying to avoid having to disable LPI RX clock-stop on suspend
> > > > by ensuring that it was enabled at resume time. I think that's
> > > > valid, but you've brought up another similar scenario:
> > > >
> > > > - device is brought up, configures RX clock stop
> > > > - links with media, negotiates EEE
> > > > - driver is unloaded, link doesn't go down, but due to no traffic goes
> > > >   into idle, so RX clock is stopped
> > > > - driver reloaded, RX clock still stopped, reset fails
> > > >
> > > > I would like to solve that so we can get the power savings from
> > > > stopping the clock, but still have reset work when necessary.
> > > >
> > > I would be happy to test the patches ;)
> > >
> > > > I'm guessing that the "DMA reset fails" refers to this path:
> > > >
> > > > stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
> > > > stmmac_init_dma_engine() -> stmmac_reset() ?
> > > >
> > > Yes.
> > >
> > > > In other words, when the device is being brought back up
> > > > adminsitratively?
> > > >
> > > > What happens if you (replace $if):
> > > >
> > > > # ip li set dev $if down
> > > > # ip li set dev $if up
> > > >
> > > > Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?
> > > >
> > > Logs without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > > --------------------------------------------------------------
> > > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > > [   33.606549] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > > [   37.356992] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > > [   43.974803] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-0
> > > [   43.983189] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-1
> > > [   43.991155] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-2
> > > [   43.999128] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-3
> > > [   44.072079] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > [   44.094605] dwmac4: Master AXI performs fixed burst length
> > > [   44.100138] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > > Features support found
> > > [   44.107748] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > > Advanced Timestamp supported
> > > [   44.116725] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > > [   44.123352] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > > phy/rgmii-id link mode
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~# ip li set dev eth1[   47.207761] renesas-gbeth
> > > 15c40000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off ^C
> > > root@rzv2h-evk-alpha:~# ^C root@rzv2h-evk-alpha:~# ip li set dev eth0
> > > up
> > > [   55.636722] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-0
> > > [   55.645139] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-1
> > > [   55.653111] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-2
> > > [   55.661073] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-3
> > > [   55.732087] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > [   55.754612] dwmac4: Master AXI performs fixed burst length
> > > [   55.760143] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > > Features support found
> > > [   55.767740] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > > Advanced Timestamp supported
> > > [   55.776705] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > > [   55.783333] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > > phy/rgmii-id link mode
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~# [   58.855844] renesas-gbeth 15c30000.ethernet
> > > eth0: tx_clk_stop=1
> > > [   58.861989] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > > 1Gbps/Full - flow control rx/tx
> > >
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~#
> > >
> > > Logs with STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > > --------------------------------------------------------------
> > > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > > [   30.053790] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > > [   35.366935] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > > [   40.448563] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-0
> > > [   40.456725] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-1
> > > [   40.464893] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-2
> > > [   40.472840] renesas-gbeth 15c40000.ethernet eth1: Register
> > > MEM_TYPE_PAGE_POOL RxQ-3
> > > [   40.543895] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > [   40.566419] dwmac4: Master AXI performs fixed burst length
> > > [   40.571949] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > > Features support found
> > > [   40.579550] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > > Advanced Timestamp supported
> > > [   40.588505] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > > [   40.595135] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > > phy/rgmii-id link mode
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~# [   43.687551] renesas-gbeth 15c40000.ethernet
> > > eth1: Link is Up - 1Gbps/Full - flow control off
> > >
> > > root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> > > [   49.644479] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-0
> > > [   49.652719] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-1
> > > [   49.660681] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-2
> > > [   49.669059] renesas-gbeth 15c30000.ethernet eth0: Register
> > > MEM_TYPE_PAGE_POOL RxQ-3
> > > [   49.740011] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > [   49.762518] dwmac4: Master AXI performs fixed burst length
> > > [   49.768057] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > > Features support found
> > > [   49.775655] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > > Advanced Timestamp supported
> > > [   49.784609] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > > [   49.791236] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > > phy/rgmii-id link mode
> > > root@rzv2h-evk-alpha:~#
> > > root@rzv2h-evk-alpha:~# [   52.871635] renesas-gbeth 15c30000.ethernet
> > > eth0: tx_clk_stop=1
> > > [   52.877777] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > > 1Gbps/Full - flow control rx/tx
> > 
> down/upping the interface but it seems you get different behaviour.
> > 
> > I'd like to understand why that is, because at the moment I'm wondering whether my patches that
> > address the suspend/resume need further work before I send them - but in order to assess that, I need
> > to work out why your issue only seems to occur in the module removal/insertion and not down/up as well
> > as I'd expect.
> 
> FYI, With linux next, On RZ/G3E SoC which has similar IP as RZ/V2H,i
> ethernet works during suspend entry/exit
> Even though STR is not fully functional yet.

For the failure to happen, you need to check whether EEE is being used:

# ethtool --show-eee ethX

and check whether it states that EEE is enabled and active, and Tx LPI
also shows the timer value.

You need a PHY that does stop it's receive clock when the link enters
low-power mode. PHYs are not required to have this ability implemented,
and there's no way for software to know whether it is or not.

Then, you need to be certain that your link partner does actually
support EEE and signals LPI from its side, rather than just advertising
EEE. Lastly, you need to ensure that there is no traffic over the cable
when you're resuming for the period of the reset timeout for the
failure to occur. If the link wakes up, the clock will be started and
reset will complete.

One can rule out some of the above by checking the LPI status bits,
either in the DWMAC or PHY which indicates whether transmit and/or
receive seeing LPI signalled.

If the link doesn't enter low power, then the receive clock won't be
stopped, and reset will complete. If the link wakes up during reset,
then the clock will be restarted, and reset will complete before the
timeout expires.

So, the possibility for a successful test is quite high.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

RE: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Biju Das]

> -----Original Message-----
> From: Russell King <linux@armlinux.org.uk>
> Sent: 04 March 2025 10:00
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Lad, Prabhakar <prabhakar.csengg@gmail.com>; Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>;
> Conor Dooley <conor+dt@kernel.org>; Philipp Zabel <p.zabel@pengutronix.de>; Geert Uytterhoeven
> <geert+renesas@glider.be>; Giuseppe Cavallaro <peppe.cavallaro@st.com>; Jose Abreu
> <joabreu@synopsys.com>; Alexandre Torgue <alexandre.torgue@foss.st.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-soc@vger.kernel.org; Fabrizio
> Castro <fabrizio.castro.jz@renesas.com>; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> Subject: Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
> 
> On Tue, Mar 04, 2025 at 06:58:44AM +0000, Biju Das wrote:
> > Hi Russel king,
> >
> > > -----Original Message-----
> > > From: Russell King <linux@armlinux.org.uk>
> > > Sent: 03 March 2025 11:19
> > > Subject: Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for
> > > Renesas GBETH
> > >
> > > On Sun, Mar 02, 2025 at 10:02:15PM +0000, Lad, Prabhakar wrote:
> > > > Hi Russell,
> > > >
> > > > On Sun, Mar 2, 2025 at 9:44 PM Russell King (Oracle)
> > > > <linux@armlinux.org.uk> wrote:
> > > > >
> > > > > On Sun, Mar 02, 2025 at 09:20:49PM +0000, Lad, Prabhakar wrote:
> > > > > > Hi Russell,
> > > > > > > What is the reason for setting this flag? If it's because of
> > > > > > > suspend/ resume failures, does my "net: stmmac: fix resume
> > > > > > > failures due to RX clock" series solve this for you without requiring this flag?
> > > > > > >
> > > > > > Ive set this flag based on the configuration supported by this IP.
> > > > > > Unfortunately the platform which I am working on doesn't
> > > > > > support s2r yet so I cannot test suspend/resume path yet. But
> > > > > > I do see an issue when I unload and load just the glue module the DMA reset fails.
> > > > >
> > > > > Thanks for that feedback - that's a scenario I hadn't considered.
> > > > >
> > > > > I was trying to avoid having to disable LPI RX clock-stop on
> > > > > suspend by ensuring that it was enabled at resume time. I think
> > > > > that's valid, but you've brought up another similar scenario:
> > > > >
> > > > > - device is brought up, configures RX clock stop
> > > > > - links with media, negotiates EEE
> > > > > - driver is unloaded, link doesn't go down, but due to no traffic goes
> > > > >   into idle, so RX clock is stopped
> > > > > - driver reloaded, RX clock still stopped, reset fails
> > > > >
> > > > > I would like to solve that so we can get the power savings from
> > > > > stopping the clock, but still have reset work when necessary.
> > > > >
> > > > I would be happy to test the patches ;)
> > > >
> > > > > I'm guessing that the "DMA reset fails" refers to this path:
> > > > >
> > > > > stmmac_open() -> __stmmac_open() -> stmmac_hw_setup() ->
> > > > > stmmac_init_dma_engine() -> stmmac_reset() ?
> > > > >
> > > > Yes.
> > > >
> > > > > In other words, when the device is being brought back up
> > > > > adminsitratively?
> > > > >
> > > > > What happens if you (replace $if):
> > > > >
> > > > > # ip li set dev $if down
> > > > > # ip li set dev $if up
> > > > >
> > > > > Does that also fail without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI set?
> > > > >
> > > > Logs without STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > > > --------------------------------------------------------------
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > > > [   33.606549] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > > > [   37.356992] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > > > [   43.974803] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-0
> > > > [   43.983189] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-1
> > > > [   43.991155] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-2
> > > > [   43.999128] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-3
> > > > [   44.072079] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > > [   44.094605] dwmac4: Master AXI performs fixed burst length
> > > > [   44.100138] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > > > Features support found
> > > > [   44.107748] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > > > Advanced Timestamp supported
> > > > [   44.116725] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > > > [   44.123352] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > > > phy/rgmii-id link mode
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth1[   47.207761] renesas-gbeth
> > > > 15c40000.ethernet eth1: Link is Up - 1Gbps/Full - flow control off
> > > > ^C root@rzv2h-evk-alpha:~# ^C root@rzv2h-evk-alpha:~# ip li set
> > > > dev eth0 up
> > > > [   55.636722] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-0
> > > > [   55.645139] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-1
> > > > [   55.653111] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-2
> > > > [   55.661073] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-3
> > > > [   55.732087] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > > [   55.754612] dwmac4: Master AXI performs fixed burst length
> > > > [   55.760143] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > > > Features support found
> > > > [   55.767740] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > > > Advanced Timestamp supported
> > > > [   55.776705] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > > > [   55.783333] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > > > phy/rgmii-id link mode
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~# [   58.855844] renesas-gbeth 15c30000.ethernet
> > > > eth0: tx_clk_stop=1
> > > > [   58.861989] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > > > 1Gbps/Full - flow control rx/tx
> > > >
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~#
> > > >
> > > > Logs with STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag set:
> > > > --------------------------------------------------------------
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth1 down
> > > > [   30.053790] renesas-gbeth 15c40000.ethernet eth1: Link is Down
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth0 down
> > > > [   35.366935] renesas-gbeth 15c30000.ethernet eth0: Link is Down
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth1 up
> > > > [   40.448563] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-0
> > > > [   40.456725] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-1
> > > > [   40.464893] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-2
> > > > [   40.472840] renesas-gbeth 15c40000.ethernet eth1: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-3
> > > > [   40.543895] renesas-gbeth 15c40000.ethernet eth1: PHY [stmmac-1:00]
> > > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > > [   40.566419] dwmac4: Master AXI performs fixed burst length
> > > > [   40.571949] renesas-gbeth 15c40000.ethernet eth1: No Safety
> > > > Features support found
> > > > [   40.579550] renesas-gbeth 15c40000.ethernet eth1: IEEE 1588-2008
> > > > Advanced Timestamp supported
> > > > [   40.588505] renesas-gbeth 15c40000.ethernet eth1: registered PTP clock
> > > > [   40.595135] renesas-gbeth 15c40000.ethernet eth1: configuring for
> > > > phy/rgmii-id link mode
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~# [   43.687551] renesas-gbeth 15c40000.ethernet
> > > > eth1: Link is Up - 1Gbps/Full - flow control off
> > > >
> > > > root@rzv2h-evk-alpha:~# ip li set dev eth0 up
> > > > [   49.644479] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-0
> > > > [   49.652719] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-1
> > > > [   49.660681] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-2
> > > > [   49.669059] renesas-gbeth 15c30000.ethernet eth0: Register
> > > > MEM_TYPE_PAGE_POOL RxQ-3
> > > > [   49.740011] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > > > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > > > [   49.762518] dwmac4: Master AXI performs fixed burst length
> > > > [   49.768057] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > > > Features support found
> > > > [   49.775655] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > > > Advanced Timestamp supported
> > > > [   49.784609] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > > > [   49.791236] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > > > phy/rgmii-id link mode
> > > > root@rzv2h-evk-alpha:~#
> > > > root@rzv2h-evk-alpha:~# [   52.871635] renesas-gbeth 15c30000.ethernet
> > > > eth0: tx_clk_stop=1
> > > > [   52.877777] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > > > 1Gbps/Full - flow control rx/tx
> > >
> > down/upping the interface but it seems you get different behaviour.
> > >
> > > I'd like to understand why that is, because at the moment I'm
> > > wondering whether my patches that address the suspend/resume need
> > > further work before I send them - but in order to assess that, I
> > > need to work out why your issue only seems to occur in the module removal/insertion and not
> down/up as well as I'd expect.
> >
> > FYI, With linux next, On RZ/G3E SoC which has similar IP as RZ/V2H,i
> > ethernet works during suspend entry/exit Even though STR is not fully
> > functional yet.
> 
> For the failure to happen, you need to check whether EEE is being used:
> 
> # ethtool --show-eee ethX
> 
> and check whether it states that EEE is enabled and active, and Tx LPI also shows the timer value.
> 
> You need a PHY that does stop it's receive clock when the link enters low-power mode. PHYs are not
> required to have this ability implemented, and there's no way for software to know whether it is or
> not.
> 
> Then, you need to be certain that your link partner does actually support EEE and signals LPI from its
> side, rather than just advertising EEE. Lastly, you need to ensure that there is no traffic over the
> cable when you're resuming for the period of the reset timeout for the failure to occur. If the link
> wakes up, the clock will be started and reset will complete.
> 
> One can rule out some of the above by checking the LPI status bits, either in the DWMAC or PHY which
> indicates whether transmit and/or receive seeing LPI signalled.
> 
> If the link doesn't enter low power, then the receive clock won't be stopped, and reset will complete.
> If the link wakes up during reset, then the clock will be restarted, and reset will complete before
> the timeout expires.
> 
> So, the possibility for a successful test is quite high.


This what I get on next. It is showing enabled , but inactive.

root@smarc-rzg3e:~# ethtool --show-eee eth0
EEE settings for eth0:
        EEE status: enabled - inactive
        Tx LPI: 1000000 (us)
        Supported EEE link modes:  100baseT/Full
                                   1000baseT/Full
        Advertised EEE link modes:  100baseT/Full
                                    1000baseT/Full
        Link partner advertised EEE link modes:  Not reported
root@smarc-rzg3e:~# [ 4255.568190] PM: suspend entry (s2idle)
[ 4255.572235] Filesystems sync: 0.000 seconds
[ 4255.577786] Freezing user space processes
[ 4255.585187] Freezing user space processes completed (elapsed 0.003 seconds)
[ 4255.592143] OOM killer disabled.
[ 4255.595375] Freezing remaining freezable tasks
[ 4255.600907] Freezing remaining freezable tasks completed (elapsed 0.001 seconds)
[ 4255.608297] printk: Suspending console(s) (use no_console_suspend to debug)
ÿ[ 4255.657045] renesas-gbeth 15c30000.ethernet eth0: Link is Down

...
...
[ 4255.659896] PM: Some devices failed to suspend, or early wake event detected
[ 4255.661252] renesas-gbeth 15c30000.ethernet eth0: configuring for phy/rgmii-id link mode
[ 4255.665681] dwmac4: Master AXI performs fixed burst length
[ 4255.665763] renesas-gbeth 15c30000.ethernet eth0: No Safety Features support found
[ 4255.665823] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[ 4255.776289] OOM killer enabled.
[ 4255.779431] Restarting tasks ... done.
[ 4255.783936] random: crng reseeded on system resumption
[ 4255.789595] PM: suspend exit
[ 4255.793070] PM: suspend entry (s2idle)
[ 4255.797174] Filesystems sync: 0.000 seconds
[ 4255.801734] Freezing user space processes
[ 4255.806464] Freezing user space processes completed (elapsed 0.004 seconds)
[ 4255.813431] OOM killer disabled.
[ 4255.816644] Freezing remaining freezable tasks
[ 4256.055857] Freezing remaining freezable tasks completed (elapsed 0.234 seconds)
[ 4256.063427] printk: Suspending console(s) (use no_console_suspend to debug)
[ 4256.101633] gpio-keys keys: failed to configure IRQ 51 as wakeup source: -38
[ 4256.101683] gpio-keys keys: PM: dpm_run_callback(): platform_pm_suspend returns -38
[ 4256.101755] gpio-keys keys: PM: failed to suspend: error -38
[ 4256.101792] PM: Some devices failed to suspend, or early wake event detected
[ 4256.103095] renesas-gbeth 15c30000.ethernet eth0: configuring for phy/rgmii-id link mode
[ 4256.107110] dwmac4: Master AXI performs fixed burst length
[ 4256.107192] renesas-gbeth 15c30000.ethernet eth0: No Safety Features support found
[ 4256.107254] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[ 4256.215317] OOM killer enabled.
[ 4256.218475] Restarting tasks ... done.
[ 4256.222866] random: crng reseeded on system resumption
[ 4256.228101] PM: suspend exit
[ 4259.113502] renesas-gbeth 15c30000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx

root@smarc-rzg3e:~# [ 4260.514428] 8021q: 802.1Q VLAN Support v1.8
[ 4260.518689] 8021q: adding VLAN 0 to HW filter on device eth0

root@smarc-rzg3e:~#

Cheers,
Biju

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Tue, Mar 04, 2025 at 10:56:39AM +0000, Biju Das wrote:
> > For the failure to happen, you need to check whether EEE is being used:
> > 
> > # ethtool --show-eee ethX
> > 
> > and check whether it states that EEE is enabled and active, and Tx LPI also shows the timer value.
> > 
> > You need a PHY that does stop it's receive clock when the link enters low-power mode. PHYs are not
> > required to have this ability implemented, and there's no way for software to know whether it is or
> > not.
> > 
> > Then, you need to be certain that your link partner does actually support EEE and signals LPI from its
> > side, rather than just advertising EEE. Lastly, you need to ensure that there is no traffic over the
> > cable when you're resuming for the period of the reset timeout for the failure to occur. If the link
> > wakes up, the clock will be started and reset will complete.
> > 
> > One can rule out some of the above by checking the LPI status bits, either in the DWMAC or PHY which
> > indicates whether transmit and/or receive seeing LPI signalled.
> > 
> > If the link doesn't enter low power, then the receive clock won't be stopped, and reset will complete.
> > If the link wakes up during reset, then the clock will be restarted, and reset will complete before
> > the timeout expires.
> > 
> > So, the possibility for a successful test is quite high.
> 
> 
> This what I get on next. It is showing enabled , but inactive.
> 
> root@smarc-rzg3e:~# ethtool --show-eee eth0
> EEE settings for eth0:
>         EEE status: enabled - inactive
>         Tx LPI: 1000000 (us)
>         Supported EEE link modes:  100baseT/Full
>                                    1000baseT/Full
>         Advertised EEE link modes:  100baseT/Full
>                                     1000baseT/Full
>         Link partner advertised EEE link modes:  Not reported

That means your link partner doesn't support EEE (or has EEE disabled)
so the issue we're discussing in this thread doesn't occur for your
setup.

In order to do a valid test for the issue in this thread, you need a
link partner that supports EEE and has EEE enabled.

Note that also with an EEE capable setup, with the LPI timer set to
one second, you need to have not transmitted any packets for one
second before the transmit path enters LPI. Although this is the
default for stmmac, it seems to me to be an excessively long default,
and may even be masking some problems.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Geert Uytterhoeven]

Hi Russell,

On Tue, 4 Mar 2025 at 11:00, Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
> For the failure to happen, you need to check whether EEE is being used:
>
> # ethtool --show-eee ethX

Doh, that's also not supported on Starlight (BeagleV beta).

I tried unbind/rebind regardless, and it works (sort of),
using the old Beagle V Fedora rootfs on microSD:

[root@fedora-starfive starfive-dwmac]# echo 10020000.ethernet > unbind
starfive-dwmac 10020000.ethernet eth0: stmmac_dvr_remove: removing driver
starfive-dwmac 10020000.ethernet eth0: Link is Down
[root@fedora-starfive starfive-dwmac]# echo 10020000.ethernet > bind
starfive-dwmac 10020000.ethernet: IRQ eth_lpi not found
starfive-dwmac 10020000.ethernet: IRQ sfty not found
starfive-dwmac 10020000.ethernet: Hash table entries set to unexpected value 32
starfive-dwmac 10020000.ethernet: User ID: 0x59, Synopsys ID: 0x37
starfive-dwmac 10020000.ethernet:        DWMAC1000
starfive-dwmac 10020000.ethernet: DMA HW capability register supported
starfive-dwmac 10020000.ethernet: RX Checksum Offload Engine supported
starfive-dwmac 10020000.ethernet: COE Type 2
starfive-dwmac 10020000.ethernet: Wake-Up On Lan supported
starfive-dwmac 10020000.ethernet: Enhanced/Alternate descriptors
starfive-dwmac 10020000.ethernet: Enabled extended descriptors
starfive-dwmac 10020000.ethernet: Chain mode enabled
starfive-dwmac 10020000.ethernet: Enable RX Mitigation via HW Watchdog Timer
starfive-dwmac 10020000.ethernet: device MAC address fa:58:39:0a:37:37
libphy: get_phy_c22_id: mii_bus stmmac phy_id = 0x00221622
starfive-dwmac 10020000.ethernet eth0: Register MEM_TYPE_PAGE_POOL RxQ-0
starfive-dwmac 10020000.ethernet eth0: PHY [stmmac-0:07] driver
[Micrel KSZ9031 Gigabit PHY] (irq=POLL)
dwmac1000: Master AXI performs fixed burst length
starfive-dwmac 10020000.ethernet eth0: No Safety Features support found
starfive-dwmac 10020000.ethernet eth0: No MAC Management Counters available
starfive-dwmac 10020000.ethernet eth0: IEEE 1588-2008 Advanced
Timestamp supported
starfive-dwmac 10020000.ethernet eth0: configuring for phy/rgmii-id link mode
starfive-dwmac 10020000.ethernet eth0: Link is Up - 1Gbps/Full - flow
control off

Apparently the MAC address has changed, so the board got a different
IP address from my DHCP server :-(

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Russell,

On Sun, Mar 2, 2025 at 9:20 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Russell,
>
> On Sun, Mar 2, 2025 at 7:33 PM Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > On Sun, Mar 02, 2025 at 06:18:08PM +0000, Prabhakar wrote:
> > > +     gbeth->dev = dev;
> > > +     gbeth->regs = stmmac_res.addr;
> > > +     plat_dat->bsp_priv = gbeth;
> > > +     plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
> >
> > Thanks for using that!
> >
> Yep, it shortens the glue driver further.
>
> > > +     plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> > > +                        STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |
> >
> > I would like to know what value tx_clk_stop is in
> > stmmac_mac_enable_tx_lpi() for your setup. Ideally, stmmac should
> > use the capability report from the PHY to decide whether the
> > transmit clock can be gated, but sadly we haven't had any support
> > in phylib/phylink for that until recently, and I haven't modified
> > stmmac to allow use of that. However, it would be good to gain
> > knowledge in this area.
> >
> tx_clk_stop =1,
>
> root@rzv2h-evk-alpha:~# ifconfig eth0 up
> [  587.830436] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-0
> [  587.838636] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-1
> [  587.846792] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-2
> [  587.854734] renesas-gbeth 15c30000.ethernet eth0: Register
> MEM_TYPE_PAGE_POOL RxQ-3
> [  587.926860] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> [  587.949380] dwmac4: Master AXI performs fixed burst length
> [  587.954910] renesas-gbeth 15c30000.ethernet eth0: No Safety
> Features support found
> [  587.962556] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> Advanced Timestamp supported
> [  587.971420] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> [  587.978004] renesas-gbeth 15c30000.ethernet eth0: configuring for
> phy/rgmii-id link mode
> root@rzv2h-evk-alpha:~# [  591.070448] renesas-gbeth 15c30000.ethernet
> eth0: tx_clk_stop=1
> [  591.076590] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> 1Gbps/Full - flow control rx/tx
>
> With the below diff:
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index aec230353ac4..68f1954e6eea 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1100,6 +1100,7 @@ static int stmmac_mac_enable_tx_lpi(struct
> phylink_config *config, u32 timer,
>         struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
>         int ret;
>
> +       netdev_err(priv->dev, "tx_clk_stop=%d\n", tx_clk_stop);
>         priv->tx_lpi_timer = timer;
>         priv->eee_active = true;
>
> > > +                        STMMAC_FLAG_RX_CLK_RUNS_IN_LPI |
> >
I got some feedback from the HW team, based on the feedback this flag
depends on the PHY device. I wonder if we should create a DT property
for this. Please share your thoughts.

Cheers,
Prabhakar

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Mon, Mar 03, 2025 at 09:41:13AM +0000, Lad, Prabhakar wrote:
> Hi Russell,
> 
> On Sun, Mar 2, 2025 at 9:20 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> >
> > Hi Russell,
> >
> > On Sun, Mar 2, 2025 at 7:33 PM Russell King (Oracle)
> > <linux@armlinux.org.uk> wrote:
> > >
> > > On Sun, Mar 02, 2025 at 06:18:08PM +0000, Prabhakar wrote:
> > > > +     gbeth->dev = dev;
> > > > +     gbeth->regs = stmmac_res.addr;
> > > > +     plat_dat->bsp_priv = gbeth;
> > > > +     plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
> > >
> > > Thanks for using that!
> > >
> > Yep, it shortens the glue driver further.
> >
> > > > +     plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> > > > +                        STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |
> > >
> > > I would like to know what value tx_clk_stop is in
> > > stmmac_mac_enable_tx_lpi() for your setup. Ideally, stmmac should
> > > use the capability report from the PHY to decide whether the
> > > transmit clock can be gated, but sadly we haven't had any support
> > > in phylib/phylink for that until recently, and I haven't modified
> > > stmmac to allow use of that. However, it would be good to gain
> > > knowledge in this area.
> > >
> > tx_clk_stop =1,
> >
> > root@rzv2h-evk-alpha:~# ifconfig eth0 up
> > [  587.830436] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-0
> > [  587.838636] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-1
> > [  587.846792] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-2
> > [  587.854734] renesas-gbeth 15c30000.ethernet eth0: Register
> > MEM_TYPE_PAGE_POOL RxQ-3
> > [  587.926860] renesas-gbeth 15c30000.ethernet eth0: PHY [stmmac-0:00]
> > driver [Microchip KSZ9131 Gigabit PHY] (irq=POLL)
> > [  587.949380] dwmac4: Master AXI performs fixed burst length
> > [  587.954910] renesas-gbeth 15c30000.ethernet eth0: No Safety
> > Features support found
> > [  587.962556] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008
> > Advanced Timestamp supported
> > [  587.971420] renesas-gbeth 15c30000.ethernet eth0: registered PTP clock
> > [  587.978004] renesas-gbeth 15c30000.ethernet eth0: configuring for
> > phy/rgmii-id link mode
> > root@rzv2h-evk-alpha:~# [  591.070448] renesas-gbeth 15c30000.ethernet
> > eth0: tx_clk_stop=1
> > [  591.076590] renesas-gbeth 15c30000.ethernet eth0: Link is Up -
> > 1Gbps/Full - flow control rx/tx
> >
> > With the below diff:
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index aec230353ac4..68f1954e6eea 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -1100,6 +1100,7 @@ static int stmmac_mac_enable_tx_lpi(struct
> > phylink_config *config, u32 timer,
> >         struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev));
> >         int ret;
> >
> > +       netdev_err(priv->dev, "tx_clk_stop=%d\n", tx_clk_stop);
> >         priv->tx_lpi_timer = timer;
> >         priv->eee_active = true;
> >
> > > > +                        STMMAC_FLAG_RX_CLK_RUNS_IN_LPI |
> > >
> I got some feedback from the HW team, based on the feedback this flag
> depends on the PHY device. I wonder if we should create a DT property
> for this. Please share your thoughts.

Not sure exactly which flag you're referring to, because you first
quote the code that you added to dump the _transmit_ clock stop,
and then you named the _receive_ clock flag.

I assume you're referring to STMMAC_FLAG_EN_TX_LPI_CLOCKGATING, which
is currently used by the driver because it didn't know any better to
check the capabilities of the PHY - and phylib didn't expose an
interface to do that.

tx_clk_stop is basically the flag from the PHY indicating whether the
MAC may be permitted to stop its transmit clock. Unfortunately, we
can't just switch over to using that in stmmac because of it's dumb
history as that may cause regressions. As we haven't used this flag
from the PHY before, we have no idea whether it's reliable or not,
and if it isn't reliable, then using it will cause regressions.

I think that the way forward would be to introduce yet another flag
(maybe STMMAC_FLAG_LPI_TX_CLK_PHY_CAP) and:

	if (priv->plat->flags & STMMAC_FLAG_LPI_TX_CLK_PHY_CAP)
		priv->tx_lpi_clk_stop = tx_clk_stop;
	else
		priv->tx_lpi_clk_stop = priv->plat->flags &
					STMMAC_FLAG_EN_TX_LPI_CLOCKGATING;

and then where STMMAC_FLAG_EN_TX_LPI_CLOCKGATING is checked, that
becomes:

	ret = stmmac_set_lpi_mode(priv, priv->hw, STMMAC_LPI_TIMER,
				  priv->tx_lpi_clk_stop,
				  priv->tx_lpi_timer);

It's rather annoying to have to include a flag to say "use the 802.3
standard behaviour" but given that we want to avoid regressions I don't
see any other choice. It would've been nice to have had the driver
using the PHY capability, but that horse has already bolted. We can now
only try to encourage platform glue authors to try setting
STMMAC_FLAG_LPI_TX_CLK_PHY_CAP with the above in place.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Mon, Mar 03, 2025 at 09:58:16AM +0000, Russell King (Oracle) wrote:
> I think that the way forward would be to introduce yet another flag
> (maybe STMMAC_FLAG_LPI_TX_CLK_PHY_CAP) and:
> 
> 	if (priv->plat->flags & STMMAC_FLAG_LPI_TX_CLK_PHY_CAP)
> 		priv->tx_lpi_clk_stop = tx_clk_stop;
> 	else
> 		priv->tx_lpi_clk_stop = priv->plat->flags &
> 					STMMAC_FLAG_EN_TX_LPI_CLOCKGATING;
> 
> and then where STMMAC_FLAG_EN_TX_LPI_CLOCKGATING is checked, that
> becomes:
> 
> 	ret = stmmac_set_lpi_mode(priv, priv->hw, STMMAC_LPI_TIMER,
> 				  priv->tx_lpi_clk_stop,
> 				  priv->tx_lpi_timer);

I'm thinking something like the following:

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 3a00a988cb36..04197496ee87 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -307,6 +307,7 @@ struct stmmac_priv {
 	struct timer_list eee_ctrl_timer;
 	int lpi_irq;
 	u32 tx_lpi_timer;
+	bool tx_lpi_clk_stop;
 	bool eee_enabled;
 	bool eee_active;
 	bool eee_sw_timer_en;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 7d10e58e009e..7709d431e950 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -461,8 +461,7 @@ static void stmmac_try_to_start_sw_lpi(struct stmmac_priv *priv)
 	/* Check and enter in LPI mode */
 	if (!priv->tx_path_in_lpi_mode)
 		stmmac_set_lpi_mode(priv, priv->hw, STMMAC_LPI_FORCED,
-			priv->plat->flags & STMMAC_FLAG_EN_TX_LPI_CLOCKGATING,
-			0);
+				    priv->tx_lpi_clk_stop, 0);
 }
 
 /**
@@ -1110,13 +1109,18 @@ static int stmmac_mac_enable_tx_lpi(struct phylink_config *config, u32 timer,
 
 	priv->eee_enabled = true;
 
+	/* Update the transmit clock stop according to PHY capability if
+	 * the platform allows
+	 */
+	if (priv->plat->flags & STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP)
+		priv->tx_lpi_clk_stop = tx_clk_stop;
+
 	stmmac_set_eee_timer(priv, priv->hw, STMMAC_DEFAULT_LIT_LS,
 			     STMMAC_DEFAULT_TWT_LS);
 
 	/* Try to cnfigure the hardware timer. */
 	ret = stmmac_set_lpi_mode(priv, priv->hw, STMMAC_LPI_TIMER,
-				  priv->plat->flags & STMMAC_FLAG_EN_TX_LPI_CLOCKGATING,
-				  priv->tx_lpi_timer);
+				  priv->tx_lpi_clk_stop, priv->tx_lpi_timer);
 
 	if (ret) {
 		/* Hardware timer mode not supported, or value out of range.
@@ -1262,6 +1266,10 @@ static int stmmac_phy_setup(struct stmmac_priv *priv)
 	if (!(priv->plat->flags & STMMAC_FLAG_RX_CLK_RUNS_IN_LPI))
 		priv->phylink_config.eee_rx_clk_stop_enable = true;
 
+	/* Set the default transmit clock stop bit based on the platform glue */
+	priv->tx_lpi_clk_stop = priv->plat->flags &
+				STMMAC_FLAG_EN_TX_LPI_CLOCKGATING;
+
 	mdio_bus_data = priv->plat->mdio_bus_data;
 	if (mdio_bus_data)
 		priv->phylink_config.default_an_inband =
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index cd0d1383df87..102de1aeac17 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -183,7 +183,8 @@ struct dwmac4_addrs {
 #define STMMAC_FLAG_INT_SNAPSHOT_EN		BIT(9)
 #define STMMAC_FLAG_RX_CLK_RUNS_IN_LPI		BIT(10)
 #define STMMAC_FLAG_EN_TX_LPI_CLOCKGATING	BIT(11)
-#define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY	BIT(12)
+#define STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP	BIT(12)
+#define STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY	BIT(13)
 
 struct plat_stmmacenet_data {
 	int bus_id;

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Geert Uytterhoeven]

Hi Prabhakar,

On Sun, 2 Mar 2025 at 19:18, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Renesas RZ/V2H(P) SoC is equipped with Synopsys DesignWare Ethernet
> Quality-of-Service IP block version 5.20. This commit adds DWMAC glue
> layer for the Renesas GBETH found on the RZ/V2H(P) SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c

> +static int renesas_gbeth_probe(struct platform_device *pdev)
> +{
> +       struct plat_stmmacenet_data *plat_dat;
> +       struct stmmac_resources stmmac_res;
> +       struct device *dev = &pdev->dev;
> +       struct renesas_gbeth *gbeth;
> +       struct reset_control *rstc;
> +       unsigned int i;
> +       int err;
> +
> +       err = stmmac_get_platform_resources(pdev, &stmmac_res);
> +       if (err)
> +               return dev_err_probe(dev, err,
> +                                    "failed to get resources\n");
> +
> +       plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
> +       if (IS_ERR(plat_dat))
> +               return dev_err_probe(dev, PTR_ERR(plat_dat),
> +                                    "dt configuration failed\n");
> +
> +       gbeth = devm_kzalloc(dev, sizeof(*gbeth), GFP_KERNEL);
> +       if (!gbeth)
> +               return -ENOMEM;
> +
> +       plat_dat->clk_tx_i = devm_clk_get_enabled(dev, "tx");

drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c:52:17:
error: ‘struct plat_stmmacenet_data’ has no member named ‘clk_tx_i’

Also not in next-20250228.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Geert,

On Mon, Mar 3, 2025 at 10:40 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Sun, 2 Mar 2025 at 19:18, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Renesas RZ/V2H(P) SoC is equipped with Synopsys DesignWare Ethernet
> > Quality-of-Service IP block version 5.20. This commit adds DWMAC glue
> > layer for the Renesas GBETH found on the RZ/V2H(P) SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
>
> > +static int renesas_gbeth_probe(struct platform_device *pdev)
> > +{
> > +       struct plat_stmmacenet_data *plat_dat;
> > +       struct stmmac_resources stmmac_res;
> > +       struct device *dev = &pdev->dev;
> > +       struct renesas_gbeth *gbeth;
> > +       struct reset_control *rstc;
> > +       unsigned int i;
> > +       int err;
> > +
> > +       err = stmmac_get_platform_resources(pdev, &stmmac_res);
> > +       if (err)
> > +               return dev_err_probe(dev, err,
> > +                                    "failed to get resources\n");
> > +
> > +       plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > +       if (IS_ERR(plat_dat))
> > +               return dev_err_probe(dev, PTR_ERR(plat_dat),
> > +                                    "dt configuration failed\n");
> > +
> > +       gbeth = devm_kzalloc(dev, sizeof(*gbeth), GFP_KERNEL);
> > +       if (!gbeth)
> > +               return -ENOMEM;
> > +
> > +       plat_dat->clk_tx_i = devm_clk_get_enabled(dev, "tx");
>
> drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c:52:17:
> error: ‘struct plat_stmmacenet_data’ has no member named ‘clk_tx_i’
>
> Also not in next-20250228.
>
This patch is based on net-next. Patch [0] is the one which adds
clk_tx_i member.

[0] https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=dea5c8ec20be

Cheers,
Prabhakar

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Russell King (Oracle)]

On Mon, Mar 03, 2025 at 11:40:15AM +0100, Geert Uytterhoeven wrote:
> > +       err = stmmac_get_platform_resources(pdev, &stmmac_res);
> > +       if (err)
> > +               return dev_err_probe(dev, err,
> > +                                    "failed to get resources\n");
> > +
> > +       plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > +       if (IS_ERR(plat_dat))
> > +               return dev_err_probe(dev, PTR_ERR(plat_dat),
> > +                                    "dt configuration failed\n");
> > +
> > +       gbeth = devm_kzalloc(dev, sizeof(*gbeth), GFP_KERNEL);
> > +       if (!gbeth)
> > +               return -ENOMEM;
> > +
> > +       plat_dat->clk_tx_i = devm_clk_get_enabled(dev, "tx");
> 
> drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c:52:17:
> error: ‘struct plat_stmmacenet_data’ has no member named ‘clk_tx_i’

False error. You need to build netdev patches marked as net-next
against the net-next tree, not the outdated next tree.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Geert Uytterhoeven]

Hi Prabhakar,

On Sun, 2 Mar 2025 at 19:18, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Renesas RZ/V2H(P) SoC is equipped with Synopsys DesignWare Ethernet
> Quality-of-Service IP block version 5.20. This commit adds DWMAC glue
> layer for the Renesas GBETH found on the RZ/V2H(P) SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

A few early comments...

> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -131,6 +131,17 @@ config DWMAC_QCOM_ETHQOS
>           This selects the Qualcomm ETHQOS glue layer support for the
>           stmmac device driver.
>
> +config DWMAC_RENESAS_GBETH
> +       tristate "Renesas RZ/V2H(P) GBETH support"
> +       default ARCH_RENESAS

This auto-enables DWMAC_RENESAS_GBETH when building a kernel for e.g
RZ/N1D, which uses stmmac with DWMAC_RZN1.  So I'll have to disable
this explicitly in shmobile_defconfig.  This is not a big issue,
we already have similar constructs (DRM_RCAR_USE_MIPI_DSI defaults to
DRM_RCAR_DU, but is not used on R-Car Gen1/2).

> +       depends on OF && (ARCH_RENESAS || COMPILE_TEST)
> +       help
> +         Support for Gigabit Ethernet Interface (GBETH) on Renesas
> +         RZ/V2H(P) SoCs.
> +
> +         This selects the Renesas RZ/V2H(P) Soc specific glue layer support
> +         for the stmmac device driver.
> +
>  config DWMAC_ROCKCHIP
>         tristate "Rockchip dwmac support"
>         default ARCH_ROCKCHIP

> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c

> +static const char *const renesas_gbeth_clks[] __initconst = {

WARNING: modpost: vmlinux: section mismatch in reference:
renesas_gbeth_probe+0x1e0 (section: .text) -> renesas_gbeth_clks
(section: .init.rodata)

Please drop the __initconst.

> +       "rx", "rx-180", "tx-180",
> +};

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Thu, Mar 6, 2025 at 1:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Sun, 2 Mar 2025 at 19:18, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Renesas RZ/V2H(P) SoC is equipped with Synopsys DesignWare Ethernet
> > Quality-of-Service IP block version 5.20. This commit adds DWMAC glue
> > layer for the Renesas GBETH found on the RZ/V2H(P) SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Thanks for your patch!
>
> A few early comments...
>
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -131,6 +131,17 @@ config DWMAC_QCOM_ETHQOS
> >           This selects the Qualcomm ETHQOS glue layer support for the
> >           stmmac device driver.
> >
> > +config DWMAC_RENESAS_GBETH
> > +       tristate "Renesas RZ/V2H(P) GBETH support"
> > +       default ARCH_RENESAS
>
> This auto-enables DWMAC_RENESAS_GBETH when building a kernel for e.g
> RZ/N1D, which uses stmmac with DWMAC_RZN1.  So I'll have to disable
> this explicitly in shmobile_defconfig.  This is not a big issue,
> we already have similar constructs (DRM_RCAR_USE_MIPI_DSI defaults to
> DRM_RCAR_DU, but is not used on R-Car Gen1/2).
>
I added this based on the recent comments received while add WDT
support for RZ/G3E.

> > +       depends on OF && (ARCH_RENESAS || COMPILE_TEST)
> > +       help
> > +         Support for Gigabit Ethernet Interface (GBETH) on Renesas
> > +         RZ/V2H(P) SoCs.
> > +
> > +         This selects the Renesas RZ/V2H(P) Soc specific glue layer support
> > +         for the stmmac device driver.
> > +
> >  config DWMAC_ROCKCHIP
> >         tristate "Rockchip dwmac support"
> >         default ARCH_ROCKCHIP
>
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
>
> > +static const char *const renesas_gbeth_clks[] __initconst = {
>
> WARNING: modpost: vmlinux: section mismatch in reference:
> renesas_gbeth_probe+0x1e0 (section: .text) -> renesas_gbeth_clks
> (section: .init.rodata)
>
> Please drop the __initconst.
>
Ok, I will drop that.

Cheers,
Prabhakar