in general, is sane.  three problems...


Christoph Hellwig wrote:
+       /* As default we do not probe for EISA or ISA controllers */
+       if (probe_eisa_isa) {
+               /* scanning for controllers, at first: ISA controller */
+#ifdef CONFIG_ISA
+               for (isa_bios = 0xc8000UL; isa_bios <= 0xd8000UL;
+                            isa_bios += 0x8000UL) {
+                       if (gdth_ctr_count >= MAXHA)
+                               break;
+                       gdth_isa_probe_one(isa_bios);
+               }
+#endif
+#ifdef CONFIG_EISA
+               for (eisa_slot = 0x1000; eisa_slot <= 0x8000; eisa_slot += 
0x1000) {
+                       if (gdth_ctr_count >= MAXHA)
+                               break;
+                       gdth_eisa_probe_one(eisa_slot);
+               }
+       }
+#endif

1) endif should be above the brace


+static void __exit gdth_exit(void)
+{
+       gdth_ha_str *ha;
+
+       list_for_each_entry(ha, &gdth_instances, list)
+               gdth_remove_one(ha);

2) I think it's nicer to have two loops: scsi_remove_host() for all ha-attached devices, and then the cleanup. More pragmatic: gives hardware more time to "settle" before turning off DMA engines. A safer approach, and one that scsi_module.c follows.

3) This introduces a modicum of confusion between the gdth_ctr_tab[] stuff and the gdth_instances list. While not strictly broken, this separation creates a foundation where the two can easily get out of sync.

        Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to