libata-sff: clean up BMDMA initialization

When BMDMA initialization failed or BMDMA was not available for
whatever reason, bmdma_addr was left at zero and used as an indication
that BMDMA shouldn't be used.  This leads to the following problems.

p1. For BMDMA drivers which don't use traditional BMDMA register,
    ata_bmdma_mode_filter() incorrectly inhibits DMA modes.  Those
    drivers either have to inherit from ata_sff_port_ops or clear
    ->mode_filter explicitly.

p2. non-BMDMA drivers call into BMDMA PRD table allocation.  It
    doesn't actually allocate PRD table if bmdma_addr is not
    initialized but is still confusing.

p3. For BMDMA drivers which don't use traditional BMDMA register, some
    methods might not be invoked as expected (e.g. bmdma_stop from
    ata_sff_post_internal_cmd()).

p4. SFF drivers w/ custom DMA interface implement noop BMDMA ops
    worrying libata core might call into one of them.

These problems are caused by the muddy line between SFF and BMDMA and
the assumption that all BMDMA controllers initialize bmdma_addr.

This patch fixes p1 and p2 by removing the bmdma_addr assumption and
moving prd allocation to BMDMA port start.  Later patches will fix the
remaining issues.

This patch improves BMDMA initialization such that

* When BMDMA register initialization fails, falls back to PIO instead
  of failing.  ata_pci_bmdma_init() never fails now.

* When ata_pci_bmdma_init() falls back to PIO, it clears
  ap->mwdma_mask and udma_mask instead of depending on
  ata_bmdma_mode_filter().  This makes ata_bmdma_mode_filter()
  unnecessary thus resolving p1.

* ata_port_start() which actually is BMDMA specific is moved to
  ata_bmdma_port_start().  ata_port_start() and ata_sff_port_start()
  are killed.

* ata_sff_port_start32() is moved and renamed to
  ata_bmdma_port_start32().

Drivers which no longer call into PRD table allocation are...

  pdc_adma, sata_inic162x, sata_qstor, sata_sx4, pata_cmd640 and all
  drivers which inherit from ata_sff_port_ops.

pata_icside sets ->port_start to ATA_OP_NULL as it doesn't need PRD
but is a BMDMA controller and doesn't have custom port_start like
other such controllers.

Note that with the previous patch which makes all and only BMDMA
drivers inherit from ata_bmdma_port_ops, this change doesn't break
drivers which need PRD table.

Signed-off-by: Tejun Heo <[email protected]>
Signed-off-by: Jeff Garzik <[email protected]>

drivers/ata/libata-core.c

@@ -5505,30 +5505,6 @@ void ata_host_resume(struct ata_host *host)
 }
 #endif
 
-/**
- *	ata_port_start - Set port up for dma.
- *	@ap: Port to initialize
- *
- *	Called just after data structures for each port are
- *	initialized.  Allocates space for PRD table.
- *
- *	May be used as the port_start() entry in ata_port_operations.
- *
- *	LOCKING:
- *	Inherited from caller.
- */
-int ata_port_start(struct ata_port *ap)
-{
-	struct device *dev = ap->dev;
-
-	ap->prd = dmam_alloc_coherent(dev, ATA_PRD_TBL_SZ, &ap->prd_dma,
-				      GFP_KERNEL);
-	if (!ap->prd)
-		return -ENOMEM;
-
-	return 0;
-}
-
 /**
  *	ata_dev_init - Initialize an ata_device structure
  *	@dev: Device structure to initialize
@@ -6757,7 +6733,6 @@ EXPORT_SYMBOL_GPL(ata_xfer_mode2mask);
 EXPORT_SYMBOL_GPL(ata_xfer_mode2shift);
 EXPORT_SYMBOL_GPL(ata_mode_string);
 EXPORT_SYMBOL_GPL(ata_id_xfermask);
-EXPORT_SYMBOL_GPL(ata_port_start);
 EXPORT_SYMBOL_GPL(ata_do_set_mode);
 EXPORT_SYMBOL_GPL(ata_std_qc_defer);
 EXPORT_SYMBOL_GPL(ata_noop_qc_prep);

drivers/ata/libata-sff.c

@@ -66,8 +66,6 @@ const struct ata_port_operations ata_sff_port_ops = {
 	.sff_irq_clear		= ata_sff_irq_clear,
 
 	.lost_interrupt		= ata_sff_lost_interrupt,
-
-	.port_start		= ata_sff_port_start,
 };
 EXPORT_SYMBOL_GPL(ata_sff_port_ops);
 
@@ -2443,50 +2441,6 @@ void ata_sff_post_internal_cmd(struct ata_queued_cmd *qc)
 }
 EXPORT_SYMBOL_GPL(ata_sff_post_internal_cmd);
 
-/**
- *	ata_sff_port_start - Set port up for dma.
- *	@ap: Port to initialize
- *
- *	Called just after data structures for each port are
- *	initialized.  Allocates space for PRD table if the device
- *	is DMA capable SFF.
- *
- *	May be used as the port_start() entry in ata_port_operations.
- *
- *	LOCKING:
- *	Inherited from caller.
- */
-int ata_sff_port_start(struct ata_port *ap)
-{
-	if (ap->ioaddr.bmdma_addr)
-		return ata_port_start(ap);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(ata_sff_port_start);
-
-/**
- *	ata_sff_port_start32 - Set port up for dma.
- *	@ap: Port to initialize
- *
- *	Called just after data structures for each port are
- *	initialized.  Allocates space for PRD table if the device
- *	is DMA capable SFF.
- *
- *	May be used as the port_start() entry in ata_port_operations for
- *	devices that are capable of 32bit PIO.
- *
- *	LOCKING:
- *	Inherited from caller.
- */
-int ata_sff_port_start32(struct ata_port *ap)
-{
-	ap->pflags |= ATA_PFLAG_PIO32 | ATA_PFLAG_PIO32CHANGE;
-	if (ap->ioaddr.bmdma_addr)
-		return ata_port_start(ap);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(ata_sff_port_start32);
-
 /**
  *	ata_sff_std_ports - initialize ioaddr with standard port offsets.
  *	@ioaddr: IO address structure to be initialized
@@ -2646,21 +2600,12 @@ int ata_pci_sff_prepare_host(struct pci_dev *pdev,
 		goto err_out;
 
 	/* init DMA related stuff */
-	rc = ata_pci_bmdma_init(host);
-	if (rc)
-		goto err_bmdma;
+	ata_pci_bmdma_init(host);
 
 	devres_remove_group(&pdev->dev, NULL);
 	*r_host = host;
 	return 0;
 
-err_bmdma:
-	/* This is necessary because PCI and iomap resources are
-	 * merged and releasing the top group won't release the
-	 * acquired resources if some of those have been acquired
-	 * before entering this function.
-	 */
-	pcim_iounmap_regions(pdev, 0xf);
 err_out:
 	devres_release_group(&pdev->dev, NULL);
 	return rc;
@@ -2843,35 +2788,23 @@ EXPORT_SYMBOL_GPL(ata_pci_sff_init_one);
 const struct ata_port_operations ata_bmdma_port_ops = {
 	.inherits		= &ata_sff_port_ops,
 
-	.mode_filter		= ata_bmdma_mode_filter,
-
 	.bmdma_setup		= ata_bmdma_setup,
 	.bmdma_start		= ata_bmdma_start,
 	.bmdma_stop		= ata_bmdma_stop,
 	.bmdma_status		= ata_bmdma_status,
+
+	.port_start		= ata_bmdma_port_start,
 };
 EXPORT_SYMBOL_GPL(ata_bmdma_port_ops);
 
 const struct ata_port_operations ata_bmdma32_port_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 
 	.sff_data_xfer		= ata_sff_data_xfer32,
-	.port_start		= ata_sff_port_start32,
+	.port_start		= ata_bmdma_port_start32,
 };
 EXPORT_SYMBOL_GPL(ata_bmdma32_port_ops);
 
-unsigned long ata_bmdma_mode_filter(struct ata_device *adev,
-				    unsigned long xfer_mask)
-{
-	/* Filter out DMA modes if the device has been configured by
-	   the BIOS as PIO only */
-
-	if (adev->link->ap->ioaddr.bmdma_addr == NULL)
-		xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
-	return xfer_mask;
-}
-EXPORT_SYMBOL_GPL(ata_bmdma_mode_filter);
-
 /**
  *	ata_bmdma_setup - Set up PCI IDE BMDMA transaction
  *	@qc: Info associated with this ATA transaction.
@@ -2976,6 +2909,53 @@ u8 ata_bmdma_status(struct ata_port *ap)
 }
 EXPORT_SYMBOL_GPL(ata_bmdma_status);
 
+
+/**
+ *	ata_bmdma_port_start - Set port up for bmdma.
+ *	@ap: Port to initialize
+ *
+ *	Called just after data structures for each port are
+ *	initialized.  Allocates space for PRD table.
+ *
+ *	May be used as the port_start() entry in ata_port_operations.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+int ata_bmdma_port_start(struct ata_port *ap)
+{
+	if (ap->mwdma_mask || ap->udma_mask) {
+		ap->prd = dmam_alloc_coherent(ap->host->dev, ATA_PRD_TBL_SZ,
+					      &ap->prd_dma, GFP_KERNEL);
+		if (!ap->prd)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ata_bmdma_port_start);
+
+/**
+ *	ata_bmdma_port_start32 - Set port up for dma.
+ *	@ap: Port to initialize
+ *
+ *	Called just after data structures for each port are
+ *	initialized.  Enables 32bit PIO and allocates space for PRD
+ *	table.
+ *
+ *	May be used as the port_start() entry in ata_port_operations for
+ *	devices that are capable of 32bit PIO.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ */
+int ata_bmdma_port_start32(struct ata_port *ap)
+{
+	ap->pflags |= ATA_PFLAG_PIO32 | ATA_PFLAG_PIO32CHANGE;
+	return ata_bmdma_port_start(ap);
+}
+EXPORT_SYMBOL_GPL(ata_bmdma_port_start32);
+
 #ifdef CONFIG_PCI
 
 /**
@@ -3004,6 +2984,19 @@ int ata_pci_bmdma_clear_simplex(struct pci_dev *pdev)
 }
 EXPORT_SYMBOL_GPL(ata_pci_bmdma_clear_simplex);
 
+static void ata_bmdma_nodma(struct ata_host *host, const char *reason)
+{
+	int i;
+
+	dev_printk(KERN_ERR, host->dev, "BMDMA: %s, falling back to PIO\n",
+		   reason);
+
+	for (i = 0; i < 2; i++) {
+		host->ports[i]->mwdma_mask = 0;
+		host->ports[i]->udma_mask = 0;
+	}
+}
+
 /**
  *	ata_pci_bmdma_init - acquire PCI BMDMA resources and init ATA host
  *	@host: target ATA host
@@ -3012,33 +3005,40 @@ EXPORT_SYMBOL_GPL(ata_pci_bmdma_clear_simplex);
  *
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
- *
- *	RETURNS:
- *	0 on success, -errno otherwise.
  */
-int ata_pci_bmdma_init(struct ata_host *host)
+void ata_pci_bmdma_init(struct ata_host *host)
 {
 	struct device *gdev = host->dev;
 	struct pci_dev *pdev = to_pci_dev(gdev);
 	int i, rc;
 
 	/* No BAR4 allocation: No DMA */
-	if (pci_resource_start(pdev, 4) == 0)
-		return 0;
+	if (pci_resource_start(pdev, 4) == 0) {
+		ata_bmdma_nodma(host, "BAR4 is zero");
+		return;
+	}
 
-	/* TODO: If we get no DMA mask we should fall back to PIO */
+	/*
+	 * Some controllers require BMDMA region to be initialized
+	 * even if DMA is not in use to clear IRQ status via
+	 * ->sff_irq_clear method.  Try to initialize bmdma_addr
+	 * regardless of dma masks.
+	 */
 	rc = pci_set_dma_mask(pdev, ATA_DMA_MASK);
 	if (rc)
-		return rc;
-	rc = pci_set_consistent_dma_mask(pdev, ATA_DMA_MASK);
-	if (rc)
-		return rc;
+		ata_bmdma_nodma(host, "failed to set dma mask");
+	if (!rc) {
+		rc = pci_set_consistent_dma_mask(pdev, ATA_DMA_MASK);
+		if (rc)
+			ata_bmdma_nodma(host,
+					"failed to set consistent dma mask");
+	}
 
 	/* request and iomap DMA region */
 	rc = pcim_iomap_regions(pdev, 1 << 4, dev_driver_string(gdev));
 	if (rc) {
-		dev_printk(KERN_ERR, gdev, "failed to request/iomap BAR4\n");
-		return -ENOMEM;
+		ata_bmdma_nodma(host, "failed to request/iomap BAR4");
+		return;
 	}
 	host->iomap = pcim_iomap_table(pdev);
 
@@ -3057,8 +3057,6 @@ int ata_pci_bmdma_init(struct ata_host *host)
 		ata_port_desc(ap, "bmdma 0x%llx",
 		    (unsigned long long)pci_resource_start(pdev, 4) + 8 * i);
 	}
-
-	return 0;
 }
 EXPORT_SYMBOL_GPL(ata_pci_bmdma_init);
 

drivers/ata/pata_acpi.c

@@ -101,7 +101,7 @@ static unsigned long pacpi_discover_modes(struct ata_port *ap, struct ata_device
 static unsigned long pacpi_mode_filter(struct ata_device *adev, unsigned long mask)
 {
 	struct pata_acpi *acpi = adev->link->ap->private_data;
-	return ata_bmdma_mode_filter(adev, mask & acpi->mask[adev->devno]);
+	return mask & acpi->mask[adev->devno];
 }
 
 /**
@@ -205,7 +205,7 @@ static int pacpi_port_start(struct ata_port *ap)
 		return -ENOMEM;
 	acpi->mask[0] = pacpi_discover_modes(ap, &ap->link.device[0]);
 	acpi->mask[1] = pacpi_discover_modes(ap, &ap->link.device[1]);
-	ret = ata_sff_port_start(ap);
+	ret = ata_bmdma_port_start(ap);
 	if (ret < 0)
 		return ret;
 

drivers/ata/pata_ali.c

@@ -124,7 +124,7 @@ static unsigned long ali_20_filter(struct ata_device *adev, unsigned long mask)
 	ata_id_c_string(adev->id, model_num, ATA_ID_PROD, sizeof(model_num));
 	if (strstr(model_num, "WDC"))
 		return mask &= ~ATA_MASK_UDMA;
-	return ata_bmdma_mode_filter(adev, mask);
+	return mask;
 }
 
 /**

drivers/ata/pata_at91.c

@@ -202,7 +202,6 @@ static struct ata_port_operations pata_at91_port_ops = {
 	.sff_data_xfer	= pata_at91_data_xfer_noirq,
 	.set_piomode	= pata_at91_set_piomode,
 	.cable_detect	= ata_cable_40wire,
-	.port_start	= ATA_OP_NULL,
 };
 
 static int __devinit pata_at91_probe(struct platform_device *pdev)

drivers/ata/pata_bf54x.c

@@ -1450,8 +1450,6 @@ static struct ata_port_operations bfin_pata_ops = {
 
 	.port_start		= bfin_port_start,
 	.port_stop		= bfin_port_stop,
-
-	.mode_filter		= ATA_OP_NULL,	/* will be removed soon */
 };
 
 static struct ata_port_info bfin_port_info[] = {

drivers/ata/pata_cmd640.c

@@ -153,16 +153,12 @@ static int cmd640_port_start(struct ata_port *ap)
 	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
 	struct cmd640_reg *timing;
 
-	int ret = ata_sff_port_start(ap);
-	if (ret < 0)
-		return ret;
-
 	timing = devm_kzalloc(&pdev->dev, sizeof(struct cmd640_reg), GFP_KERNEL);
 	if (timing == NULL)
 		return -ENOMEM;
 	timing->last = -1;	/* Force a load */
 	ap->private_data = timing;
-	return ret;
+	return 0;
 }
 
 static struct scsi_host_template cmd640_sht = {

drivers/ata/pata_hpt366.c

@@ -182,7 +182,7 @@ static unsigned long hpt366_filter(struct ata_device *adev, unsigned long mask)
 	} else if (adev->class == ATA_DEV_ATAPI)
 		mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
 
-	return ata_bmdma_mode_filter(adev, mask);
+	return mask;
 }
 
 static int hpt36x_cable_detect(struct ata_port *ap)

drivers/ata/pata_hpt37x.c

@@ -282,7 +282,7 @@ static unsigned long hpt370_filter(struct ata_device *adev, unsigned long mask)
 		if (hpt_dma_blacklisted(adev, "UDMA100", bad_ata100_5))
 			mask &= ~(0xE0 << ATA_SHIFT_UDMA);
 	}
-	return ata_bmdma_mode_filter(adev, mask);
+	return mask;
 }
 
 /**
@@ -298,7 +298,7 @@ static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask)
 		if (hpt_dma_blacklisted(adev, "UDMA100", bad_ata100_5))
 			mask &= ~(0xE0 << ATA_SHIFT_UDMA);
 	}
-	return ata_bmdma_mode_filter(adev, mask);
+	return mask;
 }
 
 /**

drivers/ata/pata_icside.c

@@ -335,7 +335,7 @@ static struct ata_port_operations pata_icside_port_ops = {
 	.postreset		= pata_icside_postreset,
 	.post_internal_cmd	= pata_icside_bmdma_stop,
 
-	.mode_filter		= ATA_OP_NULL,	/* will be removed soon */
+	.port_start		= ATA_OP_NULL,	/* don't need PRD table */
 };
 
 static void __devinit