[PATCH v2] net: fec: allow disable coalescing

[PATCH v2] net: fec: allow disable coalescing

[Jonas Rebmann]

In the current implementation, IP coalescing is always enabled and
cannot be disabled.

As setting maximum frames to 0 or 1, or setting delay to zero implies
immediate delivery of single packets/IRQs, disable coalescing in
hardware in these cases.

This also guarantees that coalescing is never enabled with ICFT or ICTT
set to zero, a configuration that could lead to unpredictable behaviour
according to i.MX8MP reference manual.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Changes in v2:
- Adjust type of rx_itr, tx_itr (Thanks, Wei)
- Set multiple FEC_ITR_ flags in one line for more compact code (Thanks, Wei)
- Commit Message: mention ICFT/CTT fields constraints (Thanks, Andrew)
- Link to v1: https://lore.kernel.org/r/20250625-fec_deactivate_coalescing-v1-1-57a1e41a45d3@pengutronix.de
---
 drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++++----------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 63dac42720453a8b8a847bdd1eec76ac072030bf..d4eed252ad4098a7962f615bce98338bc3d12f5c 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3121,27 +3121,25 @@ static int fec_enet_us_to_itr_clock(struct net_device *ndev, int us)
 static void fec_enet_itr_coal_set(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
-	int rx_itr, tx_itr;
+	u32 rx_itr = 0, tx_itr = 0;
+	int rx_ictt, tx_ictt;
 
-	/* Must be greater than zero to avoid unpredictable behavior */
-	if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
-	    !fep->tx_time_itr || !fep->tx_pkts_itr)
-		return;
-
-	/* Select enet system clock as Interrupt Coalescing
-	 * timer Clock Source
-	 */
-	rx_itr = FEC_ITR_CLK_SEL;
-	tx_itr = FEC_ITR_CLK_SEL;
+	rx_ictt = fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr);
+	tx_ictt = fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr);
 
-	/* set ICFT and ICTT */
-	rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
-	rx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr));
-	tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
-	tx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr));
+	if (rx_ictt > 0 && fep->rx_pkts_itr > 1) {
+		/* Enable with enet system clock as Interrupt Coalescing timer Clock Source */
+		rx_itr = FEC_ITR_EN | FEC_ITR_CLK_SEL;
+		rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
+		rx_itr |= FEC_ITR_ICTT(rx_ictt);
+	}
 
-	rx_itr |= FEC_ITR_EN;
-	tx_itr |= FEC_ITR_EN;
+	if (tx_ictt > 0 && fep->tx_pkts_itr > 1) {
+		/* Enable with enet system clock as Interrupt Coalescing timer Clock Source */
+		tx_itr = FEC_ITR_EN | FEC_ITR_CLK_SEL;
+		tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
+		tx_itr |= FEC_ITR_ICTT(tx_ictt);
+	}
 
 	writel(tx_itr, fep->hwp + FEC_TXIC0);
 	writel(rx_itr, fep->hwp + FEC_RXIC0);

---
base-commit: 8dacfd92dbefee829ca555a860e86108fdd1d55b
change-id: 20250625-fec_deactivate_coalescing-c6f8d14a1d66

Best regards,
-- 
Jonas Rebmann <jre@pengutronix.de>

RE: [PATCH v2] net: fec: allow disable coalescing

[Wei Fang]

> In the current implementation, IP coalescing is always enabled and
> cannot be disabled.
>
> As setting maximum frames to 0 or 1, or setting delay to zero implies
> immediate delivery of single packets/IRQs, disable coalescing in
> hardware in these cases.
>
> This also guarantees that coalescing is never enabled with ICFT or ICTT
> set to zero, a configuration that could lead to unpredictable behaviour
> according to i.MX8MP reference manual.
>
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
> Changes in v2:
> - Adjust type of rx_itr, tx_itr (Thanks, Wei)
> - Set multiple FEC_ITR_ flags in one line for more compact code (Thanks, Wei)
> - Commit Message: mention ICFT/CTT fields constraints (Thanks, Andrew)
> - Link to v1:
> https://lore.kern/
> el.org%2Fr%2F20250625-fec_deactivate_coalescing-v1-1-57a1e41a45d3%40pe
> ngutronix.de&data=05%7C02%7Cwei.fang%40nxp.com%7C49dd91b2cd334451
> 91a608ddb4b78819%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6
> 38865422543116485%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRy
> dWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D
> %3D%7C0%7C%7C%7C&sdata=t0upws9HvxuIzBe5INmHspMnBpZ4%2B61hZAG
> pfikHb74%3D&reserved=0
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 34 +++++++++++++++----------------
>  1 file changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index
> 63dac42720453a8b8a847bdd1eec76ac072030bf..d4eed252ad4098a7962f615b
> ce98338bc3d12f5c 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3121,27 +3121,25 @@ static int fec_enet_us_to_itr_clock(struct
> net_device *ndev, int us)
>  static void fec_enet_itr_coal_set(struct net_device *ndev)
>  {
>       struct fec_enet_private *fep = netdev_priv(ndev);
> -     int rx_itr, tx_itr;
> +     u32 rx_itr = 0, tx_itr = 0;
> +     int rx_ictt, tx_ictt;
>
> -     /* Must be greater than zero to avoid unpredictable behavior */
> -     if (!fep->rx_time_itr || !fep->rx_pkts_itr ||
> -         !fep->tx_time_itr || !fep->tx_pkts_itr)
> -             return;
> -
> -     /* Select enet system clock as Interrupt Coalescing
> -      * timer Clock Source
> -      */
> -     rx_itr = FEC_ITR_CLK_SEL;
> -     tx_itr = FEC_ITR_CLK_SEL;
> +     rx_ictt = fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr);
> +     tx_ictt = fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr);
>
> -     /* set ICFT and ICTT */
> -     rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
> -     rx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->rx_time_itr));
> -     tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
> -     tx_itr |= FEC_ITR_ICTT(fec_enet_us_to_itr_clock(ndev, fep->tx_time_itr));
> +     if (rx_ictt > 0 && fep->rx_pkts_itr > 1) {
> +             /* Enable with enet system clock as Interrupt Coalescing timer Clock
> Source */
> +             rx_itr = FEC_ITR_EN | FEC_ITR_CLK_SEL;
> +             rx_itr |= FEC_ITR_ICFT(fep->rx_pkts_itr);
> +             rx_itr |= FEC_ITR_ICTT(rx_ictt);
> +     }
>
> -     rx_itr |= FEC_ITR_EN;
> -     tx_itr |= FEC_ITR_EN;
> +     if (tx_ictt > 0 && fep->tx_pkts_itr > 1) {
> +             /* Enable with enet system clock as Interrupt Coalescing timer Clock
> Source */
> +             tx_itr = FEC_ITR_EN | FEC_ITR_CLK_SEL;
> +             tx_itr |= FEC_ITR_ICFT(fep->tx_pkts_itr);
> +             tx_itr |= FEC_ITR_ICTT(tx_ictt);
> +     }
>
>       writel(tx_itr, fep->hwp + FEC_TXIC0);
>       writel(rx_itr, fep->hwp + FEC_RXIC0);
>
> ---

Reviewed-by: Wei Fang <wei.fang@nxp.com>

Re: [PATCH v2] net: fec: allow disable coalescing

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 26 Jun 2025 15:44:02 +0200 you wrote:
> In the current implementation, IP coalescing is always enabled and
> cannot be disabled.
> 
> As setting maximum frames to 0 or 1, or setting delay to zero implies
> immediate delivery of single packets/IRQs, disable coalescing in
> hardware in these cases.
> 
> [...]

Here is the summary with links:
  - [v2] net: fec: allow disable coalescing
    https://git.kernel.org/netdev/net-next/c/b7ad21258f9e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html