Commit 01365633 authored by Ahmed S. Darwish's avatar Ahmed S. Darwish Committed by Jakub Kicinski

net: arcnet: Fix RESET flag handling

The main arcnet interrupt handler calls arcnet_close() then
arcnet_open(), if the RESET status flag is encountered.

This is invalid:

  1) In general, interrupt handlers should never call ->ndo_stop() and
     ->ndo_open() functions. They are usually full of blocking calls and
     other methods that are expected to be called only from drivers
     init and exit code paths.

  2) arcnet_close() contains a del_timer_sync(). If the irq handler
     interrupts the to-be-deleted timer, del_timer_sync() will just loop
     forever.

  3) arcnet_close() also calls tasklet_kill(), which has a warning if
     called from irq context.

  4) For device reset, the sequence "arcnet_close(); arcnet_open();" is
     not complete.  Some children arcnet drivers have special init/exit
     code sequences, which then embed a call to arcnet_open() and
     arcnet_close() accordingly. Check drivers/net/arcnet/com20020.c.

Run the device RESET sequence from a scheduled workqueue instead.
Signed-off-by: default avatarAhmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: default avatarSebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/r/20210128194802.727770-1-a.darwish@linutronix.deSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 06cc6e5d
......@@ -332,7 +332,7 @@ static int __init arc_rimi_init(void)
dev->irq = 9;
if (arcrimi_probe(dev)) {
free_netdev(dev);
free_arcdev(dev);
return -EIO;
}
......@@ -349,7 +349,7 @@ static void __exit arc_rimi_exit(void)
iounmap(lp->mem_start);
release_mem_region(dev->mem_start, dev->mem_end - dev->mem_start + 1);
free_irq(dev->irq, dev);
free_netdev(dev);
free_arcdev(dev);
}
#ifndef MODULE
......
......@@ -298,6 +298,10 @@ struct arcnet_local {
int excnak_pending; /* We just got an excesive nak interrupt */
/* RESET flag handling */
int reset_in_progress;
struct work_struct reset_work;
struct {
uint16_t sequence; /* sequence number (incs with each packet) */
__be16 aborted_seq;
......@@ -350,7 +354,9 @@ void arcnet_dump_skb(struct net_device *dev, struct sk_buff *skb, char *desc)
void arcnet_unregister_proto(struct ArcProto *proto);
irqreturn_t arcnet_interrupt(int irq, void *dev_id);
struct net_device *alloc_arcdev(const char *name);
void free_arcdev(struct net_device *dev);
int arcnet_open(struct net_device *dev);
int arcnet_close(struct net_device *dev);
......
......@@ -387,10 +387,44 @@ static void arcnet_timer(struct timer_list *t)
struct arcnet_local *lp = from_timer(lp, t, timer);
struct net_device *dev = lp->dev;
if (!netif_carrier_ok(dev)) {
spin_lock_irq(&lp->lock);
if (!lp->reset_in_progress && !netif_carrier_ok(dev)) {
netif_carrier_on(dev);
netdev_info(dev, "link up\n");
}
spin_unlock_irq(&lp->lock);
}
static void reset_device_work(struct work_struct *work)
{
struct arcnet_local *lp;
struct net_device *dev;
lp = container_of(work, struct arcnet_local, reset_work);
dev = lp->dev;
/* Do not bring the network interface back up if an ifdown
* was already done.
*/
if (!netif_running(dev) || !lp->reset_in_progress)
return;
rtnl_lock();
/* Do another check, in case of an ifdown that was triggered in
* the small race window between the exit condition above and
* acquiring RTNL.
*/
if (!netif_running(dev) || !lp->reset_in_progress)
goto out;
dev_close(dev);
dev_open(dev, NULL);
out:
rtnl_unlock();
}
static void arcnet_reply_tasklet(unsigned long data)
......@@ -452,12 +486,25 @@ struct net_device *alloc_arcdev(const char *name)
lp->dev = dev;
spin_lock_init(&lp->lock);
timer_setup(&lp->timer, arcnet_timer, 0);
INIT_WORK(&lp->reset_work, reset_device_work);
}
return dev;
}
EXPORT_SYMBOL(alloc_arcdev);
void free_arcdev(struct net_device *dev)
{
struct arcnet_local *lp = netdev_priv(dev);
/* Do not cancel this at ->ndo_close(), as the workqueue itself
* indirectly calls the ifdown path through dev_close().
*/
cancel_work_sync(&lp->reset_work);
free_netdev(dev);
}
EXPORT_SYMBOL(free_arcdev);
/* Open/initialize the board. This is called sometime after booting when
* the 'ifconfig' program is run.
*
......@@ -587,6 +634,10 @@ int arcnet_close(struct net_device *dev)
/* shut down the card */
lp->hw.close(dev);
/* reset counters */
lp->reset_in_progress = 0;
module_put(lp->hw.owner);
return 0;
}
......@@ -820,6 +871,9 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id)
spin_lock_irqsave(&lp->lock, flags);
if (lp->reset_in_progress)
goto out;
/* RESET flag was enabled - if device is not running, we must
* clear it right away (but nothing else).
*/
......@@ -852,11 +906,14 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id)
if (status & RESETflag) {
arc_printk(D_NORMAL, dev, "spurious reset (status=%Xh)\n",
status);
arcnet_close(dev);
arcnet_open(dev);
lp->reset_in_progress = 1;
netif_stop_queue(dev);
netif_carrier_off(dev);
schedule_work(&lp->reset_work);
/* get out of the interrupt handler! */
break;
goto out;
}
/* RX is inhibited - we must have received something.
* Prepare to receive into the next buffer.
......@@ -1052,6 +1109,7 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id)
udelay(1);
lp->hw.intmask(dev, lp->intmask);
out:
spin_unlock_irqrestore(&lp->lock, flags);
return retval;
}
......
......@@ -169,7 +169,7 @@ static int __init com20020_init(void)
dev->irq = 9;
if (com20020isa_probe(dev)) {
free_netdev(dev);
free_arcdev(dev);
return -EIO;
}
......@@ -182,7 +182,7 @@ static void __exit com20020_exit(void)
unregister_netdev(my_dev);
free_irq(my_dev->irq, my_dev);
release_region(my_dev->base_addr, ARCNET_TOTAL_SIZE);
free_netdev(my_dev);
free_arcdev(my_dev);
}
#ifndef MODULE
......
......@@ -291,7 +291,7 @@ static void com20020pci_remove(struct pci_dev *pdev)
unregister_netdev(dev);
free_irq(dev->irq, dev);
free_netdev(dev);
free_arcdev(dev);
}
}
......
......@@ -177,7 +177,7 @@ static void com20020_detach(struct pcmcia_device *link)
dev = info->dev;
if (dev) {
dev_dbg(&link->dev, "kfree...\n");
free_netdev(dev);
free_arcdev(dev);
}
dev_dbg(&link->dev, "kfree2...\n");
kfree(info);
......
......@@ -396,7 +396,7 @@ static int __init com90io_init(void)
err = com90io_probe(dev);
if (err) {
free_netdev(dev);
free_arcdev(dev);
return err;
}
......@@ -419,7 +419,7 @@ static void __exit com90io_exit(void)
free_irq(dev->irq, dev);
release_region(dev->base_addr, ARCNET_TOTAL_SIZE);
free_netdev(dev);
free_arcdev(dev);
}
module_init(com90io_init)
......
......@@ -554,7 +554,7 @@ static int __init com90xx_found(int ioaddr, int airq, u_long shmem,
err_release_mem:
release_mem_region(dev->mem_start, dev->mem_end - dev->mem_start + 1);
err_free_dev:
free_netdev(dev);
free_arcdev(dev);
return -EIO;
}
......@@ -672,7 +672,7 @@ static void __exit com90xx_exit(void)
release_region(dev->base_addr, ARCNET_TOTAL_SIZE);
release_mem_region(dev->mem_start,
dev->mem_end - dev->mem_start + 1);
free_netdev(dev);
free_arcdev(dev);
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment