Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] net: stmmac: Cleanup rx coalescing computation when using RIWT
@ 2026-08-02 11:40 Maxime Chevallier
  2026-08-02 11:40 ` [PATCH net-next 1/2] net: stmmac: ethtool: Comment the magic numbers in RIWT computation Maxime Chevallier
  2026-08-02 11:40 ` [PATCH net-next 2/2] net: stmmac: ethtool: Address off-by-one when reading the coal rx-usecs Maxime Chevallier
  0 siblings, 2 replies; 5+ messages in thread
From: Maxime Chevallier @ 2026-08-02 11:40 UTC (permalink / raw)
  To: Andrew Lunn, Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni,
	Simon Horman, Maxime Coquelin, Alexandre Torgue, Russell King
  Cc: Maxime Chevallier, thomas.petazzoni, Alexis Lothoré, netdev,
	linux-kernel, linux-arm-kernel, linux-stm32

Currently when configuring interrupt coalescing on devices that relies
on the Receive Interrupt Watchdog Timer feature of dwmac, the
computation of the RIWT timings leads to off-by-one values when
reporting the timings back to userspace.

RIWT works by arming a watchdog timer upon receiving frames with the RI
bit not set in the descriptor. The timer duration is expressed in units
of 256 stmmac clock ticks, and therefore requires a bit of computation
to derive it :

riwt = (rx_usecs * n_clk_ticks_per_usec) / 256

and conversely

rx_usecs = (riwt * 256) / n_clk_ticks_per_usec

This computation as-is leads to a consistent off-by-one when setting
then getting back the rx-usecs value due to rounding errors (by truncation):

ethtool -C eth1 rx-usecs 42
ethtool -c eth1
 -> reports rx-usecs: 41

Let's use DIV_ROUND_CLOSEST instead for the computations. It does have
one side effect, the accepted boundaries for rx-usecs also shifts by one
now, going from [16us, 246us] to [15us, 245us]. For that reason, I'm not
targeting the net tree here, and it's overall a very small issue.

Maxime

Maxime Chevallier (2):
  net: stmmac: ethtool: Comment the magic numbers in RIWT computation
  net: stmmac: ethtool: Address off-by-one when reading the coal
    rx-usecs

 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.55.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 1/2] net: stmmac: ethtool: Comment the magic numbers in RIWT computation
  2026-08-02 11:40 [PATCH net-next 0/2] net: stmmac: Cleanup rx coalescing computation when using RIWT Maxime Chevallier
@ 2026-08-02 11:40 ` Maxime Chevallier
  2026-08-02 15:02   ` Andrew Lunn
  2026-08-02 11:40 ` [PATCH net-next 2/2] net: stmmac: ethtool: Address off-by-one when reading the coal rx-usecs Maxime Chevallier
  1 sibling, 1 reply; 5+ messages in thread
From: Maxime Chevallier @ 2026-08-02 11:40 UTC (permalink / raw)
  To: Andrew Lunn, Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni,
	Simon Horman, Maxime Coquelin, Alexandre Torgue, Russell King
  Cc: Maxime Chevallier, thomas.petazzoni, Alexis Lothoré, netdev,
	linux-kernel, linux-arm-kernel, linux-stm32

Receive Interrupt Watchdog Timer is an RX interrupt coalescing mechanism
used by some variants of dwmac. It allows waiting a bit before
triggering the rx interrupts, allowing for batch processing.

The RIWT is configured with a granularity of 256 stmmac clk ticks. Let's
add a comment for that and wrap the raw "1000000" into USEC_PER_SEC, as
we're computing "how many clock cycles in one microsec" with that step.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 92585d27ab88..eebfd6976aaa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -759,7 +759,10 @@ static u32 stmmac_usec2riwt(u32 usec, struct stmmac_priv *priv)
 			return 0;
 	}
 
-	return (usec * (clk / 1000000)) / 256;
+	/* Receive Interrupt Watchdog Timer (riwt) has a resolution of 256
+	 * ticks.
+	 */
+	return (usec * (clk / USEC_PER_SEC)) / 256;
 }
 
 static u32 stmmac_riwt2usec(u32 riwt, struct stmmac_priv *priv)
@@ -772,7 +775,7 @@ static u32 stmmac_riwt2usec(u32 riwt, struct stmmac_priv *priv)
 			return 0;
 	}
 
-	return (riwt * 256) / (clk / 1000000);
+	return (riwt * 256) / (clk / USEC_PER_SEC);
 }
 
 static int __stmmac_get_coalesce(struct net_device *dev,
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH net-next 2/2] net: stmmac: ethtool: Address off-by-one when reading the coal rx-usecs
  2026-08-02 11:40 [PATCH net-next 0/2] net: stmmac: Cleanup rx coalescing computation when using RIWT Maxime Chevallier
  2026-08-02 11:40 ` [PATCH net-next 1/2] net: stmmac: ethtool: Comment the magic numbers in RIWT computation Maxime Chevallier
@ 2026-08-02 11:40 ` Maxime Chevallier
  2026-08-02 15:05   ` Andrew Lunn
  1 sibling, 1 reply; 5+ messages in thread
From: Maxime Chevallier @ 2026-08-02 11:40 UTC (permalink / raw)
  To: Andrew Lunn, Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni,
	Simon Horman, Maxime Coquelin, Alexandre Torgue, Russell King
  Cc: Maxime Chevallier, thomas.petazzoni, Alexis Lothoré, netdev,
	linux-kernel, linux-arm-kernel, linux-stm32

When reading the rx-usecs coalescing parameters on a dwmac variant that
uses the RIWT for RX interrupt coalescing, we convert the riwt value to
usecs :
 - One riwt cycle is 256 clock ticks, we compute how many ticks in $riwt
   cycles
 - divide that by how many ticks in a microsecond, and we get the
   rx-usecs.

The opposite computation is done when setting the rx-usecs param.

Because of the 256 ratio, we're subjected to off-by-one errors in the
value read-back, which can be reliably measured on i.mx8MP :

$ ethtool -C eth1 rx-usecs 102

$ ethtool -c eth1
  Coalesce parameters for eth1:
  [...]
  rx-usecs: 101

Let's be more explicit about the rounding for the riwt to usec
computations by using DIV_ROUND_CLOSEST, which solves the off-by-one.

This does change the boundaries of accepted rx-usecs parameters, as the
previously accepted values were in the 16-246 us range, and now fall
into the 15-245 range on imx8mp.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index eebfd6976aaa..154cc0c7623d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -762,7 +762,7 @@ static u32 stmmac_usec2riwt(u32 usec, struct stmmac_priv *priv)
 	/* Receive Interrupt Watchdog Timer (riwt) has a resolution of 256
 	 * ticks.
 	 */
-	return (usec * (clk / USEC_PER_SEC)) / 256;
+	return DIV_ROUND_CLOSEST(usec * (clk / USEC_PER_SEC), 256);
 }
 
 static u32 stmmac_riwt2usec(u32 riwt, struct stmmac_priv *priv)
@@ -775,7 +775,7 @@ static u32 stmmac_riwt2usec(u32 riwt, struct stmmac_priv *priv)
 			return 0;
 	}
 
-	return (riwt * 256) / (clk / USEC_PER_SEC);
+	return DIV_ROUND_CLOSEST(riwt * 256, clk / USEC_PER_SEC);
 }
 
 static int __stmmac_get_coalesce(struct net_device *dev,
-- 
2.55.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 1/2] net: stmmac: ethtool: Comment the magic numbers in RIWT computation
  2026-08-02 11:40 ` [PATCH net-next 1/2] net: stmmac: ethtool: Comment the magic numbers in RIWT computation Maxime Chevallier
@ 2026-08-02 15:02   ` Andrew Lunn
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2026-08-02 15:02 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Andrew Lunn, Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni,
	Simon Horman, Maxime Coquelin, Alexandre Torgue, Russell King,
	thomas.petazzoni, Alexis Lothoré, netdev, linux-kernel,
	linux-arm-kernel, linux-stm32

On Sun, Aug 02, 2026 at 01:40:13PM +0200, Maxime Chevallier wrote:
> Receive Interrupt Watchdog Timer is an RX interrupt coalescing mechanism
> used by some variants of dwmac. It allows waiting a bit before
> triggering the rx interrupts, allowing for batch processing.
> 
> The RIWT is configured with a granularity of 256 stmmac clk ticks. Let's
> add a comment for that and wrap the raw "1000000" into USEC_PER_SEC, as
> we're computing "how many clock cycles in one microsec" with that step.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

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

    Andrew


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 2/2] net: stmmac: ethtool: Address off-by-one when reading the coal rx-usecs
  2026-08-02 11:40 ` [PATCH net-next 2/2] net: stmmac: ethtool: Address off-by-one when reading the coal rx-usecs Maxime Chevallier
@ 2026-08-02 15:05   ` Andrew Lunn
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2026-08-02 15:05 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Andrew Lunn, Jakub Kicinski, davem, Eric Dumazet, Paolo Abeni,
	Simon Horman, Maxime Coquelin, Alexandre Torgue, Russell King,
	thomas.petazzoni, Alexis Lothoré, netdev, linux-kernel,
	linux-arm-kernel, linux-stm32

> This does change the boundaries of accepted rx-usecs parameters, as the
> previously accepted values were in the 16-246 us range, and now fall
> into the 15-245 range on imx8mp.

With this change in place, what happens when trying to set 246?
EINVAL?

	Andrew


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-02 15:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-02 11:40 [PATCH net-next 0/2] net: stmmac: Cleanup rx coalescing computation when using RIWT Maxime Chevallier
2026-08-02 11:40 ` [PATCH net-next 1/2] net: stmmac: ethtool: Comment the magic numbers in RIWT computation Maxime Chevallier
2026-08-02 15:02   ` Andrew Lunn
2026-08-02 11:40 ` [PATCH net-next 2/2] net: stmmac: ethtool: Address off-by-one when reading the coal rx-usecs Maxime Chevallier
2026-08-02 15:05   ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox