stmmac: fix concurrency in eee initialization.
authorGiuseppe CAVALLARO <peppe.cavallaro@st.com>
Tue, 4 Nov 2014 16:08:08 +0000 (17:08 +0100)
committerDavid S. Miller <davem@davemloft.net>
Wed, 5 Nov 2014 21:22:57 +0000 (16:22 -0500)
This patch aims to fix the concurrency in eee initialization
inside the stmmac driver and related warnings when enable
DEBUG_ATOMIC_SLEEP.

Prior this patch, the stmmac_eee_init could be called in several places
as shown below:

stmmac_open  stmmac_resume         PHY Layer
    |            |                     |
  stmmac_hw_setup           stmmac_adjust_link
    |                                  |           stmmac ethtool
    |__________________________|______________|
                                       |
                                 stmmac_eee_init

The patch removes the stmmac_eee_init call inside the stmmac_hw_setup
that is unnecessary. It is sufficient to call it in the adjust_link to
always guarantee that EEE is always configured at mac level too.

Fixing the lock protection now it is covered another case (not
considered before). The stmmac_eee_init could be called by the ethtool
so critical sections must be protected inside this function too.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

index 27598738c9cd667001342c6e6a28dadd1ba3fa21..9c79bf23d0b8bb4c93b47ffc912d5ddaaadda2e2 100644 (file)
@@ -276,6 +276,7 @@ static void stmmac_eee_ctrl_timer(unsigned long arg)
 bool stmmac_eee_init(struct stmmac_priv *priv)
 {
        char *phy_bus_name = priv->plat->phy_bus_name;
+       unsigned long flags;
        bool ret = false;
 
        /* Using PCS we cannot dial with the phy registers at this stage
@@ -300,6 +301,7 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
                         * changed).
                         * In that case the driver disable own timers.
                         */
+                       spin_lock_irqsave(&priv->lock, flags);
                        if (priv->eee_active) {
                                pr_debug("stmmac: disable EEE\n");
                                del_timer_sync(&priv->eee_ctrl_timer);
@@ -307,9 +309,11 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
                                                             tx_lpi_timer);
                        }
                        priv->eee_active = 0;
+                       spin_unlock_irqrestore(&priv->lock, flags);
                        goto out;
                }
                /* Activate the EEE and start timers */
+               spin_lock_irqsave(&priv->lock, flags);
                if (!priv->eee_active) {
                        priv->eee_active = 1;
                        init_timer(&priv->eee_ctrl_timer);
@@ -325,9 +329,10 @@ bool stmmac_eee_init(struct stmmac_priv *priv)
                /* Set HW EEE according to the speed */
                priv->hw->mac->set_eee_pls(priv->hw, priv->phydev->link);
 
-               pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
-
                ret = true;
+               spin_unlock_irqrestore(&priv->lock, flags);
+
+               pr_debug("stmmac: Energy-Efficient Ethernet initialized\n");
        }
 out:
        return ret;
@@ -760,12 +765,12 @@ static void stmmac_adjust_link(struct net_device *dev)
        if (new_state && netif_msg_link(priv))
                phy_print_status(phydev);
 
+       spin_unlock_irqrestore(&priv->lock, flags);
+
        /* At this stage, it could be needed to setup the EEE or adjust some
         * MAC related HW registers.
         */
        priv->eee_enabled = stmmac_eee_init(priv);
-
-       spin_unlock_irqrestore(&priv->lock, flags);
 }
 
 /**
@@ -1705,8 +1710,6 @@ static int stmmac_hw_setup(struct net_device *dev)
        }
        priv->tx_lpi_timer = STMMAC_DEFAULT_TWT_LS;
 
-       priv->eee_enabled = stmmac_eee_init(priv);
-
        stmmac_init_tx_coalesce(priv);
 
        if ((priv->use_riwt) && (priv->hw->dma->rx_watchdog)) {