Skip to content

Commit

Permalink
gianfar: Fix Tx int miss, dont write IC on-the-fly
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Claudiu Manoil authored and David S. Miller committed Feb 25, 2014
1 parent 0851133 commit f19015b
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 88 deletions.
99 changes: 41 additions & 58 deletions drivers/net/ethernet/freescale/gianfar.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
66 changes: 36 additions & 30 deletions drivers/net/ethernet/freescale/gianfar_ethtool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit f19015b

Please sign in to comment.