This patch provides callbacks for struct net_device_ops for MTIP
L2 switch.
Signed-off-by: Lukasz Majewski <lukma@denx.de>
---
Changes for v13:
- New patch - created by excluding some code from large (i.e. v12 and
earlier) MTIP driver
Changes for v14:
- Add read memory barier (rmb) before reading current descriptor
- Use proper locking primitives
Changes for v15 - v15:
- None
Changes for v16:
- Enable MTIP ports to support bridge offloading
- Use dev_err_ratelimited() instead of plain dev_err()
- Move skb storage and tx ring buffer modifications after
dma mapping code.
- Do not increase tx_errors when frames are dropped after
failed dma_mapping.
- Refactor the code for better readability
- Remove legacy call to netif_trans_update()
- Remove not needed rmb() - synchronized data read already assured by
coherent DMA allocation
- Replace spin_{un}lock() with _bh variant
---
.../net/ethernet/freescale/mtipsw/mtipl2sw.c | 284 ++++++++++++++++++
1 file changed, 284 insertions(+)
diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
index a4668047ed11..4321dd1615fa 100644
--- a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
+++ b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw.c
@@ -43,6 +43,15 @@
#include "mtipl2sw.h"
+static void swap_buffer(void *bufaddr, int len)
+{
+ int i;
+ unsigned int *buf = bufaddr;
+
+ for (i = 0; i < len; i += 4, buf++)
+ swab32s(buf);
+}
+
/* Set the last buffer to wrap */
static void mtip_set_last_buf_to_wrap(struct cbd_t *bdp)
{
@@ -436,6 +445,124 @@ static void mtip_config_switch(struct switch_enet_private *fep)
fep->hwp + ESW_IMR);
}
+static netdev_tx_t mtip_start_xmit_port(struct sk_buff *skb,
+ struct net_device *dev, int port)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+ struct switch_enet_private *fep = priv->fep;
+ unsigned short status;
+ struct cbd_t *bdp;
+ void *bufaddr;
+
+ spin_lock_bh(&fep->hw_lock);
+
+ if (!fep->link[0] && !fep->link[1]) {
+ /* Link is down or autonegotiation is in progress. */
+ netif_stop_queue(dev);
+ spin_unlock_bh(&fep->hw_lock);
+ return NETDEV_TX_BUSY;
+ }
+
+ /* Fill in a Tx ring entry */
+ bdp = fep->cur_tx;
+ status = bdp->cbd_sc;
+
+ if (status & BD_ENET_TX_READY) {
+ /* All transmit buffers are full. Bail out.
+ * This should not happen, since dev->tbusy should be set.
+ */
+ netif_stop_queue(dev);
+ dev_err_ratelimited(&fep->pdev->dev, "%s: tx queue full!.\n",
+ dev->name);
+ spin_unlock(&fep->hw_lock);
+ return NETDEV_TX_BUSY;
+ }
+
+ /* Clear all of the status flags */
+ status &= ~BD_ENET_TX_STATS;
+
+ /* Set buffer length and buffer pointer */
+ bufaddr = skb->data;
+ bdp->cbd_datlen = skb->len;
+
+ /* On some FEC implementations data must be aligned on
+ * 4-byte boundaries. Use bounce buffers to copy data
+ * and get it aligned.spin
+ */
+ if ((unsigned long)bufaddr & MTIP_ALIGNMENT) {
+ unsigned int index;
+
+ index = bdp - fep->tx_bd_base;
+ memcpy(fep->tx_bounce[index], skb->data, skb->len);
+ bufaddr = fep->tx_bounce[index];
+ }
+
+ if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
+ swap_buffer(bufaddr, skb->len);
+
+ /* 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);
+ if (unlikely(dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr))) {
+ dev_err(&fep->pdev->dev,
+ "Failed to map descriptor tx buffer\n");
+ dev->stats.tx_dropped++;
+ dev_kfree_skb_any(skb);
+ goto err;
+ }
+
+ /* Save skb pointer. */
+ fep->tx_skbuff[fep->skb_cur] = skb;
+ fep->skb_cur = (fep->skb_cur + 1) & TX_RING_MOD_MASK;
+
+ /* Send it on its way. Tell FEC it's ready, interrupt when done,
+ * it's the last BD of the frame, and to put the CRC on the end.
+ */
+
+ status |= (BD_ENET_TX_READY | BD_ENET_TX_INTR | BD_ENET_TX_LAST |
+ BD_ENET_TX_TC);
+
+ /* Synchronize all descriptor writes */
+ wmb();
+ bdp->cbd_sc = status;
+
+ skb_tx_timestamp(skb);
+
+ /* Trigger transmission start */
+ writel(MCF_ESW_TDAR_X_DES_ACTIVE, fep->hwp + ESW_TDAR);
+
+ dev->stats.tx_bytes += skb->len;
+ /* If this was the last BD in the ring,
+ * start at the beginning again.
+ */
+ if (status & BD_ENET_TX_WRAP)
+ bdp = fep->tx_bd_base;
+ else
+ bdp++;
+
+ if (bdp == fep->dirty_tx) {
+ fep->tx_full = 1;
+ netif_stop_queue(dev);
+ }
+
+ fep->cur_tx = bdp;
+ err:
+ spin_unlock_bh(&fep->hw_lock);
+
+ return NETDEV_TX_OK;
+}
+
+static netdev_tx_t mtip_start_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+
+ return mtip_start_xmit_port(skb, dev, priv->portnum);
+}
+
static void mtip_configure_enet_mii(struct switch_enet_private *fep, int port)
{
struct phy_device *phydev = fep->phy_dev[port - 1];
@@ -593,6 +720,70 @@ static void mtip_switch_restart(struct net_device *dev, int duplex0,
mtip_config_switch(fep);
}
+static void mtip_timeout(struct net_device *dev, unsigned int txqueue)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+ struct switch_enet_private *fep = priv->fep;
+ struct cbd_t *bdp;
+ int i;
+
+ dev->stats.tx_errors++;
+
+ if (IS_ENABLED(CONFIG_SWITCH_DEBUG)) {
+ spin_lock_bh(&fep->hw_lock);
+ dev_info(&dev->dev, "%s: transmit timed out.\n", dev->name);
+ dev_info(&dev->dev,
+ "Ring data: cur_tx %lx%s, dirty_tx %lx cur_rx: %lx\n",
+ (unsigned long)fep->cur_tx,
+ fep->tx_full ? " (full)" : "",
+ (unsigned long)fep->dirty_tx,
+ (unsigned long)fep->cur_rx);
+
+ bdp = fep->tx_bd_base;
+ dev_info(&dev->dev, " tx: %u buffers\n", TX_RING_SIZE);
+ for (i = 0; i < TX_RING_SIZE; i++) {
+ dev_info(&dev->dev, " %08lx: %04x %04x %08x\n",
+ (kernel_ulong_t)bdp, bdp->cbd_sc,
+ bdp->cbd_datlen, (int)bdp->cbd_bufaddr);
+ bdp++;
+ }
+
+ bdp = fep->rx_bd_base;
+ dev_info(&dev->dev, " rx: %lu buffers\n",
+ (unsigned long)RX_RING_SIZE);
+ for (i = 0 ; i < RX_RING_SIZE; i++) {
+ dev_info(&dev->dev, " %08lx: %04x %04x %08x\n",
+ (kernel_ulong_t)bdp,
+ bdp->cbd_sc, bdp->cbd_datlen,
+ (int)bdp->cbd_bufaddr);
+ bdp++;
+ }
+ spin_unlock_bh(&fep->hw_lock);
+ }
+
+ schedule_work(&priv->tx_timeout_work);
+}
+
+static void mtip_timeout_work(struct work_struct *work)
+{
+ struct mtip_ndev_priv *priv =
+ container_of(work, struct mtip_ndev_priv, tx_timeout_work);
+ struct switch_enet_private *fep = priv->fep;
+ struct net_device *dev = priv->dev;
+
+ rtnl_lock();
+ if (netif_device_present(dev) || netif_running(dev)) {
+ napi_disable(&fep->napi);
+ netif_tx_lock_bh(dev);
+ mtip_switch_restart(dev, fep->full_duplex[0],
+ fep->full_duplex[1]);
+ netif_tx_wake_all_queues(dev);
+ netif_tx_unlock_bh(dev);
+ napi_enable(&fep->napi);
+ }
+ rtnl_unlock();
+}
+
static irqreturn_t mtip_interrupt(int irq, void *ptr_fep)
{
struct switch_enet_private *fep = ptr_fep;
@@ -1068,6 +1259,92 @@ static int mtip_close(struct net_device *dev)
return 0;
}
+#define FEC_HASH_BITS 6 /* #bits in hash */
+static void mtip_set_multicast_list(struct net_device *dev)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+ unsigned int hash_high = 0, hash_low = 0, crc;
+ struct switch_enet_private *fep = priv->fep;
+ void __iomem *enet_addr = fep->enet_addr;
+ struct netdev_hw_addr *ha;
+ unsigned char hash;
+
+ if (priv->portnum == 2)
+ enet_addr += MCF_ESW_ENET_PORT_OFFSET;
+
+ if (dev->flags & IFF_PROMISC) {
+ /* Promisc mode is required for switch - it is
+ * already enabled during driver's probe.
+ */
+ dev_dbg(&dev->dev, "%s: IFF_PROMISC\n", __func__);
+ return;
+ }
+
+ if (dev->flags & IFF_ALLMULTI) {
+ dev_dbg(&dev->dev, "%s: IFF_ALLMULTI\n", __func__);
+
+ /* Allow all multicast addresses */
+ writel(0xFFFFFFFF, enet_addr + MCF_FEC_GRP_HASH_TABLE_HIGH);
+ writel(0xFFFFFFFF, enet_addr + MCF_FEC_GRP_HASH_TABLE_LOW);
+
+ return;
+ }
+
+ netdev_for_each_mc_addr(ha, dev) {
+ /* Calculate crc32 value of mac address */
+ crc = ether_crc_le(dev->addr_len, ha->addr);
+
+ /* Only upper 6 bits (FEC_HASH_BITS) are used
+ * which point to specific bit in the hash registers
+ */
+ hash = (crc >> (32 - FEC_HASH_BITS)) & 0x3F;
+
+ if (hash > 31)
+ hash_high |= 1 << (hash - 32);
+ else
+ hash_low |= 1 << hash;
+ }
+
+ writel(hash_high, enet_addr + MCF_FEC_GRP_HASH_TABLE_HIGH);
+ writel(hash_low, enet_addr + MCF_FEC_GRP_HASH_TABLE_LOW);
+}
+
+static int mtip_set_mac_address(struct net_device *dev, void *p)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(dev);
+ struct switch_enet_private *fep = priv->fep;
+ void __iomem *enet_addr = fep->enet_addr;
+ struct sockaddr *addr = p;
+
+ if (!is_valid_ether_addr(addr->sa_data))
+ return -EADDRNOTAVAIL;
+ eth_hw_addr_set(dev, addr->sa_data);
+
+ if (priv->portnum == 2)
+ enet_addr += MCF_ESW_ENET_PORT_OFFSET;
+
+ writel(dev->dev_addr[3] | (dev->dev_addr[2] << 8) |
+ (dev->dev_addr[1] << 16) | (dev->dev_addr[0] << 24),
+ enet_addr + MCF_FEC_PALR);
+ writel((dev->dev_addr[5] << 16) | (dev->dev_addr[4] << 24),
+ enet_addr + MCF_FEC_PAUR);
+
+ return mtip_update_atable_static((unsigned char *)dev->dev_addr,
+ 7, 7, fep);
+}
+
+static int mtip_get_port_parent_id(struct net_device *ndev,
+ struct netdev_phys_item_id *ppid)
+{
+ struct mtip_ndev_priv *priv = netdev_priv(ndev);
+ struct switch_enet_private *fep = priv->fep;
+
+ ppid->id_len = sizeof(fep->mac[0]);
+ memcpy(&ppid->id, &fep->mac[0], ppid->id_len);
+
+ return 0;
+}
+
static const struct ethtool_ops mtip_ethtool_ops = {
.get_link_ksettings = phy_ethtool_get_link_ksettings,
.set_link_ksettings = phy_ethtool_set_link_ksettings,
@@ -1079,6 +1356,11 @@ static const struct ethtool_ops mtip_ethtool_ops = {
static const struct net_device_ops mtip_netdev_ops = {
.ndo_open = mtip_open,
.ndo_stop = mtip_close,
+ .ndo_start_xmit = mtip_start_xmit,
+ .ndo_set_rx_mode = mtip_set_multicast_list,
+ .ndo_tx_timeout = mtip_timeout,
+ .ndo_set_mac_address = mtip_set_mac_address,
+ .ndo_get_port_parent_id = mtip_get_port_parent_id,
};
bool mtip_is_switch_netdev_port(const struct net_device *ndev)
@@ -1181,6 +1463,8 @@ static int mtip_ndev_init(struct switch_enet_private *fep,
break;
}
+ INIT_WORK(&priv->tx_timeout_work, mtip_timeout_work);
+
dev_dbg(&fep->ndev[i]->dev, "%s: MTIP eth L2 switch %pM\n",
fep->ndev[i]->name, fep->ndev[i]->dev_addr);
}
--
2.39.5On Fri, 25 Jul 2025 00:33:12 +0200 Lukasz Majewski wrote:
> +static void swap_buffer(void *bufaddr, int len)
> +{
> + int i;
> + unsigned int *buf = bufaddr;
> +
nit: reverse xmas tree
> + if (status & BD_ENET_TX_READY) {
> + /* All transmit buffers are full. Bail out.
> + * This should not happen, since dev->tbusy should be set.
> + */
> + netif_stop_queue(dev);
> + dev_err_ratelimited(&fep->pdev->dev, "%s: tx queue full!.\n",
> + dev->name);
> + spin_unlock(&fep->hw_lock);
As we discussed on previous revision you have many to one mapping
of netdevs to queues. I think the warning should only be printed
if the drivers is in "single netdev" mode. Otherwise it _will_
trigger.
BTW you should put the print after the unlock, console writes are slow.
> +static void mtip_timeout(struct net_device *dev, unsigned int txqueue)
> +{
> + struct mtip_ndev_priv *priv = netdev_priv(dev);
> + struct switch_enet_private *fep = priv->fep;
> + struct cbd_t *bdp;
> + int i;
> +
> + dev->stats.tx_errors++;
> +
> + if (IS_ENABLED(CONFIG_SWITCH_DEBUG)) {
why are you hiding the debug info under a CONFIG_ ?
(which BTW appears not to be defined at all)
Seems useful to know the state of the HW when the queue hung.
You can use a DO_ONCE() if you want to avoid spamming logs
> + /* Set buffer length and buffer pointer */
> + bufaddr = skb->data;
You should call skb_cow_data() if you want to write to the skb data.
> +static void mtip_timeout(struct net_device *dev, unsigned int txqueue)
> +{
> + struct mtip_ndev_priv *priv = netdev_priv(dev);
> + struct switch_enet_private *fep = priv->fep;
> + struct cbd_t *bdp;
> + int i;
> +
> + dev->stats.tx_errors++;
timeouts are already counted by the stack, I think the statistic
is exposed per-queue in sysfs
> + spin_lock_bh(&fep->hw_lock);
> + dev_info(&dev->dev, "%s: transmit timed out.\n", dev->name);
> + dev_info(&dev->dev,
> + "Ring data: cur_tx %lx%s, dirty_tx %lx cur_rx: %lx\n",
> + (unsigned long)fep->cur_tx,
> + fep->tx_full ? " (full)" : "",
> + (unsigned long)fep->dirty_tx,
> + (unsigned long)fep->cur_rx);
> +
> + bdp = fep->tx_bd_base;
> + dev_info(&dev->dev, " tx: %u buffers\n", TX_RING_SIZE);
> + for (i = 0; i < TX_RING_SIZE; i++) {
> + dev_info(&dev->dev, " %08lx: %04x %04x %08x\n",
> + (kernel_ulong_t)bdp, bdp->cbd_sc,
> + bdp->cbd_datlen, (int)bdp->cbd_bufaddr);
> + bdp++;
> + }
> +
> + bdp = fep->rx_bd_base;
> + dev_info(&dev->dev, " rx: %lu buffers\n",
> + (unsigned long)RX_RING_SIZE);
> + for (i = 0 ; i < RX_RING_SIZE; i++) {
> + dev_info(&dev->dev, " %08lx: %04x %04x %08x\n",
> + (kernel_ulong_t)bdp,
> + bdp->cbd_sc, bdp->cbd_datlen,
> + (int)bdp->cbd_bufaddr);
> + bdp++;
> + }
> + spin_unlock_bh(&fep->hw_lock);
Hi Jakub,
> On Fri, 25 Jul 2025 00:33:12 +0200 Lukasz Majewski wrote:
> > +static void swap_buffer(void *bufaddr, int len)
> > +{
> > + int i;
> > + unsigned int *buf = bufaddr;
> > +
>
> nit: reverse xmas tree
Ok.
>
> > + if (status & BD_ENET_TX_READY) {
> > + /* All transmit buffers are full. Bail out.
> > + * This should not happen, since dev->tbusy should
> > be set.
> > + */
> > + netif_stop_queue(dev);
> > + dev_err_ratelimited(&fep->pdev->dev, "%s: tx queue
> > full!.\n",
> > + dev->name);
> > + spin_unlock(&fep->hw_lock);
>
> As we discussed on previous revision you have many to one mapping
> of netdevs to queues. I think the warning should only be printed
> if the drivers is in "single netdev" mode. Otherwise it _will_
> trigger.
It will not trigger (as far as I understand the HW) because no matter
which interface will call this function, there will be next, available
descriptor provided to perform the transmission to switch port 0.
>
> BTW you should put the print after the unlock, console writes are
> slow.
+1
>
> > +static void mtip_timeout(struct net_device *dev, unsigned int
> > txqueue) +{
> > + struct mtip_ndev_priv *priv = netdev_priv(dev);
> > + struct switch_enet_private *fep = priv->fep;
> > + struct cbd_t *bdp;
> > + int i;
> > +
> > + dev->stats.tx_errors++;
> > +
> > + if (IS_ENABLED(CONFIG_SWITCH_DEBUG)) {
>
> why are you hiding the debug info under a CONFIG_ ?
> (which BTW appears not to be defined at all)
The CONFIG_SWITCH_DEBUG is rather a "local" variable.
> Seems useful to know the state of the HW when the queue hung.
> You can use a DO_ONCE() if you want to avoid spamming logs
Hmmm.... Good point. I will rewrite it.
>
> > + /* Set buffer length and buffer pointer */
> > + bufaddr = skb->data;
>
> You should call skb_cow_data() if you want to write to the skb data.
This is the place where I do send data... (the *xmit_port function).
For reading I do have pool of pages and then I do copy the data to skb.
>
> > +static void mtip_timeout(struct net_device *dev, unsigned int
> > txqueue) +{
> > + struct mtip_ndev_priv *priv = netdev_priv(dev);
> > + struct switch_enet_private *fep = priv->fep;
> > + struct cbd_t *bdp;
> > + int i;
> > +
> > + dev->stats.tx_errors++;
>
> timeouts are already counted by the stack, I think the statistic
> is exposed per-queue in sysfs
Ok. I will remove it.
>
> > + spin_lock_bh(&fep->hw_lock);
> > + dev_info(&dev->dev, "%s: transmit timed out.\n",
> > dev->name);
> > + dev_info(&dev->dev,
> > + "Ring data: cur_tx %lx%s, dirty_tx %lx
> > cur_rx: %lx\n",
> > + (unsigned long)fep->cur_tx,
> > + fep->tx_full ? " (full)" : "",
> > + (unsigned long)fep->dirty_tx,
> > + (unsigned long)fep->cur_rx);
> > +
> > + bdp = fep->tx_bd_base;
> > + dev_info(&dev->dev, " tx: %u buffers\n",
> > TX_RING_SIZE);
> > + for (i = 0; i < TX_RING_SIZE; i++) {
> > + dev_info(&dev->dev, " %08lx: %04x %04x
> > %08x\n",
> > + (kernel_ulong_t)bdp, bdp->cbd_sc,
> > + bdp->cbd_datlen,
> > (int)bdp->cbd_bufaddr);
> > + bdp++;
> > + }
> > +
> > + bdp = fep->rx_bd_base;
> > + dev_info(&dev->dev, " rx: %lu buffers\n",
> > + (unsigned long)RX_RING_SIZE);
> > + for (i = 0 ; i < RX_RING_SIZE; i++) {
> > + dev_info(&dev->dev, " %08lx: %04x %04x
> > %08x\n",
> > + (kernel_ulong_t)bdp,
> > + bdp->cbd_sc, bdp->cbd_datlen,
> > + (int)bdp->cbd_bufaddr);
> > + bdp++;
> > + }
> > + spin_unlock_bh(&fep->hw_lock);
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Johanna Denk,
Tabea Lutz HRB 165235 Munich, Office: Kirchenstr.5, D-82194
Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Fri, Jul 25, 2025 at 12:33:12AM +0200, Lukasz Majewski wrote:
> This patch provides callbacks for struct net_device_ops for MTIP
> L2 switch.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
...
> +static netdev_tx_t mtip_start_xmit_port(struct sk_buff *skb,
> + struct net_device *dev, int port)
> +{
> + struct mtip_ndev_priv *priv = netdev_priv(dev);
> + struct switch_enet_private *fep = priv->fep;
> + unsigned short status;
> + struct cbd_t *bdp;
> + void *bufaddr;
> +
> + spin_lock_bh(&fep->hw_lock);
> +
> + if (!fep->link[0] && !fep->link[1]) {
> + /* Link is down or autonegotiation is in progress. */
> + netif_stop_queue(dev);
> + spin_unlock_bh(&fep->hw_lock);
> + return NETDEV_TX_BUSY;
> + }
> +
> + /* Fill in a Tx ring entry */
> + bdp = fep->cur_tx;
> + status = bdp->cbd_sc;
> +
> + if (status & BD_ENET_TX_READY) {
> + /* All transmit buffers are full. Bail out.
> + * This should not happen, since dev->tbusy should be set.
> + */
> + netif_stop_queue(dev);
> + dev_err_ratelimited(&fep->pdev->dev, "%s: tx queue full!.\n",
> + dev->name);
> + spin_unlock(&fep->hw_lock);
Sorry be the one to point out this needle in a haystack,
but this should be spin_unlock_bh()
Flagged by Smatch.
> + return NETDEV_TX_BUSY;
> + }
> +
> + /* Clear all of the status flags */
> + status &= ~BD_ENET_TX_STATS;
> +
> + /* Set buffer length and buffer pointer */
> + bufaddr = skb->data;
> + bdp->cbd_datlen = skb->len;
> +
> + /* On some FEC implementations data must be aligned on
> + * 4-byte boundaries. Use bounce buffers to copy data
> + * and get it aligned.spin
> + */
> + if ((unsigned long)bufaddr & MTIP_ALIGNMENT) {
> + unsigned int index;
> +
> + index = bdp - fep->tx_bd_base;
> + memcpy(fep->tx_bounce[index], skb->data, skb->len);
> + bufaddr = fep->tx_bounce[index];
> + }
> +
> + if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> + swap_buffer(bufaddr, skb->len);
> +
> + /* 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);
> + if (unlikely(dma_mapping_error(&fep->pdev->dev, bdp->cbd_bufaddr))) {
> + dev_err(&fep->pdev->dev,
> + "Failed to map descriptor tx buffer\n");
> + dev->stats.tx_dropped++;
> + dev_kfree_skb_any(skb);
> + goto err;
> + }
> +
> + /* Save skb pointer. */
> + fep->tx_skbuff[fep->skb_cur] = skb;
> + fep->skb_cur = (fep->skb_cur + 1) & TX_RING_MOD_MASK;
> +
> + /* Send it on its way. Tell FEC it's ready, interrupt when done,
> + * it's the last BD of the frame, and to put the CRC on the end.
> + */
> +
> + status |= (BD_ENET_TX_READY | BD_ENET_TX_INTR | BD_ENET_TX_LAST |
> + BD_ENET_TX_TC);
> +
> + /* Synchronize all descriptor writes */
> + wmb();
> + bdp->cbd_sc = status;
> +
> + skb_tx_timestamp(skb);
> +
> + /* Trigger transmission start */
> + writel(MCF_ESW_TDAR_X_DES_ACTIVE, fep->hwp + ESW_TDAR);
> +
> + dev->stats.tx_bytes += skb->len;
> + /* If this was the last BD in the ring,
> + * start at the beginning again.
> + */
> + if (status & BD_ENET_TX_WRAP)
> + bdp = fep->tx_bd_base;
> + else
> + bdp++;
> +
> + if (bdp == fep->dirty_tx) {
> + fep->tx_full = 1;
> + netif_stop_queue(dev);
> + }
> +
> + fep->cur_tx = bdp;
> + err:
> + spin_unlock_bh(&fep->hw_lock);
> +
> + return NETDEV_TX_OK;
> +}
...
Hi Simon,
> On Fri, Jul 25, 2025 at 12:33:12AM +0200, Lukasz Majewski wrote:
> > This patch provides callbacks for struct net_device_ops for MTIP
> > L2 switch.
> >
> > Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> ...
>
> > +static netdev_tx_t mtip_start_xmit_port(struct sk_buff *skb,
> > + struct net_device *dev,
> > int port) +{
> > + struct mtip_ndev_priv *priv = netdev_priv(dev);
> > + struct switch_enet_private *fep = priv->fep;
> > + unsigned short status;
> > + struct cbd_t *bdp;
> > + void *bufaddr;
> > +
> > + spin_lock_bh(&fep->hw_lock);
> > +
> > + if (!fep->link[0] && !fep->link[1]) {
> > + /* Link is down or autonegotiation is in progress.
> > */
> > + netif_stop_queue(dev);
> > + spin_unlock_bh(&fep->hw_lock);
> > + return NETDEV_TX_BUSY;
> > + }
> > +
> > + /* Fill in a Tx ring entry */
> > + bdp = fep->cur_tx;
> > + status = bdp->cbd_sc;
> > +
> > + if (status & BD_ENET_TX_READY) {
> > + /* All transmit buffers are full. Bail out.
> > + * This should not happen, since dev->tbusy should
> > be set.
> > + */
> > + netif_stop_queue(dev);
> > + dev_err_ratelimited(&fep->pdev->dev, "%s: tx queue
> > full!.\n",
> > + dev->name);
> > + spin_unlock(&fep->hw_lock);
>
> Sorry be the one to point out this needle in a haystack,
> but this should be spin_unlock_bh()
I must have overlooked it when working on the code.
Anyway, I need to wait if there are other comments from Jakub or Paolo
- as they had some comments (addressed already by this patch set) for
previous version.
>
> Flagged by Smatch.
>
> > + return NETDEV_TX_BUSY;
> > + }
> > +
> > + /* Clear all of the status flags */
> > + status &= ~BD_ENET_TX_STATS;
> > +
> > + /* Set buffer length and buffer pointer */
> > + bufaddr = skb->data;
> > + bdp->cbd_datlen = skb->len;
> > +
> > + /* On some FEC implementations data must be aligned on
> > + * 4-byte boundaries. Use bounce buffers to copy data
> > + * and get it aligned.spin
> > + */
> > + if ((unsigned long)bufaddr & MTIP_ALIGNMENT) {
> > + unsigned int index;
> > +
> > + index = bdp - fep->tx_bd_base;
> > + memcpy(fep->tx_bounce[index], skb->data, skb->len);
> > + bufaddr = fep->tx_bounce[index];
> > + }
> > +
> > + if (fep->quirks & FEC_QUIRK_SWAP_FRAME)
> > + swap_buffer(bufaddr, skb->len);
> > +
> > + /* 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);
> > + if (unlikely(dma_mapping_error(&fep->pdev->dev,
> > bdp->cbd_bufaddr))) {
> > + dev_err(&fep->pdev->dev,
> > + "Failed to map descriptor tx buffer\n");
> > + dev->stats.tx_dropped++;
> > + dev_kfree_skb_any(skb);
> > + goto err;
> > + }
> > +
> > + /* Save skb pointer. */
> > + fep->tx_skbuff[fep->skb_cur] = skb;
> > + fep->skb_cur = (fep->skb_cur + 1) & TX_RING_MOD_MASK;
> > +
> > + /* Send it on its way. Tell FEC it's ready, interrupt
> > when done,
> > + * it's the last BD of the frame, and to put the CRC on
> > the end.
> > + */
> > +
> > + status |= (BD_ENET_TX_READY | BD_ENET_TX_INTR |
> > BD_ENET_TX_LAST |
> > + BD_ENET_TX_TC);
> > +
> > + /* Synchronize all descriptor writes */
> > + wmb();
> > + bdp->cbd_sc = status;
> > +
> > + skb_tx_timestamp(skb);
> > +
> > + /* Trigger transmission start */
> > + writel(MCF_ESW_TDAR_X_DES_ACTIVE, fep->hwp + ESW_TDAR);
> > +
> > + dev->stats.tx_bytes += skb->len;
> > + /* If this was the last BD in the ring,
> > + * start at the beginning again.
> > + */
> > + if (status & BD_ENET_TX_WRAP)
> > + bdp = fep->tx_bd_base;
> > + else
> > + bdp++;
> > +
> > + if (bdp == fep->dirty_tx) {
> > + fep->tx_full = 1;
> > + netif_stop_queue(dev);
> > + }
> > +
> > + fep->cur_tx = bdp;
> > + err:
> > + spin_unlock_bh(&fep->hw_lock);
> > +
> > + return NETDEV_TX_OK;
> > +}
>
> ...
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Johanna Denk,
Tabea Lutz HRB 165235 Munich, Office: Kirchenstr.5, D-82194
Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
© 2016 - 2025 Red Hat, Inc.