gianfar: Fix Tx int miss, dont write IC on-the-fly

Programming the interrupt coalescing (IC) registers while
the controller/DMA is on may incur the loss of one Tx
confirmation interrupt, under certain conditions.  This is
a subtle hw race because it does not occur during a burst
of Tx packets.  It has been observed on p2020 devices that,
if just one packet is being xmit'ed, the Tx confirmation
doesn't trigger and BQL evetually blocks the Tx queues,
followed by Tx timeout and an un-responsive device.
This issue was not apparent prior to introducing BQL
support, as a late Tx confirmation was not an issue back then
and the next burst of Tx frames would have triggered the
Tx confirmation/ Tx ring cleanup anyway.

Bottom line, the hw specifications state that the IC registers
should not be programmed while the Rx/Tx blocks (the DMA) are
enabled. Further more, these registers are currently re-written
with the same values on the processing path, over and over again.
To fix this, rewriting the IC registers has been removed from
the processing path (napi poll).  A complete MAC reset procedure
has been implemented for the ethtool -c option instead, to
reliably update these registers while the controller is stopped.

Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 4eac25f66605fecd24408a71f6411516b0f115fc..c5b9320f76298678b94291040c7412ae458dc5ea 100644 (file)
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -412,6 +412,47 @@ static void gfar_mac_tx_config(struct gfar_private *priv)
        gfar_write(&regs->tctrl, tctrl);
 }
 
+static void gfar_configure_coalescing(struct gfar_private *priv,
+                              unsigned long tx_mask, unsigned long rx_mask)
+{
+       struct gfar __iomem *regs = priv->gfargrp[0].regs;
+       u32 __iomem *baddr;
+
+       if (priv->mode == MQ_MG_MODE) {
+               int i = 0;
+
+               baddr = &regs->txic0;
+               for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
+                       gfar_write(baddr + i, 0);
+                       if (likely(priv->tx_queue[i]->txcoalescing))
+                               gfar_write(baddr + i, priv->tx_queue[i]->txic);
+               }
+
+               baddr = &regs->rxic0;
+               for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
+                       gfar_write(baddr + i, 0);
+                       if (likely(priv->rx_queue[i]->rxcoalescing))
+                               gfar_write(baddr + i, priv->rx_queue[i]->rxic);
+               }
+       } else {
+               /* Backward compatible case -- even if we enable
+                * multiple queues, there's only single reg to program
+                */
+               gfar_write(&regs->txic, 0);
+               if (likely(priv->tx_queue[0]->txcoalescing))
+                       gfar_write(&regs->txic, priv->tx_queue[0]->txic);
+
+               gfar_write(&regs->rxic, 0);
+               if (unlikely(priv->rx_queue[0]->rxcoalescing))
+                       gfar_write(&regs->rxic, priv->rx_queue[0]->rxic);
+       }
+}
+
+void gfar_configure_coalescing_all(struct gfar_private *priv)
+{
+       gfar_configure_coalescing(priv, 0xFF, 0xFF);
+}
+
 static struct net_device_stats *gfar_get_stats(struct net_device *dev)
 {
        struct gfar_private *priv = netdev_priv(dev);
@@ -1837,47 +1878,6 @@ void gfar_start(struct gfar_private *priv)
        priv->ndev->trans_start = jiffies; /* prevent tx timeout */
 }
 
-static void gfar_configure_coalescing(struct gfar_private *priv,
-                              unsigned long tx_mask, unsigned long rx_mask)
-{
-       struct gfar __iomem *regs = priv->gfargrp[0].regs;
-       u32 __iomem *baddr;
-
-       if (priv->mode == MQ_MG_MODE) {
-               int i = 0;
-
-               baddr = &regs->txic0;
-               for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
-                       gfar_write(baddr + i, 0);
-                       if (likely(priv->tx_queue[i]->txcoalescing))
-                               gfar_write(baddr + i, priv->tx_queue[i]->txic);
-               }
-
-               baddr = &regs->rxic0;
-               for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
-                       gfar_write(baddr + i, 0);
-                       if (likely(priv->rx_queue[i]->rxcoalescing))
-                               gfar_write(baddr + i, priv->rx_queue[i]->rxic);
-               }
-       } else {
-               /* Backward compatible case -- even if we enable
-                * multiple queues, there's only single reg to program
-                */
-               gfar_write(&regs->txic, 0);
-               if (likely(priv->tx_queue[0]->txcoalescing))
-                       gfar_write(&regs->txic, priv->tx_queue[0]->txic);
-
-               gfar_write(&regs->rxic, 0);
-               if (unlikely(priv->rx_queue[0]->rxcoalescing))
-                       gfar_write(&regs->rxic, priv->rx_queue[0]->rxic);
-       }
-}
-
-void gfar_configure_coalescing_all(struct gfar_private *priv)
-{
-       gfar_configure_coalescing(priv, 0xFF, 0xFF);
-}
-
 static void free_grp_irqs(struct gfar_priv_grp *grp)
 {
        free_irq(gfar_irq(grp, TX)->irq, grp);
@@ -2812,17 +2812,6 @@ static int gfar_poll_sq(struct napi_struct *napi, int budget)
                gfar_write(&regs->rstat, gfargrp->rstat);
 
                gfar_write(&regs->imask, IMASK_DEFAULT);
-
-               /* If we are coalescing interrupts, update the timer
-                * Otherwise, clear it
-                */
-               gfar_write(&regs->txic, 0);
-               if (likely(tx_queue->txcoalescing))
-                       gfar_write(&regs->txic, tx_queue->txic);
-
-               gfar_write(&regs->rxic, 0);
-               if (unlikely(rx_queue->rxcoalescing))
-                       gfar_write(&regs->rxic, rx_queue->rxic);
        }
 
        return work_done;
@@ -2892,12 +2881,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
                gfar_write(&regs->rstat, gfargrp->rstat);
 
                gfar_write(&regs->imask, IMASK_DEFAULT);
-
-               /* If we are coalescing interrupts, update the timer
-                * Otherwise, clear it
-                */
-               gfar_configure_coalescing(priv, gfargrp->rx_bit_map,
-                                         gfargrp->tx_bit_map);
        }
 
        return work_done;

diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 45219d4d09b412be287895a37df7c3c87b2ad96e..891dbee6e6c14d2394cc2dff00092f448faf3dc2 100644 (file)
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -360,25 +360,11 @@ static int gfar_scoalesce(struct net_device *dev,
                          struct ethtool_coalesce *cvals)
 {
        struct gfar_private *priv = netdev_priv(dev);
-       int i = 0;
+       int i, err = 0;
 
        if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_COALESCE))
                return -EOPNOTSUPP;
 
-       /* Set up rx coalescing */
-       /* As of now, we will enable/disable coalescing for all
-        * queues together in case of eTSEC2, this will be modified
-        * along with the ethtool interface
-        */
-       if ((cvals->rx_coalesce_usecs == 0) ||
-           (cvals->rx_max_coalesced_frames == 0)) {
-               for (i = 0; i < priv->num_rx_queues; i++)
-                       priv->rx_queue[i]->rxcoalescing = 0;
-       } else {
-               for (i = 0; i < priv->num_rx_queues; i++)
-                       priv->rx_queue[i]->rxcoalescing = 1;
-       }
-
        if (NULL == priv->phydev)
                return -ENODEV;
 
@@ -395,6 +381,32 @@ static int gfar_scoalesce(struct net_device *dev,
                return -EINVAL;
        }
 
+       /* Check the bounds of the values */
+       if (cvals->tx_coalesce_usecs > GFAR_MAX_COAL_USECS) {
+               netdev_info(dev, "Coalescing is limited to %d microseconds\n",
+                           GFAR_MAX_COAL_USECS);
+               return -EINVAL;
+       }
+
+       if (cvals->tx_max_coalesced_frames > GFAR_MAX_COAL_FRAMES) {
+               netdev_info(dev, "Coalescing is limited to %d frames\n",
+                           GFAR_MAX_COAL_FRAMES);
+               return -EINVAL;
+       }
+
+       while (test_and_set_bit_lock(GFAR_RESETTING, &priv->state))
+               cpu_relax();
+
+       /* Set up rx coalescing */
+       if ((cvals->rx_coalesce_usecs == 0) ||
+           (cvals->rx_max_coalesced_frames == 0)) {
+               for (i = 0; i < priv->num_rx_queues; i++)
+                       priv->rx_queue[i]->rxcoalescing = 0;
+       } else {
+               for (i = 0; i < priv->num_rx_queues; i++)
+                       priv->rx_queue[i]->rxcoalescing = 1;
+       }
+
        for (i = 0; i < priv->num_rx_queues; i++) {
                priv->rx_queue[i]->rxic = mk_ic_value(
                        cvals->rx_max_coalesced_frames,
@@ -411,28 +423,22 @@ static int gfar_scoalesce(struct net_device *dev,
                        priv->tx_queue[i]->txcoalescing = 1;
        }
 
-       /* Check the bounds of the values */
-       if (cvals->tx_coalesce_usecs > GFAR_MAX_COAL_USECS) {
-               netdev_info(dev, "Coalescing is limited to %d microseconds\n",
-                           GFAR_MAX_COAL_USECS);
-               return -EINVAL;
-       }
-
-       if (cvals->tx_max_coalesced_frames > GFAR_MAX_COAL_FRAMES) {
-               netdev_info(dev, "Coalescing is limited to %d frames\n",
-                           GFAR_MAX_COAL_FRAMES);
-               return -EINVAL;
-       }
-
        for (i = 0; i < priv->num_tx_queues; i++) {
                priv->tx_queue[i]->txic = mk_ic_value(
                        cvals->tx_max_coalesced_frames,
                        gfar_usecs2ticks(priv, cvals->tx_coalesce_usecs));
        }
 
-       gfar_configure_coalescing_all(priv);
+       if (dev->flags & IFF_UP) {
+               stop_gfar(dev);
+               err = startup_gfar(dev);
+       } else {
+               gfar_mac_reset(priv);
+       }
 
-       return 0;
+       clear_bit_unlock(GFAR_RESETTING, &priv->state);
+
+       return err;
 }
 
 /* Fills in rvals with the current ring parameters.  Currently,