[PATCH net v2] net: spacemit: Clarify stat timeout comments and messages

[PATCH net v2] net: spacemit: Clarify stat timeout comments and messages

[Vivian Wang]

Someone did run into this timeout in the wild [1], and it turns out to
be related to the PHY reference clock stopping.

Improve the comments and error message prints around this to reflect the
better understanding of how this could happen.

This patch isn't a fix for the problem per se, since it is hardware
behavior. The new message and comments, however, should direct those
running into this on new hardware towards a fix.

Link: https://lore.kernel.org/r/20260119141620.1318102-1-amadeus@jmu.edu.cn/ # [1]
Fixes: bfec6d7f2001 ("net: spacemit: Add K1 Ethernet MAC")
Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn>
---
Changes in v2:
- Add "Fixes" and retarget to net (Andrew)
- Link to v1: https://lore.kernel.org/r/20260120-k1-ethernet-clarify-stat-timeout-v1-1-108cf928d1b3@iscas.ac.cn
---
 drivers/net/ethernet/spacemit/k1_emac.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/spacemit/k1_emac.c b/drivers/net/ethernet/spacemit/k1_emac.c
index 220eb5ce7583..387f26ff7714 100644
--- a/drivers/net/ethernet/spacemit/k1_emac.c
+++ b/drivers/net/ethernet/spacemit/k1_emac.c
@@ -1099,7 +1099,14 @@ static int emac_read_stat_cnt(struct emac_priv *priv, u8 cnt, u32 *res,
 					100, 10000);
 
 	if (ret) {
-		netdev_err(priv->ndev, "Read stat timeout\n");
+		/*
+		 * If you run into this, one possibility is that even though the
+		 * interface is up the PHY may have stopped its clock anyway for
+		 * power saving. This MAC doesn't like that, so configure your
+		 * PHY to not do that.
+		 */
+		dev_err_ratelimited(&priv->ndev->dev,
+				    "Read stat timeout. PHY clock stopped?\n");
 		return ret;
 	}
 
@@ -1148,16 +1155,20 @@ static void emac_stats_update(struct emac_priv *priv)
 	assert_spin_locked(&priv->stats_lock);
 
 	if (!netif_running(priv->ndev) || !netif_device_present(priv->ndev)) {
-		/* Not up, don't try to update */
+		/*
+		 * Not up, don't try to update. If the PHY is stopped, reading
+		 * stats would time out.
+		 */
 		return;
 	}
 
 	for (i = 0; i < sizeof(priv->tx_stats) / sizeof(*tx_stats); i++) {
 		/*
-		 * If reading stats times out, everything is broken and there's
-		 * nothing we can do. Reading statistics also can't return an
-		 * error, so just return without updating and without
-		 * rescheduling.
+		 * If reading stats times out anyway, the stat registers will be
+		 * stuck, and we can't really recover from that.
+		 *
+		 * Reading statistics also can't return an error, so just return
+		 * without updating and without rescheduling.
 		 */
 		if (emac_tx_read_stat_cnt(priv, i, &res))
 			return;

---
base-commit: a9f470594c50ab1ddf25b21a00ca4d3166057f3b
change-id: 20260120-k1-ethernet-clarify-stat-timeout-d221fcd3aaa8

Best regards,
-- 
Vivian "dramforever" Wang

Re: [PATCH net v2] net: spacemit: Clarify stat timeout comments and messages

[Jakub Kicinski]

On Wed, 21 Jan 2026 10:17:18 +0800 Vivian Wang wrote:
> This patch isn't a fix for the problem per se, since it is hardware
> behavior. The new message and comments, however, should direct those
> running into this on new hardware towards a fix.

Is it not possible to improve the situation?
Is the refclock disappearing because of power saving?
Or because the link is down?
Because if it's the latter we should be able to skip reading the stats
when link is down (still racy but better than waiting in cases we can
detect?)

Re: [PATCH net v2] net: spacemit: Clarify stat timeout comments and messages

[Vivian Wang]

On 1/22/26 12:04, Jakub Kicinski wrote:
> On Wed, 21 Jan 2026 10:17:18 +0800 Vivian Wang wrote:
>> This patch isn't a fix for the problem per se, since it is hardware
>> behavior. The new message and comments, however, should direct those
>> running into this on new hardware towards a fix.
> Is it not possible to improve the situation?

Maybe I should have made it clearer, but this comment and messages
improvement was proposed by Andrew in the linked thread [1].

As an aside, saying that made me realize this patch should be:

Suggested-by: Andrew Lunn <andrew@lunn.ch>

> Is the refclock disappearing because of power saving?
> Or because the link is down?

I'm not really well-versed in what's allowed from the PHY here, but at
least the Motorcomm and Realtek PHYs seem to only stop this clock while
the link is down. So from what I can tell it's PHY stopping the clock
because of power saving for when the link is down. I'm not sure how that
should translate into an answer to your question here...

> Because if it's the latter we should be able to skip reading the stats
> when link is down (still racy but better than waiting in cases we can
> detect?)

If checking the link is definitely up as a condition for updating stats
would be acceptable, then maybe the original patch from the linked
thread [2] can also be adopted?

Vivian "dramforever" Wang

[1]: https://lore.kernel.org/r/48757af2-bbea-4185-8cc9-2ef51dbc8373@lunn.ch/
[2]: https://lore.kernel.org/r/20260118100000.224793-1-amadeus@jmu.edu.cn/

Re: [PATCH net v2] net: spacemit: Clarify stat timeout comments and messages

[Paolo Abeni]

On 1/22/26 11:39 AM, Vivian Wang wrote:
> On 1/22/26 12:04, Jakub Kicinski wrote:
>> On Wed, 21 Jan 2026 10:17:18 +0800 Vivian Wang wrote:
>>> This patch isn't a fix for the problem per se, since it is hardware
>>> behavior. The new message and comments, however, should direct those
>>> running into this on new hardware towards a fix.
>> Is it not possible to improve the situation?
> 
> Maybe I should have made it clearer, but this comment and messages
> improvement was proposed by Andrew in the linked thread [1].
> 
> As an aside, saying that made me realize this patch should be:
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> 
>> Is the refclock disappearing because of power saving?
>> Or because the link is down?
> 
> I'm not really well-versed in what's allowed from the PHY here, but at
> least the Motorcomm and Realtek PHYs seem to only stop this clock while
> the link is down. So from what I can tell it's PHY stopping the clock
> because of power saving for when the link is down. I'm not sure how that
> should translate into an answer to your question here...
> 
>> Because if it's the latter we should be able to skip reading the stats
>> when link is down (still racy but better than waiting in cases we can
>> detect?)
> 
> If checking the link is definitely up as a condition for updating stats
> would be acceptable, then maybe the original patch from the linked
> thread [2] can also be adopted?

My understanding is that such option would fit Jakub's suggestion. Note
that the commit message should be expanded and clarified WRT the
original submission.

/P

Re: [PATCH net v2] net: spacemit: Clarify stat timeout comments and messages

[Jakub Kicinski]

On Thu, 22 Jan 2026 18:39:06 +0800 Vivian Wang wrote:
> > Because if it's the latter we should be able to skip reading the stats
> > when link is down (still racy but better than waiting in cases we can
> > detect?)  
> 
> If checking the link is definitely up as a condition for updating stats
> would be acceptable, then maybe the original patch from the linked
> thread [2] can also be adopted?

Yes, speaking under correction if Andrew disagrees, but could you
squash those two patches ([2] and the v2) into one?