[PATCH] libata-sff: Undo bug introduced with pci_iomap changes

[PATCH] libata-sff: Undo bug introduced with pci_iomap changes

[Alan Cox]

If you have a controller with one channel disabled and unmapped the new
iomap code blindly tries to iomap unconfigured BARs. Later on the code
does the right thing and checks for unmapped bars but it is done in the
wrong order

Reorder the checks and make the iomap conditional

Tejun: I think the code below is now correct but would appreciate you
giving it a review.

Signed-off-by: Alan Cox <alan@redhat.com>

--- drivers/ata/libata-sff.c~	2007-05-01 12:03:39.634876384 +0100
+++ drivers/ata/libata-sff.c	2007-05-01 12:03:39.634876384 +0100
@@ -557,12 +557,30 @@
 	int i, p = 0;
 	void __iomem * const *iomap;
 
+	/* Discard disabled ports. Some controllers show their
+	   unused channels this way */
+	if (ata_resources_present(pdev, 0) == 0)
+		ports &= ~ATA_PORT_PRIMARY;
+	if (ata_resources_present(pdev, 1) == 0)
+		ports &= ~ATA_PORT_SECONDARY;
+
 	/* iomap BARs */
-	for (i = 0; i < 4; i++) {
-		if (pcim_iomap(pdev, i, 0) == NULL) {
-			dev_printk(KERN_ERR, &pdev->dev,
-				   "failed to iomap PCI BAR %d\n", i);
-			return NULL;
+	if (ports & ATA_PORT_PRIMARY) {
+		for (i = 0; i <= 1; i++) {
+			if (pcim_iomap(pdev, i, 0) == NULL) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "failed to iomap PCI BAR %d\n", i);
+				return NULL;
+			}
+		}
+	}
+	if (ports & ATA_PORT_SECONDARY) {
+		for (i = 2; i <= 3; i++) {
+			if (pcim_iomap(pdev, i, 0) == NULL) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "failed to iomap PCI BAR %d\n", i);
+				return NULL;
+			}
 		}
 	}
 
@@ -577,13 +595,6 @@
 	probe_ent->irq = pdev->irq;
 	probe_ent->irq_flags = IRQF_SHARED;
 
-	/* Discard disabled ports. Some controllers show their
-	   unused channels this way */
-	if (ata_resources_present(pdev, 0) == 0)
-		ports &= ~ATA_PORT_PRIMARY;
-	if (ata_resources_present(pdev, 1) == 0)
-		ports &= ~ATA_PORT_SECONDARY;
-
 	if (ports & ATA_PORT_PRIMARY) {
 		probe_ent->port[p].cmd_addr = iomap[0];
 		probe_ent->port[p].altstatus_addr =

Re: [PATCH] libata-sff: Undo bug introduced with pci_iomap changes

[Tejun Heo]

Hello, Alan.

Alan Cox wrote:
> If you have a controller with one channel disabled and unmapped the new
> iomap code blindly tries to iomap unconfigured BARs. Later on the code
> does the right thing and checks for unmapped bars but it is done in the
> wrong order
> 
> Reorder the checks and make the iomap conditional
> 
> Tejun: I think the code below is now correct but would appreciate you
> giving it a review.
>
> Signed-off-by: Alan Cox <alan@redhat.com>

Acked-by: Tejun Heo <htejun@gmail.com>

Yeap, the patch looks good.  I got tunnel visioned while converting to
iomap and mechanically mapped IO BAR access to iomap.  Thanks for
catching this.  This is already fixed in #upstream while converting to
new-init-model (the code is in ata_pci_init_native_host() and does about
the same thing as your patch does), so I think this patch is only needed
for -stable.  Cc'ing stable team.  The original patch can be reached at...

  http://article.gmane.org/gmane.linux.kernel/523335/raw

Thanks.

-- 
tejun