| Message ID | 20250302181808.728734-4-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add GBETH glue layer driver for Renesas RZ/V2H(P) SoC | expand |
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.
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
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
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?
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
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
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.
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
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
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.
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;
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.
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
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.
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");