Skip to content

Commit 4f1547a

Browse files
Marek Vasutgregkh
authored andcommitted
net: ks8851-ml: Fix IRQ handling and locking
[ Upstream commit 4434341 ] The KS8851 requires that packet RX and TX are mutually exclusive. Currently, the driver hopes to achieve this by disabling interrupt from the card by writing the card registers and by disabling the interrupt on the interrupt controller. This however is racy on SMP. Replace this approach by expanding the spinlock used around the ks_start_xmit() TX path to ks_irq() RX path to assure true mutual exclusion and remove the interrupt enabling/disabling, which is now not needed anymore. Furthermore, disable interrupts also in ks_net_stop(), which was missing before. Note that a massive improvement here would be to re-use the KS8851 driver approach, which is to move the TX path into a worker thread, interrupt handling to threaded interrupt, and synchronize everything with mutexes, but that would be a much bigger rework, for a separate patch. Signed-off-by: Marek Vasut <[email protected]> Cc: David S. Miller <[email protected]> Cc: Lukas Wunner <[email protected]> Cc: Petr Stetiar <[email protected]> Cc: YueHaibing <[email protected]> Signed-off-by: David S. Miller <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent c0d470e commit 4f1547a

File tree

1 file changed

+8
-6
lines changed

1 file changed

+8
-6
lines changed

drivers/net/ethernet/micrel/ks8851_mll.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -831,14 +831,17 @@ static irqreturn_t ks_irq(int irq, void *pw)
831831
{
832832
struct net_device *netdev = pw;
833833
struct ks_net *ks = netdev_priv(netdev);
834+
unsigned long flags;
834835
u16 status;
835836

837+
spin_lock_irqsave(&ks->statelock, flags);
836838
/*this should be the first in IRQ handler */
837839
ks_save_cmd_reg(ks);
838840

839841
status = ks_rdreg16(ks, KS_ISR);
840842
if (unlikely(!status)) {
841843
ks_restore_cmd_reg(ks);
844+
spin_unlock_irqrestore(&ks->statelock, flags);
842845
return IRQ_NONE;
843846
}
844847

@@ -864,6 +867,7 @@ static irqreturn_t ks_irq(int irq, void *pw)
864867
ks->netdev->stats.rx_over_errors++;
865868
/* this should be the last in IRQ handler*/
866869
ks_restore_cmd_reg(ks);
870+
spin_unlock_irqrestore(&ks->statelock, flags);
867871
return IRQ_HANDLED;
868872
}
869873

@@ -933,6 +937,7 @@ static int ks_net_stop(struct net_device *netdev)
933937

934938
/* shutdown RX/TX QMU */
935939
ks_disable_qmu(ks);
940+
ks_disable_int(ks);
936941

937942
/* set powermode to soft power down to save power */
938943
ks_set_powermode(ks, PMECR_PM_SOFTDOWN);
@@ -989,10 +994,9 @@ static netdev_tx_t ks_start_xmit(struct sk_buff *skb, struct net_device *netdev)
989994
{
990995
netdev_tx_t retv = NETDEV_TX_OK;
991996
struct ks_net *ks = netdev_priv(netdev);
997+
unsigned long flags;
992998

993-
disable_irq(netdev->irq);
994-
ks_disable_int(ks);
995-
spin_lock(&ks->statelock);
999+
spin_lock_irqsave(&ks->statelock, flags);
9961000

9971001
/* Extra space are required:
9981002
* 4 byte for alignment, 4 for status/length, 4 for CRC
@@ -1006,9 +1010,7 @@ static netdev_tx_t ks_start_xmit(struct sk_buff *skb, struct net_device *netdev)
10061010
dev_kfree_skb(skb);
10071011
} else
10081012
retv = NETDEV_TX_BUSY;
1009-
spin_unlock(&ks->statelock);
1010-
ks_enable_int(ks);
1011-
enable_irq(netdev->irq);
1013+
spin_unlock_irqrestore(&ks->statelock, flags);
10121014
return retv;
10131015
}
10141016

0 commit comments

Comments
 (0)