From 1bd33bf0fe6d3012410db0302187199871b510a0 Mon Sep 17 00:00:00 2001 From: Esben Haabendal Date: Thu, 23 May 2019 14:02:20 +0200 Subject: net: ll_temac: Prepare indirect register access for multicast support With .ndo_set_rx_mode/temac_set_multicast_list() being called in atomic context (holding addr_list_lock), and temac_set_multicast_list() needing to access temac indirect registers, the mutex used to synchronize indirect register is a no-no. Replace it with a spinlock, and avoid sleeping in temac_indirect_busywait(). To avoid excessive holding of the lock, which is now a spinlock, the temac_device_reset() function is changed to only hold the lock for short periods. With timeouts, it could be holding the spinlock for more than 2 seconds. Signed-off-by: Esben Haabendal Signed-off-by: David S. Miller --- drivers/net/ethernet/xilinx/ll_temac.h | 5 +- drivers/net/ethernet/xilinx/ll_temac_main.c | 240 +++++++++++++++++++--------- drivers/net/ethernet/xilinx/ll_temac_mdio.c | 20 ++- 3 files changed, 177 insertions(+), 88 deletions(-) (limited to 'drivers') diff --git a/drivers/net/ethernet/xilinx/ll_temac.h b/drivers/net/ethernet/xilinx/ll_temac.h index 1aeda084b8f1..276292bca334 100644 --- a/drivers/net/ethernet/xilinx/ll_temac.h +++ b/drivers/net/ethernet/xilinx/ll_temac.h @@ -361,7 +361,7 @@ struct temac_local { /* For synchronization of indirect register access. Must be * shared mutex between interfaces in same TEMAC block. */ - struct mutex *indirect_mutex; + spinlock_t *indirect_lock; u32 options; /* Current options word */ int last_link; unsigned int temac_features; @@ -388,8 +388,9 @@ struct temac_local { /* xilinx_temac.c */ int temac_indirect_busywait(struct temac_local *lp); u32 temac_indirect_in32(struct temac_local *lp, int reg); +u32 temac_indirect_in32_locked(struct temac_local *lp, int reg); void temac_indirect_out32(struct temac_local *lp, int reg, u32 value); - +void temac_indirect_out32_locked(struct temac_local *lp, int reg, u32 value); /* xilinx_temac_mdio.c */ int temac_mdio_setup(struct temac_local *lp, struct platform_device *pdev); diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c b/drivers/net/ethernet/xilinx/ll_temac_main.c index 65fb549241b2..cc58bd8c12f6 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_main.c +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include "ll_temac.h" @@ -84,51 +85,118 @@ static void _temac_iow_le(struct temac_local *lp, int offset, u32 value) return iowrite32(value, lp->regs + offset); } +static bool hard_acs_rdy(struct temac_local *lp) +{ + return temac_ior(lp, XTE_RDY0_OFFSET) & XTE_RDY0_HARD_ACS_RDY_MASK; +} + +static bool hard_acs_rdy_or_timeout(struct temac_local *lp, ktime_t timeout) +{ + ktime_t cur = ktime_get(); + + return hard_acs_rdy(lp) || ktime_after(cur, timeout); +} + +/* Poll for maximum 20 ms. This is similar to the 2 jiffies @ 100 Hz + * that was used before, and should cover MDIO bus speed down to 3200 + * Hz. + */ +#define HARD_ACS_RDY_POLL_NS (20 * NSEC_PER_MSEC) + +/** + * temac_indirect_busywait - Wait for current indirect register access + * to complete. + */ int temac_indirect_busywait(struct temac_local *lp) { - unsigned long end = jiffies + 2; + ktime_t timeout = ktime_add_ns(ktime_get(), HARD_ACS_RDY_POLL_NS); - while (!(temac_ior(lp, XTE_RDY0_OFFSET) & XTE_RDY0_HARD_ACS_RDY_MASK)) { - if (time_before_eq(end, jiffies)) { - WARN_ON(1); - return -ETIMEDOUT; - } - usleep_range(500, 1000); - } - return 0; + spin_until_cond(hard_acs_rdy_or_timeout(lp, timeout)); + if (WARN_ON(!hard_acs_rdy(lp))) + return -ETIMEDOUT; + else + return 0; } /** - * temac_indirect_in32 - * - * lp->indirect_mutex must be held when calling this function + * temac_indirect_in32 - Indirect register read access. This function + * must be called without lp->indirect_lock being held. */ u32 temac_indirect_in32(struct temac_local *lp, int reg) { - u32 val; + unsigned long flags; + int val; + + spin_lock_irqsave(lp->indirect_lock, flags); + val = temac_indirect_in32_locked(lp, reg); + spin_unlock_irqrestore(lp->indirect_lock, flags); + return val; +} - if (temac_indirect_busywait(lp)) +/** + * temac_indirect_in32_locked - Indirect register read access. This + * function must be called with lp->indirect_lock being held. Use + * this together with spin_lock_irqsave/spin_lock_irqrestore to avoid + * repeated lock/unlock and to ensure uninterrupted access to indirect + * registers. + */ +u32 temac_indirect_in32_locked(struct temac_local *lp, int reg) +{ + /* This initial wait should normally not spin, as we always + * try to wait for indirect access to complete before + * releasing the indirect_lock. + */ + if (WARN_ON(temac_indirect_busywait(lp))) return -ETIMEDOUT; + /* Initiate read from indirect register */ temac_iow(lp, XTE_CTL0_OFFSET, reg); - if (temac_indirect_busywait(lp)) + /* Wait for indirect register access to complete. We really + * should not see timeouts, and could even end up causing + * problem for following indirect access, so let's make a bit + * of WARN noise. + */ + if (WARN_ON(temac_indirect_busywait(lp))) return -ETIMEDOUT; - val = temac_ior(lp, XTE_LSW0_OFFSET); - - return val; + /* Value is ready now */ + return temac_ior(lp, XTE_LSW0_OFFSET); } /** - * temac_indirect_out32 - * - * lp->indirect_mutex must be held when calling this function + * temac_indirect_out32 - Indirect register write access. This function + * must be called without lp->indirect_lock being held. */ void temac_indirect_out32(struct temac_local *lp, int reg, u32 value) { - if (temac_indirect_busywait(lp)) + unsigned long flags; + + spin_lock_irqsave(lp->indirect_lock, flags); + temac_indirect_out32_locked(lp, reg, value); + spin_unlock_irqrestore(lp->indirect_lock, flags); +} + +/** + * temac_indirect_out32_locked - Indirect register write access. This + * function must be called with lp->indirect_lock being held. Use + * this together with spin_lock_irqsave/spin_lock_irqrestore to avoid + * repeated lock/unlock and to ensure uninterrupted access to indirect + * registers. + */ +void temac_indirect_out32_locked(struct temac_local *lp, int reg, u32 value) +{ + /* As in temac_indirect_in32_locked(), we should normally not + * spin here. And if it happens, we actually end up silently + * ignoring the write request. Ouch. + */ + if (WARN_ON(temac_indirect_busywait(lp))) return; + /* Initiate write to indirect register */ temac_iow(lp, XTE_LSW0_OFFSET, value); temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg); - temac_indirect_busywait(lp); + /* As in temac_indirect_in32_locked(), we should not see timeouts + * here. And if it happens, we continue before the write has + * completed. Not good. + */ + WARN_ON(temac_indirect_busywait(lp)); } /** @@ -344,20 +412,21 @@ out: static void temac_do_set_mac_address(struct net_device *ndev) { struct temac_local *lp = netdev_priv(ndev); + unsigned long flags; /* set up unicast MAC address filter set its mac address */ - mutex_lock(lp->indirect_mutex); - temac_indirect_out32(lp, XTE_UAW0_OFFSET, - (ndev->dev_addr[0]) | - (ndev->dev_addr[1] << 8) | - (ndev->dev_addr[2] << 16) | - (ndev->dev_addr[3] << 24)); + spin_lock_irqsave(lp->indirect_lock, flags); + temac_indirect_out32_locked(lp, XTE_UAW0_OFFSET, + (ndev->dev_addr[0]) | + (ndev->dev_addr[1] << 8) | + (ndev->dev_addr[2] << 16) | + (ndev->dev_addr[3] << 24)); /* There are reserved bits in EUAW1 * so don't affect them Set MAC bits [47:32] in EUAW1 */ - temac_indirect_out32(lp, XTE_UAW1_OFFSET, - (ndev->dev_addr[4] & 0x000000ff) | - (ndev->dev_addr[5] << 8)); - mutex_unlock(lp->indirect_mutex); + temac_indirect_out32_locked(lp, XTE_UAW1_OFFSET, + (ndev->dev_addr[4] & 0x000000ff) | + (ndev->dev_addr[5] << 8)); + spin_unlock_irqrestore(lp->indirect_lock, flags); } static int temac_init_mac_address(struct net_device *ndev, const void *address) @@ -383,42 +452,56 @@ static int temac_set_mac_address(struct net_device *ndev, void *p) static void temac_set_multicast_list(struct net_device *ndev) { struct temac_local *lp = netdev_priv(ndev); - u32 multi_addr_msw, multi_addr_lsw, val; + u32 multi_addr_msw, multi_addr_lsw; int i; + unsigned long flags; + bool promisc_mode_disabled = false; - mutex_lock(lp->indirect_mutex); - if (ndev->flags & (IFF_ALLMULTI | IFF_PROMISC) || - netdev_mc_count(ndev) > MULTICAST_CAM_TABLE_NUM) { + if (ndev->flags & (IFF_PROMISC | IFF_ALLMULTI) || + (netdev_mc_count(ndev) > MULTICAST_CAM_TABLE_NUM)) { temac_indirect_out32(lp, XTE_AFM_OFFSET, XTE_AFM_EPPRM_MASK); dev_info(&ndev->dev, "Promiscuous mode enabled.\n"); - } else if (!netdev_mc_empty(ndev)) { + return; + } + + spin_lock_irqsave(lp->indirect_lock, flags); + + if (!netdev_mc_empty(ndev)) { struct netdev_hw_addr *ha; i = 0; netdev_for_each_mc_addr(ha, ndev) { - if (i >= MULTICAST_CAM_TABLE_NUM) + if (WARN_ON(i >= MULTICAST_CAM_TABLE_NUM)) break; multi_addr_msw = ((ha->addr[3] << 24) | (ha->addr[2] << 16) | (ha->addr[1] << 8) | (ha->addr[0])); - temac_indirect_out32(lp, XTE_MAW0_OFFSET, - multi_addr_msw); + temac_indirect_out32_locked(lp, XTE_MAW0_OFFSET, + multi_addr_msw); multi_addr_lsw = ((ha->addr[5] << 8) | (ha->addr[4]) | (i << 16)); - temac_indirect_out32(lp, XTE_MAW1_OFFSET, - multi_addr_lsw); + temac_indirect_out32_locked(lp, XTE_MAW1_OFFSET, + multi_addr_lsw); i++; } } else { - val = temac_indirect_in32(lp, XTE_AFM_OFFSET); - temac_indirect_out32(lp, XTE_AFM_OFFSET, - val & ~XTE_AFM_EPPRM_MASK); - temac_indirect_out32(lp, XTE_MAW0_OFFSET, 0); - temac_indirect_out32(lp, XTE_MAW1_OFFSET, 0); - dev_info(&ndev->dev, "Promiscuous mode disabled.\n"); + temac_indirect_out32_locked(lp, XTE_MAW0_OFFSET, 0); + temac_indirect_out32_locked(lp, XTE_MAW1_OFFSET, i << 16); + } } - mutex_unlock(lp->indirect_mutex); + + /* Enable address filter block if currently disabled */ + if (temac_indirect_in32_locked(lp, XTE_AFM_OFFSET) + & XTE_AFM_EPPRM_MASK) { + temac_indirect_out32_locked(lp, XTE_AFM_OFFSET, 0); + promisc_mode_disabled = true; + } + + spin_unlock_irqrestore(lp->indirect_lock, flags); + + if (promisc_mode_disabled) + dev_info(&ndev->dev, "Promiscuous mode disabled.\n"); } static struct temac_option { @@ -509,17 +592,19 @@ static u32 temac_setoptions(struct net_device *ndev, u32 options) struct temac_local *lp = netdev_priv(ndev); struct temac_option *tp = &temac_options[0]; int reg; + unsigned long flags; - mutex_lock(lp->indirect_mutex); + spin_lock_irqsave(lp->indirect_lock, flags); while (tp->opt) { - reg = temac_indirect_in32(lp, tp->reg) & ~tp->m_or; - if (options & tp->opt) + reg = temac_indirect_in32_locked(lp, tp->reg) & ~tp->m_or; + if (options & tp->opt) { reg |= tp->m_or; - temac_indirect_out32(lp, tp->reg, reg); + temac_indirect_out32_locked(lp, tp->reg, reg); + } tp++; } + spin_unlock_irqrestore(lp->indirect_lock, flags); lp->options |= options; - mutex_unlock(lp->indirect_mutex); return 0; } @@ -530,6 +615,7 @@ static void temac_device_reset(struct net_device *ndev) struct temac_local *lp = netdev_priv(ndev); u32 timeout; u32 val; + unsigned long flags; /* Perform a software reset */ @@ -538,7 +624,6 @@ static void temac_device_reset(struct net_device *ndev) dev_dbg(&ndev->dev, "%s()\n", __func__); - mutex_lock(lp->indirect_mutex); /* Reset the receiver and wait for it to finish reset */ temac_indirect_out32(lp, XTE_RXC1_OFFSET, XTE_RXC1_RXRST_MASK); timeout = 1000; @@ -564,8 +649,11 @@ static void temac_device_reset(struct net_device *ndev) } /* Disable the receiver */ - val = temac_indirect_in32(lp, XTE_RXC1_OFFSET); - temac_indirect_out32(lp, XTE_RXC1_OFFSET, val & ~XTE_RXC1_RXEN_MASK); + spin_lock_irqsave(lp->indirect_lock, flags); + val = temac_indirect_in32_locked(lp, XTE_RXC1_OFFSET); + temac_indirect_out32_locked(lp, XTE_RXC1_OFFSET, + val & ~XTE_RXC1_RXEN_MASK); + spin_unlock_irqrestore(lp->indirect_lock, flags); /* Reset Local Link (DMA) */ lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST); @@ -585,12 +673,12 @@ static void temac_device_reset(struct net_device *ndev) "temac_device_reset descriptor allocation failed\n"); } - temac_indirect_out32(lp, XTE_RXC0_OFFSET, 0); - temac_indirect_out32(lp, XTE_RXC1_OFFSET, 0); - temac_indirect_out32(lp, XTE_TXC_OFFSET, 0); - temac_indirect_out32(lp, XTE_FCC_OFFSET, XTE_FCC_RXFLO_MASK); - - mutex_unlock(lp->indirect_mutex); + spin_lock_irqsave(lp->indirect_lock, flags); + temac_indirect_out32_locked(lp, XTE_RXC0_OFFSET, 0); + temac_indirect_out32_locked(lp, XTE_RXC1_OFFSET, 0); + temac_indirect_out32_locked(lp, XTE_TXC_OFFSET, 0); + temac_indirect_out32_locked(lp, XTE_FCC_OFFSET, XTE_FCC_RXFLO_MASK); + spin_unlock_irqrestore(lp->indirect_lock, flags); /* Sync default options with HW * but leave receiver and transmitter disabled. */ @@ -614,13 +702,14 @@ static void temac_adjust_link(struct net_device *ndev) struct phy_device *phy = ndev->phydev; u32 mii_speed; int link_state; + unsigned long flags; /* hash together the state values to decide if something has changed */ link_state = phy->speed | (phy->duplex << 1) | phy->link; - mutex_lock(lp->indirect_mutex); if (lp->last_link != link_state) { - mii_speed = temac_indirect_in32(lp, XTE_EMCFG_OFFSET); + spin_lock_irqsave(lp->indirect_lock, flags); + mii_speed = temac_indirect_in32_locked(lp, XTE_EMCFG_OFFSET); mii_speed &= ~XTE_EMCFG_LINKSPD_MASK; switch (phy->speed) { @@ -630,11 +719,12 @@ static void temac_adjust_link(struct net_device *ndev) } /* Write new speed setting out to TEMAC */ - temac_indirect_out32(lp, XTE_EMCFG_OFFSET, mii_speed); + temac_indirect_out32_locked(lp, XTE_EMCFG_OFFSET, mii_speed); + spin_unlock_irqrestore(lp->indirect_lock, flags); + lp->last_link = link_state; phy_print_status(phy); } - mutex_unlock(lp->indirect_mutex); } #ifdef CONFIG_64BIT @@ -1096,17 +1186,17 @@ static int temac_probe(struct platform_device *pdev) /* Setup mutex for synchronization of indirect register access */ if (pdata) { - if (!pdata->indirect_mutex) { + if (!pdata->indirect_lock) { dev_err(&pdev->dev, - "indirect_mutex missing in platform_data\n"); + "indirect_lock missing in platform_data\n"); return -EINVAL; } - lp->indirect_mutex = pdata->indirect_mutex; + lp->indirect_lock = pdata->indirect_lock; } else { - lp->indirect_mutex = devm_kmalloc(&pdev->dev, - sizeof(*lp->indirect_mutex), - GFP_KERNEL); - mutex_init(lp->indirect_mutex); + lp->indirect_lock = devm_kmalloc(&pdev->dev, + sizeof(*lp->indirect_lock), + GFP_KERNEL); + spin_lock_init(lp->indirect_lock); } /* map device registers */ diff --git a/drivers/net/ethernet/xilinx/ll_temac_mdio.c b/drivers/net/ethernet/xilinx/ll_temac_mdio.c index a4667326f745..6fd2dea4e60f 100644 --- a/drivers/net/ethernet/xilinx/ll_temac_mdio.c +++ b/drivers/net/ethernet/xilinx/ll_temac_mdio.c @@ -25,14 +25,15 @@ static int temac_mdio_read(struct mii_bus *bus, int phy_id, int reg) { struct temac_local *lp = bus->priv; u32 rc; + unsigned long flags; /* Write the PHY address to the MIIM Access Initiator register. * When the transfer completes, the PHY register value will appear * in the LSW0 register */ - mutex_lock(lp->indirect_mutex); + spin_lock_irqsave(lp->indirect_lock, flags); temac_iow(lp, XTE_LSW0_OFFSET, (phy_id << 5) | reg); - rc = temac_indirect_in32(lp, XTE_MIIMAI_OFFSET); - mutex_unlock(lp->indirect_mutex); + rc = temac_indirect_in32_locked(lp, XTE_MIIMAI_OFFSET); + spin_unlock_irqrestore(lp->indirect_lock, flags); dev_dbg(lp->dev, "temac_mdio_read(phy_id=%i, reg=%x) == %x\n", phy_id, reg, rc); @@ -43,6 +44,7 @@ static int temac_mdio_read(struct mii_bus *bus, int phy_id, int reg) static int temac_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val) { struct temac_local *lp = bus->priv; + unsigned long flags; dev_dbg(lp->dev, "temac_mdio_write(phy_id=%i, reg=%x, val=%x)\n", phy_id, reg, val); @@ -50,10 +52,10 @@ static int temac_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 val) /* First write the desired value into the write data register * and then write the address into the access initiator register */ - mutex_lock(lp->indirect_mutex); - temac_indirect_out32(lp, XTE_MGTDR_OFFSET, val); - temac_indirect_out32(lp, XTE_MIIMAI_OFFSET, (phy_id << 5) | reg); - mutex_unlock(lp->indirect_mutex); + spin_lock_irqsave(lp->indirect_lock, flags); + temac_indirect_out32_locked(lp, XTE_MGTDR_OFFSET, val); + temac_indirect_out32_locked(lp, XTE_MIIMAI_OFFSET, (phy_id << 5) | reg); + spin_unlock_irqrestore(lp->indirect_lock, flags); return 0; } @@ -87,9 +89,7 @@ int temac_mdio_setup(struct temac_local *lp, struct platform_device *pdev) /* Enable the MDIO bus by asserting the enable bit and writing * in the clock config */ - mutex_lock(lp->indirect_mutex); temac_indirect_out32(lp, XTE_MC_OFFSET, 1 << 6 | clk_div); - mutex_unlock(lp->indirect_mutex); bus = devm_mdiobus_alloc(&pdev->dev); if (!bus) @@ -116,10 +116,8 @@ int temac_mdio_setup(struct temac_local *lp, struct platform_device *pdev) if (rc) return rc; - mutex_lock(lp->indirect_mutex); dev_dbg(lp->dev, "MDIO bus registered; MC:%x\n", temac_indirect_in32(lp, XTE_MC_OFFSET)); - mutex_unlock(lp->indirect_mutex); return 0; } -- cgit v1.2.3-59-g8ed1b