3.16.46-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Doug Berger <[email protected]>

commit 07c52d6a0b955a8a28834f9354793cfc4b81d0e9 upstream.

Add a spinlock to ensure that irq0_stat is not unintentionally altered
as the result of preemption.  Also removed unserviced irq0 interrupts
and removed irq1_stat since there is no bottom half service for those
interrupts.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Signed-off-by: Doug Berger <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
[bwh: Backported to 3.16:
 - bcmgenet_irq_task() only handles UMAC_IRQ_LINK_UP and UMAC_IRQ_LINK_DOWN
   status flags
 - Adjust context]
Signed-off-by: Ben Hutchings <[email protected]>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 73 ++++++++++++++------------
 drivers/net/ethernet/broadcom/genet/bcmgenet.h |  6 ++-
 2 files changed, 44 insertions(+), 35 deletions(-)

--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1858,42 +1858,46 @@ static int bcmgenet_poll(struct napi_str
 /* Interrupt bottom half */
 static void bcmgenet_irq_task(struct work_struct *work)
 {
+       unsigned long flags;
+       unsigned int status;
        struct bcmgenet_priv *priv = container_of(
                        work, struct bcmgenet_priv, bcmgenet_irq_work);
 
        netif_dbg(priv, intr, priv->dev, "%s\n", __func__);
 
+       spin_lock_irqsave(&priv->lock, flags);
+       status = priv->irq0_stat;
+       priv->irq0_stat = 0;
+       spin_unlock_irqrestore(&priv->lock, flags);
+
        /* Link UP/DOWN event */
        if ((priv->hw_params->flags & GENET_HAS_MDIO_INTR) &&
-               (priv->irq0_stat & (UMAC_IRQ_LINK_UP|UMAC_IRQ_LINK_DOWN))) {
+               (status & (UMAC_IRQ_LINK_UP|UMAC_IRQ_LINK_DOWN)))
                phy_mac_interrupt(priv->phydev,
-                       priv->irq0_stat & UMAC_IRQ_LINK_UP);
-               priv->irq0_stat &= ~(UMAC_IRQ_LINK_UP|UMAC_IRQ_LINK_DOWN);
-       }
+                       status & UMAC_IRQ_LINK_UP);
 }
 
 /* bcmgenet_isr1: interrupt handler for ring buffer. */
 static irqreturn_t bcmgenet_isr1(int irq, void *dev_id)
 {
        struct bcmgenet_priv *priv = dev_id;
-       unsigned int index;
+       unsigned int index, status;
 
-       /* Save irq status for bottom-half processing. */
-       priv->irq1_stat =
-               bcmgenet_intrl2_1_readl(priv, INTRL2_CPU_STAT) &
+       /* Read irq status */
+       status = bcmgenet_intrl2_1_readl(priv, INTRL2_CPU_STAT) &
                ~priv->int1_mask;
        /* clear inerrupts*/
-       bcmgenet_intrl2_1_writel(priv, priv->irq1_stat, INTRL2_CPU_CLEAR);
+       bcmgenet_intrl2_1_writel(priv, status, INTRL2_CPU_CLEAR);
 
        netif_dbg(priv, intr, priv->dev,
-               "%s: IRQ=0x%x\n", __func__, priv->irq1_stat);
+               "%s: IRQ=0x%x\n", __func__, status);
        /* Check the MBDONE interrupts.
         * packet is done, reclaim descriptors
         */
-       if (priv->irq1_stat & 0x0000ffff) {
+       if (status & 0x0000ffff) {
                index = 0;
                for (index = 0; index < 16; index++) {
-                       if (priv->irq1_stat & (1 << index))
+                       if (status & (1 << index))
                                bcmgenet_tx_reclaim(priv->dev,
                                                &priv->tx_rings[index]);
                }
@@ -1905,18 +1909,19 @@ static irqreturn_t bcmgenet_isr1(int irq
 static irqreturn_t bcmgenet_isr0(int irq, void *dev_id)
 {
        struct bcmgenet_priv *priv = dev_id;
+       unsigned int status;
+       unsigned long flags;
 
-       /* Save irq status for bottom-half processing. */
-       priv->irq0_stat =
-               bcmgenet_intrl2_0_readl(priv, INTRL2_CPU_STAT) &
+       /* Read irq status */
+       status = bcmgenet_intrl2_0_readl(priv, INTRL2_CPU_STAT) &
                ~bcmgenet_intrl2_0_readl(priv, INTRL2_CPU_MASK_STATUS);
        /* clear inerrupts*/
-       bcmgenet_intrl2_0_writel(priv, priv->irq0_stat, INTRL2_CPU_CLEAR);
+       bcmgenet_intrl2_0_writel(priv, status, INTRL2_CPU_CLEAR);
 
        netif_dbg(priv, intr, priv->dev,
-               "IRQ=0x%x\n", priv->irq0_stat);
+               "IRQ=0x%x\n", status);
 
-       if (priv->irq0_stat & (UMAC_IRQ_RXDMA_BDONE | UMAC_IRQ_RXDMA_PDONE)) {
+       if (status & (UMAC_IRQ_RXDMA_BDONE | UMAC_IRQ_RXDMA_PDONE)) {
                /* We use NAPI(software interrupt throttling, if
                 * Rx Descriptor throttling is not used.
                 * Disable interrupt, will be enabled in the poll method.
@@ -1927,28 +1932,27 @@ static irqreturn_t bcmgenet_isr0(int irq
                        __napi_schedule(&priv->napi);
                }
        }
-       if (priv->irq0_stat &
+       if (status &
                        (UMAC_IRQ_TXDMA_BDONE | UMAC_IRQ_TXDMA_PDONE)) {
                /* Tx reclaim */
                bcmgenet_tx_reclaim(priv->dev, &priv->tx_rings[DESC_INDEX]);
        }
-       if (priv->irq0_stat & (UMAC_IRQ_PHY_DET_R |
-                               UMAC_IRQ_PHY_DET_F |
-                               UMAC_IRQ_LINK_UP |
-                               UMAC_IRQ_LINK_DOWN |
-                               UMAC_IRQ_HFB_SM |
-                               UMAC_IRQ_HFB_MM |
-                               UMAC_IRQ_MPD_R)) {
-               /* all other interested interrupts handled in bottom half */
-               schedule_work(&priv->bcmgenet_irq_work);
-       }
-
        if ((priv->hw_params->flags & GENET_HAS_MDIO_INTR) &&
-               priv->irq0_stat & (UMAC_IRQ_MDIO_DONE | UMAC_IRQ_MDIO_ERROR)) {
-               priv->irq0_stat &= ~(UMAC_IRQ_MDIO_DONE | UMAC_IRQ_MDIO_ERROR);
+               status & (UMAC_IRQ_MDIO_DONE | UMAC_IRQ_MDIO_ERROR)) {
                wake_up(&priv->wq);
        }
 
+       /* all other interested interrupts handled in bottom half */
+       status &= UMAC_IRQ_LINK_UP | UMAC_IRQ_LINK_DOWN;
+       if (status) {
+               /* Save irq status for bottom-half processing. */
+               spin_lock_irqsave(&priv->lock, flags);
+               priv->irq0_stat |= status;
+               spin_unlock_irqrestore(&priv->lock, flags);
+
+               schedule_work(&priv->bcmgenet_irq_work);
+       }
+
        return IRQ_HANDLED;
 }
 
@@ -2531,6 +2535,8 @@ static int bcmgenet_probe(struct platfor
                goto err;
        }
 
+       spin_lock_init(&priv->lock);
+
        SET_NETDEV_DEV(dev, &pdev->dev);
        dev_set_drvdata(&pdev->dev, dev);
        ether_addr_copy(dev->dev_addr, macaddr);
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -571,8 +571,10 @@ struct bcmgenet_priv {
        struct work_struct bcmgenet_irq_work;
        int irq0;
        int irq1;
+
+       /* shared status */
+       spinlock_t lock;
        unsigned int irq0_stat;
-       unsigned int irq1_stat;
 
        /* HW descriptors/checksum variables */
        bool desc_64b_en;

Reply via email to