Skip to content

Commit

Permalink
net: tlan: Replace in_irq() usage
Browse files Browse the repository at this point in the history 
The driver uses in_irq() to determine if the tlan_priv::lock has to be
acquired in tlan_mii_read_reg() and tlan_mii_write_reg().

The interrupt handler acquires the lock outside of these functions so the
in_irq() check is meant to prevent a lock recursion deadlock. But this
check is incorrect when interrupt force threading is enabled because then
the handler runs in thread context and in_irq() correctly returns false.

The usage of in_*() in drivers is phased out and Linus clearly requested
that code which changes behaviour depending on context should either be
seperated or the context be conveyed in an argument passed by the caller,
which usually knows the context.

tlan_set_timer() has this conditional as well, but this function is only
invoked from task context or the timer callback itself. So it always has to
lock and the check can be removed.

tlan_mii_read_reg(), tlan_mii_write_reg() and tlan_phy_print() are invoked
from interrupt and other contexts.

Split out the actual function body into helper variants which are called
from interrupt context and make the original functions wrappers which
acquire tlan_priv::lock unconditionally.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Samuel Chessman <chessman@tux.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Sebastian Andrzej Siewior authored and Jakub Kicinski committed Oct 31, 2020
1 parent dc5e8bf commit beca928
Showing 1 changed file with 57 additions and 41 deletions.
98 changes: 57 additions & 41 deletions drivers/net/ethernet/ti/tlan.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ static void tlan_reset_adapter(struct net_device *);
static void tlan_finish_reset(struct net_device *);
static void tlan_set_mac(struct net_device *, int areg, char *mac);

static void __tlan_phy_print(struct net_device *);
static void tlan_phy_print(struct net_device *);
static void tlan_phy_detect(struct net_device *);
static void tlan_phy_power_down(struct net_device *);
Expand All @@ -201,9 +202,11 @@ static void tlan_phy_finish_auto_neg(struct net_device *);
static int tlan_phy_dp83840a_check(struct net_device *);
*/

static bool tlan_mii_read_reg(struct net_device *, u16, u16, u16 *);
static bool __tlan_mii_read_reg(struct net_device *, u16, u16, u16 *);
static void tlan_mii_read_reg(struct net_device *, u16, u16, u16 *);
static void tlan_mii_send_data(u16, u32, unsigned);
static void tlan_mii_sync(u16);
static void __tlan_mii_write_reg(struct net_device *, u16, u16, u16);
static void tlan_mii_write_reg(struct net_device *, u16, u16, u16);

static void tlan_ee_send_start(u16);
Expand Down Expand Up @@ -242,23 +245,20 @@ static u32
tlan_handle_rx_eoc
};

static inline void
static void
tlan_set_timer(struct net_device *dev, u32 ticks, u32 type)
{
struct tlan_priv *priv = netdev_priv(dev);
unsigned long flags = 0;

if (!in_irq())
spin_lock_irqsave(&priv->lock, flags);
spin_lock_irqsave(&priv->lock, flags);
if (priv->timer.function != NULL &&
priv->timer_type != TLAN_TIMER_ACTIVITY) {
if (!in_irq())
spin_unlock_irqrestore(&priv->lock, flags);
spin_unlock_irqrestore(&priv->lock, flags);
return;
}
priv->timer.function = tlan_timer;
if (!in_irq())
spin_unlock_irqrestore(&priv->lock, flags);
spin_unlock_irqrestore(&priv->lock, flags);

priv->timer_set_at = jiffies;
priv->timer_type = type;
Expand Down Expand Up @@ -1703,22 +1703,22 @@ static u32 tlan_handle_status_check(struct net_device *dev, u16 host_int)
dev->name, (unsigned) net_sts);
}
if ((net_sts & TLAN_NET_STS_MIRQ) && (priv->phy_num == 0)) {
tlan_mii_read_reg(dev, phy, TLAN_TLPHY_STS, &tlphy_sts);
tlan_mii_read_reg(dev, phy, TLAN_TLPHY_CTL, &tlphy_ctl);
__tlan_mii_read_reg(dev, phy, TLAN_TLPHY_STS, &tlphy_sts);
__tlan_mii_read_reg(dev, phy, TLAN_TLPHY_CTL, &tlphy_ctl);
if (!(tlphy_sts & TLAN_TS_POLOK) &&
!(tlphy_ctl & TLAN_TC_SWAPOL)) {
tlphy_ctl |= TLAN_TC_SWAPOL;
tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
tlphy_ctl);
__tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
tlphy_ctl);
} else if ((tlphy_sts & TLAN_TS_POLOK) &&
(tlphy_ctl & TLAN_TC_SWAPOL)) {
tlphy_ctl &= ~TLAN_TC_SWAPOL;
tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
tlphy_ctl);
__tlan_mii_write_reg(dev, phy, TLAN_TLPHY_CTL,
tlphy_ctl);
}

if (debug)
tlan_phy_print(dev);
__tlan_phy_print(dev);
}
}

Expand Down Expand Up @@ -2379,7 +2379,7 @@ ThunderLAN driver PHY layer routines


/*********************************************************************
* tlan_phy_print
* __tlan_phy_print
*
* Returns:
* Nothing
Expand All @@ -2391,11 +2391,13 @@ ThunderLAN driver PHY layer routines
*
********************************************************************/

static void tlan_phy_print(struct net_device *dev)
static void __tlan_phy_print(struct net_device *dev)
{
struct tlan_priv *priv = netdev_priv(dev);
u16 i, data0, data1, data2, data3, phy;

lockdep_assert_held(&priv->lock);

phy = priv->phy[priv->phy_num];

if (priv->adapter->flags & TLAN_ADAPTER_UNMANAGED_PHY) {
Expand All @@ -2404,10 +2406,10 @@ static void tlan_phy_print(struct net_device *dev)
netdev_info(dev, "PHY 0x%02x\n", phy);
pr_info(" Off. +0 +1 +2 +3\n");
for (i = 0; i < 0x20; i += 4) {
tlan_mii_read_reg(dev, phy, i, &data0);
tlan_mii_read_reg(dev, phy, i + 1, &data1);
tlan_mii_read_reg(dev, phy, i + 2, &data2);
tlan_mii_read_reg(dev, phy, i + 3, &data3);
__tlan_mii_read_reg(dev, phy, i, &data0);
__tlan_mii_read_reg(dev, phy, i + 1, &data1);
__tlan_mii_read_reg(dev, phy, i + 2, &data2);
__tlan_mii_read_reg(dev, phy, i + 3, &data3);
pr_info(" 0x%02x 0x%04hx 0x%04hx 0x%04hx 0x%04hx\n",
i, data0, data1, data2, data3);
}
Expand All @@ -2417,7 +2419,15 @@ static void tlan_phy_print(struct net_device *dev)

}

static void tlan_phy_print(struct net_device *dev)
{
struct tlan_priv *priv = netdev_priv(dev);
unsigned long flags;

spin_lock_irqsave(&priv->lock, flags);
__tlan_phy_print(dev);
spin_unlock_irqrestore(&priv->lock, flags);
}


/*********************************************************************
Expand Down Expand Up @@ -2795,7 +2805,7 @@ these routines are based on the information in chap. 2 of the


/***************************************************************
* tlan_mii_read_reg
* __tlan_mii_read_reg
*
* Returns:
* false if ack received ok
Expand All @@ -2819,23 +2829,21 @@ these routines are based on the information in chap. 2 of the
**************************************************************/

static bool
tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)
__tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)
{
u8 nack;
u16 sio, tmp;
u32 i;
bool err;
int minten;
struct tlan_priv *priv = netdev_priv(dev);
unsigned long flags = 0;

lockdep_assert_held(&priv->lock);

err = false;
outw(TLAN_NET_SIO, dev->base_addr + TLAN_DIO_ADR);
sio = dev->base_addr + TLAN_DIO_DATA + TLAN_NET_SIO;

if (!in_irq())
spin_lock_irqsave(&priv->lock, flags);

tlan_mii_sync(dev->base_addr);

minten = tlan_get_bit(TLAN_NET_SIO_MINTEN, sio);
Expand Down Expand Up @@ -2881,15 +2889,19 @@ tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg, u16 *val)

*val = tmp;

if (!in_irq())
spin_unlock_irqrestore(&priv->lock, flags);

return err;

}

static void tlan_mii_read_reg(struct net_device *dev, u16 phy, u16 reg,
u16 *val)
{
struct tlan_priv *priv = netdev_priv(dev);
unsigned long flags;


spin_lock_irqsave(&priv->lock, flags);
__tlan_mii_read_reg(dev, phy, reg, val);
spin_unlock_irqrestore(&priv->lock, flags);
}

/***************************************************************
* tlan_mii_send_data
Expand Down Expand Up @@ -2971,7 +2983,7 @@ static void tlan_mii_sync(u16 base_port)


/***************************************************************
* tlan_mii_write_reg
* __tlan_mii_write_reg
*
* Returns:
* Nothing
Expand All @@ -2991,19 +3003,17 @@ static void tlan_mii_sync(u16 base_port)
**************************************************************/

static void
tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
__tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
{
u16 sio;
int minten;
unsigned long flags = 0;
struct tlan_priv *priv = netdev_priv(dev);

lockdep_assert_held(&priv->lock);

outw(TLAN_NET_SIO, dev->base_addr + TLAN_DIO_ADR);
sio = dev->base_addr + TLAN_DIO_DATA + TLAN_NET_SIO;

if (!in_irq())
spin_lock_irqsave(&priv->lock, flags);

tlan_mii_sync(dev->base_addr);

minten = tlan_get_bit(TLAN_NET_SIO_MINTEN, sio);
Expand All @@ -3024,12 +3034,18 @@ tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
if (minten)
tlan_set_bit(TLAN_NET_SIO_MINTEN, sio);

if (!in_irq())
spin_unlock_irqrestore(&priv->lock, flags);

}

static void
tlan_mii_write_reg(struct net_device *dev, u16 phy, u16 reg, u16 val)
{
struct tlan_priv *priv = netdev_priv(dev);
unsigned long flags;

spin_lock_irqsave(&priv->lock, flags);
__tlan_mii_write_reg(dev, phy, reg, val);
spin_unlock_irqrestore(&priv->lock, flags);
}


/*****************************************************************************
Expand Down

0 comments on commit beca928

Please sign in to comment.