Re: [net-next,v20,4/7] net: mtip: Add net_device_ops functions to the L2 switch driver

Hi Jakub,

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see
https://netdev-ai.bots.linux.dev/ai-local.html ---
net: mtip: Add net_device_ops functions to the L2 switch driver

This patch adds the net_device_ops callback implementations for the
MTIP L2 switch driver including transmit, timeout handling, multicast
setup, MAC address configuration, and port parent ID retrieval.

quoted

diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c

b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c index
9c21a3d29c1c..a6424ee1a9d9 100644 ---
a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c +++
b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c  

[ ... ]

quoted

+static netdev_tx_t mtip_start_xmit_port(struct sk_buff *skb,
+					struct net_device *dev,
int port) +{  

[ ... ]

quoted

+	/* Push the data cache so the CPM does not get stale memory
+	 * data.
+	 */
+	bdp->cbd_bufaddr = dma_map_single(&fep->pdev->dev, bufaddr,
+					  MTIP_SWITCH_TX_FRSIZE,
+					  DMA_TO_DEVICE);  

The dma_map_single() call here creates a DMA mapping, but
mtip_switch_tx() at this point in the series is an empty stub that
never calls dma_unmap_single().

Yes.

Is the intention for this to be
resolved by a later patch in the series? Looking ahead, a4c3fa1c66c9
("net: mtip: Add mtip_switch_{rx|tx} functions to the L2 switch
driver") does add the full TX completion handler with the
dma_unmap_single() call. However, this commit on its own could cause
DMA resource exhaustion if used standalone (e.g., during bisection).

The MTIP Kconfig define would not be available at this point of the
series.

The split is done as part of the agreement of how that large driver
shall be added.

The mtip_switch_rx|tx is a stub at this point to ease the review.

Would it make sense to merge these two patches together, or at minimum
have a note in the commit message explaining the dependency?

@Jakub - If possible I would keep things as they are now.

[ ... ]

-- 
Best regards,

Łukasz Majewski