Re: [PATCH 7/7] gdth: switch to modern scsi host registration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux