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

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

[Jakub Kicinski]

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.

> 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

[ ... ]

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

[ ... ]

> +	/* 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(). 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).

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

[ ... ]

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

[Łukasz Majewski]

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.
> 
> > 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  
> 
> [ ... ]
> 
> > +static netdev_tx_t mtip_start_xmit_port(struct sk_buff *skb,
> > +					struct net_device *dev,
> > int port) +{  
> 
> [ ... ]
> 
> > +	/* 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

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

[Jakub Kicinski]

On Wed, 28 Jan 2026 22:55:33 +0100 Łukasz Majewski wrote:
> > 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.

Not ideal, but okay, we can leave it as is.