netdev - Re: [PATCH RFC net-next v2 0/3] net: stmmac: approach 2 to solve EEE LPI reset issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6bfc94a7-0215-4433-9f51-1bfb1cdd1e43@nvidia.com>
Date: Tue, 11 Mar 2025 17:32:10 +0000
From: Jon Hunter <jonathanh@...dia.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Thierry Reding <treding@...dia.com>,
 "Lad, Prabhakar" <prabhakar.csengg@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>, Andrew Lunn
 <andrew@...n.ch>, Andrew Lunn <andrew+netdev@...n.ch>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Heiner Kallweit <hkallweit1@...il.com>, Jakub Kicinski <kuba@...nel.org>,
 linux-arm-kernel@...ts.infradead.org,
 linux-stm32@...md-mailman.stormreply.com,
 Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
 Paolo Abeni <pabeni@...hat.com>,
 "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH RFC net-next v2 0/3] net: stmmac: approach 2 to solve EEE
 LPI reset issues


On 11/03/2025 13:58, Russell King (Oracle) wrote:

...

> I'm wondering whether there's something else which needs the RX clock
> running in order to take effect.
> 
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index e2146d3aee74..48a646b76a29 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -3109,10 +3109,7 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
>>          if (priv->extend_desc && (priv->mode == STMMAC_RING_MODE))
>>                  priv->plat->dma_cfg->atds = 1;
>> -       /* Note that the PHY clock must be running for reset to complete. */
>> -       phylink_rx_clk_stop_block(priv->phylink);
>>          ret = stmmac_reset(priv, priv->ioaddr);
>> -       phylink_rx_clk_stop_unblock(priv->phylink);
>>          if (ret) {
>>                  netdev_err(priv->dev, "Failed to reset the dma\n");
>>                  return ret;
>> @@ -7951,6 +7948,8 @@ int stmmac_resume(struct device *dev)
>>          rtnl_lock();
>>          mutex_lock(&priv->lock);
>> +       /* Note that the PHY clock must be running for reset to complete. */
>> +       phylink_rx_clk_stop_block(priv->phylink);
>>          stmmac_reset_queues_param(priv);
>>          stmmac_free_tx_skbufs(priv);
>> @@ -7961,6 +7960,7 @@ int stmmac_resume(struct device *dev)
>>          stmmac_set_rx_mode(ndev);
>>          stmmac_restore_hw_vlan_rx_fltr(priv, ndev, priv->hw);
>> +       phylink_rx_clk_stop_unblock(priv->phylink);
>>          stmmac_enable_all_queues(priv);
>>          stmmac_enable_all_dma_irq(priv);
> 
> If you haven't already, can you try shrinking down the number of
> functions that are within the block..unblock region please?

It seems that at a minimum I need to block/unblock around the following
functions ...

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e2146d3aee74..46c343088b1f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3109,10 +3109,7 @@ static int stmmac_init_dma_engine(struct stmmac_priv *priv)
         if (priv->extend_desc && (priv->mode == STMMAC_RING_MODE))
                 priv->plat->dma_cfg->atds = 1;
  
-       /* Note that the PHY clock must be running for reset to complete. */
-       phylink_rx_clk_stop_block(priv->phylink);
         ret = stmmac_reset(priv, priv->ioaddr);
-       phylink_rx_clk_stop_unblock(priv->phylink);
         if (ret) {
                 netdev_err(priv->dev, "Failed to reset the dma\n");
                 return ret;
@@ -7953,10 +7950,13 @@ int stmmac_resume(struct device *dev)
  
         stmmac_reset_queues_param(priv);
  
+       /* Note that the PHY clock must be running for reset to complete. */
+       phylink_rx_clk_stop_block(priv->phylink);
         stmmac_free_tx_skbufs(priv);
         stmmac_clear_descriptors(priv, &priv->dma_conf);
  
         stmmac_hw_setup(ndev, false);
+       phylink_rx_clk_stop_unblock(priv->phylink);
         stmmac_init_coalesce(priv);
         stmmac_set_rx_mode(ndev);

> Looking at the functions called:
> 
> stmmac_reset_queues_param()
> stmmac_free_tx_skbufs()
> stmmac_clear_descriptors()
> 	These look like it's only manipulating software state

So it appears that the last two need to be in the block/unblock region
and ...
  
> stmmac_hw_setup()
> 	We know this calls stmmac_reset() and thus needs the blocking

... this one, which is no surprise, but the others are OK. Please
note so far I have only tested on the Tegra186 board which seems
to be the most sensitive.

Cheers
Jon

-- 
nvpublic