Re: [PATCH][ATM]: [ambassador] eliminate pci_find_device()

In message [ref],Christoph Hellwig writes:

quoted

   // Did one of our cards generate the interrupt?

this debug check is superflous.

i know that.  however, i didnt want to change the original more than
necessary.  dropping this check doesnt bother me.

quoted

@@ -1554,19 +1556,19 @@
   if (housekeeping.data) {
-    while (dev) {
+    list_for_each(p, &amb_devs) {
+      dev = list_entry(p, struct amb_dev, entry);

this really should be a per-device timer - or even better using
schedule_delayed_work()

With these two changes you could get rid of amb_devs easily.  If you still
want to keep it for some reason at least use list_for_each_entry.

my primary reason for keeping it is that i cant easily test the changes
to the driver.  i have a programmable card that i set to the correct
vendor/device id.  it doesnt do any 'real' work.  i dont really know
anyone with a real ambassador (or horizon) card.  this driver is essentially
"reference only" as far as i can tell.  i can change to list_for_each_entry.

quoted

+	list_del(&dev->entry);
+    
+	PRINTD(DBG_INFO|DBG_INIT, "closing %p (atm_dev = %p)", dev, dev->atm_dev);
+	// the drain should not be necessary
+	drain_rx_pools(dev);
+	interrupts_off(dev);
+	amb_reset(dev, 0);
+	free_irq(dev->irq, dev);
+	pci_disable_device(pci_dev);
+	destroy_queues(dev);
+	atm_dev_deregister(dev->atm_dev);
+	kfree(dev);
+	pci_release_region(pci_dev, 1);

this doesn't look correct (without knowing the atm subsystem in detail), in
general you need to unregister from the subsystem first so you don't get
new work, and then you can start tearing the hardware down.

still that what was the order in the original driver.  i am sure it
races with housekeeping as well if that makes you feel any better.
(yes, i know, seperate timers wouldnt race.)