[PATCH net-next 0/2] net: stmmac: approach 2 to solve EEE LPI reset issues

[PATCH net-next 0/2] net: stmmac: approach 2 to solve EEE LPI reset issues

[Russell King (Oracle)]

Hi,

This is a second approach to solving the STMMAC reset issues caused by
the lack of receive clock from the PHY where the media is in low power
mode with a PHY that supports receive clock-stop.

The first approach centred around only addressing the issue in the
resume path, but it seems to also happen when the platform glue module
is removed and re-inserted (Jon - can you check whether that's also
the case for you please?)

As this is more targetted, I've dropped the patches from this series
which move the call to phylink_resume(), so the link may still come
up too early on resume - but that's something I also intend to fix.

This is experimental - so I value test reports for this change.

As mentioned recently, the reset timeout will only occur if the PHY
receive clock is actually stopped at the moment that stmmac_reset()
is called and remains stopped for the duration of the timeout.
Network activity can wake up the link, causing the PHY to restart
its receive clock and allow reset to complete. So, careful testing
with and without these patches is necessary.

Change from RFC: drop unnecessary first patch.

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |  3 ++
 drivers/net/phy/phylink.c                         | 50 +++++++++++++++++++++++
 include/linux/phylink.h                           |  3 ++
 3 files changed, 56 insertions(+)

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

[PATCH net-next 1/2] net: phylink: add functions to block/unblock rx clock stop

[Russell King (Oracle)]

Some MACs require the PHY receive clock to be running to complete setup
actions. This may fail if the PHY has negotiated EEE, the MAC supports
receive clock stop, and the link has entered LPI state. Provide a pair
of APIs that MAC drivers can use to temporarily block the PHY disabling
the receive clock.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/phy/phylink.c | 50 +++++++++++++++++++++++++++++++++++++++
 include/linux/phylink.h   |  3 +++
 2 files changed, 53 insertions(+)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index a3b186ab3854..8f93b597d019 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -88,6 +88,7 @@ struct phylink {
 	bool mac_enable_tx_lpi;
 	bool mac_tx_clk_stop;
 	u32 mac_tx_lpi_timer;
+	u8 mac_rx_clk_stop_blocked;
 
 	struct sfp_bus *sfp_bus;
 	bool sfp_may_have_phy;
@@ -2592,6 +2593,55 @@ void phylink_stop(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_stop);
 
+
+void phylink_rx_clk_stop_block(struct phylink *pl)
+{
+	ASSERT_RTNL();
+
+	if (pl->mac_rx_clk_stop_blocked == U8_MAX) {
+		phylink_warn(pl, "%s called too many times - ignoring\n",
+			     __func__);
+		dump_stack();
+		return;
+	}
+
+	/* Disable PHY receive clock stop if this is the first time this
+	 * 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->config->eee_rx_clk_stop_enable)
+		phy_eee_rx_clock_stop(pl->phydev, false);
+}
+
+/**
+ * phylink_rx_clk_stop_unblock() - unblock PHY ability to stop receive clock
+ * @pl: a pointer to a &struct phylink returned from phylink_create()
+ *
+ * All calls to phylink_rx_clk_stop_block() must be balanced with a
+ * corresponding call to phylink_rx_clk_stop_unblock() to restore the PHYs
+ * clock stop ability.
+ */
+void phylink_rx_clk_stop_unblock(struct phylink *pl)
+{
+	ASSERT_RTNL();
+
+	if (pl->mac_rx_clk_stop_blocked == 0) {
+		phylink_warn(pl, "%s called too many times - ignoring\n",
+			     __func__);
+		dump_stack();
+		return;
+	}
+
+	/* Re-enable PHY receive clock stop if the number of unblocks matches
+	 * the number of calls to the block function above.
+	 */
+	if (--pl->mac_rx_clk_stop_blocked == 0 &&
+	    pl->mac_supports_eee_ops && pl->phydev &&
+	    pl->config->eee_rx_clk_stop_enable)
+		phy_eee_rx_clock_stop(pl->phydev, true);
+}
+
 /**
  * phylink_suspend() - handle a network device suspend event
  * @pl: a pointer to a &struct phylink returned from phylink_create()
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index 08df65f6867a..249c437d6b7b 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -698,6 +698,9 @@ int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs);
 void phylink_start(struct phylink *);
 void phylink_stop(struct phylink *);
 
+void phylink_rx_clk_stop_block(struct phylink *);
+void phylink_rx_clk_stop_unblock(struct phylink *);
+
 void phylink_suspend(struct phylink *pl, bool mac_wol);
 void phylink_resume(struct phylink *pl);
 
-- 
2.30.2

[PATCH net-next 2/2] net: stmmac: block PHY rx clock-stop over reset

[Russell King (Oracle)]

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6924df893e42..037039a9a33b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3096,7 +3096,10 @@ 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;
-- 
2.30.2

Re: [PATCH net-next 1/2] net: phylink: add functions to block/unblock rx clock stop

[Maxime Chevallier]

Hello Russell,

On Wed, 05 Mar 2025 18:00:39 +0000
"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote:

> Some MACs require the PHY receive clock to be running to complete setup
> actions. This may fail if the PHY has negotiated EEE, the MAC supports
> receive clock stop, and the link has entered LPI state. Provide a pair
> of APIs that MAC drivers can use to temporarily block the PHY disabling
> the receive clock.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

I only have comments on the implementation, see below :)

> ---
>  drivers/net/phy/phylink.c | 50 +++++++++++++++++++++++++++++++++++++++
>  include/linux/phylink.h   |  3 +++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index a3b186ab3854..8f93b597d019 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -88,6 +88,7 @@ struct phylink {
>  	bool mac_enable_tx_lpi;
>  	bool mac_tx_clk_stop;
>  	u32 mac_tx_lpi_timer;
> +	u8 mac_rx_clk_stop_blocked;
>  
>  	struct sfp_bus *sfp_bus;
>  	bool sfp_may_have_phy;
> @@ -2592,6 +2593,55 @@ void phylink_stop(struct phylink *pl)
>  }
>  EXPORT_SYMBOL_GPL(phylink_stop);
>  
> +
> +void phylink_rx_clk_stop_block(struct phylink *pl)
> +{
> +	ASSERT_RTNL();
> +
> +	if (pl->mac_rx_clk_stop_blocked == U8_MAX) {
> +		phylink_warn(pl, "%s called too many times - ignoring\n",
> +			     __func__);
> +		dump_stack();
> +		return;
> +	}
> +
> +	/* Disable PHY receive clock stop if this is the first time this
> +	 * 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->config->eee_rx_clk_stop_enable)

Looks like there's an extra closing ')' here

> +		phy_eee_rx_clock_stop(pl->phydev, false);
> +}

Do you need an EXPORT_SYMBOL_GPL here as this will be used by MAC
drivers?

> +
> +/**
> + * phylink_rx_clk_stop_unblock() - unblock PHY ability to stop receive clock
> + * @pl: a pointer to a &struct phylink returned from phylink_create()
> + *
> + * All calls to phylink_rx_clk_stop_block() must be balanced with a
> + * corresponding call to phylink_rx_clk_stop_unblock() to restore the PHYs
> + * clock stop ability.
> + */
> +void phylink_rx_clk_stop_unblock(struct phylink *pl)
> +{
> +	ASSERT_RTNL();
> +
> +	if (pl->mac_rx_clk_stop_blocked == 0) {
> +		phylink_warn(pl, "%s called too many times - ignoring\n",
> +			     __func__);
> +		dump_stack();
> +		return;
> +	}
> +
> +	/* Re-enable PHY receive clock stop if the number of unblocks matches
> +	 * the number of calls to the block function above.
> +	 */
> +	if (--pl->mac_rx_clk_stop_blocked == 0 &&
> +	    pl->mac_supports_eee_ops && pl->phydev &&
> +	    pl->config->eee_rx_clk_stop_enable)
> +		phy_eee_rx_clock_stop(pl->phydev, true);
> +}

Same for the EXPORT_SYMBOL_GPL

Thanks,

Maxime

Re: [PATCH net-next 1/2] net: phylink: add functions to block/unblock rx clock stop

[kernel test robot]

Hi Russell,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-phylink-add-functions-to-block-unblock-rx-clock-stop/20250306-060346
base:   net-next/main
patch link:    https://lore.kernel.org/r/E1tpt2t-005UNB-MC%40rmk-PC.armlinux.org.uk
patch subject: [PATCH net-next 1/2] net: phylink: add functions to block/unblock rx clock stop
config: arm-exynos_defconfig (https://download.01.org/0day-ci/archive/20250307/202503070329.D6SVd1ez-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project 14170b16028c087ca154878f5ed93d3089a965c6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250307/202503070329.D6SVd1ez-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503070329.D6SVd1ez-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

>> drivers/net/phy/phylink.c:2613:40: error: expected ';' after expression
    2613 |             pl->config->eee_rx_clk_stop_enable)
         |                                               ^
         |                                               ;
>> drivers/net/phy/phylink.c:2613:40: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
   drivers/net/phy/phylink.c:2611:2: note: previous statement is here
    2611 |         if (pl->mac_rx_clk_stop_blocked++ == 0 &&
         |         ^
>> drivers/net/phy/phylink.c:2613:40: error: expected expression
    2613 |             pl->config->eee_rx_clk_stop_enable)
         |                                               ^
>> drivers/net/phy/phylink.c:2613:18: warning: expression result unused [-Wunused-value]
    2613 |             pl->config->eee_rx_clk_stop_enable)
         |             ~~~~~~~~~~  ^~~~~~~~~~~~~~~~~~~~~~
   2 warnings and 2 errors generated.


vim +2613 drivers/net/phy/phylink.c

  2595	
  2596	
  2597	void phylink_rx_clk_stop_block(struct phylink *pl)
  2598	{
  2599		ASSERT_RTNL();
  2600	
  2601		if (pl->mac_rx_clk_stop_blocked == U8_MAX) {
  2602			phylink_warn(pl, "%s called too many times - ignoring\n",
  2603				     __func__);
  2604			dump_stack();
  2605			return;
  2606		}
  2607	
  2608		/* Disable PHY receive clock stop if this is the first time this
  2609		 * function has been called and clock-stop was previously enabled.
  2610		 */
  2611		if (pl->mac_rx_clk_stop_blocked++ == 0 &&
  2612		    pl->mac_supports_eee_ops && pl->phydev)
> 2613		    pl->config->eee_rx_clk_stop_enable)
  2614			phy_eee_rx_clock_stop(pl->phydev, false);
  2615	}
  2616	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 1/2] net: phylink: add functions to block/unblock rx clock stop

[kernel test robot]

Hi Russell,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Russell-King-Oracle/net-phylink-add-functions-to-block-unblock-rx-clock-stop/20250306-060346
base:   net-next/main
patch link:    https://lore.kernel.org/r/E1tpt2t-005UNB-MC%40rmk-PC.armlinux.org.uk
patch subject: [PATCH net-next 1/2] net: phylink: add functions to block/unblock rx clock stop
config: riscv-randconfig-001-20250307 (https://download.01.org/0day-ci/archive/20250307/202503070916.gDJMqkeF-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250307/202503070916.gDJMqkeF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503070916.gDJMqkeF-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/net/phy/phylink.c: In function 'phylink_rx_clk_stop_block':
>> drivers/net/phy/phylink.c:2613:23: warning: statement with no effect [-Wunused-value]
    2613 |             pl->config->eee_rx_clk_stop_enable)
         |             ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/phy/phylink.c:2613:47: error: expected ';' before ')' token
    2613 |             pl->config->eee_rx_clk_stop_enable)
         |                                               ^
         |                                               ;
>> drivers/net/phy/phylink.c:2611:9: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
    2611 |         if (pl->mac_rx_clk_stop_blocked++ == 0 &&
         |         ^~
   drivers/net/phy/phylink.c:2613:47: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
    2613 |             pl->config->eee_rx_clk_stop_enable)
         |                                               ^
>> drivers/net/phy/phylink.c:2613:47: error: expected statement before ')' token


vim +2613 drivers/net/phy/phylink.c

  2595	
  2596	
  2597	void phylink_rx_clk_stop_block(struct phylink *pl)
  2598	{
  2599		ASSERT_RTNL();
  2600	
  2601		if (pl->mac_rx_clk_stop_blocked == U8_MAX) {
  2602			phylink_warn(pl, "%s called too many times - ignoring\n",
  2603				     __func__);
  2604			dump_stack();
  2605			return;
  2606		}
  2607	
  2608		/* Disable PHY receive clock stop if this is the first time this
  2609		 * function has been called and clock-stop was previously enabled.
  2610		 */
> 2611		if (pl->mac_rx_clk_stop_blocked++ == 0 &&
  2612		    pl->mac_supports_eee_ops && pl->phydev)
> 2613		    pl->config->eee_rx_clk_stop_enable)
  2614			phy_eee_rx_clock_stop(pl->phydev, false);
  2615	}
  2616	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki