[PATCH v6 net-next 0/6] net: fec: add the Jumbo frame support

[PATCH v6 net-next 0/6] net: fec: add the Jumbo frame support

[Shenwei Wang]

Changes in v6:
 - address the comments from Frank and Jakub.
 - only allow changing mtu when the adaptor is not running, simplifying
   the configuration logic.

Changes in v5:
 - move the macro FEC_DRV_RESERVE_SPACE to fec.h
 - improve the comments for patch #0004

Changes in v4:
 - configure the MAX_FL according to the MTU value, and correct the
   comments for patch #3.
 - in change_mtu function, revert to original setting when buffer
   allocation fails in patch #4
 - only enable the FIFO cut-through mode when mtu greater than
   (PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN)

Changes in v3:
 - modify the OPT_FRAME_SIZE definition and drop the condition logic
 - address the review comments from Wei Fang

Changes in v2:
 - split the v1 patch per Andrew's feedback.

Shenwei Wang (6):
  net: fec: use a member variable for maximum buffer size
  net: fec: add pagepool_order to support variable page size
  net: fec: update MAX_FL based on the current MTU
  net: fec: add rx_frame_size to support configurable RX length
  net: fec: add change_mtu to support dynamic buffer allocation
  net: fec: enable the Jumbo frame support for i.MX8QM

 drivers/net/ethernet/freescale/fec.h      | 11 +++-
 drivers/net/ethernet/freescale/fec_main.c | 68 ++++++++++++++++++-----
 2 files changed, 63 insertions(+), 16 deletions(-)

--
2.43.0

[PATCH v6 net-next 1/6] net: fec: use a member variable for maximum buffer size

[Shenwei Wang]

Refactor code to support Jumbo frame functionality by adding a member
variable in the fec_enet_private structure to store PKT_MAXBUF_SIZE.

Remove the OPT_FRAME_SIZE and define a new macro OPT_ARCH_HAS_MAX_FL to
indicate architectures that support configurable maximum frame length.
And update the MAX_FL register value to max_buf_size when
OPT_ARCH_HAS_MAX_FL is defined.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Frank Li <frank.li@nxp.com>
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/fec.h      |  1 +
 drivers/net/ethernet/freescale/fec_main.c | 17 ++++++++++-------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 5c8fdcef759b..2969088dda09 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -619,6 +619,7 @@ struct fec_enet_private {

 	unsigned int total_tx_ring_size;
 	unsigned int total_rx_ring_size;
+	unsigned int max_buf_size;

 	struct	platform_device *pdev;

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 1383918f8a3f..9d348a8edf02 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -253,9 +253,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
     defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
     defined(CONFIG_ARM64)
-#define	OPT_FRAME_SIZE	(PKT_MAXBUF_SIZE << 16)
-#else
-#define	OPT_FRAME_SIZE	0
+#define	OPT_ARCH_HAS_MAX_FL
 #endif

 /* FEC MII MMFR bits definition */
@@ -1083,7 +1081,7 @@ static void fec_enet_enable_ring(struct net_device *ndev)
 	for (i = 0; i < fep->num_rx_queues; i++) {
 		rxq = fep->rx_queue[i];
 		writel(rxq->bd.dma, fep->hwp + FEC_R_DES_START(i));
-		writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_R_BUFF_SIZE(i));
+		writel(fep->max_buf_size, fep->hwp + FEC_R_BUFF_SIZE(i));

 		/* enable DMA1/2 */
 		if (i)
@@ -1145,9 +1143,13 @@ static void
 fec_restart(struct net_device *ndev)
 {
 	struct fec_enet_private *fep = netdev_priv(ndev);
-	u32 rcntl = OPT_FRAME_SIZE | FEC_RCR_MII;
+	u32 rcntl = FEC_RCR_MII;
 	u32 ecntl = FEC_ECR_ETHEREN;

+#ifdef OPT_ARCH_HAS_MAX_FL
+	rcntl |= fep->max_buf_size << 16;
+#endif
+
 	if (fep->bufdesc_ex)
 		fec_ptp_save_state(fep);

@@ -1191,7 +1193,7 @@ fec_restart(struct net_device *ndev)
 		else
 			val &= ~FEC_RACC_OPTIONS;
 		writel(val, fep->hwp + FEC_RACC);
-		writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_FTRL);
+		writel(fep->max_buf_size, fep->hwp + FEC_FTRL);
 	}
 #endif

@@ -4559,7 +4561,8 @@ fec_probe(struct platform_device *pdev)
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&pdev->dev);

-	ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN;
+	fep->max_buf_size = PKT_MAXBUF_SIZE;
+	ndev->max_mtu = fep->max_buf_size - ETH_HLEN - ETH_FCS_LEN;

 	ret = register_netdev(ndev);
 	if (ret)
--
2.43.0

[PATCH v6 net-next 2/6] net: fec: add pagepool_order to support variable page size

[Shenwei Wang]

Add a new pagepool_order member in the fec_enet_private struct
to allow dynamic configuration of page size for an instance. This
change clears the hardcoded page size assumptions.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Wei Fang <wei.fang@nxp.com>
Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/fec.h      | 1 +
 drivers/net/ethernet/freescale/fec_main.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 2969088dda09..47317346b2f3 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -620,6 +620,7 @@ struct fec_enet_private {
 	unsigned int total_tx_ring_size;
 	unsigned int total_rx_ring_size;
 	unsigned int max_buf_size;
+	unsigned int pagepool_order;
 
 	struct	platform_device *pdev;
 
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9d348a8edf02..642e19187128 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1782,7 +1782,7 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
 	 * These get messed up if we get called due to a busy condition.
 	 */
 	bdp = rxq->bd.cur;
-	xdp_init_buff(&xdp, PAGE_SIZE, &rxq->xdp_rxq);
+	xdp_init_buff(&xdp, PAGE_SIZE << fep->pagepool_order, &rxq->xdp_rxq);
 
 	while (!((status = fec16_to_cpu(bdp->cbd_sc)) & BD_ENET_RX_EMPTY)) {
 
@@ -1852,7 +1852,8 @@ fec_enet_rx_queue(struct net_device *ndev, u16 queue_id, int budget)
 		 * include that when passing upstream as it messes up
 		 * bridging applications.
 		 */
-		skb = build_skb(page_address(page), PAGE_SIZE);
+		skb = build_skb(page_address(page),
+				PAGE_SIZE << fep->pagepool_order);
 		if (unlikely(!skb)) {
 			page_pool_recycle_direct(rxq->page_pool, page);
 			ndev->stats.rx_dropped++;
@@ -4561,6 +4562,7 @@ fec_probe(struct platform_device *pdev)
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&pdev->dev);
 
+	fep->pagepool_order = 0;
 	fep->max_buf_size = PKT_MAXBUF_SIZE;
 	ndev->max_mtu = fep->max_buf_size - ETH_HLEN - ETH_FCS_LEN;
 
-- 
2.43.0

[PATCH v6 net-next 3/6] net: fec: update MAX_FL based on the current MTU

[Shenwei Wang]

Configure the MAX_FL (Maximum Frame Length) register according to the
current MTU value, which ensures that packets exceeding the configured MTU
trigger an RX error.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 642e19187128..5b71c4cf86bc 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1147,9 +1147,8 @@ fec_restart(struct net_device *ndev)
 	u32 ecntl = FEC_ECR_ETHEREN;
 
 #ifdef OPT_ARCH_HAS_MAX_FL
-	rcntl |= fep->max_buf_size << 16;
+	rcntl |= (fep->netdev->mtu + ETH_HLEN + ETH_FCS_LEN) << 16;
 #endif
-
 	if (fep->bufdesc_ex)
 		fec_ptp_save_state(fep);
 
-- 
2.43.0

[PATCH v6 net-next 4/6] net: fec: add rx_frame_size to support configurable RX length

[Shenwei Wang]

Add a new rx_frame_size member in the fec_enet_private structure to
track the RX buffer size. On the Jumbo frame enabled system, the value
will be recalculated whenever the MTU is updated, allowing the driver
to allocate RX buffer efficiently.

Configure the TRUNC_FL (Frame Truncation Length) based on the smaller
value between max_buf_size and the rx_frame_size to maintain consistent
RX error behavior, regardless of whether Jumbo frames are enabled.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/fec.h      | 1 +
 drivers/net/ethernet/freescale/fec_main.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 47317346b2f3..f1032a11aa76 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -621,6 +621,7 @@ struct fec_enet_private {
 	unsigned int total_rx_ring_size;
 	unsigned int max_buf_size;
 	unsigned int pagepool_order;
+	unsigned int rx_frame_size;
 
 	struct	platform_device *pdev;
 
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5b71c4cf86bc..df8b69af5296 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1192,7 +1192,7 @@ fec_restart(struct net_device *ndev)
 		else
 			val &= ~FEC_RACC_OPTIONS;
 		writel(val, fep->hwp + FEC_RACC);
-		writel(fep->max_buf_size, fep->hwp + FEC_FTRL);
+		writel(min(fep->rx_frame_size, fep->max_buf_size), fep->hwp + FEC_FTRL);
 	}
 #endif
 
@@ -4562,6 +4562,7 @@ fec_probe(struct platform_device *pdev)
 	pinctrl_pm_select_sleep_state(&pdev->dev);
 
 	fep->pagepool_order = 0;
+	fep->rx_frame_size = FEC_ENET_RX_FRSIZE;
 	fep->max_buf_size = PKT_MAXBUF_SIZE;
 	ndev->max_mtu = fep->max_buf_size - ETH_HLEN - ETH_FCS_LEN;
 
-- 
2.43.0

[PATCH v6 net-next 5/6] net: fec: add change_mtu to support dynamic buffer allocation

[Shenwei Wang]

Add a fec_change_mtu() handler to recalculate the pagepool_order based on
the new_mtu value. And update the rx_frame_size accordingly when
pagepool_order changes.

MTU changes are only allowed when the adater is not running.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/fec.h      |  5 +++--
 drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index f1032a11aa76..0127cfa5529f 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -348,10 +348,11 @@ struct bufdesc_ex {
  * the skbuffer directly.
  */
 
+#define FEC_DRV_RESERVE_SPACE (XDP_PACKET_HEADROOM + \
+		SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
 #define FEC_ENET_XDP_HEADROOM	(XDP_PACKET_HEADROOM)
 #define FEC_ENET_RX_PAGES	256
-#define FEC_ENET_RX_FRSIZE	(PAGE_SIZE - FEC_ENET_XDP_HEADROOM \
-		- SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+#define FEC_ENET_RX_FRSIZE	(PAGE_SIZE - FEC_DRV_RESERVE_SPACE)
 #define FEC_ENET_RX_FRPPG	(PAGE_SIZE / FEC_ENET_RX_FRSIZE)
 #define RX_RING_SIZE		(FEC_ENET_RX_FRPPG * FEC_ENET_RX_PAGES)
 #define FEC_ENET_TX_FRSIZE	2048
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index df8b69af5296..a2aa08afa4bd 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -468,14 +468,14 @@ fec_enet_create_page_pool(struct fec_enet_private *fep,
 {
 	struct bpf_prog *xdp_prog = READ_ONCE(fep->xdp_prog);
 	struct page_pool_params pp_params = {
-		.order = 0,
+		.order = fep->pagepool_order,
 		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
 		.pool_size = size,
 		.nid = dev_to_node(&fep->pdev->dev),
 		.dev = &fep->pdev->dev,
 		.dma_dir = xdp_prog ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE,
 		.offset = FEC_ENET_XDP_HEADROOM,
-		.max_len = FEC_ENET_RX_FRSIZE,
+		.max_len = fep->rx_frame_size,
 	};
 	int err;
 
@@ -4022,6 +4022,23 @@ static int fec_hwtstamp_set(struct net_device *ndev,
 	return fec_ptp_set(ndev, config, extack);
 }
 
+static int fec_change_mtu(struct net_device *ndev, int new_mtu)
+{
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	int order;
+
+	if (netif_running(ndev))
+		return -EBUSY;
+
+	order = get_order(new_mtu + ETH_HLEN + ETH_FCS_LEN
+			  + FEC_DRV_RESERVE_SPACE);
+	fep->rx_frame_size = (PAGE_SIZE << order) - FEC_DRV_RESERVE_SPACE;
+	fep->pagepool_order = order;
+	WRITE_ONCE(ndev->mtu, new_mtu);
+
+	return 0;
+}
+
 static const struct net_device_ops fec_netdev_ops = {
 	.ndo_open		= fec_enet_open,
 	.ndo_stop		= fec_enet_close,
@@ -4031,6 +4048,7 @@ static const struct net_device_ops fec_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= fec_timeout,
 	.ndo_set_mac_address	= fec_set_mac_address,
+	.ndo_change_mtu		= fec_change_mtu,
 	.ndo_eth_ioctl		= phy_do_ioctl_running,
 	.ndo_set_features	= fec_set_features,
 	.ndo_bpf		= fec_enet_bpf,
-- 
2.43.0

[PATCH v6 net-next 6/6] net: fec: enable the Jumbo frame support for i.MX8QM

[Shenwei Wang]

Certain i.MX SoCs, such as i.MX8QM and i.MX8QXP, feature enhanced
FEC hardware that supports Ethernet Jumbo frames with packet sizes
up to 16K bytes.

When Jumbo frames are supported, the TX FIFO may not be large enough
to hold an entire frame. To handle this, the FIFO is configured to
operate in cut-through mode when the frame size exceeds
(PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN), which allows transmission
to begin once the FIFO reaches a certain threshold.

Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
---
 drivers/net/ethernet/freescale/fec.h      |  3 +++
 drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++----
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 0127cfa5529f..41e0d85d15da 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -514,6 +514,9 @@ struct bufdesc_ex {
  */
 #define FEC_QUIRK_HAS_MDIO_C45		BIT(24)
 
+/* Jumbo Frame support */
+#define FEC_QUIRK_JUMBO_FRAME		BIT(25)
+
 struct bufdesc_prop {
 	int qid;
 	/* Address of Rx and Tx buffers */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index a2aa08afa4bd..329320395285 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -167,7 +167,8 @@ static const struct fec_devinfo fec_imx8qm_info = {
 		  FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE |
 		  FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE |
 		  FEC_QUIRK_CLEAR_SETUP_MII | FEC_QUIRK_HAS_MULTI_QUEUES |
-		  FEC_QUIRK_DELAYED_CLKS_SUPPORT | FEC_QUIRK_HAS_MDIO_C45,
+		  FEC_QUIRK_DELAYED_CLKS_SUPPORT | FEC_QUIRK_HAS_MDIO_C45 |
+		  FEC_QUIRK_JUMBO_FRAME,
 };
 
 static const struct fec_devinfo fec_s32v234_info = {
@@ -233,6 +234,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
  * 2048 byte skbufs are allocated. However, alignment requirements
  * varies between FEC variants. Worst case is 64, so round down by 64.
  */
+#define MAX_JUMBO_BUF_SIZE	(round_down(16384 - FEC_DRV_RESERVE_SPACE - 64, 64))
 #define PKT_MAXBUF_SIZE		(round_down(2048 - 64, 64))
 #define PKT_MINBUF_SIZE		64
 
@@ -1279,8 +1281,18 @@ fec_restart(struct net_device *ndev)
 	if (fep->quirks & FEC_QUIRK_ENET_MAC) {
 		/* enable ENET endian swap */
 		ecntl |= FEC_ECR_BYTESWP;
-		/* enable ENET store and forward mode */
-		writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
+
+		/* When Jumbo Frame is enabled, the FIFO may not be large enough
+		 * to hold an entire frame. In such cases, if the MTU exceeds
+		 * (PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN), configure the interface
+		 * to operate in cut-through mode, triggered by the FIFO threshold.
+		 * Otherwise, enable the ENET store-and-forward mode.
+		 */
+		if ((fep->quirks & FEC_QUIRK_JUMBO_FRAME) &&
+		    (ndev->mtu > (PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN)))
+			writel(0xF, fep->hwp + FEC_X_WMRK);
+		else
+			writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
 	}
 
 	if (fep->bufdesc_ex)
@@ -4581,7 +4593,12 @@ fec_probe(struct platform_device *pdev)
 
 	fep->pagepool_order = 0;
 	fep->rx_frame_size = FEC_ENET_RX_FRSIZE;
-	fep->max_buf_size = PKT_MAXBUF_SIZE;
+
+	if (fep->quirks & FEC_QUIRK_JUMBO_FRAME)
+		fep->max_buf_size = MAX_JUMBO_BUF_SIZE;
+	else
+		fep->max_buf_size = PKT_MAXBUF_SIZE;
+
 	ndev->max_mtu = fep->max_buf_size - ETH_HLEN - ETH_FCS_LEN;
 
 	ret = register_netdev(ndev);
-- 
2.43.0

RE: [PATCH v6 net-next 1/6] net: fec: use a member variable for maximum buffer size

[Wei Fang]

> @@ -253,9 +253,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC
> address");  #if defined(CONFIG_M523x) || defined(CONFIG_M527x) ||
> defined(CONFIG_M528x) || \
>      defined(CONFIG_M520x) || defined(CONFIG_M532x) ||
> defined(CONFIG_ARM) || \
>      defined(CONFIG_ARM64)
> -#define	OPT_FRAME_SIZE	(PKT_MAXBUF_SIZE << 16)
> -#else
> -#define	OPT_FRAME_SIZE	0
> +#define	OPT_ARCH_HAS_MAX_FL

In the fec driver, I noticed that the expression "#if defined (CONFIG_M523x) ||
defined(CONFIG_M527x) || ..." is used in four different places. I think we could
add a separate patch to define a new macro to replace these occurrences. This
new macro should be more generic than OPT_ARCH_HAS_MAX_FL.

>  #endif
> 
>  /* FEC MII MMFR bits definition */
> @@ -1083,7 +1081,7 @@ static void fec_enet_enable_ring(struct net_device
> *ndev)
>  	for (i = 0; i < fep->num_rx_queues; i++) {
>  		rxq = fep->rx_queue[i];
>  		writel(rxq->bd.dma, fep->hwp + FEC_R_DES_START(i));
> -		writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_R_BUFF_SIZE(i));
> +		writel(fep->max_buf_size, fep->hwp + FEC_R_BUFF_SIZE(i));
> 
>  		/* enable DMA1/2 */
>  		if (i)
> @@ -1145,9 +1143,13 @@ static void
>  fec_restart(struct net_device *ndev)
>  {
>  	struct fec_enet_private *fep = netdev_priv(ndev);
> -	u32 rcntl = OPT_FRAME_SIZE | FEC_RCR_MII;
> +	u32 rcntl = FEC_RCR_MII;

Nit: please follow reverse xmas tree rule, move this line under ecntl.

RE: [PATCH v6 net-next 3/6] net: fec: update MAX_FL based on the current MTU

[Wei Fang]

> Configure the MAX_FL (Maximum Frame Length) register according to the
> current MTU value, which ensures that packets exceeding the configured MTU
> trigger an RX error.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 642e19187128..5b71c4cf86bc 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1147,9 +1147,8 @@ fec_restart(struct net_device *ndev)
>  	u32 ecntl = FEC_ECR_ETHEREN;
> 
>  #ifdef OPT_ARCH_HAS_MAX_FL
> -	rcntl |= fep->max_buf_size << 16;
> +	rcntl |= (fep->netdev->mtu + ETH_HLEN + ETH_FCS_LEN) << 16;
>  #endif
> -
>  	if (fep->bufdesc_ex)
>  		fec_ptp_save_state(fep);
> 
> --
> 2.43.0

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

RE: [PATCH v6 net-next 4/6] net: fec: add rx_frame_size to support configurable RX length

[Wei Fang]

> Add a new rx_frame_size member in the fec_enet_private structure to
> track the RX buffer size. On the Jumbo frame enabled system, the value
> will be recalculated whenever the MTU is updated, allowing the driver
> to allocate RX buffer efficiently.
> 
> Configure the TRUNC_FL (Frame Truncation Length) based on the smaller
> value between max_buf_size and the rx_frame_size to maintain consistent
> RX error behavior, regardless of whether Jumbo frames are enabled.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec.h      | 1 +
>  drivers/net/ethernet/freescale/fec_main.c | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index 47317346b2f3..f1032a11aa76 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -621,6 +621,7 @@ struct fec_enet_private {
>  	unsigned int total_rx_ring_size;
>  	unsigned int max_buf_size;
>  	unsigned int pagepool_order;
> +	unsigned int rx_frame_size;
> 
>  	struct	platform_device *pdev;
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index 5b71c4cf86bc..df8b69af5296 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1192,7 +1192,7 @@ fec_restart(struct net_device *ndev)
>  		else
>  			val &= ~FEC_RACC_OPTIONS;
>  		writel(val, fep->hwp + FEC_RACC);
> -		writel(fep->max_buf_size, fep->hwp + FEC_FTRL);
> +		writel(min(fep->rx_frame_size, fep->max_buf_size), fep->hwp +
> FEC_FTRL);
>  	}
>  #endif
> 
> @@ -4562,6 +4562,7 @@ fec_probe(struct platform_device *pdev)
>  	pinctrl_pm_select_sleep_state(&pdev->dev);
> 
>  	fep->pagepool_order = 0;
> +	fep->rx_frame_size = FEC_ENET_RX_FRSIZE;
>  	fep->max_buf_size = PKT_MAXBUF_SIZE;
>  	ndev->max_mtu = fep->max_buf_size - ETH_HLEN - ETH_FCS_LEN;
> 
> --
> 2.43.0

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

RE: [PATCH v6 net-next 6/6] net: fec: enable the Jumbo frame support for i.MX8QM

[Wei Fang]

> Certain i.MX SoCs, such as i.MX8QM and i.MX8QXP, feature enhanced
> FEC hardware that supports Ethernet Jumbo frames with packet sizes
> up to 16K bytes.
> 
> When Jumbo frames are supported, the TX FIFO may not be large enough
> to hold an entire frame. To handle this, the FIFO is configured to
> operate in cut-through mode when the frame size exceeds
> (PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN), which allows transmission
> to begin once the FIFO reaches a certain threshold.
> 
> Signed-off-by: Shenwei Wang <shenwei.wang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/fec.h      |  3 +++
>  drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++----
>  2 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec.h
> b/drivers/net/ethernet/freescale/fec.h
> index 0127cfa5529f..41e0d85d15da 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -514,6 +514,9 @@ struct bufdesc_ex {
>   */
>  #define FEC_QUIRK_HAS_MDIO_C45		BIT(24)
> 
> +/* Jumbo Frame support */
> +#define FEC_QUIRK_JUMBO_FRAME		BIT(25)
> +
>  struct bufdesc_prop {
>  	int qid;
>  	/* Address of Rx and Tx buffers */
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index a2aa08afa4bd..329320395285 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -167,7 +167,8 @@ static const struct fec_devinfo fec_imx8qm_info = {
>  		  FEC_QUIRK_ERR007885 | FEC_QUIRK_BUG_CAPTURE |
>  		  FEC_QUIRK_HAS_RACC | FEC_QUIRK_HAS_COALESCE |
>  		  FEC_QUIRK_CLEAR_SETUP_MII | FEC_QUIRK_HAS_MULTI_QUEUES
> |
> -		  FEC_QUIRK_DELAYED_CLKS_SUPPORT |
> FEC_QUIRK_HAS_MDIO_C45,
> +		  FEC_QUIRK_DELAYED_CLKS_SUPPORT |
> FEC_QUIRK_HAS_MDIO_C45 |
> +		  FEC_QUIRK_JUMBO_FRAME,
>  };
> 
>  static const struct fec_devinfo fec_s32v234_info = {
> @@ -233,6 +234,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC
> address");
>   * 2048 byte skbufs are allocated. However, alignment requirements
>   * varies between FEC variants. Worst case is 64, so round down by 64.
>   */
> +#define MAX_JUMBO_BUF_SIZE	(round_down(16384 -
> FEC_DRV_RESERVE_SPACE - 64, 64))
>  #define PKT_MAXBUF_SIZE		(round_down(2048 - 64, 64))
>  #define PKT_MINBUF_SIZE		64
> 
> @@ -1279,8 +1281,18 @@ fec_restart(struct net_device *ndev)
>  	if (fep->quirks & FEC_QUIRK_ENET_MAC) {
>  		/* enable ENET endian swap */
>  		ecntl |= FEC_ECR_BYTESWP;
> -		/* enable ENET store and forward mode */
> -		writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
> +
> +		/* When Jumbo Frame is enabled, the FIFO may not be large enough
> +		 * to hold an entire frame. In such cases, if the MTU exceeds
> +		 * (PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN), configure the
> interface
> +		 * to operate in cut-through mode, triggered by the FIFO threshold.
> +		 * Otherwise, enable the ENET store-and-forward mode.
> +		 */
> +		if ((fep->quirks & FEC_QUIRK_JUMBO_FRAME) &&
> +		    (ndev->mtu > (PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN)))
> +			writel(0xF, fep->hwp + FEC_X_WMRK);
> +		else
> +			writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
>  	}
> 
>  	if (fep->bufdesc_ex)
> @@ -4581,7 +4593,12 @@ fec_probe(struct platform_device *pdev)
> 
>  	fep->pagepool_order = 0;
>  	fep->rx_frame_size = FEC_ENET_RX_FRSIZE;
> -	fep->max_buf_size = PKT_MAXBUF_SIZE;
> +
> +	if (fep->quirks & FEC_QUIRK_JUMBO_FRAME)
> +		fep->max_buf_size = MAX_JUMBO_BUF_SIZE;
> +	else
> +		fep->max_buf_size = PKT_MAXBUF_SIZE;
> +
>  	ndev->max_mtu = fep->max_buf_size - ETH_HLEN - ETH_FCS_LEN;
> 
>  	ret = register_netdev(ndev);
> --
> 2.43.0

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

RE: [PATCH v6 net-next 5/6] net: fec: add change_mtu to support dynamic buffer allocation

[Wei Fang]

> Add a fec_change_mtu() handler to recalculate the pagepool_order based on
> the new_mtu value. And update the rx_frame_size accordingly when
> pagepool_order changes.
> 
> MTU changes are only allowed when the adater is not running.
> 

It seems further improvements are needed to support runtime modification
of MTU.

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

RE: [PATCH v6 net-next 1/6] net: fec: use a member variable for maximum buffer size

[Shenwei Wang]

> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Monday, September 8, 2025 9:12 PM
> To: Shenwei Wang <shenwei.wang@nxp.com>
> Cc: Clark Wang <xiaoning.wang@nxp.com>; Stanislav Fomichev
> <sdf@fomichev.me>; imx@lists.linux.dev; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Andrew Lunn
> <andrew@lunn.ch>; Frank Li <frank.li@nxp.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Alexei Starovoitov <ast@kernel.org>; Daniel
> Borkmann <daniel@iogearbox.net>; Jesper Dangaard Brouer
> <hawk@kernel.org>; John Fastabend <john.fastabend@gmail.com>
> Subject: RE: [PATCH v6 net-next 1/6] net: fec: use a member variable for
> maximum buffer size
> 
> > @@ -253,9 +253,7 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC
> > address");  #if defined(CONFIG_M523x) || defined(CONFIG_M527x) ||
> > defined(CONFIG_M528x) || \
> >      defined(CONFIG_M520x) || defined(CONFIG_M532x) ||
> > defined(CONFIG_ARM) || \
> >      defined(CONFIG_ARM64)
> > -#define	OPT_FRAME_SIZE	(PKT_MAXBUF_SIZE << 16)
> > -#else
> > -#define	OPT_FRAME_SIZE	0
> > +#define	OPT_ARCH_HAS_MAX_FL
> 
> In the fec driver, I noticed that the expression "#if defined (CONFIG_M523x) ||
> defined(CONFIG_M527x) || ..." is used in four different places. I think we could
> add a separate patch to define a new macro to replace these occurrences. This
> new macro should be more generic than OPT_ARCH_HAS_MAX_FL.
> 

That is outside the scope of this patch and should be addressed in a follow-up 
cleanup patch later.

Thanks,
Shenwei

> >  #endif
> >
> >  /* FEC MII MMFR bits definition */

Re: [PATCH v6 net-next 1/6] net: fec: use a member variable for maximum buffer size

[Jakub Kicinski]

On Mon,  8 Sep 2025 11:17:50 -0500 Shenwei Wang wrote:
> -#define	OPT_FRAME_SIZE	(PKT_MAXBUF_SIZE << 16)
> -#else
> -#define	OPT_FRAME_SIZE	0
> +#define	OPT_ARCH_HAS_MAX_FL
>  #endif

> +#ifdef OPT_ARCH_HAS_MAX_FL
> +	rcntl |= fep->max_buf_size << 16;
> +#endif

We try to avoid ifdefs inside C functions, they make compilation
coverage harder. Could you define OPT_ARCH_HAS_MAX_FL to 0 or 1
depending on the platform, and then use:

	if (OPT_ARCH_HAS_MAX_FL)
		rcntl |= fep->max_buf_size << 16;

The compiler will eliminate the condition if it's a constant,
but its slightly easier to read and lets the compiler see the code
regardless of the arch.

RE: [PATCH v6 net-next 1/6] net: fec: use a member variable for maximum buffer size

[Wei Fang]

> > In the fec driver, I noticed that the expression "#if defined
> > (CONFIG_M523x) ||
> > defined(CONFIG_M527x) || ..." is used in four different places. I
> > think we could add a separate patch to define a new macro to replace
> > these occurrences. This new macro should be more generic than
> OPT_ARCH_HAS_MAX_FL.
> >
> 
> That is outside the scope of this patch and should be addressed in a follow-up
> cleanup patch later.
> 

Okay, make sense.