netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: 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,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Fabrizio Castro <fabrizio.castro.jz@renesas.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
Date: Mon, 3 Mar 2025 11:19:01 +0000	[thread overview]
Message-ID: <Z8WQJQo5kW9QV-wV@shell.armlinux.org.uk> (raw)
In-Reply-To: <CA+V-a8vykhxqP30iTwN6yrqDgT8YRVE_MadjiTFp653rHVqMNg@mail.gmail.com>

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!

  reply	other threads:[~2025-03-03 11:19 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-02 18:18 [PATCH 0/3] Add GBETH glue layer driver for Renesas RZ/V2H(P) SoC Prabhakar
2025-03-02 18:18 ` [PATCH 1/3] dt-bindings: net: dwmac: Increase 'maxItems' for 'interrupts' and 'interrupt-names' Prabhakar
2025-03-03 15:26   ` Rob Herring
2025-03-03 15:58     ` Lad, Prabhakar
2025-03-03 16:34       ` Andrew Lunn
2025-03-03 20:40         ` Lad, Prabhakar
2025-03-02 18:18 ` [PATCH 2/3] dt-bindings: net: Document GBETH bindings for Renesas RZ/V2H(P) SoC Prabhakar
2025-03-02 19:10   ` Andrew Lunn
2025-03-02 19:28     ` Russell King (Oracle)
2025-03-02 21:33       ` Andrew Lunn
2025-03-03 15:15         ` Rob Herring
2025-03-02 19:25   ` Andrew Lunn
2025-03-02 20:41     ` Lad, Prabhakar
2025-03-02 21:01       ` Russell King (Oracle)
2025-03-02 21:22         ` Lad, Prabhakar
2025-03-02 21:39           ` Andrew Lunn
2025-03-02 21:43             ` Lad, Prabhakar
2025-03-02 21:49               ` Russell King (Oracle)
2025-03-02 21:51                 ` Russell King (Oracle)
2025-03-02 22:03                   ` Lad, Prabhakar
2025-03-02 18:18 ` [PATCH 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH Prabhakar
2025-03-02 19:33   ` Russell King (Oracle)
2025-03-02 20:05     ` Russell King (Oracle)
2025-03-02 21:20     ` Lad, Prabhakar
2025-03-02 21:44       ` Russell King (Oracle)
2025-03-02 22:02         ` Lad, Prabhakar
2025-03-03 11:19           ` Russell King (Oracle) [this message]
2025-03-03 16:04             ` Lad, Prabhakar
2025-03-03 16:32               ` Russell King (Oracle)
2025-03-05 21:26                 ` Lad, Prabhakar
2025-03-06  0:31                   ` Russell King (Oracle)
2025-03-08 13:20                     ` Lad, Prabhakar
2025-03-04  6:58             ` Biju Das
2025-03-04 10:00               ` Russell King (Oracle)
2025-03-04 10:56                 ` Biju Das
2025-03-04 11:16                   ` Russell King (Oracle)
2025-03-04 14:04                 ` Geert Uytterhoeven
2025-03-03  9:41       ` Lad, Prabhakar
2025-03-03  9:58         ` Russell King (Oracle)
2025-03-03 10:58           ` Russell King (Oracle)
2025-03-03 10:40   ` Geert Uytterhoeven
2025-03-03 10:44     ` Lad, Prabhakar
2025-03-03 10:54     ` Russell King (Oracle)
2025-03-06 13:11   ` Geert Uytterhoeven
2025-03-08 12:44     ` Lad, Prabhakar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z8WQJQo5kW9QV-wV@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=conor+dt@kernel.org \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=fabrizio.castro.jz@renesas.com \
    --cc=geert+renesas@glider.be \
    --cc=joabreu@synopsys.com \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=pabeni@redhat.com \
    --cc=peppe.cavallaro@st.com \
    --cc=prabhakar.csengg@gmail.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).