On 20.01.2017 22:05, Timur Tabi wrote:
On 01/20/2017 02:44 PM, Lino Sanfilippo wrote:


On 18.01.2017 22:42, Timur Tabi wrote:
@@ -1029,8 +1017,6 @@ void emac_mac_down(struct emac_adapter *adpt)
       */
      writel(DIS_INT, adpt->base + EMAC_INT_STATUS);
      writel(0, adpt->base + EMAC_INT_MASK);
-    synchronize_irq(adpt->irq.irq);

There is no reason to remove the irq synchronization, is it?
Note that the desriptors are freed after that so we must be sure that
the irq handler is not running any more.

I'm moving it to stay with the free_irq().

@@ -283,6 +292,9 @@ static int emac_close(struct net_device *netdev)

     mutex_lock(&adpt->reset_lock);

+    synchronize_irq(adpt->irq.irq);
+    free_irq(adpt->irq.irq, &adpt->irq);
+

However, I'll admit that I don't know why we call synchronize_irq() at all.


free_irq() will call synchronize_irq() if necessary, so it is pointless to call synchronize_irq()
right before free_irq().
In emac_mac_down() however we need synchronize_irq(), since it ensures that the irq
handler is not running any more when it (synchronize_irq) returns.

Regards,
Lino


Reply via email to