lib8390: comment on locking by Alan Cox

Additional explanation of problems with locking by Alan Cox.

Signed-off-by: Jarek Poplawski <jarkao2@o2.pl>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Paul Gortmaker <p_gortmaker@yahoo.com>
Cc: Jeff Garzik <jeff@garzik.org>
Signed-off-by: Jeff Garzik <jeff@garzik.org>

1 files changed, 46 insertions, 0 deletions

diff --git a/drivers/net/lib8390.c b/drivers/net/lib8390.c
index 721ee38d2241..c429a5002dd6 100644
--- a/drivers/net/lib8390.c
+++ b/drivers/net/lib8390.c
@@ -143,6 +143,52 @@ static void __NS8390_init(struct net_device *dev, int startp);
  *      annoying the transmit function is called bh atomic. That places
  *      restrictions on the user context callers as disable_irq won't save
  *      them.
+ *
+ *      Additional explanation of problems with locking by Alan Cox:
+ *
+ *      "The author (me) didn't use spin_lock_irqsave because the slowness of the
+ *      card means that approach caused horrible problems like losing serial data
+ *      at 38400 baud on some chips. Rememeber many 8390 nics on PCI were ISA
+ *      chips with FPGA front ends.
+ *      
+ *      Ok the logic behind the 8390 is very simple:
+ *      
+ *      Things to know
+ *              - IRQ delivery is asynchronous to the PCI bus
+ *              - Blocking the local CPU IRQ via spin locks was too slow
+ *              - The chip has register windows needing locking work
+ *      
+ *      So the path was once (I say once as people appear to have changed it
+ *      in the mean time and it now looks rather bogus if the changes to use
+ *      disable_irq_nosync_irqsave are disabling the local IRQ)
+ *      
+ *      
+ *              Take the page lock
+ *              Mask the IRQ on chip
+ *              Disable the IRQ (but not mask locally- someone seems to have
+ *                      broken this with the lock validator stuff)
+ *                      [This must be _nosync as the page lock may otherwise
+ *                              deadlock us]
+ *              Drop the page lock and turn IRQs back on
+ *              
+ *              At this point an existing IRQ may still be running but we can't
+ *              get a new one
+ *      
+ *              Take the lock (so we know the IRQ has terminated) but don't mask
+ *      the IRQs on the processor
+ *              Set irqlock [for debug]
+ *      
+ *              Transmit (slow as ****)
+ *      
+ *              re-enable the IRQ
+ *      
+ *      
+ *      We have to use disable_irq because otherwise you will get delayed
+ *      interrupts on the APIC bus deadlocking the transmit path.
+ *      
+ *      Quite hairy but the chip simply wasn't designed for SMP and you can't
+ *      even ACK an interrupt without risking corrupting other parallel
+ *      activities on the chip." [lkml, 25 Jul 2007]
  */