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: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/r/20210128194802.727770-1-a.darwish@linutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/arcnet/arc-rimi.c b/drivers/net/arcnet/arc-rimi.c
index 98df38fe553ced05b714767d4ad628dcba45819a..12d085405bd052c3621280d97bd3b92575466379 100644 (file)
--- a/drivers/net/arcnet/arc-rimi.c
+++ b/drivers/net/arcnet/arc-rimi.c
@@ -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

diff --git a/drivers/net/arcnet/arcdevice.h b/drivers/net/arcnet/arcdevice.h
index 22a49c6d7ae6ef590b3b0e2d8b6bcf2a10f8b0b7..5d4a4c7efbbff3763193844e2052ca32dcb27490 100644 (file)
--- a/drivers/net/arcnet/arcdevice.h
+++ b/drivers/net/arcnet/arcdevice.h
@@ -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);

diff --git a/drivers/net/arcnet/arcnet.c b/drivers/net/arcnet/arcnet.c
index e04efc0a5c977ee8b33a0ff77fe15c85ee322119..d76dd7d14299e198f305f4effed8611a8c9b480f 100644 (file)
--- a/drivers/net/arcnet/arcnet.c
+++ b/drivers/net/arcnet/arcnet.c
@@ -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;
 }

diff --git a/drivers/net/arcnet/com20020-isa.c b/drivers/net/arcnet/com20020-isa.c
index f983c4ce6b07f186fd2b6cc136df9eabfa7c8df4..be618e4b9ed5e0aa20f949ea23abf416ce5239b6 100644 (file)
--- a/drivers/net/arcnet/com20020-isa.c
+++ b/drivers/net/arcnet/com20020-isa.c
@@ -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

diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
index eb7f76753c9c0dbdd2ca07985d9c407b433ab506..8bdc44b7e09a188e678eaade1faf5fe035fd0434 100644 (file)
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -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);
        }
 }
 

diff --git a/drivers/net/arcnet/com20020_cs.c b/drivers/net/arcnet/com20020_cs.c
index cf607ffcf358e95eab6a63c3c7bb63382cd57c49..9cc5eb6a8e9051aaaf16e876b34a3803c94d571a 100644 (file)
--- a/drivers/net/arcnet/com20020_cs.c
+++ b/drivers/net/arcnet/com20020_cs.c
@@ -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);

diff --git a/drivers/net/arcnet/com90io.c b/drivers/net/arcnet/com90io.c
index cf214b7306715487b84cd2301fca4de1bada6f48..3856b447d38ed27ca5ef98a1d0b4191f92152d31 100644 (file)
--- a/drivers/net/arcnet/com90io.c
+++ b/drivers/net/arcnet/com90io.c
@@ -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)

diff --git a/drivers/net/arcnet/com90xx.c b/drivers/net/arcnet/com90xx.c
index 3dc3d533cb19a258e72f57d46443b9b52d1b8df7..d8dfb9ea0de89bf9929ee334d9f332c9156cef18 100644 (file)
--- a/drivers/net/arcnet/com90xx.c
+++ b/drivers/net/arcnet/com90xx.c
@@ -554,7 +554,7 @@ err_free_irq:
 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);
        }
 }