[PATCHSET] libata: implement new initialization model w/ iomap support

[PATCHSET] libata: implement new initialization model w/ iomap support

[Tejun Heo]

Hello, all.

This is the first take of new-init-model patchset.  This patchset
implements new init model for libata LLDs.  The new init model has the
following benefits.

* Consistency across different LLDs

  Provide helpers such that common codes live in one place and init
  sequence doesn't have to be creative.

* Flexibility and easy code sharing with suspend/resume

  Instead of using probe_ent, allocate host_set and initialize it
  directly.  This removes many restrictions of probe_ent and allows
  LLDs to share init/remove codes with suspend/resume easily as they
  use the same target data structure.

  IRQ management is also moved out of libata-core into libata-pci
  helpers which LLDs can use or override at will.

* More bookkeeping in libata-core, less in LLDDs

  Acquired resources are tracked by libata-core and helpers.  e.g. LLD
  doesn't have to remember which resources it has acquired, it can
  simply tell libata to release all resources on error path and driver
  detachments.  This simplies LLDs and fixes several dangling resource
  and double-free bugs.

* Support for iomap

  As with all other PCI resources, both PIO and mmio areas are managed
  by libata PCI helpers.  iomaps are automatically created on IO
  resource acqusition and destroyed on release.  LLDs can simply
  request which BARs it intends to use.  The rest is handled by
  libata PCI helpers.

This patchset has been tested on

* ata_piix	: ICH7R, most combinations and w/ induced resource
		  allocation errors
* ahci		: ICH7R
* sata_sil	: sil3112, sil3114
* sata_sil24	: sil3124, sil3132
* sata_via	: vt6420 (via C3)
* pdc_adma	: as broken as previous
* sata_promise	: PDC20375, tested w/ PATA port too (PATA bits not
		  included in this post)

If you have any controller which use one of the following drivers,
please test and report.

* sata_svw
* sata_sx4
* sata_uli
* sata_vsc
* sata_sis
* sata_qstor
* sata_nv (I can do this later)
* sata_mv

If this patchset is agreed upon, I'll update #pata-drivers
accordingly.

This patchset is against

[U] upstream
[1] + #upstream-fixes + several-fixes-against-upstream-fixes merged
[2] + improve-initialization-and-legacy-handling patchset, take #2
[3] + ahci-remove-irq-clearing-from-init_controller

[U] + [1] + [2] is #tj-upstream which is available at the following
URLs.

  http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=tj-upstream
  git://htj.dyndns.org/libata-tj tj-upstream 

Thanks.

--
tejun

[U] 236a686b56428a8967a057a2396f9be74e2ee652
[1] http://article.gmane.org/gmane.linux.ide/12393
[2] http://article.gmane.org/gmane.linux.ide/12422
[3] http://article.gmane.org/gmane.linux.ide/12423

[PATCH 04/17] libata: implement several LLD init helpers

[Tejun Heo]

Implement the following init helpers.

* ata_host_set_alloc_pinfo():
	alloc host_set and init with the given port_info.
* ata_host_set_alloc_pinfo_ar():
	alloc host_set and init with the given port_info array.
* ata_host_set_add_ports_pinfo():
	add ports to host_set and init with the given port_info.
* ata_host_set_add_ports_pinfo_ar():
	add ports to host_set and init with the given port_info array.
* ata_host_set_request_irq():
	prep host_set for IRQ enabling and request IRQ.

These helpers will be used in new LLD init model.

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

---

 drivers/scsi/libata-core.c |  201 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |   17 +++-
 2 files changed, 215 insertions(+), 3 deletions(-)

23919795368e0b7ed49f63b7aeb36fc5217c6ec7
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index eccc6bc..78ee73a 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5369,6 +5369,147 @@ int ata_host_set_add_ports(struct ata_ho
 	return 0;
 }
 
+static void __ata_host_set_init_pinfo(struct ata_host_set *host_set,
+				      const struct ata_port_info **pinfo,
+				      int n_ports, int pi_is_ar)
+{
+	int i;
+
+	if (host_set->private_data == NULL)
+		host_set->private_data = pinfo[0]->private_data;
+	host_set->ops = pinfo[0]->port_ops;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+		const struct ata_port_info *pi;
+
+		if (pi_is_ar)
+			pi = pinfo[i];
+		else
+			pi = pinfo[0];
+
+		ap->pio_mask = pi->pio_mask;
+		ap->mwdma_mask = pi->mwdma_mask;
+		ap->udma_mask = pi->udma_mask;
+		ap->flags |= pi->host_flags;
+		ap->ops = pi->port_ops;
+
+		WARN_ON(pi->private_data &&
+			pi->private_data != host_set->private_data);
+	}
+}
+
+/**
+ *	ata_host_set_alloc_pinfo - allocate host_set and init with port_info
+ *	@dev: generic device this host_set is associated with
+ *	@pinfo: ATA port_info to initialize host_set with
+ *	@n_ports: number of ATA ports attached to this host_set
+ *
+ *	Allocate ATA host_set and initialize with info from @pi.
+ *
+ *	RETURNS:
+ *	Allocate ATA host_set on success, NULL on failure.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+struct ata_host_set *
+ata_host_set_alloc_pinfo(struct device *dev,
+			 const struct ata_port_info *pinfo, int n_ports)
+{
+	struct ata_host_set *host_set;
+
+	if (!n_ports)
+		return NULL;
+
+	host_set = ata_host_set_alloc(dev, pinfo->sht, n_ports);
+	if (host_set)
+		__ata_host_set_init_pinfo(host_set, &pinfo, n_ports, 0);
+	return host_set;
+}
+
+/**
+ *	ata_host_set_alloc_pinfo_ar - alloc host_set and init with port_info ar
+ *	@dev: generic device this host_set is associated with
+ *	@pinfo_ar: array of ATA port_info to initialize host_set with
+ *	@n_ports: number of ATA ports attached to this host_set
+ *
+ *	Allocate ATA host_set and initialize with info from @pinfo_ar.
+ *
+ *	RETURNS:
+ *	Allocate ATA host_set on success, NULL on failure.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+struct ata_host_set *
+ata_host_set_alloc_pinfo_ar(struct device *dev,
+			    const struct ata_port_info **pinfo_ar, int n_ports)
+{
+	struct ata_host_set *host_set;
+
+	if (!n_ports)
+		return NULL;
+
+	host_set = ata_host_set_alloc(dev, pinfo_ar[0]->sht, n_ports);
+	if (host_set)
+		__ata_host_set_init_pinfo(host_set, pinfo_ar, n_ports, 1);
+	return host_set;
+}
+
+/**
+ *	ata_host_set_add_ports_pinfo - add ports and init with port_info
+ *	@host_set: target ATA host_set
+ *	@pinfo: ATA port_info to initialize host_set with
+ *	@n_ports: number of ATA ports attached to this host_set
+ *
+ *	Add @n_ports ports to @host_set and initialize @host_set with
+ *	info from @pinfo.
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+int ata_host_set_add_ports_pinfo(struct ata_host_set *host_set,
+				 const struct ata_port_info *pinfo, int n_ports)
+{
+	int rc;
+
+	rc = ata_host_set_add_ports(host_set, pinfo->sht, n_ports);
+	if (rc == 0)
+		__ata_host_set_init_pinfo(host_set, &pinfo, n_ports, 0);
+	return rc;
+}
+
+/**
+ *	ata_host_set_add_ports_pinfo_ar - add ports and init with port_info ar
+ *	@host_set: target ATA host_set
+ *	@pinfo_ar: array of ATA port_info to initialize host_set with
+ *	@n_ports: number of ATA ports attached to this host_set
+ *
+ *	Add @n_ports ports to @host_set and initialize @host_set with
+ *	info from @pinfo_ar.
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+int ata_host_set_add_ports_pinfo_ar(struct ata_host_set *host_set,
+				    const struct ata_port_info **pinfo_ar,
+				    int n_ports)
+{
+	int rc;
+
+	rc = ata_host_set_add_ports(host_set, pinfo_ar[0]->sht, n_ports);
+	if (rc == 0)
+		__ata_host_set_init_pinfo(host_set, pinfo_ar, n_ports, 1);
+	return rc;
+}
+
 /**
  *	ata_host_set_start - start ports of an ATA host_set
  *	@host_set: ATA host_set to start ports for
@@ -5417,6 +5558,61 @@ int ata_host_set_start(struct ata_host_s
 }
 
 /**
+ *	ata_host_set_request_irq - request IRQ helper
+ *	@host_set: ATA host_set requesting IRQ for
+ *	@irq: irq to request
+ *	@handler: irq handler
+ *	@irq_flags: irq flags
+ *	@reason: out arg for error message
+ *
+ *	Freeze all ports and request IRQ with given parameters.
+ *	dev_id for the IRQ will be @host_set and devname the name of
+ *	the associated LLD.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer.
+ *
+ *	RETURNS:
+ *	Return value of request_irq().
+ */
+int ata_host_set_request_irq(struct ata_host_set *host_set,
+			unsigned int irq,
+			irqreturn_t (*handler)(int, void *, struct pt_regs *),
+			unsigned long irq_flags, const char **reason)
+{
+	const char *drv_name;
+	int i, rc;
+
+	/* make sure ports are started */
+	rc = ata_host_set_start(host_set);
+	if (rc) {
+		*reason = "failed to start host_set";
+		return rc;
+	}
+
+	/* freeze */
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		ata_chk_status(ap);
+		host_set->ops->irq_clear(ap);
+		ata_eh_freeze_port(ap);	/* freeze port before requesting IRQ */
+	}
+
+	/* request irq */
+	drv_name = dev_driver_string(host_set->dev);
+	if (drv_name[0] == '\0')
+		drv_name = DRV_NAME;
+
+	host_set->irq = irq;
+
+	rc = request_irq(irq, handler, irq_flags, drv_name, host_set);
+	if (rc)
+		*reason = "failed to request IRQ";
+	return rc;
+}
+
+/**
  *	ata_host_set_attach - attach initialized ATA host_set
  *	@host_set: ATA host_set to attach
  *
@@ -6117,7 +6313,12 @@ EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_std_ports);
 EXPORT_SYMBOL_GPL(ata_host_set_alloc);
 EXPORT_SYMBOL_GPL(ata_host_set_add_ports);
+EXPORT_SYMBOL_GPL(ata_host_set_alloc_pinfo);
+EXPORT_SYMBOL_GPL(ata_host_set_alloc_pinfo_ar);
+EXPORT_SYMBOL_GPL(ata_host_set_add_ports_pinfo);
+EXPORT_SYMBOL_GPL(ata_host_set_add_ports_pinfo_ar);
 EXPORT_SYMBOL_GPL(ata_host_set_start);
+EXPORT_SYMBOL_GPL(ata_host_set_request_irq);
 EXPORT_SYMBOL_GPL(ata_host_set_attach);
 EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_set_detach);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index fed12ae..91b1fc5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -692,10 +692,21 @@ extern int ata_pci_device_resume(struct 
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);
 #endif /* CONFIG_PCI */
 extern struct ata_host_set *ata_host_set_alloc(struct device *dev,
-					       struct scsi_host_template *sht,
-					       int n_ports);
+			struct scsi_host_template *sht, int n_ports);
 extern int ata_host_set_add_ports(struct ata_host_set *host_set,
-				  struct scsi_host_template *sht, int n_ports);
+			struct scsi_host_template *sht, int n_ports);
+extern struct ata_host_set *ata_host_set_alloc_pinfo(struct device *dev,
+			const struct ata_port_info *pinfo, int n_ports);
+extern struct ata_host_set *ata_host_set_alloc_pinfo_ar(struct device *dev,
+			const struct ata_port_info **pinfo_ar, int n_ports);
+extern int ata_host_set_add_ports_pinfo(struct ata_host_set *host_set,
+			const struct ata_port_info *pinfo, int n_ports);
+extern int ata_host_set_add_ports_pinfo_ar(struct ata_host_set *host_set,
+			const struct ata_port_info **pinfo_ar, int n_ports);
+extern int ata_host_set_request_irq(struct ata_host_set *host_set,
+			unsigned int irq,
+			irqreturn_t (*handler)(int, void *, struct pt_regs *),
+			unsigned long irq_flags, const char **reason);
 extern int ata_host_set_start(struct ata_host_set *host_set);
 extern int ata_host_set_attach(struct ata_host_set *host_set);
 extern int ata_device_add(const struct ata_probe_ent *ent);
-- 
1.3.2

Re: [PATCH 04/17] libata: implement several LLD init helpers

[Jeff Garzik]

Tejun Heo wrote:
> Implement the following init helpers.
> 
> * ata_host_set_alloc_pinfo():
> 	alloc host_set and init with the given port_info.
> * ata_host_set_alloc_pinfo_ar():
> 	alloc host_set and init with the given port_info array.
> * ata_host_set_add_ports_pinfo():
> 	add ports to host_set and init with the given port_info.
> * ata_host_set_add_ports_pinfo_ar():
> 	add ports to host_set and init with the given port_info array.
> * ata_host_set_request_irq():
> 	prep host_set for IRQ enabling and request IRQ.
> 
> These helpers will be used in new LLD init model.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

OK except the names are too long

Re: [PATCH 04/17] libata: implement several LLD init helpers

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> Implement the following init helpers.
>>
>> * ata_host_set_alloc_pinfo():
>>     alloc host_set and init with the given port_info.
>> * ata_host_set_alloc_pinfo_ar():
>>     alloc host_set and init with the given port_info array.
>> * ata_host_set_add_ports_pinfo():
>>     add ports to host_set and init with the given port_info.
>> * ata_host_set_add_ports_pinfo_ar():
>>     add ports to host_set and init with the given port_info array.
>> * ata_host_set_request_irq():
>>     prep host_set for IRQ enabling and request IRQ.
>>
>> These helpers will be used in new LLD init model.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> OK except the names are too long

hoping to do s/host_set/host/g....

-- 
tejun

[PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Tejun Heo]

Implement the following PCI init helpers for new LLD init model.

* ata_pci_host_set_init_native(): init ports for native PCI host_set
* ata_pci_legacy_mask(): obtain legacy mask of ATA PCI device
* ata_pci_set_dma_mask(): set DMA mask helper
* ata_pci_acquire_resources(): acquire generic ATA PCI resources for host_set
* ata_pci_request_irq(): prep host_set for IRQ and request IRQ.
* ata_pci_release_resources(): release generic ATA PCI resources including IRQs
* ata_pci_host_set_destroy(): release associated resources and destroy host_set

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

---

 drivers/scsi/libata-bmdma.c |  390 +++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libata-core.c  |    7 +
 include/linux/libata.h      |   36 ++++
 3 files changed, 431 insertions(+), 2 deletions(-)

139908fea67d9fa2c835b1c92e76112ab45e295c
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index c4cd578..341d9b5 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -797,6 +797,396 @@ void ata_bmdma_post_internal_cmd(struct 
 }
 
 #ifdef CONFIG_PCI
+static const unsigned long ata_legacy_addr_tbl[][2] = {
+	{ ATA_PRIMARY_CMD, ATA_PRIMARY_CTL },
+	{ ATA_SECONDARY_CMD, ATA_SECONDARY_CTL}
+};
+
+static void ata_pci_port_init(struct ata_port *ap, unsigned long cmd_addr,
+			      unsigned long ctl_addr, unsigned long bmdma_addr)
+{
+	struct ata_ioports *ioaddr = &ap->ioaddr;
+
+	ioaddr->cmd_addr = cmd_addr;
+	ioaddr->altstatus_addr = ctl_addr;
+	ioaddr->ctl_addr = ctl_addr;
+
+	if (bmdma_addr) {
+		if (inb(bmdma_addr + 2) & 0x80)
+			ap->host_set->flags |= ATA_HOST_SIMPLEX;
+		ioaddr->bmdma_addr = bmdma_addr;
+	}
+	ata_std_ports(ioaddr);
+}
+
+static void ata_pci_port_init_native(struct ata_port *ap, int port)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
+	unsigned long cmd_addr, ctl_addr, bmdma_addr;
+
+	cmd_addr = pci_resource_start(pdev, port * 2);
+	ctl_addr = pci_resource_start(pdev, port * 2 + 1) | ATA_PCI_CTL_OFS;
+	bmdma_addr = pci_resource_start(pdev, 4);
+	if (bmdma_addr)
+		bmdma_addr += port * 8;
+
+	ata_pci_port_init(ap, cmd_addr, ctl_addr, bmdma_addr);
+}
+
+static void ata_pci_port_init_legacy(struct ata_port *ap, int port)
+{
+	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
+	unsigned long cmd_addr, ctl_addr, bmdma_addr;
+
+	cmd_addr = ata_legacy_addr_tbl[port][0];
+	ctl_addr = ata_legacy_addr_tbl[port][1];
+	bmdma_addr = pci_resource_start(pdev, 4);
+	if (bmdma_addr)
+		bmdma_addr += port * 8;
+
+	ata_pci_port_init(ap, cmd_addr, ctl_addr, bmdma_addr);
+}
+
+/**
+ *	ata_pci_host_set_init_native - init host_set for native PCI ATA
+ *	@host_set: target ATA host_set
+ *
+ *	Initialize @host_set for native mode PCI ATA.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+void ata_pci_host_set_init_native(struct ata_host_set *host_set)
+{
+	ata_pci_port_init_native(host_set->ports[0], 0);
+	if (host_set->n_ports > 1)
+		ata_pci_port_init_native(host_set->ports[1], 1);
+}
+
+/**
+ *	ata_pci_legacy_mask - obatin legacy mask from PCI IDE device
+ *	@pdev: target PCI device
+ *
+ *	Obtain legacy mask from @pdev.
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	Obtained legacy mask.
+ */
+unsigned int ata_pci_legacy_mask(struct pci_dev *pdev)
+{
+	unsigned int mask = 0;
+	u8 tmp8;
+
+	if (pdev->class >> 8 != PCI_CLASS_STORAGE_IDE)
+		return 0;
+
+	pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
+
+	if (!(tmp8 & (1 << 0)))
+		mask |= ATA_PORT_PRIMARY;
+
+	if (!(tmp8 & (1 << 2)))
+		mask |= ATA_PORT_SECONDARY;
+
+	return mask;
+}
+
+static int ata_pci_acquire_legacy(int port, unsigned long *host_set_flags)
+{
+	unsigned long cmd_addr = ata_legacy_addr_tbl[port][0];
+
+	if (request_region(cmd_addr, 8, "libata") != NULL)
+		*host_set_flags |= ATA_HOST_RES_LEGACY_PRI << port;
+	else {
+		struct resource *conflict, res;
+
+		res.start = cmd_addr;
+		res.end = cmd_addr + 8 - 1;
+		conflict = ____request_resource(&ioport_resource, &res);
+
+		if (strcmp(conflict->name, "libata")) {
+			printk(KERN_WARNING "ata: 0x%0lX IDE port busy\n",
+			       cmd_addr);
+			*host_set_flags |= ATA_HOST_PCI_DEV_BUSY;
+			return -EBUSY;
+		}
+		printk("ata: 0x%0lX IDE port preallocated\n", cmd_addr);
+	}
+
+	return 0;
+}
+
+/**
+ *	ata_pci_set_dma_mask - PCI DMA mask set helper
+ *	@pdev: target PCI device
+ *	@dma_mask: DMA mask
+ *	@p_reason: output arg for error message
+ *
+ *	Helper to set PCI DMA mask.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+int ata_pci_set_dma_mask(struct pci_dev *pdev, u64 dma_mask,
+			 const char **p_reason)
+{
+	const char *reason;
+	int rc = 0;
+
+	if (dma_mask == DMA_64BIT_MASK) {
+		if (pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+			if (rc) {
+				rc = pci_set_consistent_dma_mask(pdev,
+							DMA_32BIT_MASK);
+				if (rc) {
+					reason = "64-bit DMA enable failed";
+					goto err;
+				}
+			}
+		} else {
+			rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				reason = "32-bit DMA enable failed";
+				goto err;
+			}
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				reason = "32-bit consistent DMA enable failed";
+				goto err;
+			}
+		}
+	} else if (dma_mask) {
+		reason = "failed to set DMA mask";
+		rc = pci_set_dma_mask(pdev, dma_mask);
+		if (rc)
+			goto err;
+		rc = pci_set_consistent_dma_mask(pdev, dma_mask);
+		if (rc)
+			goto err;
+	}
+
+	return 0;
+
+ err:
+	if (p_reason)
+		*p_reason = reason;
+	return rc;
+}
+
+/**
+ *	ata_pci_acquire_resources - acquire default PCI resources
+ *	@host_set: target ATA host_set to acquire PCI resources for
+ *	@dma_mask: DMA mask
+ *	@reason: output arg for error message
+ *
+ *	Acquire default ATA PCI resources.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+int ata_pci_acquire_resources(struct ata_host_set *host_set, u64 dma_mask,
+			      const char **reason)
+{
+	struct pci_dev *pdev = to_pci_dev(host_set->dev);
+	int nr_dummy, i, rc;
+
+	/* acquire generic resources */
+
+	/* FIXME: Really for ATA it isn't safe because the device may
+	 * be multi-purpose and we want to leave it alone if it was
+	 * already enabled. Secondly for shared use as Arjan says we
+	 * want refcounting
+	 *
+	 * Checking dev->is_enabled is insufficient as this is not set
+	 * at boot for the primary video which is BIOS enabled
+         */
+	rc = pci_enable_device(pdev);
+	if (rc) {
+		*reason = "failed to enable PCI device";
+		goto err;
+	}
+
+	rc = pci_request_regions(pdev, DRV_NAME);
+	if (rc) {
+		host_set->flags |= ATA_HOST_PCI_DEV_BUSY;
+		*reason = "failed to request PCI regions";
+		goto err;
+	}
+
+	host_set->flags |= ATA_HOST_PCI_RES_GEN;
+
+	/* set DMA mask */
+	/* FIXME: If we get no DMA mask we should fall back to PIO */
+	rc = ata_pci_set_dma_mask(pdev, dma_mask, reason);
+	if (rc)
+		goto err;
+
+	/* Acquire legacy resources.  For legacy ports, host_set must
+	 * be popultaed with ports before calling this function.
+	 */
+	nr_dummy = 0;
+	for (i = 0; i < 2; i++) {
+		if (!(host_set->flags & (ATA_HOST_LEGACY_PRI << i)))
+			continue;
+		BUG_ON(i >= host_set->n_ports);
+
+		if (ata_pci_acquire_legacy(i, &host_set->flags)) {
+			host_set->ports[i]->ops = &ata_dummy_port_ops;
+			nr_dummy++;
+		}
+	}
+
+	if (nr_dummy && nr_dummy == host_set->n_ports) {
+		*reason = "no available port";
+		rc = -ENODEV;
+		goto err;
+	}
+
+	return 0;
+
+ err:
+	ata_pci_release_resources(host_set);
+	return rc;
+}
+
+/**
+ *	ata_pci_request_irq - request PCI irq for ATA host_set
+ *	@host_set: ATA host_set to request PCI irq for
+ *	@irq_handler: irq_handler
+ *	@flags: ATA_PCI_IRQ_* flags
+ *	@reason: output arg for error message
+ *
+ *	Request PCI irq for @host_set.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer.
+ *
+ *	RETURNS:
+ *	Return value of request_irq().
+ */
+int ata_pci_request_irq(struct ata_host_set *host_set,
+		irqreturn_t (*irq_handler)(int, void *, struct pt_regs *),
+		unsigned int flags, const char **reason)
+{
+	struct pci_dev *pdev = to_pci_dev(host_set->dev);
+	unsigned int irq_flags = 0;
+	int rc;
+
+	if (flags & ATA_PCI_IRQ_MSI) {
+		flags &= ~ATA_PCI_IRQ_INTX;
+
+		if (pci_enable_msi(pdev) == 0)
+			host_set->flags |= ATA_HOST_PCI_RES_MSI;
+		else
+			flags |= ATA_PCI_IRQ_INTX;
+	}
+
+	if (flags & ATA_PCI_IRQ_INTX) {
+		pci_intx(pdev, 1);
+		host_set->flags |= ATA_HOST_PCI_RES_INTX;
+	}
+
+	if (!(flags & ATA_PCI_IRQ_EXCL))
+		irq_flags |= IRQF_SHARED;
+
+	rc = ata_host_set_request_irq(host_set, pdev->irq, irq_handler,
+				      irq_flags, reason);
+	if (rc == 0)
+		host_set->flags |= ATA_HOST_PCI_RES_IRQ;
+
+	return rc;
+}
+
+/**
+ *	ata_pci_release_resources - release all ATA PCI resources
+ *	@pdev: target ATA host_set
+ *
+ *	Release all ATA PCI resources.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+void ata_pci_release_resources(struct ata_host_set *host_set)
+{
+	struct pci_dev *pdev = to_pci_dev(host_set->dev);
+	int i;
+
+	/* stop all ports */
+	ata_host_set_stop(host_set);
+
+	/* release IRQs */
+	if (host_set->flags & ATA_HOST_PCI_RES_IRQ15) {
+		free_irq(15, host_set);
+		host_set->flags &= ~ATA_HOST_PCI_RES_IRQ15;
+	}
+
+	if (host_set->flags & ATA_HOST_PCI_RES_IRQ14) {
+		free_irq(14, host_set);
+		host_set->flags &= ~ATA_HOST_PCI_RES_IRQ14;
+	}
+
+	if (host_set->flags & ATA_HOST_PCI_RES_IRQ) {
+		free_irq(pdev->irq, host_set);
+		host_set->flags &= ~ATA_HOST_PCI_RES_IRQ;
+	}
+
+	if (host_set->flags & ATA_HOST_PCI_RES_INTX) {
+		pci_intx(pdev, 0);
+		host_set->flags &= ~ATA_HOST_PCI_RES_INTX;
+	}
+
+	if (host_set->flags & ATA_HOST_PCI_RES_MSI) {
+		pci_disable_msi(pdev);
+		host_set->flags &= ~ATA_HOST_PCI_RES_MSI;
+	}
+
+	/* release legacy resources */
+	for (i = 0; i < 2; i++) {
+		if (host_set->flags & (ATA_HOST_RES_LEGACY_PRI << i)) {
+			release_region(ata_legacy_addr_tbl[i][0], 8);
+			host_set->flags &= ~(ATA_HOST_RES_LEGACY_PRI << i);
+		}
+	}
+
+	/* release generic resources */
+	if (host_set->flags & ATA_HOST_PCI_RES_GEN) {
+		pci_release_regions(pdev);
+
+		if (!(host_set->flags & ATA_HOST_PCI_DEV_BUSY))
+			pci_disable_device(pdev);
+
+		host_set->flags &= ~ATA_HOST_PCI_RES_GEN;
+	}
+}
+
+/**
+ *	ata_pci_host_set_destroy - release all PCI resources and free host_set
+ *	@host_set: target ATA host_set
+ *
+ *	Release all ATA PCI resources associated with @host_set and
+ *	free it.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+void ata_pci_host_set_destroy(struct ata_host_set *host_set)
+{
+	if (host_set) {
+		ata_pci_release_resources(host_set);
+		ata_host_set_free(host_set);
+	}
+}
+
 static struct ata_probe_ent *
 ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port)
 {
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 78ee73a..d44c1df 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -6403,6 +6403,13 @@ EXPORT_SYMBOL_GPL(ata_timing_merge);
 #ifdef CONFIG_PCI
 EXPORT_SYMBOL_GPL(pci_test_config_bits);
 EXPORT_SYMBOL_GPL(ata_pci_host_stop);
+EXPORT_SYMBOL_GPL(ata_pci_host_set_init_native);
+EXPORT_SYMBOL_GPL(ata_pci_legacy_mask);
+EXPORT_SYMBOL_GPL(ata_pci_set_dma_mask);
+EXPORT_SYMBOL_GPL(ata_pci_acquire_resources);
+EXPORT_SYMBOL_GPL(ata_pci_request_irq);
+EXPORT_SYMBOL_GPL(ata_pci_release_resources);
+EXPORT_SYMBOL_GPL(ata_pci_host_set_destroy);
 EXPORT_SYMBOL_GPL(ata_pci_init_native_mode);
 EXPORT_SYMBOL_GPL(ata_pci_init_one);
 EXPORT_SYMBOL_GPL(ata_pci_remove_one);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 91b1fc5..0ae0fbb 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -200,8 +200,24 @@ enum {
 	ATA_QCFLAG_EH_SCHEDULED = (1 << 18), /* EH scheduled (obsolete) */
 
 	/* host set flags */
-	ATA_HOST_SIMPLEX	= (1 << 0),	/* Host is simplex, one DMA channel per host_set only */
-	
+	/* keep ATA_HOST_*_LEGACY in sync with ATA_PORT_PRIMARY/SECONDARY */
+	ATA_HOST_LEGACY_PRI	= (1 << 0), /* primary port is legacy */
+	ATA_HOST_LEGACY_SEC	= (1 << 1), /* secondary port is legacy */
+	ATA_HOST_LEGACY_MASK	= ATA_HOST_LEGACY_PRI | ATA_HOST_LEGACY_SEC,
+
+	ATA_HOST_SIMPLEX	= (1 << 2), /* Host is simplex, one DMA channel per host_set only */
+
+	/* host_set resource flags */
+	ATA_HOST_RES_LEGACY_PRI	= (1 << 8), /* holding primary legacy */
+	ATA_HOST_RES_LEGACY_SEC	= (1 << 9), /* holding secondary legacy */
+	ATA_HOST_PCI_DEV_BUSY	= (1 << 10), /* if set, don't disable pdev */
+	ATA_HOST_PCI_RES_GEN	= (1 << 11), /* holding generic PCI resources */
+	ATA_HOST_PCI_RES_IRQ	= (1 << 12), /* holding PCI IRQ */
+	ATA_HOST_PCI_RES_IRQ14	= (1 << 13), /* holding legacy IRQ 14 */
+	ATA_HOST_PCI_RES_IRQ15	= (1 << 14), /* holding legacy IRQ 15 */
+	ATA_HOST_PCI_RES_INTX	= (1 << 15), /* enabled INTX */
+	ATA_HOST_PCI_RES_MSI	= (1 << 16), /* enabled MSI */
+
 	/* various lengths of time */
 	ATA_TMOUT_BOOT		= 30 * HZ,	/* heuristic */
 	ATA_TMOUT_BOOT_QUICK	= 7 * HZ,	/* heuristic */
@@ -248,6 +264,11 @@ enum {
 	ATA_PORT_PRIMARY	= (1 << 0),
 	ATA_PORT_SECONDARY	= (1 << 1),
 
+	/* flags for ata_pci_request_irq() */
+	ATA_PCI_IRQ_EXCL	= (1 << 0), /* don't set IRQF_SHARED */
+	ATA_PCI_IRQ_INTX	= (1 << 1), /* enable IRQ with INTX */
+	ATA_PCI_IRQ_MSI		= (1 << 2), /* try MSI (implies INTX) */
+
 	/* ering size */
 	ATA_ERING_SIZE		= 32,
 
@@ -840,6 +861,17 @@ struct pci_bits {
 };
 
 extern void ata_pci_host_stop (struct ata_host_set *host_set);
+extern void ata_pci_host_set_init_native(struct ata_host_set *host_set);
+extern unsigned int ata_pci_legacy_mask(struct pci_dev *pdev);
+extern int ata_pci_set_dma_mask(struct pci_dev *pdev, u64 dma_mask,
+				const char **p_reason);
+extern int ata_pci_acquire_resources(struct ata_host_set *host_set,
+				     u64 dma_mask, const char **reason);
+extern int ata_pci_request_irq(struct ata_host_set *host_set,
+		irqreturn_t (*irq_handler)(int, void *, struct pt_regs *),
+		unsigned int flags, const char **reason);
+extern void ata_pci_release_resources(struct ata_host_set *host_set);
+extern void ata_pci_host_set_destroy(struct ata_host_set *host_set);
 extern struct ata_probe_ent *
 ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int portmask);
 extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits);
-- 
1.3.2

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Jeff Garzik]

Tejun Heo wrote:
> Implement the following PCI init helpers for new LLD init model.
> 
> * ata_pci_host_set_init_native(): init ports for native PCI host_set
> * ata_pci_legacy_mask(): obtain legacy mask of ATA PCI device
> * ata_pci_set_dma_mask(): set DMA mask helper
> * ata_pci_acquire_resources(): acquire generic ATA PCI resources for host_set
> * ata_pci_request_irq(): prep host_set for IRQ and request IRQ.
> * ata_pci_release_resources(): release generic ATA PCI resources including IRQs
> * ata_pci_host_set_destroy(): release associated resources and destroy host_set
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/libata-bmdma.c |  390 +++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/libata-core.c  |    7 +
>  include/linux/libata.h      |   36 ++++
>  3 files changed, 431 insertions(+), 2 deletions(-)
> 
> 139908fea67d9fa2c835b1c92e76112ab45e295c
> diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
> index c4cd578..341d9b5 100644
> --- a/drivers/scsi/libata-bmdma.c
> +++ b/drivers/scsi/libata-bmdma.c
> @@ -797,6 +797,396 @@ void ata_bmdma_post_internal_cmd(struct 
>  }
>  
>  #ifdef CONFIG_PCI
> +static const unsigned long ata_legacy_addr_tbl[][2] = {
> +	{ ATA_PRIMARY_CMD, ATA_PRIMARY_CTL },
> +	{ ATA_SECONDARY_CMD, ATA_SECONDARY_CTL}
> +};
> +
> +static void ata_pci_port_init(struct ata_port *ap, unsigned long cmd_addr,
> +			      unsigned long ctl_addr, unsigned long bmdma_addr)
> +{
> +	struct ata_ioports *ioaddr = &ap->ioaddr;
> +
> +	ioaddr->cmd_addr = cmd_addr;
> +	ioaddr->altstatus_addr = ctl_addr;
> +	ioaddr->ctl_addr = ctl_addr;
> +
> +	if (bmdma_addr) {
> +		if (inb(bmdma_addr + 2) & 0x80)
> +			ap->host_set->flags |= ATA_HOST_SIMPLEX;
> +		ioaddr->bmdma_addr = bmdma_addr;
> +	}
> +	ata_std_ports(ioaddr);
> +}
> +
> +static void ata_pci_port_init_native(struct ata_port *ap, int port)
> +{
> +	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
> +	unsigned long cmd_addr, ctl_addr, bmdma_addr;
> +
> +	cmd_addr = pci_resource_start(pdev, port * 2);
> +	ctl_addr = pci_resource_start(pdev, port * 2 + 1) | ATA_PCI_CTL_OFS;
> +	bmdma_addr = pci_resource_start(pdev, 4);
> +	if (bmdma_addr)
> +		bmdma_addr += port * 8;
> +
> +	ata_pci_port_init(ap, cmd_addr, ctl_addr, bmdma_addr);
> +}
> +
> +static void ata_pci_port_init_legacy(struct ata_port *ap, int port)
> +{
> +	struct pci_dev *pdev = to_pci_dev(ap->host_set->dev);
> +	unsigned long cmd_addr, ctl_addr, bmdma_addr;
> +
> +	cmd_addr = ata_legacy_addr_tbl[port][0];
> +	ctl_addr = ata_legacy_addr_tbl[port][1];
> +	bmdma_addr = pci_resource_start(pdev, 4);
> +	if (bmdma_addr)
> +		bmdma_addr += port * 8;
> +
> +	ata_pci_port_init(ap, cmd_addr, ctl_addr, bmdma_addr);
> +}
> +
> +/**
> + *	ata_pci_host_set_init_native - init host_set for native PCI ATA
> + *	@host_set: target ATA host_set
> + *
> + *	Initialize @host_set for native mode PCI ATA.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer (may sleep).
> + */
> +void ata_pci_host_set_init_native(struct ata_host_set *host_set)
> +{
> +	ata_pci_port_init_native(host_set->ports[0], 0);
> +	if (host_set->n_ports > 1)
> +		ata_pci_port_init_native(host_set->ports[1], 1);
> +}
> +
> +/**
> + *	ata_pci_legacy_mask - obatin legacy mask from PCI IDE device
> + *	@pdev: target PCI device
> + *
> + *	Obtain legacy mask from @pdev.
> + *
> + *	LOCKING:
> + *	None.
> + *
> + *	RETURNS:
> + *	Obtained legacy mask.
> + */
> +unsigned int ata_pci_legacy_mask(struct pci_dev *pdev)
> +{
> +	unsigned int mask = 0;
> +	u8 tmp8;
> +
> +	if (pdev->class >> 8 != PCI_CLASS_STORAGE_IDE)
> +		return 0;
> +
> +	pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
> +
> +	if (!(tmp8 & (1 << 0)))
> +		mask |= ATA_PORT_PRIMARY;
> +
> +	if (!(tmp8 & (1 << 2)))
> +		mask |= ATA_PORT_SECONDARY;
> +
> +	return mask;
> +}
> +
> +static int ata_pci_acquire_legacy(int port, unsigned long *host_set_flags)
> +{
> +	unsigned long cmd_addr = ata_legacy_addr_tbl[port][0];
> +
> +	if (request_region(cmd_addr, 8, "libata") != NULL)
> +		*host_set_flags |= ATA_HOST_RES_LEGACY_PRI << port;
> +	else {
> +		struct resource *conflict, res;
> +
> +		res.start = cmd_addr;
> +		res.end = cmd_addr + 8 - 1;
> +		conflict = ____request_resource(&ioport_resource, &res);
> +
> +		if (strcmp(conflict->name, "libata")) {
> +			printk(KERN_WARNING "ata: 0x%0lX IDE port busy\n",
> +			       cmd_addr);
> +			*host_set_flags |= ATA_HOST_PCI_DEV_BUSY;
> +			return -EBUSY;
> +		}
> +		printk("ata: 0x%0lX IDE port preallocated\n", cmd_addr);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + *	ata_pci_set_dma_mask - PCI DMA mask set helper
> + *	@pdev: target PCI device
> + *	@dma_mask: DMA mask
> + *	@p_reason: output arg for error message
> + *
> + *	Helper to set PCI DMA mask.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer (may sleep).
> + *
> + *	RETURNS:
> + *	0 on success, -errno otherwise.
> + */
> +int ata_pci_set_dma_mask(struct pci_dev *pdev, u64 dma_mask,
> +			 const char **p_reason)
> +{
> +	const char *reason;
> +	int rc = 0;
> +
> +	if (dma_mask == DMA_64BIT_MASK) {
> +		if (pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
> +			rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
> +			if (rc) {
> +				rc = pci_set_consistent_dma_mask(pdev,
> +							DMA_32BIT_MASK);
> +				if (rc) {
> +					reason = "64-bit DMA enable failed";
> +					goto err;
> +				}
> +			}
> +		} else {
> +			rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> +			if (rc) {
> +				reason = "32-bit DMA enable failed";
> +				goto err;
> +			}
> +			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
> +			if (rc) {
> +				reason = "32-bit consistent DMA enable failed";
> +				goto err;
> +			}
> +		}
> +	} else if (dma_mask) {
> +		reason = "failed to set DMA mask";
> +		rc = pci_set_dma_mask(pdev, dma_mask);
> +		if (rc)
> +			goto err;
> +		rc = pci_set_consistent_dma_mask(pdev, dma_mask);
> +		if (rc)
> +			goto err;
> +	}
> +
> +	return 0;
> +
> + err:
> +	if (p_reason)
> +		*p_reason = reason;
> +	return rc;
> +}
> +
> +/**
> + *	ata_pci_acquire_resources - acquire default PCI resources
> + *	@host_set: target ATA host_set to acquire PCI resources for
> + *	@dma_mask: DMA mask
> + *	@reason: output arg for error message
> + *
> + *	Acquire default ATA PCI resources.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer (may sleep).
> + *
> + *	RETURNS:
> + *	0 on success, -errno otherwise.
> + */
> +int ata_pci_acquire_resources(struct ata_host_set *host_set, u64 dma_mask,
> +			      const char **reason)
> +{
> +	struct pci_dev *pdev = to_pci_dev(host_set->dev);
> +	int nr_dummy, i, rc;
> +
> +	/* acquire generic resources */
> +
> +	/* FIXME: Really for ATA it isn't safe because the device may
> +	 * be multi-purpose and we want to leave it alone if it was
> +	 * already enabled. Secondly for shared use as Arjan says we
> +	 * want refcounting
> +	 *
> +	 * Checking dev->is_enabled is insufficient as this is not set
> +	 * at boot for the primary video which is BIOS enabled
> +         */
> +	rc = pci_enable_device(pdev);
> +	if (rc) {
> +		*reason = "failed to enable PCI device";
> +		goto err;
> +	}
> +
> +	rc = pci_request_regions(pdev, DRV_NAME);
> +	if (rc) {
> +		host_set->flags |= ATA_HOST_PCI_DEV_BUSY;
> +		*reason = "failed to request PCI regions";
> +		goto err;
> +	}
> +
> +	host_set->flags |= ATA_HOST_PCI_RES_GEN;
> +
> +	/* set DMA mask */
> +	/* FIXME: If we get no DMA mask we should fall back to PIO */
> +	rc = ata_pci_set_dma_mask(pdev, dma_mask, reason);
> +	if (rc)
> +		goto err;
> +
> +	/* Acquire legacy resources.  For legacy ports, host_set must
> +	 * be popultaed with ports before calling this function.
> +	 */
> +	nr_dummy = 0;
> +	for (i = 0; i < 2; i++) {
> +		if (!(host_set->flags & (ATA_HOST_LEGACY_PRI << i)))
> +			continue;
> +		BUG_ON(i >= host_set->n_ports);
> +
> +		if (ata_pci_acquire_legacy(i, &host_set->flags)) {
> +			host_set->ports[i]->ops = &ata_dummy_port_ops;
> +			nr_dummy++;
> +		}
> +	}
> +
> +	if (nr_dummy && nr_dummy == host_set->n_ports) {
> +		*reason = "no available port";
> +		rc = -ENODEV;
> +		goto err;
> +	}
> +
> +	return 0;
> +
> + err:
> +	ata_pci_release_resources(host_set);
> +	return rc;
> +}
> +
> +/**
> + *	ata_pci_request_irq - request PCI irq for ATA host_set
> + *	@host_set: ATA host_set to request PCI irq for
> + *	@irq_handler: irq_handler
> + *	@flags: ATA_PCI_IRQ_* flags
> + *	@reason: output arg for error message
> + *
> + *	Request PCI irq for @host_set.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer.
> + *
> + *	RETURNS:
> + *	Return value of request_irq().
> + */
> +int ata_pci_request_irq(struct ata_host_set *host_set,
> +		irqreturn_t (*irq_handler)(int, void *, struct pt_regs *),
> +		unsigned int flags, const char **reason)
> +{
> +	struct pci_dev *pdev = to_pci_dev(host_set->dev);
> +	unsigned int irq_flags = 0;
> +	int rc;
> +
> +	if (flags & ATA_PCI_IRQ_MSI) {
> +		flags &= ~ATA_PCI_IRQ_INTX;
> +
> +		if (pci_enable_msi(pdev) == 0)
> +			host_set->flags |= ATA_HOST_PCI_RES_MSI;
> +		else
> +			flags |= ATA_PCI_IRQ_INTX;
> +	}
> +
> +	if (flags & ATA_PCI_IRQ_INTX) {
> +		pci_intx(pdev, 1);
> +		host_set->flags |= ATA_HOST_PCI_RES_INTX;
> +	}
> +
> +	if (!(flags & ATA_PCI_IRQ_EXCL))
> +		irq_flags |= IRQF_SHARED;
> +
> +	rc = ata_host_set_request_irq(host_set, pdev->irq, irq_handler,
> +				      irq_flags, reason);
> +	if (rc == 0)
> +		host_set->flags |= ATA_HOST_PCI_RES_IRQ;
> +
> +	return rc;
> +}
> +
> +/**
> + *	ata_pci_release_resources - release all ATA PCI resources
> + *	@pdev: target ATA host_set
> + *
> + *	Release all ATA PCI resources.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer (may sleep).
> + */
> +void ata_pci_release_resources(struct ata_host_set *host_set)
> +{
> +	struct pci_dev *pdev = to_pci_dev(host_set->dev);
> +	int i;
> +
> +	/* stop all ports */
> +	ata_host_set_stop(host_set);
> +
> +	/* release IRQs */
> +	if (host_set->flags & ATA_HOST_PCI_RES_IRQ15) {
> +		free_irq(15, host_set);
> +		host_set->flags &= ~ATA_HOST_PCI_RES_IRQ15;
> +	}
> +
> +	if (host_set->flags & ATA_HOST_PCI_RES_IRQ14) {
> +		free_irq(14, host_set);
> +		host_set->flags &= ~ATA_HOST_PCI_RES_IRQ14;
> +	}
> +
> +	if (host_set->flags & ATA_HOST_PCI_RES_IRQ) {
> +		free_irq(pdev->irq, host_set);
> +		host_set->flags &= ~ATA_HOST_PCI_RES_IRQ;
> +	}
> +
> +	if (host_set->flags & ATA_HOST_PCI_RES_INTX) {
> +		pci_intx(pdev, 0);
> +		host_set->flags &= ~ATA_HOST_PCI_RES_INTX;
> +	}
> +
> +	if (host_set->flags & ATA_HOST_PCI_RES_MSI) {
> +		pci_disable_msi(pdev);
> +		host_set->flags &= ~ATA_HOST_PCI_RES_MSI;
> +	}

This is definitely the wrong direction.

We don't want to keep crowding knowledge of multiple bus technologies 
into the same function.

ata_pci_request_irq() and other code above follows the same theme... 
but its an unmaintainable direction.

This sort of stuff needs to be split up, not coalesced.

Another thing to think about:  IMO it makes sense to separate out the 
PCI IDE resource handling, because that set of technology is largely static.

Most newer controllers will only have a few resources, normally MMIO, 
and may even support MSI-X (multiple messages for different event types, 
rather than a single message for all events like MSI).

So, I'd like to see some of this inside libata-bmdma.c to keep the core 
free of such nastiness.

	Jeff

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Tejun Heo]

Jeff Garzik wrote:
> This is definitely the wrong direction.

Ouch...  The whole patch?

> We don't want to keep crowding knowledge of multiple bus technologies 
> into the same function.

Do you mean PCI-IDE (legacy/BMDMA), PCI-native and newer controllers 
(including MSI stuff)?  Bus-wise they're all PCI.

> ata_pci_request_irq() and other code above follows the same theme... but 
> its an unmaintainable direction.
> 
> This sort of stuff needs to be split up, not coalesced.

ata_pci_*() functions are PCI helpers and I wanted to move most of PCI 
resource bookkeeping into helpers.  Less clutter in LLDs and less errors.

> Another thing to think about:  IMO it makes sense to separate out the 
> PCI IDE resource handling, because that set of technology is largely 
> static.
 >
> Most newer controllers will only have a few resources, normally MMIO, 
> and may even support MSI-X (multiple messages for different event types, 
> rather than a single message for all events like MSI).
> 
> So, I'd like to see some of this inside libata-bmdma.c to keep the core 
> free of such nastiness.

I'm having a little bit difficult time following what you're thinking. 
Whatever the final interface is, the goals would be similar to what's 
listed in the head message of this patchset, and I think 
alloc/init/attach model is a good way to achieve that thus killing 
probe_ent.  Can you explain in more detail how the interface should look 
like?

Thanks.

-- 
tejun

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Jeff Garzik]

Tejun Heo wrote:
> Jeff Garzik wrote:
>> This is definitely the wrong direction.
> 
> Ouch...  The whole patch?
> 
>> We don't want to keep crowding knowledge of multiple bus technologies 
>> into the same function.
> 
> Do you mean PCI-IDE (legacy/BMDMA), PCI-native and newer controllers 
> (including MSI stuff)?  Bus-wise they're all PCI.

ata_generic, ISA, VLB, and embedded controllers that use libata are not 
PCI.  libata-bmdma can drive older non-PCI controllers.

Its a bit confusing because "PCI IDE" is often used to refer to the 
legacy IDE shadow register interface.  I started to use "bmdma" to 
describe the interface, but it drives Alan crazy :)


>> ata_pci_request_irq() and other code above follows the same theme... 
>> but its an unmaintainable direction.
>>
>> This sort of stuff needs to be split up, not coalesced.
> 
> ata_pci_*() functions are PCI helpers and I wanted to move most of PCI 
> resource bookkeeping into helpers.  Less clutter in LLDs and less errors.

Putting legacy IDE resource allocation and ultra-modern (MSI) resource 
allocation in the same function is the wrong direction.

Legacy LLDDs should not be traversing code that has to worry about PCI 
MSI, and PCI MSI controllers should not be traversing code paths that 
deal with legacy IDE.

I'm looking forward to having ARM and PowerMac IDE drivers ported to 
libata soon, and we don't need to be adding special ARM/PPC bus handling 
stuff to these already-gigantic resource routines.


>> Another thing to think about:  IMO it makes sense to separate out the 
>> PCI IDE resource handling, because that set of technology is largely 
>> static.
>  >
>> Most newer controllers will only have a few resources, normally MMIO, 
>> and may even support MSI-X (multiple messages for different event 
>> types, rather than a single message for all events like MSI).
>>
>> So, I'd like to see some of this inside libata-bmdma.c to keep the 
>> core free of such nastiness.
> 
> I'm having a little bit difficult time following what you're thinking. 
> Whatever the final interface is, the goals would be similar to what's 
> listed in the head message of this patchset, and I think 
> alloc/init/attach model is a good way to achieve that thus killing 
> probe_ent.  Can you explain in more detail how the interface should look 
> like?

Killing probe_ent is a good thing, definitely.  Ideally we should have 
an interface for the LLDD like

	host_set = alloc_ata()
	[ fill in host_set ]
	rc = register_ata()
	...
	unregister_ata()
	free_ata()

Additionally, to take into account strange resource allocation stuff, 
BMDMA drivers should eventually do

	host_set = alloc_bmdma()
	[ fill in host_set ]
	rc = register_bmdma()
	...
	unregister_bmdma()
	free_bmdma()

That keeps all the legacy IDE crap out of the core completely.

Overall, your movement towards killing probe_ent and filling in host_set 
directly is a good one, as I illustrate above.  But lumping all the 
resource allocation and irq registration into a few big functions is the 
wrong direction.  Consider for example an LLDD that supports MSI-X in 
your design.  That's a good proof of concept.

	Jeff

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Alan Cox]

Ar Mer, 2006-08-09 am 02:41 -0400, ysgrifennodd Jeff Garzik:
> ata_generic, ISA, VLB, and embedded controllers that use libata are not 
> PCI.  libata-bmdma can drive older non-PCI controllers.
> 
> Its a bit confusing because "PCI IDE" is often used to refer to the 
> legacy IDE shadow register interface.  I started to use "bmdma" to 
> describe the interface, but it drives Alan crazy :)

Properly we have

	ST506		(register layout for PIO)
	BMDMA		(register layout for dma area/sg tables) [SFF]
	SFF		(full SFF - BAR4, PCI BMDMA)
	ADMA		(also in SFF)
	AHCI

	+ various weird vendor stuff

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Albert Lee]

Alan Cox wrote:
> Ar Mer, 2006-08-09 am 02:41 -0400, ysgrifennodd Jeff Garzik:
> 
>>ata_generic, ISA, VLB, and embedded controllers that use libata are not 
>>PCI.  libata-bmdma can drive older non-PCI controllers.
>>
>>Its a bit confusing because "PCI IDE" is often used to refer to the 
>>legacy IDE shadow register interface.  I started to use "bmdma" to 
>>describe the interface, but it drives Alan crazy :)
> 
> 
> Properly we have
> 
> 	ST506		(register layout for PIO)
> 	BMDMA		(register layout for dma area/sg tables) [SFF]
> 	SFF		(full SFF - BAR4, PCI BMDMA)
> 	ADMA		(also in SFF)
> 	AHCI
> 
> 	+ various weird vendor stuff
> 
> 

The first time libata-"bmdma".c was seen, it looked strange to me, too.

I remember the WD1003/ST-506 adapter on my old i386 PC, which was later replaced by
an "AT-Bus" IDE controller. The IDE controller is register level compatible
to the WD1003 ST-506 adapter, with the same legacy PIO address and irq 14.
(IDE had to be register-level compatible with the old WD1003 interface,
because the Int 13h code for the WD1003 is in the main PC/AT BIOS, not on the adapter.)

At about the 486 VL-bus days, an additional "secondary" port was added, with irq 15.
In those old 286/386/486 days, the IDE adapters were PIO only.

The BM-DMA came at about the time of the Intel Pentium/PCI-bus, with the new BM-DMA registers. 

When libata-bmdma.c is seperated out from libata-core.c, I was wondering why the
BM-DMA PRD table management codes such as ata_sg_setup() are left in libata-core.c,
while the old WD1003/ST-506 like logic ata_tf_load(), ata_exec_command() and ata_check_status()
are moved to the libata-bmdma.c.

Jeff explained the traditional WD1003 ST-506 registers can be viewed as subset of BM-DMA,
but I still feel strange to call the old WD1003-like IDE controllers on my 386/486 as "BM-DMA"...

--
albert

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Jeff Garzik]

Albert Lee wrote:
> When libata-bmdma.c is seperated out from libata-core.c, I was wondering why the
> BM-DMA PRD table management codes such as ata_sg_setup() are left in libata-core.c,
> while the old WD1003/ST-506 like logic ata_tf_load(), ata_exec_command() and ata_check_status()
> are moved to the libata-bmdma.c.
> 
> Jeff explained the traditional WD1003 ST-506 registers can be viewed as subset of BM-DMA,
> but I still feel strange to call the old WD1003-like IDE controllers on my 386/486 as "BM-DMA"...

hehe :)

Well I am more than willing to rename it, though its a bit low priority.

What would people prefer?  libata-sff.c ?

	Jeff

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Alan Cox]

Ar Iau, 2006-08-10 am 03:27 -0400, ysgrifennodd Jeff Garzik:
> hehe :)
> 
> Well I am more than willing to rename it, though its a bit low priority.
> 
> What would people prefer?  libata-sff.c ?

Looking at the non-PCI cases I think it needs a three way split

- Libata core as it is now basically (no bus specific code at all)
- Libata register compatibility stuff (ST506) in a file (no
dependancies) [libata-legacy] ??
- Libata BMDMA, SFF PCI in a file (dep CONFIG_PCI)  [libata-sff]

Alan

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Tejun Heo]

Alan Cox wrote:
> Ar Iau, 2006-08-10 am 03:27 -0400, ysgrifennodd Jeff Garzik:
>> hehe :)
>>
>> Well I am more than willing to rename it, though its a bit low priority.
>>
>> What would people prefer?  libata-sff.c ?
> 
> Looking at the non-PCI cases I think it needs a three way split
> 
> - Libata core as it is now basically (no bus specific code at all)
> - Libata register compatibility stuff (ST506) in a file (no
> dependancies) [libata-legacy] ??

As we're using 'legacy' to refer to PCI compatible mode and it's a bit 
long, how about libata-tf?

> - Libata BMDMA, SFF PCI in a file (dep CONFIG_PCI)  [libata-sff]

-- 
tejun

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Mark Lord]

>Its a bit confusing because "PCI IDE" is often used to refer to the legacy IDE shadow register >interface.  I started to use "bmdma" to describe the interface, but it drives Alan crazy  :) 

Yes, it would do that, because the official specification for those interfaces
is the "PCI IDE Controller Specification".  Need a copy?

Cheers

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Alan Cox]

Ar Mer, 2006-08-09 am 01:11 -0400, ysgrifennodd Jeff Garzik:
> > +static const unsigned long ata_legacy_addr_tbl[][2] = {
> > +	{ ATA_PRIMARY_CMD, ATA_PRIMARY_CTL },
> > +	{ ATA_SECONDARY_CMD, ATA_SECONDARY_CTL}
> > +};
> > +

NAK. I purposefully didn't do this because the defines may not be
constants. Take a look at the PPC platforms where it will eventually
have to be a call into the machine dependant function table.

Re: [PATCH 05/17] libata: implement PCI init helpers for new LLD init model

[Tejun Heo]

Alan Cox wrote:
> Ar Mer, 2006-08-09 am 01:11 -0400, ysgrifennodd Jeff Garzik:
>>> +static const unsigned long ata_legacy_addr_tbl[][2] = {
>>> +	{ ATA_PRIMARY_CMD, ATA_PRIMARY_CTL },
>>> +	{ ATA_SECONDARY_CMD, ATA_SECONDARY_CTL}
>>> +};
>>> +
> 
> NAK. I purposefully didn't do this because the defines may not be
> constants. Take a look at the PPC platforms where it will eventually
> have to be a call into the machine dependant function table.

Thanks for pointing it out.  I'll scrap the static table.

-- 
tejun

[PATCH 06/17] libata: reimplement ata_pci_init_one() using new init helpers

[Tejun Heo]

Reimplement ata_pci_init_one() using new init helpers.  New
implementation is split into to functions - ata_pci_host_set_prepare()
and ata_pci_init_one().  ata_pci_host_set_prepare() prepares host_set
for the specified pdev.  The split is to give LLDs more flexibility.

ata_pci_host_set_prepare() can handle all combinations of native and
legacy ports, but ata_pci_init_one() still applies the same
restriction as before for compatibility.

New implementation properly releases legacy resources on remove.
Other than that there is no behavior changes.

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

---

 drivers/scsi/libata-bmdma.c |  317 +++++++++++++++++++++----------------------
 drivers/scsi/libata-core.c  |    1 
 include/linux/libata.h      |    4 +
 3 files changed, 163 insertions(+), 159 deletions(-)

455f87635e37afc13700ebab64128a02e51f2a49
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index 341d9b5..d97af71 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -1281,69 +1281,159 @@ ata_pci_init_native_mode(struct pci_dev 
 	return probe_ent;
 }
 
-
-static struct ata_probe_ent *ata_pci_init_legacy_port(struct pci_dev *pdev,
-				struct ata_port_info **port, int port_mask)
+/**
+ *	ata_pci_host_set_prepare - prepare PCI ATA device for libata attach
+ *	@pdev: target PCI device
+ *	@pinfo_ar: array of pointers to ATA port_info
+ *	@mask: available port mask
+ *	@legacy_mask: legacy port mask
+ *	@r_host_set: out arg for prepared ATA host_set
+ *
+ *	Allocate and initialize ATA host_set for PCI ATA device @pdev.
+ *	Ports of @pdev can be in either native or legacy mode.  All
+ *	related PCI resources including IRQs are acquired by this
+ *	function.  Returned ATA host_set is ready to be attached using
+ *	ata_host_set_attach().
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+int ata_pci_host_set_prepare(struct pci_dev *pdev,
+			     struct ata_port_info **pinfo_ar,
+			     unsigned int mask, unsigned int legacy_mask,
+			     struct ata_host_set **r_host_set)
 {
-	struct ata_probe_ent *probe_ent;
-	unsigned long bmdma = pci_resource_start(pdev, 4);
+	static const unsigned long irq_flags[3] = { 0, 0, IRQF_SHARED };
+	static const unsigned long irq_res_mask[3] = { ATA_HOST_PCI_RES_IRQ14,
+						       ATA_HOST_PCI_RES_IRQ15,
+						       ATA_HOST_PCI_RES_IRQ };
+	const int irq[3] = { 14, 15, pdev->irq };
+	irqreturn_t (*irq_handler[3])(int, void *,
+				      struct pt_regs *) = { NULL, NULL, NULL };
+	struct ata_host_set *host_set = NULL;
+	struct ata_port_info dummy_pinfo;
+	const struct ata_port_info *pi[2];
+	int n_ports, i, rc;
+	const char *reason;
 
-	probe_ent = ata_probe_ent_alloc(pci_dev_to_dev(pdev), port[0]);
-	if (!probe_ent)
-		return NULL;
+	/* prep pi[] and count n_ports */
+	if (pinfo_ar[0])
+		dummy_pinfo = *pinfo_ar[0];
+	else
+		dummy_pinfo = *pinfo_ar[1];
+	dummy_pinfo.port_ops = &ata_dummy_port_ops;
+	pi[0] = &dummy_pinfo;
+	pi[1] = &dummy_pinfo;
+
+	n_ports = 0;
+	if (mask & ATA_PORT_PRIMARY) {
+		pi[0] = pinfo_ar[0];
+		n_ports = 1;
+	}
+	if (mask & ATA_PORT_SECONDARY) {
+		pi[1] = pinfo_ar[1];
+		n_ports = 2;
+	}
 
-	probe_ent->n_ports = 2;
-	probe_ent->private_data = port[0]->private_data;
+	/* allocate host_set */
+	host_set = ata_host_set_alloc_pinfo_ar(&pdev->dev, pi, n_ports);
+	if (!host_set) {
+		reason = "failed to allocate host_set";
+		rc = -ENOMEM;
+		goto err;
+	}
 
-	if (port_mask & ATA_PORT_PRIMARY) {
-		probe_ent->irq = 14;
-		probe_ent->port[0].cmd_addr = ATA_PRIMARY_CMD;
-		probe_ent->port[0].altstatus_addr =
-		probe_ent->port[0].ctl_addr = ATA_PRIMARY_CTL;
-		if (bmdma) {
-			probe_ent->port[0].bmdma_addr = bmdma;
-			if (inb(bmdma + 2) & 0x80)
-				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
+	host_set->flags |= legacy_mask & ATA_HOST_LEGACY_MASK;
+
+	/* acquire ATA PCI resources */
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	if (rc)
+		goto err;
+
+	/* initialize & start ports */
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		if (ata_port_is_dummy(ap))
+			continue;
+
+		if (host_set->flags & (ATA_HOST_LEGACY_PRI << i)) {
+			ata_pci_port_init_legacy(ap, i);
+			irq_handler[i] = ap->ops->irq_handler;
+		} else {
+			ata_pci_port_init_native(ap, i);
+			irq_handler[2] = ap->ops->irq_handler;
 		}
-		ata_std_ports(&probe_ent->port[0]);
-	} else
-		probe_ent->dummy_port_mask |= ATA_PORT_PRIMARY;
+	}
 
-	if (port_mask & ATA_PORT_SECONDARY) {
-		if (probe_ent->irq)
-			probe_ent->irq2 = 15;
-		else
-			probe_ent->irq = 15;
-		probe_ent->port[1].cmd_addr = ATA_SECONDARY_CMD;
-		probe_ent->port[1].altstatus_addr =
-		probe_ent->port[1].ctl_addr = ATA_SECONDARY_CTL;
-		if (bmdma) {
-			probe_ent->port[1].bmdma_addr = bmdma + 8;
-			if (inb(bmdma + 10) & 0x80)
-				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
+	rc = ata_host_set_start(host_set);
+	if (rc) {
+		reason = "failed to start host_set";
+		goto err;
+	}
+
+	/* freeze & request IRQs */
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		ata_chk_status(ap);
+		host_set->ops->irq_clear(ap);
+		ata_eh_freeze_port(ap);	/* freeze port before requesting IRQ */
+	}
+
+	for (i = 0; i < 3; i++) {
+		if (!irq_handler[i])
+			continue;
+
+		switch (i) {
+		case 0:
+			host_set->irq = irq[i];
+			break;
+		case 1:
+			host_set->irq2 = irq[i];
+			break;
+		case 2:
+			if (host_set->irq == 0)
+				host_set->irq = irq[i];
+			else
+				host_set->irq2 = irq[i];
+			break;
 		}
-		ata_std_ports(&probe_ent->port[1]);
-	} else
-		probe_ent->dummy_port_mask |= ATA_PORT_SECONDARY;
 
-	return probe_ent;
-}
+		rc = request_irq(irq[i], irq_handler[i], irq_flags[i],
+				 DRV_NAME, host_set);
+		if (rc) {
+			reason = "failed to request IRQ";
+			goto err;
+		}
 
+		host_set->flags |= irq_res_mask[i];
+	}
+
+	pci_set_master(pdev);
+
+	*r_host_set = host_set;
+	return 0;
+
+ err:
+	ata_pci_host_set_destroy(host_set);
+	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
+	return rc;
+}
 
 /**
  *	ata_pci_init_one - Initialize/register PCI IDE host controller
  *	@pdev: Controller to be initialized
- *	@port_info: Information from low-level host driver
+ *	@pinfo_ar: Information from low-level host driver
  *	@n_ports: Number of ports attached to host controller
  *
  *	This is a helper function which can be called from a driver's
  *	xxx_init_one() probe function if the hardware uses traditional
  *	IDE taskfile registers.
  *
- *	This function calls pci_enable_device(), reserves its register
- *	regions, sets the dma mask, enables bus master mode, and calls
- *	ata_device_add()
- *
  *	ASSUMPTION:
  *	Nobody makes a single channel controller that appears solely as
  *	the secondary legacy port on PCI.
@@ -1354,134 +1444,43 @@ static struct ata_probe_ent *ata_pci_ini
  *	RETURNS:
  *	Zero on success, negative on errno-based value on error.
  */
-
-int ata_pci_init_one (struct pci_dev *pdev, struct ata_port_info **port_info,
-		      unsigned int n_ports)
+int ata_pci_init_one(struct pci_dev *pdev, struct ata_port_info **pinfo_ar,
+		     unsigned int n_ports)
 {
-	struct ata_probe_ent *probe_ent = NULL;
-	struct ata_port_info *port[2];
-	u8 tmp8, mask;
-	unsigned int legacy_mode = 0;
-	int disable_dev_on_err = 1;
+	unsigned int mask = ATA_PORT_PRIMARY | ATA_PORT_SECONDARY;
+	unsigned int legacy_mask = 0;
+	struct ata_host_set *host_set;
 	int rc;
 
 	DPRINTK("ENTER\n");
 
-	port[0] = port_info[0];
-	if (n_ports > 1)
-		port[1] = port_info[1];
-	else
-		port[1] = port[0];
-
-	if ((port[0]->host_flags & ATA_FLAG_NO_LEGACY) == 0
-	    && (pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
-		/* TODO: What if one channel is in native mode ... */
-		pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
-		mask = (1 << 2) | (1 << 0);
-		if ((tmp8 & mask) != mask)
-			legacy_mode = (1 << 3);
-	}
+	/* legacy mode? */
+	if (!(pinfo_ar[0]->host_flags & ATA_FLAG_NO_LEGACY))
+		legacy_mask = ata_pci_legacy_mask(pdev);
 
-	/* FIXME... */
-	if ((!legacy_mode) && (n_ports > 2)) {
-		printk(KERN_ERR "ata: BUG: native mode, n_ports > 2\n");
-		n_ports = 2;
-		/* For now */
+	if (legacy_mask) {
+		/* XXX: all legacy or all native for the time being */
+		BUG_ON(n_ports < 2);
+		legacy_mask |= ATA_PORT_PRIMARY | ATA_PORT_SECONDARY;
+	} else {
+		/* FIXME... */
+		if (n_ports > 2)
+			printk(KERN_ERR "ata: BUG: native mode, n_ports > 2\n");
+		if (n_ports < 2)
+			mask &= ~ATA_PORT_SECONDARY;
 	}
 
-	/* FIXME: Really for ATA it isn't safe because the device may be
-	   multi-purpose and we want to leave it alone if it was already
-	   enabled. Secondly for shared use as Arjan says we want refcounting
-
-	   Checking dev->is_enabled is insufficient as this is not set at
-	   boot for the primary video which is BIOS enabled
-         */
-
-	rc = pci_enable_device(pdev);
+	/* prep */
+	rc = ata_pci_host_set_prepare(pdev, pinfo_ar, mask, legacy_mask,
+				      &host_set);
 	if (rc)
 		return rc;
 
-	rc = pci_request_regions(pdev, DRV_NAME);
-	if (rc) {
-		disable_dev_on_err = 0;
-		goto err_out;
-	}
-
-	if (legacy_mode) {
-		if (!request_region(ATA_PRIMARY_CMD, 8, "libata")) {
-			struct resource *conflict, res;
-			res.start = ATA_PRIMARY_CMD;
-			res.end = ATA_PRIMARY_CMD + 8 - 1;
-			conflict = ____request_resource(&ioport_resource, &res);
-			if (!strcmp(conflict->name, "libata"))
-				legacy_mode |= ATA_PORT_PRIMARY;
-			else {
-				disable_dev_on_err = 0;
-				printk(KERN_WARNING "ata: 0x%0X IDE port busy\n", ATA_PRIMARY_CMD);
-			}
-		} else
-			legacy_mode |= ATA_PORT_PRIMARY;
-
-		if (!request_region(ATA_SECONDARY_CMD, 8, "libata")) {
-			struct resource *conflict, res;
-			res.start = ATA_SECONDARY_CMD;
-			res.end = ATA_SECONDARY_CMD + 8 - 1;
-			conflict = ____request_resource(&ioport_resource, &res);
-			if (!strcmp(conflict->name, "libata"))
-				legacy_mode |= ATA_PORT_SECONDARY;
-			else {
-				disable_dev_on_err = 0;
-				printk(KERN_WARNING "ata: 0x%X IDE port busy\n", ATA_SECONDARY_CMD);
-			}
-		} else
-			legacy_mode |= ATA_PORT_SECONDARY;
-	}
-
-	/* we have legacy mode, but all ports are unavailable */
-	if (legacy_mode == (1 << 3)) {
-		rc = -EBUSY;
-		goto err_out_regions;
-	}
-
-	/* FIXME: If we get no DMA mask we should fall back to PIO */
-	rc = pci_set_dma_mask(pdev, ATA_DMA_MASK);
-	if (rc)
-		goto err_out_regions;
-	rc = pci_set_consistent_dma_mask(pdev, ATA_DMA_MASK);
+	/* attach */
+	rc = ata_host_set_attach(host_set);
 	if (rc)
-		goto err_out_regions;
-
-	if (legacy_mode) {
-		probe_ent = ata_pci_init_legacy_port(pdev, port, legacy_mode);
-	} else {
-		if (n_ports == 2)
-			probe_ent = ata_pci_init_native_mode(pdev, port, ATA_PORT_PRIMARY | ATA_PORT_SECONDARY);
-		else
-			probe_ent = ata_pci_init_native_mode(pdev, port, ATA_PORT_PRIMARY);
-	}
-	if (!probe_ent) {
-		rc = -ENOMEM;
-		goto err_out_regions;
-	}
-
-	pci_set_master(pdev);
-
-	/* FIXME: check ata_device_add return */
-	ata_device_add(probe_ent);
-
-	kfree(probe_ent);
-
-	return 0;
+		ata_pci_host_set_destroy(host_set);
 
-err_out_regions:
-	if (legacy_mode & ATA_PORT_PRIMARY)
-		release_region(ATA_PRIMARY_CMD, 8);
-	if (legacy_mode & ATA_PORT_SECONDARY)
-		release_region(ATA_SECONDARY_CMD, 8);
-	pci_release_regions(pdev);
-err_out:
-	if (disable_dev_on_err)
-		pci_disable_device(pdev);
 	return rc;
 }
 
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index d44c1df..cb22c04 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -6411,6 +6411,7 @@ EXPORT_SYMBOL_GPL(ata_pci_request_irq);
 EXPORT_SYMBOL_GPL(ata_pci_release_resources);
 EXPORT_SYMBOL_GPL(ata_pci_host_set_destroy);
 EXPORT_SYMBOL_GPL(ata_pci_init_native_mode);
+EXPORT_SYMBOL_GPL(ata_pci_host_set_prepare);
 EXPORT_SYMBOL_GPL(ata_pci_init_one);
 EXPORT_SYMBOL_GPL(ata_pci_remove_one);
 EXPORT_SYMBOL_GPL(ata_pci_device_do_suspend);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0ae0fbb..ed89959 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -874,6 +874,10 @@ extern void ata_pci_release_resources(st
 extern void ata_pci_host_set_destroy(struct ata_host_set *host_set);
 extern struct ata_probe_ent *
 ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int portmask);
+extern int ata_pci_host_set_prepare(struct pci_dev *pdev,
+				    struct ata_port_info **pinfo_ar,
+				    unsigned int mask, unsigned int legacy_mask,
+				    struct ata_host_set **r_host_set);
 extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits);
 extern unsigned long ata_pci_default_filter(const struct ata_port *, struct ata_device *, unsigned long);
 #endif /* CONFIG_PCI */
-- 
1.3.2

[PATCH 02/17] libata: implement ata_host_set_detach() and ata_host_set_free()

[Tejun Heo]

Implement and use ata_host_set_detach() and ata_host_set_free().
ata_host_set_detach() detaches all ports in the host_set and
supercedes ata_port_detach().  ata_host_set() makes sure all ports are
stopped (LLDs may stop ports beforehand if necessary), frees all ports
and the host_set itself.  ata_host_set_free() can also be used in the
error handling path of init functions.

This patch moves several memory deallocations but doesn't introduce
any real behavior changes.

This is a part of efforts to improve [de-]init paths and simplify
LLDs.

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

---

 drivers/scsi/ahci.c        |   13 +++------
 drivers/scsi/libata-core.c |   66 +++++++++++++++++++++++++++++++++++---------
 include/linux/libata.h     |    3 +-
 3 files changed, 60 insertions(+), 22 deletions(-)

39d12c9f4e34b2c220deec0c2dc774f7db1a2f65
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 305663b..980a63e 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -1634,30 +1634,27 @@ static void ahci_remove_one (struct pci_
 	struct device *dev = pci_dev_to_dev(pdev);
 	struct ata_host_set *host_set = dev_get_drvdata(dev);
 	struct ahci_host_priv *hpriv = host_set->private_data;
-	unsigned int i;
 	int have_msi;
 
-	for (i = 0; i < host_set->n_ports; i++)
-		ata_port_detach(host_set->ports[i]);
+	ata_host_set_detach(host_set);
 
 	have_msi = hpriv->flags & AHCI_FLAG_MSI;
 	free_irq(host_set->irq, host_set);
 
 	ata_host_set_stop(host_set);
-
-	for (i = 0; i < host_set->n_ports; i++)
-		scsi_host_put(host_set->ports[i]->host);
-	kfree(hpriv);
 	pci_iounmap(pdev, host_set->mmio_base);
-	kfree(host_set);
 
 	if (have_msi)
 		pci_disable_msi(pdev);
 	else
 		pci_intx(pdev, 0);
+
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 	dev_set_drvdata(dev, NULL);
+
+	ata_host_set_free(host_set);
+	kfree(hpriv);
 }
 
 static int __init ahci_init(void)
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index b3cc2f5..f8d6bc0 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5554,11 +5554,7 @@ int ata_device_add(const struct ata_prob
 err_out_free_irq:
 	free_irq(ent->irq, host_set);
 err_out:
-	ata_host_set_stop(host_set);
-
-	for (i = 0; i < host_set->n_ports; i++)
-		scsi_host_put(host_set->ports[i]->host);
-	kfree(host_set);
+	ata_host_set_free(host_set);
 	VPRINTK("EXIT, returning 0\n");
 	return 0;
 }
@@ -5574,7 +5570,7 @@ err_out:
  *	LOCKING:
  *	Kernel thread context (may sleep).
  */
-void ata_port_detach(struct ata_port *ap)
+static void ata_port_detach(struct ata_port *ap)
 {
 	unsigned long flags;
 	int i;
@@ -5656,6 +5652,53 @@ void ata_host_set_stop(struct ata_host_s
 }
 
 /**
+ *	ata_host_set_detach - Detach a host_set from the system
+ *	@host_set: ATA host set that was removed
+ *
+ *	Detach all objects associated with this host set.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+void ata_host_set_detach(struct ata_host_set *host_set)
+{
+	int i;
+
+	for (i = 0; i < host_set->n_ports; i++)
+		ata_port_detach(host_set->ports[i]);
+}
+
+/**
+ *	ata_host_set_free - Release a host_set
+ *	@host_set: ATA host set to be freed
+ *
+ *	Free all objects associated with this host set.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+void ata_host_set_free(struct ata_host_set *host_set)
+{
+	int i;
+
+	/* free(NULL) is supported */
+	if (!host_set)
+		return;
+
+	/* make sure it's stopped */
+	ata_host_set_stop(host_set);
+
+	/* free */
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+		if (ap)
+			scsi_host_put(ap->host);
+	}
+
+	kfree(host_set);
+}
+
+/**
  *	ata_host_set_remove - PCI layer callback for device removal
  *	@host_set: ATA host set that was removed
  *
@@ -5665,13 +5708,11 @@ void ata_host_set_stop(struct ata_host_s
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
  */
-
 void ata_host_set_remove(struct ata_host_set *host_set)
 {
 	unsigned int i;
 
-	for (i = 0; i < host_set->n_ports; i++)
-		ata_port_detach(host_set->ports[i]);
+	ata_host_set_detach(host_set);
 
 	free_irq(host_set->irq, host_set);
 	if (host_set->irq2)
@@ -5691,14 +5732,12 @@ void ata_host_set_remove(struct ata_host
 			else if (ioaddr->cmd_addr == ATA_SECONDARY_CMD)
 				release_region(ATA_SECONDARY_CMD, 8);
 		}
-
-		scsi_host_put(ap->host);
 	}
 
 	if (host_set->ops->host_stop)
 		host_set->ops->host_stop(host_set);
 
-	kfree(host_set);
+	ata_host_set_free(host_set);
 }
 
 /**
@@ -5981,8 +6020,9 @@ EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_std_ports);
 EXPORT_SYMBOL_GPL(ata_host_set_start);
 EXPORT_SYMBOL_GPL(ata_device_add);
-EXPORT_SYMBOL_GPL(ata_port_detach);
+EXPORT_SYMBOL_GPL(ata_host_set_detach);
 EXPORT_SYMBOL_GPL(ata_host_set_stop);
+EXPORT_SYMBOL_GPL(ata_host_set_free);
 EXPORT_SYMBOL_GPL(ata_host_set_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 7f1d45a..5436bd1 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -693,8 +693,9 @@ extern int ata_pci_clear_simplex(struct 
 #endif /* CONFIG_PCI */
 extern int ata_host_set_start(struct ata_host_set *host_set);
 extern int ata_device_add(const struct ata_probe_ent *ent);
-extern void ata_port_detach(struct ata_port *ap);
+extern void ata_host_set_detach(struct ata_host_set *host_set);
 extern void ata_host_set_stop(struct ata_host_set *host_set);
+extern void ata_host_set_free(struct ata_host_set *host_set);
 extern void ata_host_set_remove(struct ata_host_set *host_set);
 extern int ata_scsi_detect(struct scsi_host_template *sht);
 extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
-- 
1.3.2

Re: [PATCH 02/17] libata: implement ata_host_set_detach() and ata_host_set_free()

[Jeff Garzik]

Tejun Heo wrote:
> Implement and use ata_host_set_detach() and ata_host_set_free().
> ata_host_set_detach() detaches all ports in the host_set and
> supercedes ata_port_detach().  ata_host_set() makes sure all ports are

"ata_host_set()" ?


> stopped (LLDs may stop ports beforehand if necessary), frees all ports
> and the host_set itself.  ata_host_set_free() can also be used in the
> error handling path of init functions.
> 
> This patch moves several memory deallocations but doesn't introduce
> any real behavior changes.
> 
> This is a part of efforts to improve [de-]init paths and simplify
> LLDs.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/ahci.c        |   13 +++------
>  drivers/scsi/libata-core.c |   66 +++++++++++++++++++++++++++++++++++---------
>  include/linux/libata.h     |    3 +-
>  3 files changed, 60 insertions(+), 22 deletions(-)
> 
> 39d12c9f4e34b2c220deec0c2dc774f7db1a2f65
> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
> index 305663b..980a63e 100644
> --- a/drivers/scsi/ahci.c
> +++ b/drivers/scsi/ahci.c
> @@ -1634,30 +1634,27 @@ static void ahci_remove_one (struct pci_
>  	struct device *dev = pci_dev_to_dev(pdev);
>  	struct ata_host_set *host_set = dev_get_drvdata(dev);
>  	struct ahci_host_priv *hpriv = host_set->private_data;
> -	unsigned int i;
>  	int have_msi;
>  
> -	for (i = 0; i < host_set->n_ports; i++)
> -		ata_port_detach(host_set->ports[i]);
> +	ata_host_set_detach(host_set);
>  
>  	have_msi = hpriv->flags & AHCI_FLAG_MSI;
>  	free_irq(host_set->irq, host_set);
>  
>  	ata_host_set_stop(host_set);
> -
> -	for (i = 0; i < host_set->n_ports; i++)
> -		scsi_host_put(host_set->ports[i]->host);
> -	kfree(hpriv);
>  	pci_iounmap(pdev, host_set->mmio_base);
> -	kfree(host_set);
>  
>  	if (have_msi)
>  		pci_disable_msi(pdev);
>  	else
>  		pci_intx(pdev, 0);
> +
>  	pci_release_regions(pdev);
>  	pci_disable_device(pdev);
>  	dev_set_drvdata(dev, NULL);
> +
> +	ata_host_set_free(host_set);
> +	kfree(hpriv);
>  }
>  
>  static int __init ahci_init(void)
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index b3cc2f5..f8d6bc0 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -5554,11 +5554,7 @@ int ata_device_add(const struct ata_prob
>  err_out_free_irq:
>  	free_irq(ent->irq, host_set);
>  err_out:
> -	ata_host_set_stop(host_set);
> -
> -	for (i = 0; i < host_set->n_ports; i++)
> -		scsi_host_put(host_set->ports[i]->host);
> -	kfree(host_set);
> +	ata_host_set_free(host_set);
>  	VPRINTK("EXIT, returning 0\n");
>  	return 0;
>  }
> @@ -5574,7 +5570,7 @@ err_out:
>   *	LOCKING:
>   *	Kernel thread context (may sleep).
>   */
> -void ata_port_detach(struct ata_port *ap)
> +static void ata_port_detach(struct ata_port *ap)
>  {
>  	unsigned long flags;
>  	int i;
> @@ -5656,6 +5652,53 @@ void ata_host_set_stop(struct ata_host_s
>  }
>  
>  /**
> + *	ata_host_set_detach - Detach a host_set from the system
> + *	@host_set: ATA host set that was removed
> + *
> + *	Detach all objects associated with this host set.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer (may sleep).
> + */
> +void ata_host_set_detach(struct ata_host_set *host_set)
> +{
> +	int i;
> +
> +	for (i = 0; i < host_set->n_ports; i++)
> +		ata_port_detach(host_set->ports[i]);
> +}
> +
> +/**
> + *	ata_host_set_free - Release a host_set
> + *	@host_set: ATA host set to be freed
> + *
> + *	Free all objects associated with this host set.
> + *
> + *	LOCKING:
> + *	Inherited from calling layer (may sleep).
> + */
> +void ata_host_set_free(struct ata_host_set *host_set)
> +{
> +	int i;
> +
> +	/* free(NULL) is supported */
> +	if (!host_set)
> +		return;
> +
> +	/* make sure it's stopped */
> +	ata_host_set_stop(host_set);
> +
> +	/* free */
> +	for (i = 0; i < host_set->n_ports; i++) {
> +		struct ata_port *ap = host_set->ports[i];
> +		if (ap)
> +			scsi_host_put(ap->host);

for SAS this might need to be

	if (ap && ap->host)

otherwise OK

[PATCH 01/17] libata: implement ata_host_set_start/stop()

[Tejun Heo]

Implement [de-]init helpers ata_host_set_start/stop().  start() will
start all ports in the host_set while stop() does the opposite.  These
helpers use ATA_PFLAG_STARTED to track whether a port is started or
not and thus can be called without too much caution.

ata_host_set_stop() replaces half of ata_scsi_release() and the other
half is merged into ata_host_set_detach().

This patch introduces the follwing behavior changes.

* ->port_start/stop() can be omitted.  This will be used to simplify
  port initialization.

* ->port_disable() callback is reordered w.r.t. freeing IRQs.  The
  callback's roll and usefulness are questionable.  All LLDs use
  ata_port_disable() and libata-core directly calls ata_port_disable()
  in several places.  ->port_disable() invocation during port deinit
  doesn't provide any feature.  All are already done by EH.  The
  relocation doesn't cause any real behavior change.

This is a part of efforts to improve [de-]init paths and simplify
LLDs.

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

---

 drivers/scsi/ahci.c        |    9 +--
 drivers/scsi/libata-core.c |  135 +++++++++++++++++++++++++++++---------------
 include/linux/libata.h     |   18 +++---
 3 files changed, 103 insertions(+), 59 deletions(-)

8b0d0dbdb69bea20d32d54e0ec07b24d51c91bb4
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 68fd766..305663b 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -1643,13 +1643,10 @@ static void ahci_remove_one (struct pci_
 	have_msi = hpriv->flags & AHCI_FLAG_MSI;
 	free_irq(host_set->irq, host_set);
 
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-
-		ata_scsi_release(ap->host);
-		scsi_host_put(ap->host);
-	}
+	ata_host_set_stop(host_set);
 
+	for (i = 0; i < host_set->n_ports; i++)
+		scsi_host_put(host_set->ports[i]->host);
 	kfree(hpriv);
 	pci_iounmap(pdev, host_set->mmio_base);
 	kfree(host_set);
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 0ce7cdc..b3cc2f5 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5322,6 +5322,45 @@ static struct ata_port * ata_port_add(co
 }
 
 /**
+ *	ata_host_set_start - start ports of an ATA host_set
+ *	@host_set: ATA host_set to start ports for
+ *
+ *	Start ports of @host_set.  Port started status is recorded in
+ *	ap->pflags, so this function can be called multiple times.
+ *	Ports are guaranteed to get started only once.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ *
+ *	RETURNS:
+ *	0 if all ports are started successfully, -errno otherwise.
+ */
+int ata_host_set_start(struct ata_host_set *host_set)
+{
+	unsigned long flags;
+	int i, rc;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		if (ap->pflags & ATA_PFLAG_STARTED)
+			continue;
+
+		if (ap->ops->port_start) {
+			rc = ap->ops->port_start(ap);
+			if (rc)
+				return rc;
+		}
+
+		spin_lock_irqsave(ap->lock, flags);
+		ap->pflags |= ATA_PFLAG_STARTED;
+		spin_unlock_irqrestore(ap->lock, flags);
+	}
+
+	return 0;
+}
+
+/**
  *	ata_device_add - Register hardware device with ATA and SCSI layers
  *	@ent: Probe information describing hardware device to be registered
  *
@@ -5382,14 +5421,6 @@ int ata_device_add(const struct ata_prob
 			continue;
 		}
 
-		/* start port */
-		rc = ap->ops->port_start(ap);
-		if (rc) {
-			host_set->ports[i] = NULL;
-			scsi_host_put(ap->host);
-			goto err_out;
-		}
-
 		/* Report the secondary IRQ for second channel legacy */
 		if (i == 1 && ent->irq2)
 			irq_line = ent->irq2;
@@ -5407,6 +5438,16 @@ int ata_device_add(const struct ata_prob
 				ap->ioaddr.ctl_addr,
 				ap->ioaddr.bmdma_addr,
 				irq_line);
+	}
+
+	/* start ports */
+	rc = ata_host_set_start(host_set);
+	if (rc)
+		goto err_out;
+
+	/* freeze */
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
 
 		ata_chk_status(ap);
 		host_set->ops->irq_clear(ap);
@@ -5513,14 +5554,10 @@ int ata_device_add(const struct ata_prob
 err_out_free_irq:
 	free_irq(ent->irq, host_set);
 err_out:
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-		if (ap) {
-			ap->ops->port_stop(ap);
-			scsi_host_put(ap->host);
-		}
-	}
+	ata_host_set_stop(host_set);
 
+	for (i = 0; i < host_set->n_ports; i++)
+		scsi_host_put(host_set->ports[i]->host);
 	kfree(host_set);
 	VPRINTK("EXIT, returning 0\n");
 	return 0;
@@ -5582,6 +5619,40 @@ void ata_port_detach(struct ata_port *ap
  skip_eh:
 	/* remove the associated SCSI host */
 	scsi_remove_host(ap->host);
+
+	/* disable port */
+	ap->ops->port_disable(ap);
+}
+
+/**
+ *	ata_host_set_stop - stop ports of an ATA host_set
+ *	@host_set: ATA host_set to stop
+ *
+ *	Stop ports of @host_set.  Port started status is recorded in
+ *	ap->pflags, so this functio can be called multiple times.
+ *	Started ports are guranteed to be stopped only once.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+void ata_host_set_stop(struct ata_host_set *host_set)
+{
+	unsigned long flags;
+	int i;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		if (!ap || !(ap->pflags & ATA_PFLAG_STARTED))
+			continue;
+
+		if (ap->ops->port_stop)
+			ap->ops->port_stop(ap);
+
+		spin_lock_irqsave(ap->lock, flags);
+		ap->pflags &= ~ATA_PFLAG_STARTED;
+		spin_unlock_irqrestore(ap->lock, flags);
+	}
 }
 
 /**
@@ -5606,11 +5677,11 @@ void ata_host_set_remove(struct ata_host
 	if (host_set->irq2)
 		free_irq(host_set->irq2, host_set);
 
+	ata_host_set_stop(host_set);
+
 	for (i = 0; i < host_set->n_ports; i++) {
 		struct ata_port *ap = host_set->ports[i];
 
-		ata_scsi_release(ap->host);
-
 		if ((ap->flags & ATA_FLAG_NO_LEGACY) == 0) {
 			struct ata_ioports *ioaddr = &ap->ioaddr;
 
@@ -5631,33 +5702,6 @@ void ata_host_set_remove(struct ata_host
 }
 
 /**
- *	ata_scsi_release - SCSI layer callback hook for host unload
- *	@host: libata host to be unloaded
- *
- *	Performs all duties necessary to shut down a libata port...
- *	Kill port kthread, disable port, and release resources.
- *
- *	LOCKING:
- *	Inherited from SCSI layer.
- *
- *	RETURNS:
- *	One.
- */
-
-int ata_scsi_release(struct Scsi_Host *host)
-{
-	struct ata_port *ap = ata_shost_to_port(host);
-
-	DPRINTK("ENTER\n");
-
-	ap->ops->port_disable(ap);
-	ap->ops->port_stop(ap);
-
-	DPRINTK("EXIT\n");
-	return 1;
-}
-
-/**
  *	ata_std_ports - initialize ioaddr with standard port offsets.
  *	@ioaddr: IO address structure to be initialized
  *
@@ -5935,8 +5979,10 @@ EXPORT_SYMBOL_GPL(sata_deb_timing_long);
 EXPORT_SYMBOL_GPL(ata_dummy_port_ops);
 EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_std_ports);
+EXPORT_SYMBOL_GPL(ata_host_set_start);
 EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_port_detach);
+EXPORT_SYMBOL_GPL(ata_host_set_stop);
 EXPORT_SYMBOL_GPL(ata_host_set_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
@@ -5996,7 +6042,6 @@ EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
 EXPORT_SYMBOL_GPL(ata_scsi_slave_config);
 EXPORT_SYMBOL_GPL(ata_scsi_slave_destroy);
 EXPORT_SYMBOL_GPL(ata_scsi_change_queue_depth);
-EXPORT_SYMBOL_GPL(ata_scsi_release);
 EXPORT_SYMBOL_GPL(ata_host_intr);
 EXPORT_SYMBOL_GPL(sata_scr_valid);
 EXPORT_SYMBOL_GPL(sata_scr_read);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1b6283c..7f1d45a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -174,13 +174,14 @@ enum {
 	/* bits 24:31 of ap->flags are reserved for LLD specific flags */
 
 	/* struct ata_port pflags */
-	ATA_PFLAG_EH_PENDING	= (1 << 0), /* EH pending */
-	ATA_PFLAG_EH_IN_PROGRESS = (1 << 1), /* EH in progress */
-	ATA_PFLAG_FROZEN	= (1 << 2), /* port is frozen */
-	ATA_PFLAG_RECOVERED	= (1 << 3), /* recovery action performed */
-	ATA_PFLAG_LOADING	= (1 << 4), /* boot/loading probe */
-	ATA_PFLAG_UNLOADING	= (1 << 5), /* module is unloading */
-	ATA_PFLAG_SCSI_HOTPLUG	= (1 << 6), /* SCSI hotplug scheduled */
+	ATA_PFLAG_STARTED	= (1 << 0), /* port has been started */
+	ATA_PFLAG_EH_PENDING	= (1 << 1), /* EH pending */
+	ATA_PFLAG_EH_IN_PROGRESS = (1 << 2), /* EH in progress */
+	ATA_PFLAG_FROZEN	= (1 << 3), /* port is frozen */
+	ATA_PFLAG_RECOVERED	= (1 << 4), /* recovery action performed */
+	ATA_PFLAG_LOADING	= (1 << 5), /* boot/loading probe */
+	ATA_PFLAG_UNLOADING	= (1 << 6), /* module is unloading */
+	ATA_PFLAG_SCSI_HOTPLUG	= (1 << 7), /* SCSI hotplug scheduled */
 
 	ATA_PFLAG_FLUSH_PORT_TASK = (1 << 16), /* flush port task */
 	ATA_PFLAG_SUSPENDED	= (1 << 17), /* port is suspended (power) */
@@ -690,13 +691,14 @@ extern int ata_pci_device_suspend(struct
 extern int ata_pci_device_resume(struct pci_dev *pdev);
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);
 #endif /* CONFIG_PCI */
+extern int ata_host_set_start(struct ata_host_set *host_set);
 extern int ata_device_add(const struct ata_probe_ent *ent);
 extern void ata_port_detach(struct ata_port *ap);
+extern void ata_host_set_stop(struct ata_host_set *host_set);
 extern void ata_host_set_remove(struct ata_host_set *host_set);
 extern int ata_scsi_detect(struct scsi_host_template *sht);
 extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
 extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *));
-extern int ata_scsi_release(struct Scsi_Host *host);
 extern unsigned int ata_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc);
 extern int sata_scr_valid(struct ata_port *ap);
 extern int sata_scr_read(struct ata_port *ap, int reg, u32 *val);
-- 
1.3.2

Re: [PATCH 01/17] libata: implement ata_host_set_start/stop()

[Jeff Garzik]

Tejun Heo wrote:
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -174,13 +174,14 @@ enum {
>  	/* bits 24:31 of ap->flags are reserved for LLD specific flags */
>  
>  	/* struct ata_port pflags */
> -	ATA_PFLAG_EH_PENDING	= (1 << 0), /* EH pending */
> -	ATA_PFLAG_EH_IN_PROGRESS = (1 << 1), /* EH in progress */
> -	ATA_PFLAG_FROZEN	= (1 << 2), /* port is frozen */
> -	ATA_PFLAG_RECOVERED	= (1 << 3), /* recovery action performed */
> -	ATA_PFLAG_LOADING	= (1 << 4), /* boot/loading probe */
> -	ATA_PFLAG_UNLOADING	= (1 << 5), /* module is unloading */
> -	ATA_PFLAG_SCSI_HOTPLUG	= (1 << 6), /* SCSI hotplug scheduled */
> +	ATA_PFLAG_STARTED	= (1 << 0), /* port has been started */
> +	ATA_PFLAG_EH_PENDING	= (1 << 1), /* EH pending */
> +	ATA_PFLAG_EH_IN_PROGRESS = (1 << 2), /* EH in progress */
> +	ATA_PFLAG_FROZEN	= (1 << 3), /* port is frozen */
> +	ATA_PFLAG_RECOVERED	= (1 << 4), /* recovery action performed */
> +	ATA_PFLAG_LOADING	= (1 << 5), /* boot/loading probe */
> +	ATA_PFLAG_UNLOADING	= (1 << 6), /* module is unloading */
> +	ATA_PFLAG_SCSI_HOTPLUG	= (1 << 7), /* SCSI hotplug scheduled */
>  


Please don't obscure your change by all this bit renumbering.

Otherwise OK

[PATCH 03/17] libata: separate out ata_host_set_alloc() and ata_host_set_attach()

[Tejun Heo]

Separate out ata_host_set_alloc(), ata_host_set_add_ports() and
ata_host_set_attach() from ata_device_add().  These functions will be
used by new LLD init model where LLD allocates host_set directly, then
intializes and attaches it instead of going through probe_ent.

This patch does not introduce behavior changes.

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

---

 drivers/scsi/libata-core.c |  420 +++++++++++++++++++++++++++-----------------
 include/linux/libata.h     |    6 +
 2 files changed, 266 insertions(+), 160 deletions(-)

b84483d8b22dbbf17fed447883e9e01aacf47171
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index f8d6bc0..eccc6bc 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5200,25 +5200,36 @@ void ata_dev_init(struct ata_device *dev
 }
 
 /**
- *	ata_port_init - Initialize an ata_port structure
- *	@ap: Structure to initialize
- *	@host: associated SCSI mid-layer structure
- *	@host_set: Collection of hosts to which @ap belongs
- *	@ent: Probe information provided by low-level driver
- *	@port_no: Port number associated with this ata_port
+ *	ata_port_alloc - allocate and initialize basic ATA port resources
+ *	@host_set: ATA host_set this allocated port belongs to
+ *	@sht: template for SCSI host
  *
- *	Initialize a new ata_port structure, and its associated
- *	scsi_host.
+ *	Allocate and initialize basic ATA port resources.
+ *
+ *	RETURNS:
+ *	Allocate ATA port on success, NULL on failure.
  *
  *	LOCKING:
- *	Inherited from caller.
+ *	Inherited from calling layer (may sleep).
  */
-static void ata_port_init(struct ata_port *ap, struct Scsi_Host *host,
-			  struct ata_host_set *host_set,
-			  const struct ata_probe_ent *ent, unsigned int port_no)
+static struct ata_port *ata_port_alloc(struct ata_host_set *host_set,
+				       struct scsi_host_template *sht)
 {
-	unsigned int i;
+	struct Scsi_Host *host;
+	struct ata_port *ap;
+	int i;
+
+	DPRINTK("ENTER\n");
+
+	host = scsi_host_alloc(sht, sizeof(struct ata_port));
+	if (!host)
+		return NULL;
+
+	host->transportt = &ata_scsi_transport_template;
+
+	ap = ata_shost_to_port(host);
 
+	/* initialize basic fields */
 	host->max_id = 16;
 	host->max_lun = 1;
 	host->max_channel = 1;
@@ -5231,13 +5242,7 @@ static void ata_port_init(struct ata_por
 	ap->host = host;
 	ap->ctl = ATA_DEVCTL_OBS;
 	ap->host_set = host_set;
-	ap->dev = ent->dev;
-	ap->port_no = port_no;
-	ap->pio_mask = ent->pio_mask;
-	ap->mwdma_mask = ent->mwdma_mask;
-	ap->udma_mask = ent->udma_mask;
-	ap->flags |= ent->host_flags;
-	ap->ops = ent->port_ops;
+	ap->dev = host_set->dev;
 	ap->hw_sata_spd_limit = UINT_MAX;
 	ap->active_tag = ATA_TAG_POISON;
 	ap->last_ctl = 0xFF;
@@ -5257,10 +5262,7 @@ #endif
 	INIT_LIST_HEAD(&ap->eh_done_q);
 	init_waitqueue_head(&ap->eh_wait_q);
 
-	/* set cable type */
 	ap->cbl = ATA_CBL_NONE;
-	if (ap->flags & ATA_FLAG_SATA)
-		ap->cbl = ATA_CBL_SATA;
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
@@ -5274,51 +5276,97 @@ #ifdef ATA_IRQ_TRAP
 	ap->stats.idle_irq = 1;
 #endif
 
-	memcpy(&ap->ioaddr, &ent->port[port_no], sizeof(struct ata_ioports));
+	return ap;
 }
 
 /**
- *	ata_port_add - Attach low-level ATA driver to system
- *	@ent: Information provided by low-level driver
- *	@host_set: Collections of ports to which we add
- *	@port_no: Port number associated with this host
+ *	ata_host_set_alloc - allocate and init basic ATA host_set resources
+ *	@dev: generic device this host_set is associated with
+ *	@sht: template for SCSI host
+ *	@n_ports: number of ATA ports associated with this host_set (can be 0)
  *
- *	Attach low-level ATA driver to system.
- *
- *	LOCKING:
- *	PCI/etc. bus probe sem.
+ *	Allocate and initialize basic ATA host_set resources.  LLD
+ *	calls this function to allocate a host_set, initializes it
+ *	fully and attaches it using ata_host_set_attach().
  *
  *	RETURNS:
- *	New ata_port on success, for NULL on error.
+ *	Allocate ATA host_set on success, NULL on failure.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
  */
-
-static struct ata_port * ata_port_add(const struct ata_probe_ent *ent,
-				      struct ata_host_set *host_set,
-				      unsigned int port_no)
+struct ata_host_set *ata_host_set_alloc(struct device *dev,
+					struct scsi_host_template *sht,
+					int n_ports)
 {
-	struct Scsi_Host *host;
-	struct ata_port *ap;
+	struct ata_host_set *host_set;
+	size_t sz;
 
 	DPRINTK("ENTER\n");
 
-	if (!ent->port_ops->error_handler &&
-	    !(ent->host_flags & (ATA_FLAG_SATA_RESET | ATA_FLAG_SRST))) {
-		printk(KERN_ERR "ata%u: no reset mechanism available\n",
-		       port_no);
-		return NULL;
-	}
+	/* alloc a container for our list of ATA ports (buses) */
+	sz = sizeof(struct ata_host_set) + n_ports * sizeof(void *);
+	if (n_ports == 0)
+		sz += ATA_MAX_PORTS * sizeof(void *);
 
-	host = scsi_host_alloc(ent->sht, sizeof(struct ata_port));
-	if (!host)
+	host_set = kzalloc(sz, GFP_KERNEL);
+	if (!host_set)
 		return NULL;
 
-	host->transportt = &ata_scsi_transport_template;
+	spin_lock_init(&host_set->lock);
+	host_set->dev = dev;
 
-	ap = ata_shost_to_port(host);
+	if (ata_host_set_add_ports(host_set, sht, n_ports))
+		goto err_free;
 
-	ata_port_init(ap, host, host_set, ent, port_no);
+	return host_set;
 
-	return ap;
+ err_free:
+	ata_host_set_free(host_set);
+	return NULL;
+}
+
+/**
+ *	ata_host_set_add_ports - allocate ports to zero-port host_set
+ *	@host_set: target ATA host_set
+ *	@sht: template for SCSI host
+ *	@n_ports: number of ATA ports associated with this host_set
+ *
+ *	Allocate @n_ports ports and associate them with @host_set.
+ *	@n_ports must be less than or equal to ATA_MAX_PORTS and
+ *	@host_set must have been allocated with zero port.  This
+ *	function is used by LLDs which can't determine number of ports
+ *	at host_set allocation time.
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+int ata_host_set_add_ports(struct ata_host_set *host_set,
+			   struct scsi_host_template *sht, int n_ports)
+{
+	int i;
+
+	if (host_set->n_ports || n_ports > ATA_MAX_PORTS)
+		return -EINVAL;
+
+	/* allocate ports bound to this host_set */
+	for (i = 0; i < n_ports; i++) {
+		struct ata_port *ap;
+
+		ap = ata_port_alloc(host_set, sht);
+		if (!ap)
+			return -ENOMEM;
+
+		ap->port_no = i;
+		host_set->ports[i] = ap;
+	}
+
+	host_set->n_ports = n_ports;
+
+	return 0;
 }
 
 /**
@@ -5329,6 +5377,9 @@ static struct ata_port * ata_port_add(co
  *	ap->pflags, so this function can be called multiple times.
  *	Ports are guaranteed to get started only once.
  *
+ *	This function also initializes ap->cbl according to
+ *	ATA_FLAG_SATA if cable type isn't set yet.
+ *
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
  *
@@ -5343,6 +5394,11 @@ int ata_host_set_start(struct ata_host_s
 	for (i = 0; i < host_set->n_ports; i++) {
 		struct ata_port *ap = host_set->ports[i];
 
+		/* set cable type */
+		if (ap->cbl == ATA_CBL_NONE && (ap->flags & ATA_FLAG_SATA))
+			ap->cbl = ATA_CBL_SATA;
+
+		/* start ports */
 		if (ap->pflags & ATA_PFLAG_STARTED)
 			continue;
 
@@ -5361,6 +5417,126 @@ int ata_host_set_start(struct ata_host_s
 }
 
 /**
+ *	ata_host_set_attach - attach initialized ATA host_set
+ *	@host_set: ATA host_set to attach
+ *
+ *	Attach initialized ATA host_set.  @host_set is allocated using
+ *	ata_host_set_alloc() and fully initialized by LLD.  This
+ *	function registers @host_set with ATA and SCSI layers and
+ *	probe attached devices.
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ */
+int ata_host_set_attach(struct ata_host_set *host_set)
+{
+	int i, rc;
+
+	/* make sure ports are started */
+	rc = ata_host_set_start(host_set);
+	if (rc)
+		return rc;
+
+	/* perform each probe synchronously */
+	DPRINTK("probe begin\n");
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+		int irq_line = host_set->irq;
+		unsigned long xfer_mode_mask;
+		u32 scontrol;
+		int rc;
+
+		/* report the secondary IRQ for second channel legacy */
+		if (i == 1 && host_set->irq2)
+			irq_line = host_set->irq2;
+
+		xfer_mode_mask = (ap->udma_mask << ATA_SHIFT_UDMA) |
+				 (ap->mwdma_mask << ATA_SHIFT_MWDMA) |
+				 (ap->pio_mask << ATA_SHIFT_PIO);
+
+		/* print per-port info to dmesg */
+		if (ap->ops != &ata_dummy_port_ops)
+			ata_port_printk(ap, KERN_INFO, "%cATA max %s cmd 0x%lu "
+					"ctl 0x%lX bmdma 0x%lX irq %d\n",
+					ap->flags & ATA_FLAG_SATA ? 'S' : 'P',
+					ata_mode_string(xfer_mode_mask),
+					ap->ioaddr.cmd_addr,
+					ap->ioaddr.ctl_addr,
+					ap->ioaddr.bmdma_addr, irq_line);
+		else
+			ata_port_printk(ap, KERN_INFO, "DUMMY port\n");
+
+		/* init sata_spd_limit to the current value */
+		if (sata_scr_read(ap, SCR_CONTROL, &scontrol) == 0) {
+			int spd = (scontrol >> 4) & 0xf;
+			ap->hw_sata_spd_limit &= (1 << spd) - 1;
+		}
+		ap->sata_spd_limit = ap->hw_sata_spd_limit;
+
+		rc = scsi_add_host(ap->host, host_set->dev);
+		if (rc) {
+			ata_port_printk(ap, KERN_ERR, "scsi_add_host failed\n");
+			/* FIXME: do something useful here */
+			/* FIXME: handle unconditional calls to
+			 * scsi_scan_host and ata_host_remove, below,
+			 * at the very least
+			 */
+		}
+
+		if (ap->ops->error_handler) {
+			struct ata_eh_info *ehi = &ap->eh_info;
+			unsigned long flags;
+
+			ata_port_probe(ap);
+
+			/* kick EH for boot probing */
+			spin_lock_irqsave(ap->lock, flags);
+
+			ehi->probe_mask = (1 << ATA_MAX_DEVICES) - 1;
+			ehi->action |= ATA_EH_SOFTRESET;
+			ehi->flags |= ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET;
+
+			ap->pflags |= ATA_PFLAG_LOADING;
+			ata_port_schedule_eh(ap);
+
+			spin_unlock_irqrestore(ap->lock, flags);
+
+			/* wait for EH to finish */
+			ata_port_wait_eh(ap);
+		} else {
+			DPRINTK("ata%u: bus probe begin\n", ap->id);
+			rc = ata_bus_probe(ap);
+			DPRINTK("ata%u: bus probe end\n", ap->id);
+
+			if (rc) {
+				/* FIXME: do something useful here?
+				 * Current libata behavior will
+				 * tear down everything when
+				 * the module is removed
+				 * or the h/w is unplugged.
+				 */
+			}
+		}
+	}
+
+	/* probes are done, now scan each port's disk(s) */
+	DPRINTK("host probe begin\n");
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		ata_scsi_scan_host(ap);
+	}
+
+	dev_set_drvdata(host_set->dev, host_set);
+
+	VPRINTK("EXIT, returning %u\n", host_set->n_ports);
+	return 0; /* success */
+}
+
+/**
  *	ata_device_add - Register hardware device with ATA and SCSI layers
  *	@ent: Probe information describing hardware device to be registered
  *
@@ -5386,15 +5562,19 @@ int ata_device_add(const struct ata_prob
 	int rc;
 
 	DPRINTK("ENTER\n");
-	/* alloc a container for our list of ATA ports (buses) */
-	host_set = kzalloc(sizeof(struct ata_host_set) +
-			   (ent->n_ports * sizeof(void *)), GFP_KERNEL);
+
+	if (!ent->port_ops->error_handler &&
+	    !(ent->host_flags & (ATA_FLAG_SATA_RESET | ATA_FLAG_SRST))) {
+		dev_printk(KERN_ERR, dev, "no reset mechanism available\n");
+		return 0;
+	}
+
+	/* allocate host_set */
+	host_set = ata_host_set_alloc(dev, ent->sht, ent->n_ports);
 	if (!host_set)
 		return 0;
-	spin_lock_init(&host_set->lock);
 
-	host_set->dev = dev;
-	host_set->n_ports = ent->n_ports;
+	/* initialize host_set according to @ent */
 	host_set->irq = ent->irq;
 	host_set->irq2 = ent->irq2;
 	host_set->mmio_base = ent->mmio_base;
@@ -5402,48 +5582,29 @@ int ata_device_add(const struct ata_prob
 	host_set->ops = ent->port_ops;
 	host_set->flags = ent->host_set_flags;
 
-	/* register each port bound to this device */
 	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap;
-		unsigned long xfer_mode_mask;
-		int irq_line = ent->irq;
-
-		ap = ata_port_add(ent, host_set, i);
-		if (!ap)
-			goto err_out;
-
-		host_set->ports[i] = ap;
+		struct ata_port *ap = host_set->ports[i];
 
 		/* dummy? */
 		if (ent->dummy_port_mask & (1 << i)) {
-			ata_port_printk(ap, KERN_INFO, "DUMMY\n");
 			ap->ops = &ata_dummy_port_ops;
 			continue;
 		}
 
-		/* Report the secondary IRQ for second channel legacy */
-		if (i == 1 && ent->irq2)
-			irq_line = ent->irq2;
+		ap->pio_mask = ent->pio_mask;
+		ap->mwdma_mask = ent->mwdma_mask;
+		ap->udma_mask = ent->udma_mask;
+		ap->flags |= ent->host_flags;
+		ap->ops = ent->port_ops;
 
-		xfer_mode_mask =(ap->udma_mask << ATA_SHIFT_UDMA) |
-				(ap->mwdma_mask << ATA_SHIFT_MWDMA) |
-				(ap->pio_mask << ATA_SHIFT_PIO);
-
-		/* print per-port info to dmesg */
-		ata_port_printk(ap, KERN_INFO, "%cATA max %s cmd 0x%lX "
-				"ctl 0x%lX bmdma 0x%lX irq %d\n",
-				ap->flags & ATA_FLAG_SATA ? 'S' : 'P',
-				ata_mode_string(xfer_mode_mask),
-				ap->ioaddr.cmd_addr,
-				ap->ioaddr.ctl_addr,
-				ap->ioaddr.bmdma_addr,
-				irq_line);
+		memcpy(&ap->ioaddr, &ent->port[ap->port_no],
+		       sizeof(struct ata_ioports));
 	}
 
 	/* start ports */
 	rc = ata_host_set_start(host_set);
 	if (rc)
-		goto err_out;
+		goto err_free_host_set;
 
 	/* freeze */
 	for (i = 0; i < host_set->n_ports; i++) {
@@ -5460,7 +5621,7 @@ int ata_device_add(const struct ata_prob
 	if (rc) {
 		dev_printk(KERN_ERR, dev, "irq %lu request failed: %d\n",
 			   ent->irq, rc);
-		goto err_out;
+		goto err_free_host_set;
 	}
 
 	/* do we have a second IRQ for the other channel, eg legacy mode */
@@ -5474,86 +5635,22 @@ int ata_device_add(const struct ata_prob
 		if (rc) {
 			dev_printk(KERN_ERR, dev, "irq %lu request failed: %d\n",
 				   ent->irq2, rc);
-			goto err_out_free_irq;
+			goto err_free_irq;
 		}
 	}
 
-	/* perform each probe synchronously */
-	DPRINTK("probe begin\n");
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-		u32 scontrol;
-		int rc;
-
-		/* init sata_spd_limit to the current value */
-		if (sata_scr_read(ap, SCR_CONTROL, &scontrol) == 0) {
-			int spd = (scontrol >> 4) & 0xf;
-			ap->hw_sata_spd_limit &= (1 << spd) - 1;
-		}
-		ap->sata_spd_limit = ap->hw_sata_spd_limit;
-
-		rc = scsi_add_host(ap->host, dev);
-		if (rc) {
-			ata_port_printk(ap, KERN_ERR, "scsi_add_host failed\n");
-			/* FIXME: do something useful here */
-			/* FIXME: handle unconditional calls to
-			 * scsi_scan_host and ata_host_remove, below,
-			 * at the very least
-			 */
-		}
-
-		if (ap->ops->error_handler) {
-			struct ata_eh_info *ehi = &ap->eh_info;
-			unsigned long flags;
-
-			ata_port_probe(ap);
-
-			/* kick EH for boot probing */
-			spin_lock_irqsave(ap->lock, flags);
-
-			ehi->probe_mask = (1 << ATA_MAX_DEVICES) - 1;
-			ehi->action |= ATA_EH_SOFTRESET;
-			ehi->flags |= ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET;
-
-			ap->pflags |= ATA_PFLAG_LOADING;
-			ata_port_schedule_eh(ap);
-
-			spin_unlock_irqrestore(ap->lock, flags);
-
-			/* wait for EH to finish */
-			ata_port_wait_eh(ap);
-		} else {
-			DPRINTK("ata%u: bus probe begin\n", ap->id);
-			rc = ata_bus_probe(ap);
-			DPRINTK("ata%u: bus probe end\n", ap->id);
-
-			if (rc) {
-				/* FIXME: do something useful here?
-				 * Current libata behavior will
-				 * tear down everything when
-				 * the module is removed
-				 * or the h/w is unplugged.
-				 */
-			}
-		}
-	}
-
-	/* probes are done, now scan each port's disk(s) */
-	DPRINTK("host probe begin\n");
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-
-		ata_scsi_scan_host(ap);
-	}
-
-	dev_set_drvdata(dev, host_set);
+	rc = ata_host_set_attach(host_set);
+	if (rc)
+		goto err_free_irq2;
 
-	VPRINTK("EXIT, returning %u\n", ent->n_ports);
-	return ent->n_ports; /* success */
+	return host_set->n_ports; /* success */
 
-err_out_free_irq:
+ err_free_irq2:
+	if (ent->irq2)
+		free_irq(ent->irq2, host_set);
+ err_free_irq:
 	free_irq(ent->irq, host_set);
-err_out:
+ err_free_host_set:
 	ata_host_set_free(host_set);
 	VPRINTK("EXIT, returning 0\n");
 	return 0;
@@ -6018,7 +6115,10 @@ EXPORT_SYMBOL_GPL(sata_deb_timing_long);
 EXPORT_SYMBOL_GPL(ata_dummy_port_ops);
 EXPORT_SYMBOL_GPL(ata_std_bios_param);
 EXPORT_SYMBOL_GPL(ata_std_ports);
+EXPORT_SYMBOL_GPL(ata_host_set_alloc);
+EXPORT_SYMBOL_GPL(ata_host_set_add_ports);
 EXPORT_SYMBOL_GPL(ata_host_set_start);
+EXPORT_SYMBOL_GPL(ata_host_set_attach);
 EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_set_detach);
 EXPORT_SYMBOL_GPL(ata_host_set_stop);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5436bd1..fed12ae 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -691,7 +691,13 @@ extern int ata_pci_device_suspend(struct
 extern int ata_pci_device_resume(struct pci_dev *pdev);
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);
 #endif /* CONFIG_PCI */
+extern struct ata_host_set *ata_host_set_alloc(struct device *dev,
+					       struct scsi_host_template *sht,
+					       int n_ports);
+extern int ata_host_set_add_ports(struct ata_host_set *host_set,
+				  struct scsi_host_template *sht, int n_ports);
 extern int ata_host_set_start(struct ata_host_set *host_set);
+extern int ata_host_set_attach(struct ata_host_set *host_set);
 extern int ata_device_add(const struct ata_probe_ent *ent);
 extern void ata_host_set_detach(struct ata_host_set *host_set);
 extern void ata_host_set_stop(struct ata_host_set *host_set);
-- 
1.3.2

Re: [PATCH 03/17] libata: separate out ata_host_set_alloc() and ata_host_set_attach()

[Jeff Garzik]

OK, though obviously needs a rediff after the SAS patches in #upstream

[PATCH 11/17] libata: kill unused ->host_stop() operation and related functions

[Tejun Heo]

Now that ata_port_operations->host_stop() is replaced by
pci_driver->remove_one(), kill the method and related functions.

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

---

 drivers/scsi/libata-core.c |   19 +------------------
 include/linux/libata.h     |    4 ----
 2 files changed, 1 insertions(+), 22 deletions(-)

db0ca25f110082290e507ad3a26cbe44677cbdda
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 2350c34..f944ca0 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5161,12 +5161,6 @@ void ata_port_stop (struct ata_port *ap)
 	ata_pad_free(ap, dev);
 }
 
-void ata_host_stop (struct ata_host_set *host_set)
-{
-	if (host_set->mmio_base)
-		iounmap(host_set->mmio_base);
-}
-
 /**
  *	ata_dev_init - Initialize an ata_device structure
  *	@dev: Device structure to initialize
@@ -5902,13 +5896,6 @@ void ata_std_ports(struct ata_ioports *i
 
 #ifdef CONFIG_PCI
 
-void ata_pci_host_stop (struct ata_host_set *host_set)
-{
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-
-	pci_iounmap(pdev, host_set->mmio_base);
-}
-
 /**
  *	ata_pci_remove_one - PCI layer callback for device removal
  *	@pdev: PCI device that was removed
@@ -5930,9 +5917,7 @@ void ata_pci_remove_one(struct pci_dev *
 	ata_host_set_detach(host_set);
 	ata_host_set_stop(host_set);
 
-	if (host_set->ops->host_stop)
-		host_set->ops->host_stop(host_set);
-	else if (host_set->mmio_base)
+	if (host_set->mmio_base)
 		pci_iounmap(pdev, host_set->mmio_base);
 
 	ata_pci_host_set_destroy(host_set);
@@ -6183,7 +6168,6 @@ EXPORT_SYMBOL_GPL(ata_altstatus);
 EXPORT_SYMBOL_GPL(ata_exec_command);
 EXPORT_SYMBOL_GPL(ata_port_start);
 EXPORT_SYMBOL_GPL(ata_port_stop);
-EXPORT_SYMBOL_GPL(ata_host_stop);
 EXPORT_SYMBOL_GPL(ata_interrupt);
 EXPORT_SYMBOL_GPL(ata_mmio_data_xfer);
 EXPORT_SYMBOL_GPL(ata_pio_data_xfer);
@@ -6243,7 +6227,6 @@ EXPORT_SYMBOL_GPL(ata_timing_merge);
 
 #ifdef CONFIG_PCI
 EXPORT_SYMBOL_GPL(pci_test_config_bits);
-EXPORT_SYMBOL_GPL(ata_pci_host_stop);
 EXPORT_SYMBOL_GPL(ata_pci_host_set_init_native);
 EXPORT_SYMBOL_GPL(ata_pci_legacy_mask);
 EXPORT_SYMBOL_GPL(ata_pci_set_dma_mask);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 523a5e2..02ac266 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -618,8 +618,6 @@ struct ata_port_operations {
 	int (*port_start) (struct ata_port *ap);
 	void (*port_stop) (struct ata_port *ap);
 
-	void (*host_stop) (struct ata_host_set *host_set);
-
 	void (*bmdma_stop) (struct ata_queued_cmd *qc);
 	u8   (*bmdma_status) (struct ata_port *ap);
 };
@@ -752,7 +750,6 @@ extern u8 ata_altstatus(struct ata_port 
 extern void ata_exec_command(struct ata_port *ap, const struct ata_taskfile *tf);
 extern int ata_port_start (struct ata_port *ap);
 extern void ata_port_stop (struct ata_port *ap);
-extern void ata_host_stop (struct ata_host_set *host_set);
 extern irqreturn_t ata_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
 extern void ata_mmio_data_xfer(struct ata_device *adev, unsigned char *buf,
 			       unsigned int buflen, int write_data);
@@ -838,7 +835,6 @@ struct pci_bits {
 	unsigned long		val;
 };
 
-extern void ata_pci_host_stop (struct ata_host_set *host_set);
 extern void ata_pci_host_set_init_native(struct ata_host_set *host_set);
 extern unsigned int ata_pci_legacy_mask(struct pci_dev *pdev);
 extern int ata_pci_set_dma_mask(struct pci_dev *pdev, u64 dma_mask,
-- 
1.3.2

[PATCH 13/17] libata: move ->irq_handler from port_ops to port_info

[Tejun Heo]

libata-core now doesn't care about irq_handler.  It's responsibility
of LLD and helpers.  The only left user is PCI legacy/native init
helpers.  Move ->irq_handler to ata_port_info.

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

---

 drivers/scsi/ata_piix.c     |   10 ++++++++--
 drivers/scsi/libata-bmdma.c |    4 ++--
 include/linux/libata.h      |    3 ++-
 3 files changed, 12 insertions(+), 5 deletions(-)

b532eb39d124c6696cc965ad96ec61e880cabbb8
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 03403cc..c472348 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -254,7 +254,6 @@ static const struct ata_port_operations 
 	.error_handler		= piix_pata_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
-	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 
 	.port_start		= ata_port_start,
@@ -283,7 +282,6 @@ static const struct ata_port_operations 
 	.error_handler		= piix_sata_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
-	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 
 	.port_start		= ata_port_start,
@@ -368,6 +366,7 @@ #else
 #endif
 		.udma_mask	= ATA_UDMA_MASK_40C,
 		.port_ops	= &piix_pata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* ich5_pata */
@@ -382,6 +381,7 @@ #else
 #endif
 		.udma_mask	= 0x3f, /* udma0-5 */
 		.port_ops	= &piix_pata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* ich5_sata */
@@ -392,6 +392,7 @@ #endif
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
 		.port_ops	= &piix_sata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* i6300esb_sata */
@@ -403,6 +404,7 @@ #endif
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
 		.port_ops	= &piix_sata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* ich6_sata */
@@ -414,6 +416,7 @@ #endif
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
 		.port_ops	= &piix_sata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* ich6_sata_ahci */
@@ -426,6 +429,7 @@ #endif
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
 		.port_ops	= &piix_sata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* ich6m_sata_ahci */
@@ -438,6 +442,7 @@ #endif
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
 		.port_ops	= &piix_sata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 
 	/* ich8_sata_ahci */
@@ -450,6 +455,7 @@ #endif
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f,	/* udma0-6 */
 		.port_ops	= &piix_sata_ops,
+		.irq_handler	= ata_interrupt,
 	},
 };
 
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index 75eec7a..6b7c9b4 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -1281,10 +1281,10 @@ int ata_pci_host_set_prepare(struct pci_
 
 		if (host_set->flags & (ATA_HOST_LEGACY_PRI << i)) {
 			ata_pci_port_init_legacy(ap, i);
-			irq_handler[i] = ap->ops->irq_handler;
+			irq_handler[i] = pi[i]->irq_handler;
 		} else {
 			ata_pci_port_init_native(ap, i);
-			irq_handler[2] = ap->ops->irq_handler;
+			irq_handler[2] = pi[i]->irq_handler;
 		}
 	}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 02ac266..e2305a7 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -605,7 +605,6 @@ struct ata_port_operations {
 	void (*error_handler) (struct ata_port *ap);
 	void (*post_internal_cmd) (struct ata_queued_cmd *qc);
 
-	irqreturn_t (*irq_handler)(int, void *, struct pt_regs *);
 	void (*irq_clear) (struct ata_port *);
 
 	u32 (*scr_read) (struct ata_port *ap, unsigned int sc_reg);
@@ -630,6 +629,8 @@ struct ata_port_info {
 	unsigned long		udma_mask;
 	const struct ata_port_operations *port_ops;
 	void 			*private_data;
+	/* fields for BMDMA init */
+	irqreturn_t (*irq_handler)(int, void *, struct pt_regs *);
 };
 
 struct ata_timing {
-- 
1.3.2

[PATCH 12/17] libata: use LLD name where possible

[Tejun Heo]

When acquiring resources, use LLD name where possible.

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

---

 drivers/scsi/libata-bmdma.c |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

89c097fc1f9e41b7c2d7a03d0aa0857bed6b6b5d
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index 4cc316d..75eec7a 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -894,11 +894,12 @@ unsigned int ata_pci_legacy_mask(struct 
 	return mask;
 }
 
-static int ata_pci_acquire_legacy(int port, unsigned long *host_set_flags)
+static int ata_pci_acquire_legacy(int port, const char *name,
+				  unsigned long *host_set_flags)
 {
 	unsigned long cmd_addr = ata_legacy_addr_tbl[port][0];
 
-	if (request_region(cmd_addr, 8, "libata") != NULL)
+	if (request_region(cmd_addr, 8, name) != NULL)
 		*host_set_flags |= ATA_HOST_RES_LEGACY_PRI << port;
 	else {
 		struct resource *conflict, res;
@@ -980,6 +981,16 @@ int ata_pci_set_dma_mask(struct pci_dev 
 	return rc;
 }
 
+static const char *ata_drv_name(struct device *dev)
+{
+	const char *name = dev_driver_string(dev);
+
+	if (name[0] != '\0')
+		return name;
+
+	return DRV_NAME;
+}
+
 /**
  *	ata_pci_acquire_resources - acquire default PCI resources
  *	@host_set: target ATA host_set to acquire PCI resources for
@@ -998,6 +1009,7 @@ int ata_pci_acquire_resources(struct ata
 			      const char **reason)
 {
 	struct pci_dev *pdev = to_pci_dev(host_set->dev);
+	const char *name = ata_drv_name(&pdev->dev);
 	int nr_dummy, i, rc;
 
 	/* acquire generic resources */
@@ -1016,7 +1028,7 @@ int ata_pci_acquire_resources(struct ata
 		goto err;
 	}
 
-	rc = pci_request_regions(pdev, DRV_NAME);
+	rc = pci_request_regions(pdev, name);
 	if (rc) {
 		host_set->flags |= ATA_HOST_PCI_DEV_BUSY;
 		*reason = "failed to request PCI regions";
@@ -1040,7 +1052,7 @@ int ata_pci_acquire_resources(struct ata
 			continue;
 		BUG_ON(i >= host_set->n_ports);
 
-		if (ata_pci_acquire_legacy(i, &host_set->flags)) {
+		if (ata_pci_acquire_legacy(i, name, &host_set->flags)) {
 			host_set->ports[i]->ops = &ata_dummy_port_ops;
 			nr_dummy++;
 		}
@@ -1219,6 +1231,7 @@ int ata_pci_host_set_prepare(struct pci_
 	const int irq[3] = { 14, 15, pdev->irq };
 	irqreturn_t (*irq_handler[3])(int, void *,
 				      struct pt_regs *) = { NULL, NULL, NULL };
+	const char *name = ata_drv_name(&pdev->dev);
 	struct ata_host_set *host_set = NULL;
 	struct ata_port_info dummy_pinfo;
 	const struct ata_port_info *pi[2];
@@ -1309,8 +1322,8 @@ int ata_pci_host_set_prepare(struct pci_
 			break;
 		}
 
-		rc = request_irq(irq[i], irq_handler[i], irq_flags[i],
-				 DRV_NAME, host_set);
+		rc = request_irq(irq[i], irq_handler[i], irq_flags[i], name,
+				 host_set);
 		if (rc) {
 			reason = "failed to request IRQ";
 			goto err;
-- 
1.3.2

[PATCH 10/17] libata: kill old init helpers

[Tejun Heo]

Kill old init helpers ata_pci_init_native_mode() and ata_device_add().

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

---

 drivers/scsi/libata-bmdma.c |   94 ---------------------------------
 drivers/scsi/libata-core.c  |  122 -------------------------------------------
 include/linux/libata.h      |   23 --------
 3 files changed, 0 insertions(+), 239 deletions(-)

3890f4d1ef9e5ed37227f71914c3690f654bfd66
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index d97af71..4cc316d 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -1187,100 +1187,6 @@ void ata_pci_host_set_destroy(struct ata
 	}
 }
 
-static struct ata_probe_ent *
-ata_probe_ent_alloc(struct device *dev, const struct ata_port_info *port)
-{
-	struct ata_probe_ent *probe_ent;
-
-	probe_ent = kzalloc(sizeof(*probe_ent), GFP_KERNEL);
-	if (!probe_ent) {
-		printk(KERN_ERR DRV_NAME "(%s): out of memory\n",
-		       kobject_name(&(dev->kobj)));
-		return NULL;
-	}
-
-	INIT_LIST_HEAD(&probe_ent->node);
-	probe_ent->dev = dev;
-
-	probe_ent->sht = port->sht;
-	probe_ent->host_flags = port->host_flags;
-	probe_ent->pio_mask = port->pio_mask;
-	probe_ent->mwdma_mask = port->mwdma_mask;
-	probe_ent->udma_mask = port->udma_mask;
-	probe_ent->port_ops = port->port_ops;
-
-	return probe_ent;
-}
-
-
-/**
- *	ata_pci_init_native_mode - Initialize native-mode driver
- *	@pdev:  pci device to be initialized
- *	@port:  array[2] of pointers to port info structures.
- *	@ports: bitmap of ports present
- *
- *	Utility function which allocates and initializes an
- *	ata_probe_ent structure for a standard dual-port
- *	PIO-based IDE controller.  The returned ata_probe_ent
- *	structure can be passed to ata_device_add().  The returned
- *	ata_probe_ent structure should then be freed with kfree().
- *
- *	The caller need only pass the address of the primary port, the
- *	secondary will be deduced automatically. If the device has non
- *	standard secondary port mappings this function can be called twice,
- *	once for each interface.
- */
-
-struct ata_probe_ent *
-ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int ports)
-{
-	struct ata_probe_ent *probe_ent =
-		ata_probe_ent_alloc(pci_dev_to_dev(pdev), port[0]);
-	int p = 0;
-	unsigned long bmdma;
-
-	if (!probe_ent)
-		return NULL;
-
-	probe_ent->irq = pdev->irq;
-	probe_ent->irq_flags = IRQF_SHARED;
-	probe_ent->private_data = port[0]->private_data;
-
-	if (ports & ATA_PORT_PRIMARY) {
-		probe_ent->port[p].cmd_addr = pci_resource_start(pdev, 0);
-		probe_ent->port[p].altstatus_addr =
-		probe_ent->port[p].ctl_addr =
-			pci_resource_start(pdev, 1) | ATA_PCI_CTL_OFS;
-		bmdma = pci_resource_start(pdev, 4);
-		if (bmdma) {
-			if (inb(bmdma + 2) & 0x80)
-				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
-			probe_ent->port[p].bmdma_addr = bmdma;
-		}
-		ata_std_ports(&probe_ent->port[p]);
-		p++;
-	}
-
-	if (ports & ATA_PORT_SECONDARY) {
-		probe_ent->port[p].cmd_addr = pci_resource_start(pdev, 2);
-		probe_ent->port[p].altstatus_addr =
-		probe_ent->port[p].ctl_addr =
-			pci_resource_start(pdev, 3) | ATA_PCI_CTL_OFS;
-		bmdma = pci_resource_start(pdev, 4);
-		if (bmdma) {
-			bmdma += 8;
-			if(inb(bmdma + 2) & 0x80)
-				probe_ent->host_set_flags |= ATA_HOST_SIMPLEX;
-			probe_ent->port[p].bmdma_addr = bmdma;
-		}
-		ata_std_ports(&probe_ent->port[p]);
-		p++;
-	}
-
-	probe_ent->n_ports = p;
-	return probe_ent;
-}
-
 /**
  *	ata_pci_host_set_prepare - prepare PCI ATA device for libata attach
  *	@pdev: target PCI device
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index bc326b8..2350c34 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5733,126 +5733,6 @@ int ata_host_set_attach(struct ata_host_
 }
 
 /**
- *	ata_device_add - Register hardware device with ATA and SCSI layers
- *	@ent: Probe information describing hardware device to be registered
- *
- *	This function processes the information provided in the probe
- *	information struct @ent, allocates the necessary ATA and SCSI
- *	host information structures, initializes them, and registers
- *	everything with requisite kernel subsystems.
- *
- *	This function requests irqs, probes the ATA bus, and probes
- *	the SCSI bus.
- *
- *	LOCKING:
- *	PCI/etc. bus probe sem.
- *
- *	RETURNS:
- *	Number of ports registered.  Zero on error (no ports registered).
- */
-int ata_device_add(const struct ata_probe_ent *ent)
-{
-	unsigned int i;
-	struct device *dev = ent->dev;
-	struct ata_host_set *host_set;
-	int rc;
-
-	DPRINTK("ENTER\n");
-
-	if (!ent->port_ops->error_handler &&
-	    !(ent->host_flags & (ATA_FLAG_SATA_RESET | ATA_FLAG_SRST))) {
-		dev_printk(KERN_ERR, dev, "no reset mechanism available\n");
-		return 0;
-	}
-
-	/* allocate host_set */
-	host_set = ata_host_set_alloc(dev, ent->sht, ent->n_ports);
-	if (!host_set)
-		return 0;
-
-	/* initialize host_set according to @ent */
-	host_set->irq = ent->irq;
-	host_set->irq2 = ent->irq2;
-	host_set->mmio_base = ent->mmio_base;
-	host_set->private_data = ent->private_data;
-	host_set->ops = ent->port_ops;
-	host_set->flags = ent->host_set_flags;
-
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-
-		/* dummy? */
-		if (ent->dummy_port_mask & (1 << i)) {
-			ap->ops = &ata_dummy_port_ops;
-			continue;
-		}
-
-		ap->pio_mask = ent->pio_mask;
-		ap->mwdma_mask = ent->mwdma_mask;
-		ap->udma_mask = ent->udma_mask;
-		ap->flags |= ent->host_flags;
-		ap->ops = ent->port_ops;
-
-		memcpy(&ap->ioaddr, &ent->port[ap->port_no],
-		       sizeof(struct ata_ioports));
-	}
-
-	/* start ports */
-	rc = ata_host_set_start(host_set);
-	if (rc)
-		goto err_free_host_set;
-
-	/* freeze */
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-
-		ata_chk_status(ap);
-		host_set->ops->irq_clear(ap);
-		ata_eh_freeze_port(ap);	/* freeze port before requesting IRQ */
-	}
-
-	/* obtain irq, that may be shared between channels */
-	rc = request_irq(ent->irq, ent->port_ops->irq_handler, ent->irq_flags,
-			 DRV_NAME, host_set);
-	if (rc) {
-		dev_printk(KERN_ERR, dev, "irq %lu request failed: %d\n",
-			   ent->irq, rc);
-		goto err_free_host_set;
-	}
-
-	/* do we have a second IRQ for the other channel, eg legacy mode */
-	if (ent->irq2) {
-		/* We will get weird core code crashes later if this is true
-		   so trap it now */
-		BUG_ON(ent->irq == ent->irq2);
-
-		rc = request_irq(ent->irq2, ent->port_ops->irq_handler, ent->irq_flags,
-			 DRV_NAME, host_set);
-		if (rc) {
-			dev_printk(KERN_ERR, dev, "irq %lu request failed: %d\n",
-				   ent->irq2, rc);
-			goto err_free_irq;
-		}
-	}
-
-	rc = ata_host_set_attach(host_set);
-	if (rc)
-		goto err_free_irq2;
-
-	return host_set->n_ports; /* success */
-
- err_free_irq2:
-	if (ent->irq2)
-		free_irq(ent->irq2, host_set);
- err_free_irq:
-	free_irq(ent->irq, host_set);
- err_free_host_set:
-	ata_host_set_free(host_set);
-	VPRINTK("EXIT, returning 0\n");
-	return 0;
-}
-
-/**
  *	ata_port_detach - Detach ATA port in prepration of device removal
  *	@ap: ATA port to be detached
  *
@@ -6283,7 +6163,6 @@ EXPORT_SYMBOL_GPL(ata_host_set_add_ports
 EXPORT_SYMBOL_GPL(ata_host_set_start);
 EXPORT_SYMBOL_GPL(ata_host_set_request_irq);
 EXPORT_SYMBOL_GPL(ata_host_set_attach);
-EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_set_detach);
 EXPORT_SYMBOL_GPL(ata_host_set_stop);
 EXPORT_SYMBOL_GPL(ata_host_set_free);
@@ -6372,7 +6251,6 @@ EXPORT_SYMBOL_GPL(ata_pci_acquire_resour
 EXPORT_SYMBOL_GPL(ata_pci_request_irq);
 EXPORT_SYMBOL_GPL(ata_pci_release_resources);
 EXPORT_SYMBOL_GPL(ata_pci_host_set_destroy);
-EXPORT_SYMBOL_GPL(ata_pci_init_native_mode);
 EXPORT_SYMBOL_GPL(ata_pci_host_set_prepare);
 EXPORT_SYMBOL_GPL(ata_pci_init_one);
 EXPORT_SYMBOL_GPL(ata_pci_remove_one);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1fe1b96..523a5e2 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -367,26 +367,6 @@ struct ata_ioports {
 	unsigned long		scr_addr;
 };
 
-struct ata_probe_ent {
-	struct list_head	node;
-	struct device 		*dev;
-	const struct ata_port_operations *port_ops;
-	struct scsi_host_template *sht;
-	struct ata_ioports	port[ATA_MAX_PORTS];
-	unsigned int		n_ports;
-	unsigned int		dummy_port_mask;
-	unsigned int		pio_mask;
-	unsigned int		mwdma_mask;
-	unsigned int		udma_mask;
-	unsigned long		irq;
-	unsigned long		irq2;
-	unsigned int		irq_flags;
-	unsigned long		host_flags;
-	unsigned long		host_set_flags;
-	void __iomem		*mmio_base;
-	void			*private_data;
-};
-
 struct ata_host_set {
 	spinlock_t		lock;
 	struct device 		*dev;
@@ -730,7 +710,6 @@ extern int ata_host_set_request_irq(stru
 			unsigned long irq_flags, const char **reason);
 extern int ata_host_set_start(struct ata_host_set *host_set);
 extern int ata_host_set_attach(struct ata_host_set *host_set);
-extern int ata_device_add(const struct ata_probe_ent *ent);
 extern void ata_host_set_detach(struct ata_host_set *host_set);
 extern void ata_host_set_stop(struct ata_host_set *host_set);
 extern void ata_host_set_free(struct ata_host_set *host_set);
@@ -871,8 +850,6 @@ extern int ata_pci_request_irq(struct at
 		unsigned int flags, const char **reason);
 extern void ata_pci_release_resources(struct ata_host_set *host_set);
 extern void ata_pci_host_set_destroy(struct ata_host_set *host_set);
-extern struct ata_probe_ent *
-ata_pci_init_native_mode(struct pci_dev *pdev, struct ata_port_info **port, int portmask);
 extern int ata_pci_host_set_prepare(struct pci_dev *pdev,
 				    struct ata_port_info **pinfo_ar,
 				    unsigned int mask, unsigned int legacy_mask,
-- 
1.3.2

[PATCH 09/17] libata: use remove_one() for deinit instead of ->host_stop()

[Tejun Heo]

Update LLDs such that they use ->remove_one() for deinit instead of
->host_stop().  This is how deinit is usually done in other drivers,
more in line with how callbacks are used in libata, symmetric with
initialization, and gives LLDs more flexibility.

As mmio_base release is libata-pci's responsibility, LLDs which used
the default ata_pci_host_stop() can simply switch to stock
ata_pci_remove_one().

This patch fixes the following bugs.

* ata_piix doesn't free hpriv on removal
* sata_sil24 unmaps port_base twice on removal
* sata_uli doesn't free hpriv on removal

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

---

 drivers/scsi/ahci.c         |   20 +-------------------
 drivers/scsi/ata_piix.c     |   16 ++++++++--------
 drivers/scsi/pdc_adma.c     |   30 +++++++++++++++---------------
 drivers/scsi/sata_mv.c      |   40 +++++++++++-----------------------------
 drivers/scsi/sata_nv.c      |   26 +++++++++++++-------------
 drivers/scsi/sata_promise.c |   26 +++++++++++---------------
 drivers/scsi/sata_qstor.c   |   27 +++++++++++++--------------
 drivers/scsi/sata_sil.c     |    3 +--
 drivers/scsi/sata_sil24.c   |   30 +++++++++++++++++-------------
 drivers/scsi/sata_sis.c     |    1 -
 drivers/scsi/sata_svw.c     |    1 -
 drivers/scsi/sata_sx4.c     |   36 ++++++++++++++++++++----------------
 drivers/scsi/sata_uli.c     |   13 +++++++++++--
 drivers/scsi/sata_via.c     |    1 -
 drivers/scsi/sata_vsc.c     |    1 -
 15 files changed, 121 insertions(+), 150 deletions(-)

40c56310e61a9fc5f6bfd18bc0c6665bd26021e8
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 6d90347..87e43f2 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -1575,26 +1575,8 @@ static void ahci_remove_one (struct pci_
 	struct device *dev = pci_dev_to_dev(pdev);
 	struct ata_host_set *host_set = dev_get_drvdata(dev);
 	struct ahci_host_priv *hpriv = host_set->private_data;
-	int have_msi;
 
-	ata_host_set_detach(host_set);
-
-	have_msi = host_set->flags & ATA_HOST_PCI_RES_MSI;
-	free_irq(host_set->irq, host_set);
-
-	ata_host_set_stop(host_set);
-	pci_iounmap(pdev, host_set->mmio_base);
-
-	if (have_msi)
-		pci_disable_msi(pdev);
-	else
-		pci_intx(pdev, 0);
-
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
-	dev_set_drvdata(dev, NULL);
-
-	ata_host_set_free(host_set);
+	ata_pci_remove_one(pdev);
 	kfree(hpriv);
 }
 
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 99ed16c..03403cc 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -149,9 +149,8 @@ struct piix_host_priv {
 	const struct piix_map_db *map_db;
 };
 
-static int piix_init_one (struct pci_dev *pdev,
-				    const struct pci_device_id *ent);
-static void piix_host_stop(struct ata_host_set *host_set);
+static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static void piix_remove_one(struct pci_dev *pdev);
 static void piix_set_piomode (struct ata_port *ap, struct ata_device *adev);
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
 static void piix_pata_error_handler(struct ata_port *ap);
@@ -205,7 +204,7 @@ static struct pci_driver piix_pci_driver
 	.name			= DRV_NAME,
 	.id_table		= piix_pci_tbl,
 	.probe			= piix_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= piix_remove_one,
 	.suspend		= ata_pci_device_suspend,
 	.resume			= ata_pci_device_resume,
 };
@@ -260,7 +259,6 @@ static const struct ata_port_operations 
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= piix_host_stop,
 };
 
 static const struct ata_port_operations piix_sata_ops = {
@@ -290,7 +288,6 @@ static const struct ata_port_operations 
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= piix_host_stop,
 };
 
 static const struct piix_map_db ich5_map_db = {
@@ -930,9 +927,12 @@ static int piix_init_one (struct pci_dev
 	return ata_pci_init_one(pdev, ppinfo, 2);
 }
 
-static void piix_host_stop(struct ata_host_set *host_set)
+static void piix_remove_one(struct pci_dev *pdev)
 {
-	ata_host_stop(host_set);
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+
+	kfree(host_set->private_data);
+	ata_pci_remove_one(pdev);
 }
 
 static int __init piix_init(void)
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index 6d83181..afe1fd4 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -122,10 +122,10 @@ struct adma_port_priv {
 	adma_state_t		state;
 };
 
-static int adma_ata_init_one (struct pci_dev *pdev,
-				const struct pci_device_id *ent);
+static int adma_ata_init_one(struct pci_dev *pdev,
+			     const struct pci_device_id *ent);
+static void adma_ata_remove_one(struct pci_dev *pdev);
 static int adma_port_start(struct ata_port *ap);
-static void adma_host_stop(struct ata_host_set *host_set);
 static void adma_port_stop(struct ata_port *ap);
 static void adma_phy_reset(struct ata_port *ap);
 static void adma_qc_prep(struct ata_queued_cmd *qc);
@@ -170,7 +170,6 @@ static const struct ata_port_operations 
 	.irq_clear		= adma_irq_clear,
 	.port_start		= adma_port_start,
 	.port_stop		= adma_port_stop,
-	.host_stop		= adma_host_stop,
 	.bmdma_stop		= adma_bmdma_stop,
 	.bmdma_status		= adma_bmdma_status,
 };
@@ -198,7 +197,7 @@ static struct pci_driver adma_ata_pci_dr
 	.name			= DRV_NAME,
 	.id_table		= adma_ata_pci_tbl,
 	.probe			= adma_ata_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= adma_ata_remove_one,
 };
 
 static int adma_check_atapi_dma(struct ata_queued_cmd *qc)
@@ -592,16 +591,6 @@ static void adma_port_stop(struct ata_po
 	ata_port_stop(ap);
 }
 
-static void adma_host_stop(struct ata_host_set *host_set)
-{
-	unsigned int port_no;
-
-	for (port_no = 0; port_no < ADMA_PORTS; ++port_no)
-		adma_reset_engine(ADMA_REGS(host_set->mmio_base, port_no));
-
-	ata_pci_host_stop(host_set);
-}
-
 static void adma_host_init(unsigned int chip_id, struct ata_host_set *host_set)
 {
 	unsigned int port_no;
@@ -688,6 +677,17 @@ static int adma_ata_init_one(struct pci_
 	return rc;
 }
 
+static void adma_ata_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	unsigned int port_no;
+
+	for (port_no = 0; port_no < ADMA_PORTS; ++port_no)
+		adma_reset_engine(ADMA_REGS(host_set->mmio_base, port_no));
+
+	ata_pci_remove_one(pdev);
+}
+
 static int __init adma_ata_init(void)
 {
 	return pci_module_init(&adma_ata_pci_driver);
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index d53fd73..3da1c69 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -341,7 +341,6 @@ static u32 mv5_scr_read(struct ata_port 
 static void mv5_scr_write(struct ata_port *ap, unsigned int sc_reg_in, u32 val);
 static void mv_phy_reset(struct ata_port *ap);
 static void __mv_phy_reset(struct ata_port *ap, int can_sleep);
-static void mv_host_stop(struct ata_host_set *host_set);
 static int mv_port_start(struct ata_port *ap);
 static void mv_port_stop(struct ata_port *ap);
 static void mv_qc_prep(struct ata_queued_cmd *qc);
@@ -349,6 +348,7 @@ static void mv_qc_prep_iie(struct ata_qu
 static unsigned int mv_qc_issue(struct ata_queued_cmd *qc);
 static void mv_eng_timeout(struct ata_port *ap);
 static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static void mv_remove_one(struct pci_dev *pdev);
 
 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -415,7 +415,6 @@ static const struct ata_port_operations 
 
 	.port_start		= mv_port_start,
 	.port_stop		= mv_port_stop,
-	.host_stop		= mv_host_stop,
 };
 
 static const struct ata_port_operations mv6_ops = {
@@ -442,7 +441,6 @@ static const struct ata_port_operations 
 
 	.port_start		= mv_port_start,
 	.port_stop		= mv_port_stop,
-	.host_stop		= mv_host_stop,
 };
 
 static const struct ata_port_operations mv_iie_ops = {
@@ -468,7 +466,6 @@ static const struct ata_port_operations 
 
 	.port_start		= mv_port_start,
 	.port_stop		= mv_port_stop,
-	.host_stop		= mv_host_stop,
 };
 
 static const struct ata_port_info mv_port_info[] = {
@@ -545,7 +542,7 @@ static struct pci_driver mv_pci_driver =
 	.name			= DRV_NAME,
 	.id_table		= mv_pci_tbl,
 	.probe			= mv_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= mv_remove_one,
 };
 
 static const struct mv_hw_ops mv5xxx_ops = {
@@ -801,30 +798,6 @@ static void mv_scr_write(struct ata_port
 	}
 }
 
-/**
- *      mv_host_stop - Host specific cleanup/stop routine.
- *      @host_set: host data structure
- *
- *      Disable ints, cleanup host memory, call general purpose
- *      host_stop.
- *
- *      LOCKING:
- *      Inherited from caller.
- */
-static void mv_host_stop(struct ata_host_set *host_set)
-{
-	struct mv_host_priv *hpriv = host_set->private_data;
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-
-	if (host_set->flags & ATA_HOST_PCI_RES_MSI) {
-		pci_disable_msi(pdev);
-	} else {
-		pci_intx(pdev, 0);
-	}
-	kfree(hpriv);
-	ata_host_stop(host_set);
-}
-
 static inline void mv_priv_free(struct mv_port_priv *pp, struct device *dev)
 {
 	dma_free_coherent(dev, MV_PORT_PRIV_DMA_SZ, pp->crpb, pp->crpb_dma);
@@ -2406,6 +2379,15 @@ static int mv_init_one(struct pci_dev *p
 	return rc;
 }
 
+static void mv_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	struct mv_host_priv *hpriv = host_set->private_data;
+
+	ata_pci_remove_one(pdev);
+	kfree(hpriv);
+}
+
 static int __init mv_init(void)
 {
 	return pci_module_init(&mv_pci_driver);
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 17c4609..27085cc 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -80,8 +80,8 @@ enum {
 	NV_MCP_SATA_CFG_20_SATA_SPACE_EN = 0x04,
 };
 
-static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
-static void nv_ck804_host_stop(struct ata_host_set *host_set);
+static int nv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static void nv_remove_one(struct pci_dev *pdev);
 static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void nv_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 
@@ -145,7 +145,7 @@ static struct pci_driver nv_pci_driver =
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
 	.probe			= nv_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= nv_remove_one,
 };
 
 static struct scsi_host_template nv_sht = {
@@ -189,7 +189,6 @@ static const struct ata_port_operations 
 	.scr_write		= nv_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
 };
 
 static const struct ata_port_operations nv_nf2_ops = {
@@ -215,7 +214,6 @@ static const struct ata_port_operations 
 	.scr_write		= nv_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
 };
 
 static const struct ata_port_operations nv_ck804_ops = {
@@ -241,7 +239,6 @@ static const struct ata_port_operations 
 	.scr_write		= nv_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= nv_ck804_host_stop,
 };
 
 static struct ata_port_info nv_port_info[] = {
@@ -564,17 +561,20 @@ static int nv_init_one (struct pci_dev *
 	return rc;
 }
 
-static void nv_ck804_host_stop(struct ata_host_set *host_set)
+static void nv_remove_one(struct pci_dev *pdev)
 {
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-	u8 regval;
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
 
 	/* disable SATA space for CK804 */
-	pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
-	regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
-	pci_write_config_byte(pdev, NV_MCP_SATA_CFG_20, regval);
+	if (host_set->ops == &nv_ck804_ops) {
+		u8 regval;
+
+		pci_read_config_byte(pdev, NV_MCP_SATA_CFG_20, &regval);
+		regval &= ~NV_MCP_SATA_CFG_20_SATA_SPACE_EN;
+		pci_write_config_byte(pdev, NV_MCP_SATA_CFG_20, regval);
+	}
 
-	ata_pci_host_stop(host_set);
+	ata_pci_remove_one(pdev);
 }
 
 static int __init nv_init(void)
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index 4c61d51..3bd7753 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -97,7 +97,8 @@ struct pdc_host_priv {
 
 static u32 pdc_sata_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void pdc_sata_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
-static int pdc_ata_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
+static int pdc_ata_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static void pdc_ata_remove_one(struct pci_dev *pdev);
 static void pdc_eng_timeout(struct ata_port *ap);
 static int pdc_port_start(struct ata_port *ap);
 static void pdc_port_stop(struct ata_port *ap);
@@ -108,7 +109,6 @@ static void pdc_tf_load_mmio(struct ata_
 static void pdc_exec_command_mmio(struct ata_port *ap, const struct ata_taskfile *tf);
 static void pdc_irq_clear(struct ata_port *ap);
 static unsigned int pdc_qc_issue_prot(struct ata_queued_cmd *qc);
-static void pdc_host_stop(struct ata_host_set *host_set);
 
 
 static struct scsi_host_template pdc_ata_sht = {
@@ -149,7 +149,6 @@ static const struct ata_port_operations 
 	.scr_write		= pdc_sata_scr_write,
 	.port_start		= pdc_port_start,
 	.port_stop		= pdc_port_stop,
-	.host_stop		= pdc_host_stop,
 };
 
 static const struct ata_port_operations pdc_pata_ops = {
@@ -170,7 +169,6 @@ static const struct ata_port_operations 
 
 	.port_start		= pdc_port_start,
 	.port_stop		= pdc_port_stop,
-	.host_stop		= pdc_host_stop,
 };
 
 /* Use bits 30-31 of host_flags to encode available port numbers.
@@ -300,7 +298,7 @@ static struct pci_driver pdc_ata_pci_dri
 	.name			= DRV_NAME,
 	.id_table		= pdc_ata_pci_tbl,
 	.probe			= pdc_ata_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= pdc_ata_remove_one,
 };
 
 
@@ -350,16 +348,6 @@ static void pdc_port_stop(struct ata_por
 }
 
 
-static void pdc_host_stop(struct ata_host_set *host_set)
-{
-	struct pdc_host_priv *hp = host_set->private_data;
-
-	ata_pci_host_stop(host_set);
-
-	kfree(hp);
-}
-
-
 static void pdc_reset_port(struct ata_port *ap)
 {
 	void __iomem *mmio = (void __iomem *) ap->ioaddr.cmd_addr + PDC_CTLSTAT;
@@ -788,6 +776,14 @@ static int pdc_ata_init_one(struct pci_d
 	return rc;
 }
 
+static void pdc_ata_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	struct pdc_host_priv *hp = host_set->private_data;
+
+	ata_pci_remove_one(pdev);
+	kfree(hp);
+}
 
 static int __init pdc_ata_init(void)
 {
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index daf20a9..399fee7 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -114,8 +114,8 @@ struct qs_port_priv {
 static u32 qs_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void qs_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static int qs_ata_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
+static void qs_ata_remove_one(struct pci_dev *pdev);
 static int qs_port_start(struct ata_port *ap);
-static void qs_host_stop(struct ata_host_set *host_set);
 static void qs_port_stop(struct ata_port *ap);
 static void qs_phy_reset(struct ata_port *ap);
 static void qs_qc_prep(struct ata_queued_cmd *qc);
@@ -163,7 +163,6 @@ static const struct ata_port_operations 
 	.scr_write		= qs_scr_write,
 	.port_start		= qs_port_start,
 	.port_stop		= qs_port_stop,
-	.host_stop		= qs_host_stop,
 	.bmdma_stop		= qs_bmdma_stop,
 	.bmdma_status		= qs_bmdma_status,
 };
@@ -193,7 +192,7 @@ static struct pci_driver qs_ata_pci_driv
 	.name			= DRV_NAME,
 	.id_table		= qs_ata_pci_tbl,
 	.probe			= qs_ata_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= qs_ata_remove_one,
 };
 
 static int qs_check_atapi_dma(struct ata_queued_cmd *qc)
@@ -541,17 +540,6 @@ static void qs_port_stop(struct ata_port
 	ata_port_stop(ap);
 }
 
-static void qs_host_stop(struct ata_host_set *host_set)
-{
-	void __iomem *mmio_base = host_set->mmio_base;
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-
-	writeb(0, mmio_base + QS_HCT_CTRL); /* disable host interrupts */
-	writeb(QS_CNFG3_GSRST, mmio_base + QS_HCF_CNFG3); /* global reset */
-
-	pci_iounmap(pdev, mmio_base);
-}
-
 static void qs_host_init(unsigned int chip_id, struct ata_host_set *host_set)
 {
 	void __iomem *mmio_base = host_set->mmio_base;
@@ -688,6 +676,17 @@ static int qs_ata_init_one(struct pci_de
 	return rc;
 }
 
+static void qs_ata_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	void __iomem *mmio_base = host_set->mmio_base;
+
+	writeb(0, mmio_base + QS_HCT_CTRL); /* disable host interrupts */
+	writeb(QS_CNFG3_GSRST, mmio_base + QS_HCF_CNFG3); /* global reset */
+
+	ata_pci_remove_one(pdev);
+}
+
 static int __init qs_ata_init(void)
 {
 	return pci_module_init(&qs_ata_pci_driver);
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 3bf1cea..7c128bd 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -108,7 +108,7 @@ enum {
 	SIL_QUIRK_UDMA5MAX	= (1 << 1),
 };
 
-static int sil_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
+static int sil_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
 static int sil_pci_device_resume(struct pci_dev *pdev);
 static void sil_dev_config(struct ata_port *ap, struct ata_device *dev);
 static u32 sil_scr_read (struct ata_port *ap, unsigned int sc_reg);
@@ -208,7 +208,6 @@ static const struct ata_port_operations 
 	.scr_write		= sil_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
 };
 
 static const struct ata_port_info sil_port_info[] = {
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index a35060a..ef58622 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -336,8 +336,8 @@ static void sil24_error_handler(struct a
 static void sil24_post_internal_cmd(struct ata_queued_cmd *qc);
 static int sil24_port_start(struct ata_port *ap);
 static void sil24_port_stop(struct ata_port *ap);
-static void sil24_host_stop(struct ata_host_set *host_set);
 static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
+static void sil24_remove_one(struct pci_dev *pdev);
 static int sil24_pci_device_resume(struct pci_dev *pdev);
 
 static const struct pci_device_id sil24_pci_tbl[] = {
@@ -353,7 +353,7 @@ static struct pci_driver sil24_pci_drive
 	.name			= DRV_NAME,
 	.id_table		= sil24_pci_tbl,
 	.probe			= sil24_init_one,
-	.remove			= ata_pci_remove_one, /* safe? */
+	.remove			= sil24_remove_one,
 	.suspend		= ata_pci_device_suspend,
 	.resume			= sil24_pci_device_resume,
 };
@@ -405,7 +405,6 @@ static const struct ata_port_operations 
 
 	.port_start		= sil24_port_start,
 	.port_stop		= sil24_port_stop,
-	.host_stop		= sil24_host_stop,
 };
 
 /*
@@ -982,16 +981,6 @@ static void sil24_port_stop(struct ata_p
 	kfree(pp);
 }
 
-static void sil24_host_stop(struct ata_host_set *host_set)
-{
-	struct sil24_host_priv *hpriv = host_set->private_data;
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-
-	pci_iounmap(pdev, hpriv->host_base);
-	pci_iounmap(pdev, hpriv->port_base);
-	kfree(hpriv);
-}
-
 static void sil24_init_controller(struct ata_host_set *host_set)
 {
 	struct pci_dev *pdev = to_pci_dev(host_set->dev);
@@ -1153,6 +1142,21 @@ static int sil24_init_one(struct pci_dev
 	return rc;
 }
 
+static void sil24_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	struct sil24_host_priv *hpriv = host_set->private_data;
+
+	ata_host_set_detach(host_set);
+	ata_host_set_stop(host_set);
+
+	pci_iounmap(pdev, hpriv->host_base);
+	pci_iounmap(pdev, hpriv->port_base);
+
+	ata_pci_host_set_destroy(host_set);
+	kfree(hpriv);
+}
+
 static int sil24_pci_device_resume(struct pci_dev *pdev)
 {
 	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
diff --git a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
index 0777dce..5c9758f 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -122,7 +122,6 @@ static const struct ata_port_operations 
 	.scr_write		= sis_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_host_stop,
 };
 
 static struct ata_port_info sis_port_info = {
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index 5042019..56a74be 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -329,7 +329,6 @@ static const struct ata_port_operations 
 	.scr_write		= k2_sata_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
 };
 
 static void k2_sata_setup_port(struct ata_ioports *port, unsigned long base)
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index 9afb072..170b341 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -151,7 +151,9 @@ struct pdc_host_priv {
 };
 
 
-static int pdc_sata_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
+static int pdc_sata_init_one(struct pci_dev *pdev,
+			     const struct pci_device_id *ent);
+static void pdc_sata_remove_one(struct pci_dev *pdev);
 static void pdc_eng_timeout(struct ata_port *ap);
 static void pdc_20621_phy_reset (struct ata_port *ap);
 static int pdc_port_start(struct ata_port *ap);
@@ -159,7 +161,6 @@ static void pdc_port_stop(struct ata_por
 static void pdc20621_qc_prep(struct ata_queued_cmd *qc);
 static void pdc_tf_load_mmio(struct ata_port *ap, const struct ata_taskfile *tf);
 static void pdc_exec_command_mmio(struct ata_port *ap, const struct ata_taskfile *tf);
-static void pdc20621_host_stop(struct ata_host_set *host_set);
 static unsigned int pdc20621_dimm_init(struct ata_host_set *hs);
 static int pdc20621_detect_dimm(struct ata_host_set *hs);
 static unsigned int pdc20621_i2c_read(struct ata_host_set *hs,
@@ -209,7 +210,6 @@ static const struct ata_port_operations 
 	.irq_clear		= pdc20621_irq_clear,
 	.port_start		= pdc_port_start,
 	.port_stop		= pdc_port_stop,
-	.host_stop		= pdc20621_host_stop,
 };
 
 static const struct ata_port_info pdc_port_info[] = {
@@ -238,22 +238,10 @@ static struct pci_driver pdc_sata_pci_dr
 	.name			= DRV_NAME,
 	.id_table		= pdc_sata_pci_tbl,
 	.probe			= pdc_sata_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= pdc_sata_remove_one,
 };
 
 
-static void pdc20621_host_stop(struct ata_host_set *host_set)
-{
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-	struct pdc_host_priv *hpriv = host_set->private_data;
-	void __iomem *dimm_mmio = hpriv->dimm_mmio;
-
-	pci_iounmap(pdev, dimm_mmio);
-	kfree(hpriv);
-
-	pci_iounmap(pdev, host_set->mmio_base);
-}
-
 static int pdc_port_start(struct ata_port *ap)
 {
 	struct device *dev = ap->host_set->dev;
@@ -1455,6 +1443,22 @@ static int pdc_sata_init_one(struct pci_
 }
 
 
+static void pdc_sata_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	struct pdc_host_priv *hpriv = host_set->private_data;
+
+	ata_host_set_detach(host_set);
+	ata_host_set_stop(host_set);
+
+	pci_iounmap(pdev, host_set->mmio_base);
+	pci_iounmap(pdev, hpriv->dimm_mmio);
+
+	ata_pci_host_set_destroy(host_set);
+	kfree(hpriv);
+}
+
+
 static int __init pdc_sata_init(void)
 {
 	return pci_module_init(&pdc_sata_pci_driver);
diff --git a/drivers/scsi/sata_uli.c b/drivers/scsi/sata_uli.c
index 6461898..97e5e7e 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -57,6 +57,7 @@ struct uli_priv {
 };
 
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
+static void uli_remove_one(struct pci_dev *pdev);
 static u32 uli_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void uli_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 
@@ -72,7 +73,7 @@ static struct pci_driver uli_pci_driver 
 	.name			= DRV_NAME,
 	.id_table		= uli_pci_tbl,
 	.probe			= uli_init_one,
-	.remove			= ata_pci_remove_one,
+	.remove			= uli_remove_one,
 };
 
 static struct scsi_host_template uli_sht = {
@@ -122,7 +123,6 @@ static const struct ata_port_operations 
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_host_stop,
 };
 
 static struct ata_port_info uli_port_info = {
@@ -277,6 +277,15 @@ static int uli_init_one(struct pci_dev *
 	return rc;
 }
 
+static void uli_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
+	struct uli_priv *hpriv = host_set->private_data;
+
+	ata_pci_remove_one(pdev);
+	kfree(hpriv);
+}
+
 static int __init uli_init(void)
 {
 	return pci_module_init(&uli_pci_driver);
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index 2a3f3e8..637682f 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -137,7 +137,6 @@ static const struct ata_port_operations 
 
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_host_stop,
 };
 
 static struct ata_port_info svia_port_info = {
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index d028452..6806514 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -307,7 +307,6 @@ static const struct ata_port_operations 
 	.scr_write		= vsc_sata_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= ata_pci_host_stop,
 };
 
 static const struct ata_port_info vsc_sata_port_info = {
-- 
1.3.2

[PATCH 08/17] libata: update ata_pci_remove_one() using new PCI init helpers

[Tejun Heo]

Update ata_pci_remove_one() using new PCI init helpers.

* drvdata clearing is moved to ata_host_set_detach().

* if (!ops->host_stop) mmio_base is iounmapped.  This makes mmio_base
  release libata-pci's responsibility and will ease following
  host_stop and iomap updates.

* unused ata_host_set_remove() is killed.

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

---

 drivers/scsi/libata-core.c |   60 ++++++++------------------------------------
 include/linux/libata.h     |    1 -
 2 files changed, 11 insertions(+), 50 deletions(-)

d05753cf46cbd57821b724073778fa0f3aae5680
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index cb22c04..bc326b8 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5959,6 +5959,8 @@ void ata_host_set_detach(struct ata_host
 
 	for (i = 0; i < host_set->n_ports; i++)
 		ata_port_detach(host_set->ports[i]);
+
+	dev_set_drvdata(host_set->dev, NULL);
 }
 
 /**
@@ -5992,48 +5994,6 @@ void ata_host_set_free(struct ata_host_s
 }
 
 /**
- *	ata_host_set_remove - PCI layer callback for device removal
- *	@host_set: ATA host set that was removed
- *
- *	Unregister all objects associated with this host set. Free those
- *	objects.
- *
- *	LOCKING:
- *	Inherited from calling layer (may sleep).
- */
-void ata_host_set_remove(struct ata_host_set *host_set)
-{
-	unsigned int i;
-
-	ata_host_set_detach(host_set);
-
-	free_irq(host_set->irq, host_set);
-	if (host_set->irq2)
-		free_irq(host_set->irq2, host_set);
-
-	ata_host_set_stop(host_set);
-
-	for (i = 0; i < host_set->n_ports; i++) {
-		struct ata_port *ap = host_set->ports[i];
-
-		if ((ap->flags & ATA_FLAG_NO_LEGACY) == 0) {
-			struct ata_ioports *ioaddr = &ap->ioaddr;
-
-			/* FIXME: Add -ac IDE pci mods to remove these special cases */
-			if (ioaddr->cmd_addr == ATA_PRIMARY_CMD)
-				release_region(ATA_PRIMARY_CMD, 8);
-			else if (ioaddr->cmd_addr == ATA_SECONDARY_CMD)
-				release_region(ATA_SECONDARY_CMD, 8);
-		}
-	}
-
-	if (host_set->ops->host_stop)
-		host_set->ops->host_stop(host_set);
-
-	ata_host_set_free(host_set);
-}
-
-/**
  *	ata_std_ports - initialize ioaddr with standard port offsets.
  *	@ioaddr: IO address structure to be initialized
  *
@@ -6082,17 +6042,20 @@ void ata_pci_host_stop (struct ata_host_
  *	LOCKING:
  *	Inherited from PCI layer (may sleep).
  */
-
-void ata_pci_remove_one (struct pci_dev *pdev)
+void ata_pci_remove_one(struct pci_dev *pdev)
 {
 	struct device *dev = pci_dev_to_dev(pdev);
 	struct ata_host_set *host_set = dev_get_drvdata(dev);
 
-	ata_host_set_remove(host_set);
+	ata_host_set_detach(host_set);
+	ata_host_set_stop(host_set);
+
+	if (host_set->ops->host_stop)
+		host_set->ops->host_stop(host_set);
+	else if (host_set->mmio_base)
+		pci_iounmap(pdev, host_set->mmio_base);
 
-	pci_release_regions(pdev);
-	pci_disable_device(pdev);
-	dev_set_drvdata(dev, NULL);
+	ata_pci_host_set_destroy(host_set);
 }
 
 /* move to PCI subsystem */
@@ -6324,7 +6287,6 @@ EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_set_detach);
 EXPORT_SYMBOL_GPL(ata_host_set_stop);
 EXPORT_SYMBOL_GPL(ata_host_set_free);
-EXPORT_SYMBOL_GPL(ata_host_set_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
 EXPORT_SYMBOL_GPL(ata_hsm_move);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ed89959..1fe1b96 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -734,7 +734,6 @@ extern int ata_device_add(const struct a
 extern void ata_host_set_detach(struct ata_host_set *host_set);
 extern void ata_host_set_stop(struct ata_host_set *host_set);
 extern void ata_host_set_free(struct ata_host_set *host_set);
-extern void ata_host_set_remove(struct ata_host_set *host_set);
 extern int ata_scsi_detect(struct scsi_host_template *sht);
 extern int ata_scsi_ioctl(struct scsi_device *dev, int cmd, void __user *arg);
 extern int ata_scsi_queuecmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *));
-- 
1.3.2

[PATCH 15/17] libata: make ata_pci_acquire_resources() handle iomap

[Tejun Heo]

Make ata_pci_acquire_resources() handle iomap.  @bar_mask is added and
for each set bit respective PCI BAR is iomapped and stored in
host_set->iomap[BAR#].  All iomaps stored in host_set->iomap[] are
freed by ata_pci_release_resources().

Also, acquire_resources() also map legacy IO areas and store them in
host_set->legacy_iomap[p][a] where @p is port number
(0=PRIMARY/1=SECONDARY) and @a indicates CMD or CTL.

This function only implements the feature.

Took hints from iomap patch by Jeff Garzik <jgarzik@pobox.com>.

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

---

 drivers/scsi/ahci.c         |    2 +
 drivers/scsi/libata-bmdma.c |   65 ++++++++++++++++++++++++++++++++++++++++---
 drivers/scsi/pdc_adma.c     |    2 +
 drivers/scsi/sata_mv.c      |    2 +
 drivers/scsi/sata_nv.c      |    2 +
 drivers/scsi/sata_promise.c |    2 +
 drivers/scsi/sata_qstor.c   |    2 +
 drivers/scsi/sata_sil.c     |    2 +
 drivers/scsi/sata_sil24.c   |    2 +
 drivers/scsi/sata_sis.c     |    2 +
 drivers/scsi/sata_svw.c     |    2 +
 drivers/scsi/sata_sx4.c     |    2 +
 drivers/scsi/sata_uli.c     |    2 +
 drivers/scsi/sata_via.c     |    2 +
 drivers/scsi/sata_vsc.c     |    2 +
 include/linux/libata.h      |    5 +++
 16 files changed, 78 insertions(+), 20 deletions(-)

34fffc5383aab3af3ccbd2628ab1973d47a69f88
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 03952d1..eb3640c 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -1488,7 +1488,7 @@ static int ahci_init_one(struct pci_dev 
 	hpriv = host_set->private_data;
 
 	/* acquire ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, 0, &reason);
+	rc = ata_pci_acquire_resources(host_set, 0, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index 15d6560..388d91d 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -995,6 +995,7 @@ static const char *ata_drv_name(struct d
  *	ata_pci_acquire_resources - acquire default PCI resources
  *	@host_set: target ATA host_set to acquire PCI resources for
  *	@dma_mask: DMA mask
+ *	@bar_mask: mask of PCI bars to iomap
  *	@reason: output arg for error message
  *
  *	Acquire default ATA PCI resources.
@@ -1006,10 +1007,12 @@ static const char *ata_drv_name(struct d
  *	0 on success, -errno otherwise.
  */
 int ata_pci_acquire_resources(struct ata_host_set *host_set, u64 dma_mask,
-			      const char **reason)
+			      unsigned int bar_mask, const char **reason)
 {
 	struct pci_dev *pdev = to_pci_dev(host_set->dev);
 	const char *name = ata_drv_name(&pdev->dev);
+	void __iomem **iomap = host_set->iomap;
+	void __iomem *(*legacy_iomap)[2] = host_set->legacy_iomap;
 	int nr_dummy, i, rc;
 
 	/* acquire generic resources */
@@ -1043,11 +1046,36 @@ int ata_pci_acquire_resources(struct ata
 	if (rc)
 		goto err;
 
+	/* iomap PCI BAR */
+	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+		if (!(bar_mask & (1 << i)))
+			continue;
+
+		WARN_ON(iomap[i]);
+
+		if (!pci_resource_start(pdev, i) ||
+		    !pci_resource_len(pdev, i)) {
+			*reason = "iomap requested for null PCI BAR";
+			rc = -EIO;
+			goto err;
+		}
+
+		iomap[i] = pci_iomap(pdev, i, 0);
+		if (!iomap[i]) {
+			rc = -ENOMEM;
+			*reason = "failed to iomap PCI BAR";
+			goto err;
+		}
+	}
+
 	/* Acquire legacy resources.  For legacy ports, host_set must
 	 * be popultaed with ports before calling this function.
 	 */
 	nr_dummy = 0;
 	for (i = 0; i < 2; i++) {
+		unsigned long cmd_addr = ata_legacy_addr_tbl[i][0];
+		unsigned long ctl_addr = ata_legacy_addr_tbl[i][1];
+
 		if (!(host_set->flags & (ATA_HOST_LEGACY_PRI << i)))
 			continue;
 		BUG_ON(i >= host_set->n_ports);
@@ -1055,6 +1083,15 @@ int ata_pci_acquire_resources(struct ata
 		if (ata_pci_acquire_legacy(i, name, &host_set->flags)) {
 			host_set->ports[i]->ops = &ata_dummy_port_ops;
 			nr_dummy++;
+			continue;
+		}
+
+		legacy_iomap[i][0] = ioport_map(cmd_addr, 8);
+		legacy_iomap[i][1] = ioport_map(ctl_addr, 4);
+		if (!legacy_iomap[i][0] || (ctl_addr && !legacy_iomap[i][1])) {
+			rc = -ENOMEM;
+			*reason = "failed to iomap legacy TF";
+			goto err;
 		}
 	}
 
@@ -1131,7 +1168,9 @@ int ata_pci_request_irq(struct ata_host_
 void ata_pci_release_resources(struct ata_host_set *host_set)
 {
 	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-	int i;
+	void __iomem **iomap = host_set->iomap;
+	void __iomem *(*legacy_iomap)[2] = host_set->legacy_iomap;
+	int i, j;
 
 	/* stop all ports */
 	ata_host_set_stop(host_set);
@@ -1162,7 +1201,16 @@ void ata_pci_release_resources(struct at
 		host_set->flags &= ~ATA_HOST_PCI_RES_MSI;
 	}
 
-	/* release legacy resources */
+	/* unmap and release legacy resources */
+	for (i = 0; i < 2; i++) {
+		for (j = 0; j < 2; j++) {
+			if (legacy_iomap[i][j]) {
+				pci_iounmap(pdev, legacy_iomap[i][j]);
+				legacy_iomap[i][j] = NULL;
+			}
+		}
+	}
+
 	for (i = 0; i < 2; i++) {
 		if (host_set->flags & (ATA_HOST_RES_LEGACY_PRI << i)) {
 			release_region(ata_legacy_addr_tbl[i][0], 8);
@@ -1170,7 +1218,14 @@ void ata_pci_release_resources(struct at
 		}
 	}
 
-	/* release generic resources */
+	/* release PCI iomaps and resources */
+	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
+		if (iomap[i]) {
+			pci_iounmap(pdev, iomap[i]);
+			iomap[i] = NULL;
+		}
+	}
+
 	if (host_set->flags & ATA_HOST_PCI_RES_GEN) {
 		pci_release_regions(pdev);
 
@@ -1270,7 +1325,7 @@ int ata_pci_host_set_prepare(struct pci_
 	host_set->flags |= legacy_mask & ATA_HOST_LEGACY_MASK;
 
 	/* acquire ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index 055297e..0ad6c60 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -628,7 +628,7 @@ static int adma_ata_init_one(struct pci_
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, DMA_32BIT_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, DMA_32BIT_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index f52452e..631a175 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -2329,7 +2329,7 @@ static int mv_init_one(struct pci_dev *p
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, DMA_64BIT_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, DMA_64BIT_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 9da8782..0e66c02 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -507,7 +507,7 @@ static int nv_init_one (struct pci_dev *
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index 3f6b98e..556486a 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -714,7 +714,7 @@ static int pdc_ata_init_one(struct pci_d
 	hp = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index 2c7610e..c36e6a8 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -618,7 +618,7 @@ static int qs_ata_init_one(struct pci_de
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, 0, &reason);
+	rc = ata_pci_acquire_resources(host_set, 0, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index ef9ba6a..72297e1 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -631,7 +631,7 @@ static int sil_init_one (struct pci_dev 
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 8d1ed7c..501b32e 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -1068,7 +1068,7 @@ static int sil24_init_one(struct pci_dev
 	hpriv = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, DMA_64BIT_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, DMA_64BIT_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
index 19818c3..2017642 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -255,7 +255,7 @@ static int sis_init_one (struct pci_dev 
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index b6c7783..b303317 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -395,7 +395,7 @@ static int k2_sata_init_one (struct pci_
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index 36cf01d..cf96fba 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -1376,7 +1376,7 @@ static int pdc_sata_init_one(struct pci_
 	hpriv = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_uli.c b/drivers/scsi/sata_uli.c
index 183d4e6..0150561 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -204,7 +204,7 @@ static int uli_init_one(struct pci_dev *
 	hpriv = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index 9920050..da47a64 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -263,7 +263,7 @@ static int svia_init_one(struct pci_dev 
 	}
 
 	/* acquire generic ATA PCI resources */
-	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index b99387d..0d86e8d 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -368,7 +368,7 @@ static int __devinit vsc_sata_init_one (
 	/* Acquire generic ATA PCI resources.  Use 32 bit DMA mask,
 	 * because 64 bit address support is poor.
 	 */
-	rc = ata_pci_acquire_resources(host_set, DMA_32BIT_MASK, &reason);
+	rc = ata_pci_acquire_resources(host_set, DMA_32BIT_MASK, 0, &reason);
 	if (rc)
 		goto err;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 46e3c10..619b918 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -379,6 +379,8 @@ struct ata_host_set {
 	unsigned long		flags;
 	int			simplex_claimed;	/* Keep seperate in case we
 							   ever need to do this locked */
+	void __iomem		*iomap[DEVICE_COUNT_RESOURCE];
+	void __iomem		*legacy_iomap[2][2];
 	struct ata_port		*ports[0];
 };
 
@@ -843,7 +845,8 @@ extern unsigned int ata_pci_legacy_mask(
 extern int ata_pci_set_dma_mask(struct pci_dev *pdev, u64 dma_mask,
 				const char **p_reason);
 extern int ata_pci_acquire_resources(struct ata_host_set *host_set,
-				     u64 dma_mask, const char **reason);
+				     u64 dma_mask, unsigned int bar_mask,
+				     const char **reason);
 extern int ata_pci_request_irq(struct ata_host_set *host_set,
 		irqreturn_t (*irq_handler)(int, void *, struct pt_regs *),
 		unsigned int flags, const char **reason);
-- 
1.3.2

[PATCH 14/17] libata: make ata_host_set_alloc() take care of hpriv alloc/free

[Tejun Heo]

Transfer responsibility to allocate/init hpriv from LLD/port_info to
host_set allocation.  All host_set allocation/prepare routines now
take @hpriv_hz and initialize host_set->private_data accordingly.
hpriv area is aligned to __alignof__(long long) and zeroed.

As hpriv handling is moved to host_set allocation, pinfo->private_data
is removed.  The only user of pinfo->private_data was
ata_pci_init_one() which is updated to take @hpriv argument directly.
This change makes port_info carry one less host_set-wide info - which
is good because mismatched scope causes confusion and bug
(e.g. ata_piix private_data init bug).

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

---

 drivers/scsi/ahci.c         |   29 ++++++++---------------------
 drivers/scsi/ata_piix.c     |    9 +++------
 drivers/scsi/libata-bmdma.c |   13 +++++++++----
 drivers/scsi/libata-core.c  |   37 +++++++++++++++++++++++--------------
 drivers/scsi/pdc_adma.c     |    2 +-
 drivers/scsi/sata_mv.c      |   27 +++++++--------------------
 drivers/scsi/sata_nv.c      |    2 +-
 drivers/scsi/sata_promise.c |   29 +++++++++--------------------
 drivers/scsi/sata_qstor.c   |    3 ++-
 drivers/scsi/sata_sil.c     |    2 +-
 drivers/scsi/sata_sil24.c   |   18 +++++++-----------
 drivers/scsi/sata_sis.c     |    2 +-
 drivers/scsi/sata_svw.c     |    3 ++-
 drivers/scsi/sata_sx4.c     |   19 ++++++++-----------
 drivers/scsi/sata_uli.c     |   31 +++++++++----------------------
 drivers/scsi/sata_via.c     |    2 +-
 drivers/scsi/sata_vsc.c     |    3 ++-
 include/linux/libata.h      |   15 +++++++++------
 18 files changed, 103 insertions(+), 143 deletions(-)

62ea6f411bd27f3e723649e8397cae79bc5c7294
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 87e43f2..03952d1 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -214,7 +214,6 @@ static int ahci_port_suspend(struct ata_
 static int ahci_port_resume(struct ata_port *ap);
 static int ahci_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int ahci_pci_device_resume(struct pci_dev *pdev);
-static void ahci_remove_one (struct pci_dev *pdev);
 
 static struct scsi_host_template ahci_sht = {
 	.module			= THIS_MODULE,
@@ -374,7 +373,7 @@ static struct pci_driver ahci_pci_driver
 	.probe			= ahci_init_one,
 	.suspend		= ahci_pci_device_suspend,
 	.resume			= ahci_pci_device_resume,
-	.remove			= ahci_remove_one,
+	.remove			= ata_pci_remove_one,
 };
 
 
@@ -1453,9 +1452,9 @@ static int ahci_init_one(struct pci_dev 
 {
 	static int printed_version;
 	const struct ata_port_info *pinfo = &ahci_port_info[ent->driver_data];
-	struct ata_host_set *host_set = NULL;
-	struct ahci_host_priv *hpriv = NULL;
+	struct ata_host_set *host_set;
 	void __iomem *mmio_base = NULL;
+	struct ahci_host_priv *hpriv;
 	const char *reason;
 	u64 dma_mask;
 	u32 cap;
@@ -1479,15 +1478,14 @@ static int ahci_init_one(struct pci_dev 
 			return -ENODEV;
 	}
 
-	/* allocate host_set and private_data */
-	host_set = ata_host_set_alloc(&pdev->dev, NULL, 0);
-	hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
-	if (!host_set || !hpriv) {
-		reason = "failed to allocate host_set and private_data";
+	/* allocate host_set */
+	host_set = ata_host_set_alloc(&pdev->dev, NULL, 0, sizeof(*hpriv));
+	if (!host_set) {
+		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
 		goto err;
 	}
-	host_set->private_data = hpriv;
+	hpriv = host_set->private_data;
 
 	/* acquire ATA PCI resources */
 	rc = ata_pci_acquire_resources(host_set, 0, &reason);
@@ -1564,22 +1562,11 @@ static int ahci_init_one(struct pci_dev 
 	if (mmio_base)
 		pci_iounmap(pdev, mmio_base);
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
 
 	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
 	return rc;
 }
 
-static void ahci_remove_one (struct pci_dev *pdev)
-{
-	struct device *dev = pci_dev_to_dev(pdev);
-	struct ata_host_set *host_set = dev_get_drvdata(dev);
-	struct ahci_host_priv *hpriv = host_set->private_data;
-
-	ata_pci_remove_one(pdev);
-	kfree(hpriv);
-}
-
 static int __init ahci_init(void)
 {
 	return pci_module_init(&ahci_pci_driver);
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index c472348..00ae454 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -805,9 +805,9 @@ static void __devinit piix_init_pcs(stru
 
 static void __devinit piix_init_sata_map(struct pci_dev *pdev,
 					 struct ata_port_info *pinfo,
+					 struct piix_host_priv *hpriv,
 					 const struct piix_map_db *map_db)
 {
-	struct piix_host_priv *hpriv = pinfo[0].private_data;
 	const unsigned int *map;
 	int i, invalid_map = 0;
 	u8 map_value;
@@ -831,7 +831,6 @@ static void __devinit piix_init_sata_map
 		case IDE:
 			WARN_ON((i & 1) || map[i + 1] != IDE);
 			pinfo[i / 2] = piix_port_info[ich5_pata];
-			pinfo[i / 2].private_data = hpriv;
 			i++;
 			printk(" IDE IDE");
 			break;
@@ -890,8 +889,6 @@ static int piix_init_one (struct pci_dev
 
 	port_info[0] = piix_port_info[ent->driver_data];
 	port_info[1] = piix_port_info[ent->driver_data];
-	port_info[0].private_data = hpriv;
-	port_info[1].private_data = hpriv;
 
 	host_flags = port_info[0].host_flags;
 
@@ -907,7 +904,7 @@ static int piix_init_one (struct pci_dev
 
 	/* Initialize SATA map */
 	if (host_flags & ATA_FLAG_SATA) {
-		piix_init_sata_map(pdev, port_info,
+		piix_init_sata_map(pdev, port_info, hpriv,
 				   piix_map_db_table[ent->driver_data]);
 		piix_init_pcs(pdev, piix_map_db_table[ent->driver_data]);
 	}
@@ -930,7 +927,7 @@ static int piix_init_one (struct pci_dev
 		port_info[1].mwdma_mask = 0;
 		port_info[1].udma_mask = 0;
 	}
-	return ata_pci_init_one(pdev, ppinfo, 2);
+	return ata_pci_init_one(pdev, ppinfo, 2, hpriv);
 }
 
 static void piix_remove_one(struct pci_dev *pdev)
diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c
index 6b7c9b4..15d6560 100644
--- a/drivers/scsi/libata-bmdma.c
+++ b/drivers/scsi/libata-bmdma.c
@@ -1205,6 +1205,7 @@ void ata_pci_host_set_destroy(struct ata
  *	@pinfo_ar: array of pointers to ATA port_info
  *	@mask: available port mask
  *	@legacy_mask: legacy port mask
+ *	@hpriv_sz: size of host private data
  *	@r_host_set: out arg for prepared ATA host_set
  *
  *	Allocate and initialize ATA host_set for PCI ATA device @pdev.
@@ -1222,7 +1223,7 @@ void ata_pci_host_set_destroy(struct ata
 int ata_pci_host_set_prepare(struct pci_dev *pdev,
 			     struct ata_port_info **pinfo_ar,
 			     unsigned int mask, unsigned int legacy_mask,
-			     struct ata_host_set **r_host_set)
+			     size_t hpriv_sz, struct ata_host_set **r_host_set)
 {
 	static const unsigned long irq_flags[3] = { 0, 0, IRQF_SHARED };
 	static const unsigned long irq_res_mask[3] = { ATA_HOST_PCI_RES_IRQ14,
@@ -1258,7 +1259,8 @@ int ata_pci_host_set_prepare(struct pci_
 	}
 
 	/* allocate host_set */
-	host_set = ata_host_set_alloc_pinfo_ar(&pdev->dev, pi, n_ports);
+	host_set = ata_host_set_alloc_pinfo_ar(&pdev->dev, pi, n_ports,
+					       hpriv_sz);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
@@ -1348,6 +1350,7 @@ int ata_pci_host_set_prepare(struct pci_
  *	@pdev: Controller to be initialized
  *	@pinfo_ar: Information from low-level host driver
  *	@n_ports: Number of ports attached to host controller
+ *	@hpriv: host_set private data
  *
  *	This is a helper function which can be called from a driver's
  *	xxx_init_one() probe function if the hardware uses traditional
@@ -1364,7 +1367,7 @@ int ata_pci_host_set_prepare(struct pci_
  *	Zero on success, negative on errno-based value on error.
  */
 int ata_pci_init_one(struct pci_dev *pdev, struct ata_port_info **pinfo_ar,
-		     unsigned int n_ports)
+		     unsigned int n_ports, void *hpriv)
 {
 	unsigned int mask = ATA_PORT_PRIMARY | ATA_PORT_SECONDARY;
 	unsigned int legacy_mask = 0;
@@ -1390,11 +1393,13 @@ int ata_pci_init_one(struct pci_dev *pde
 	}
 
 	/* prep */
-	rc = ata_pci_host_set_prepare(pdev, pinfo_ar, mask, legacy_mask,
+	rc = ata_pci_host_set_prepare(pdev, pinfo_ar, mask, legacy_mask, 0,
 				      &host_set);
 	if (rc)
 		return rc;
 
+	host_set->private_data = hpriv;
+
 	/* attach */
 	rc = ata_host_set_attach(host_set);
 	if (rc)
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index f944ca0..afff768 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5278,11 +5278,16 @@ #endif
  *	@dev: generic device this host_set is associated with
  *	@sht: template for SCSI host
  *	@n_ports: number of ATA ports associated with this host_set (can be 0)
+ *	@hpriv_sz: size of host private data
  *
  *	Allocate and initialize basic ATA host_set resources.  LLD
  *	calls this function to allocate a host_set, initializes it
  *	fully and attaches it using ata_host_set_attach().
  *
+ *	If @hpriv_sz is not zero, host private data of the specified
+ *	size is allocated together with host_set.  Host private data
+ *	is aligned to __alignof__(long long).
+ *
  *	RETURNS:
  *	Allocate ATA host_set on success, NULL on failure.
  *
@@ -5291,22 +5296,27 @@ #endif
  */
 struct ata_host_set *ata_host_set_alloc(struct device *dev,
 					struct scsi_host_template *sht,
-					int n_ports)
+					int n_ports, size_t hpriv_sz)
 {
 	struct ata_host_set *host_set;
-	size_t sz;
+	size_t hs_sz;
 
 	DPRINTK("ENTER\n");
 
 	/* alloc a container for our list of ATA ports (buses) */
-	sz = sizeof(struct ata_host_set) + n_ports * sizeof(void *);
+	hs_sz = sizeof(struct ata_host_set) + n_ports * sizeof(void *);
 	if (n_ports == 0)
-		sz += ATA_MAX_PORTS * sizeof(void *);
+		hs_sz += ATA_MAX_PORTS * sizeof(void *);
+	if (hpriv_sz)
+		hs_sz = ALIGN(hs_sz, __alignof__(long long));
 
-	host_set = kzalloc(sz, GFP_KERNEL);
+	host_set = kzalloc(hs_sz + hpriv_sz, GFP_KERNEL);
 	if (!host_set)
 		return NULL;
 
+	if (hpriv_sz)
+		host_set->private_data = (void *)host_set + hs_sz;
+
 	spin_lock_init(&host_set->lock);
 	host_set->dev = dev;
 
@@ -5369,8 +5379,6 @@ static void __ata_host_set_init_pinfo(st
 {
 	int i;
 
-	if (host_set->private_data == NULL)
-		host_set->private_data = pinfo[0]->private_data;
 	host_set->ops = pinfo[0]->port_ops;
 
 	for (i = 0; i < host_set->n_ports; i++) {
@@ -5387,9 +5395,6 @@ static void __ata_host_set_init_pinfo(st
 		ap->udma_mask = pi->udma_mask;
 		ap->flags |= pi->host_flags;
 		ap->ops = pi->port_ops;
-
-		WARN_ON(pi->private_data &&
-			pi->private_data != host_set->private_data);
 	}
 }
 
@@ -5398,6 +5403,7 @@ static void __ata_host_set_init_pinfo(st
  *	@dev: generic device this host_set is associated with
  *	@pinfo: ATA port_info to initialize host_set with
  *	@n_ports: number of ATA ports attached to this host_set
+ *	@hpriv_sz: size of host private data
  *
  *	Allocate ATA host_set and initialize with info from @pi.
  *
@@ -5409,14 +5415,15 @@ static void __ata_host_set_init_pinfo(st
  */
 struct ata_host_set *
 ata_host_set_alloc_pinfo(struct device *dev,
-			 const struct ata_port_info *pinfo, int n_ports)
+			 const struct ata_port_info *pinfo, int n_ports,
+			 size_t hpriv_sz)
 {
 	struct ata_host_set *host_set;
 
 	if (!n_ports)
 		return NULL;
 
-	host_set = ata_host_set_alloc(dev, pinfo->sht, n_ports);
+	host_set = ata_host_set_alloc(dev, pinfo->sht, n_ports, hpriv_sz);
 	if (host_set)
 		__ata_host_set_init_pinfo(host_set, &pinfo, n_ports, 0);
 	return host_set;
@@ -5427,6 +5434,7 @@ ata_host_set_alloc_pinfo(struct device *
  *	@dev: generic device this host_set is associated with
  *	@pinfo_ar: array of ATA port_info to initialize host_set with
  *	@n_ports: number of ATA ports attached to this host_set
+ *	@hpriv_sz: size of host private data
  *
  *	Allocate ATA host_set and initialize with info from @pinfo_ar.
  *
@@ -5438,14 +5446,15 @@ ata_host_set_alloc_pinfo(struct device *
  */
 struct ata_host_set *
 ata_host_set_alloc_pinfo_ar(struct device *dev,
-			    const struct ata_port_info **pinfo_ar, int n_ports)
+			    const struct ata_port_info **pinfo_ar, int n_ports,
+			    size_t hpriv_sz)
 {
 	struct ata_host_set *host_set;
 
 	if (!n_ports)
 		return NULL;
 
-	host_set = ata_host_set_alloc(dev, pinfo_ar[0]->sht, n_ports);
+	host_set = ata_host_set_alloc(dev, pinfo_ar[0]->sht, n_ports, hpriv_sz);
 	if (host_set)
 		__ata_host_set_init_pinfo(host_set, pinfo_ar, n_ports, 1);
 	return host_set;
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index afe1fd4..055297e 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -620,7 +620,7 @@ static int adma_ata_init_one(struct pci_
 	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev,
 					    &adma_port_info[board_idx],
-					    ADMA_PORTS);
+					    ADMA_PORTS, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index 3da1c69..f52452e 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -348,7 +348,6 @@ static void mv_qc_prep_iie(struct ata_qu
 static unsigned int mv_qc_issue(struct ata_queued_cmd *qc);
 static void mv_eng_timeout(struct ata_port *ap);
 static int mv_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
-static void mv_remove_one(struct pci_dev *pdev);
 
 static void mv5_phy_errata(struct mv_host_priv *hpriv, void __iomem *mmio,
 			   unsigned int port);
@@ -542,7 +541,7 @@ static struct pci_driver mv_pci_driver =
 	.name			= DRV_NAME,
 	.id_table		= mv_pci_tbl,
 	.probe			= mv_init_one,
-	.remove			= mv_remove_one,
+	.remove			= ata_pci_remove_one,
 };
 
 static const struct mv_hw_ops mv5xxx_ops = {
@@ -2310,8 +2309,7 @@ static int mv_init_one(struct pci_dev *p
 	static int printed_version = 0;
 	unsigned int board_idx = (unsigned int)ent->driver_data;
 	const struct ata_port_info *pinfo = &mv_port_info[board_idx];
-	struct ata_host_set *host_set = NULL;
-	struct mv_host_priv *hpriv = NULL;
+	struct ata_host_set *host_set;
 	void __iomem *mmio_base = NULL;
 	const char *reason;
 	int n_ports, rc;
@@ -2319,17 +2317,16 @@ static int mv_init_one(struct pci_dev *p
 	if (!printed_version++)
 		dev_printk(KERN_INFO, &pdev->dev, "version " DRV_VERSION "\n");
 
-	/* alloc host_set and hpriv */
+	/* alloc host_set */
 	n_ports = MV_PORTS_PER_HC * mv_get_hc_count(pinfo->host_flags);
 
-	host_set = ata_host_set_alloc_pinfo(&pdev->dev, pinfo, n_ports);
-	hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
-	if (!host_set || !hpriv) {
-		reason = "failed to allocate host_set and private_data";
+	host_set = ata_host_set_alloc_pinfo(&pdev->dev, pinfo, n_ports,
+					    sizeof(struct mv_host_priv));
+	if (!host_set) {
+		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
 		goto err;
 	}
-	host_set->private_data = hpriv;
 
 	/* acquire generic ATA PCI resources */
 	rc = ata_pci_acquire_resources(host_set, DMA_64BIT_MASK, &reason);
@@ -2373,21 +2370,11 @@ static int mv_init_one(struct pci_dev *p
 	if (mmio_base)
 		pci_iounmap(pdev, mmio_base);
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
 
 	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
 	return rc;
 }
 
-static void mv_remove_one(struct pci_dev *pdev)
-{
-	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
-	struct mv_host_priv *hpriv = host_set->private_data;
-
-	ata_pci_remove_one(pdev);
-	kfree(hpriv);
-}
-
 static int __init mv_init(void)
 {
 	return pci_module_init(&mv_pci_driver);
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 27085cc..9da8782 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -499,7 +499,7 @@ static int nv_init_one (struct pci_dev *
 	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev,
 					    &nv_port_info[ent->driver_data],
-					    NV_PORTS);
+					    NV_PORTS, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index 3bd7753..3f6b98e 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -98,7 +98,6 @@ struct pdc_host_priv {
 static u32 pdc_sata_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void pdc_sata_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static int pdc_ata_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
-static void pdc_ata_remove_one(struct pci_dev *pdev);
 static void pdc_eng_timeout(struct ata_port *ap);
 static int pdc_port_start(struct ata_port *ap);
 static void pdc_port_stop(struct ata_port *ap);
@@ -298,7 +297,7 @@ static struct pci_driver pdc_ata_pci_dri
 	.name			= DRV_NAME,
 	.id_table		= pdc_ata_pci_tbl,
 	.probe			= pdc_ata_init_one,
-	.remove			= pdc_ata_remove_one,
+	.remove			= ata_pci_remove_one,
 };
 
 
@@ -693,8 +692,8 @@ static int pdc_ata_init_one(struct pci_d
 	static int printed_version;
 	unsigned int board_idx = (unsigned int) ent->driver_data;
 	const struct ata_port_info *pinfo = &pdc_port_info[board_idx];
-	struct ata_host_set *host_set = NULL;
-	struct pdc_host_priv *hp = NULL;
+	struct ata_host_set *host_set;
+	struct pdc_host_priv *hp;
 	void __iomem *mmio_base = NULL;
 	unsigned long base;
 	const char *reason;
@@ -703,16 +702,16 @@ static int pdc_ata_init_one(struct pci_d
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
 
-	/* allocate host_set and private_data */
+	/* allocate host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev, pinfo,
-					    PDC_FLAG2NPORTS(pinfo->host_flags));
-	hp = kzalloc(sizeof(*hp), GFP_KERNEL);
-	if (!host_set || !hp) {
-		reason = "failed to allocate host_set and private_data";
+					    PDC_FLAG2NPORTS(pinfo->host_flags),
+					    sizeof(*hp));
+	if (!host_set) {
+		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
 		goto err;
 	}
-	host_set->private_data = hp;
+	hp = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
 	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
@@ -770,21 +769,11 @@ static int pdc_ata_init_one(struct pci_d
 	if (mmio_base)
 		pci_iounmap(pdev, mmio_base);
 	ata_pci_host_set_destroy(host_set);
-	kfree(hp);
 
 	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
 	return rc;
 }
 
-static void pdc_ata_remove_one(struct pci_dev *pdev)
-{
-	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
-	struct pdc_host_priv *hp = host_set->private_data;
-
-	ata_pci_remove_one(pdev);
-	kfree(hp);
-}
-
 static int __init pdc_ata_init(void)
 {
 	return pci_module_init(&pdc_ata_pci_driver);
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index 399fee7..2c7610e 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -609,7 +609,8 @@ static int qs_ata_init_one(struct pci_de
 
 	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev,
-					    &qs_port_info[board_idx], QS_PORTS);
+					    &qs_port_info[board_idx],
+					    QS_PORTS, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 7c128bd..ef9ba6a 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -623,7 +623,7 @@ static int sil_init_one (struct pci_dev 
 	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev,
 				&sil_port_info[ent->driver_data],
-				(ent->driver_data == sil_3114) ? 4 : 2);
+				(ent->driver_data == sil_3114) ? 4 : 2, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index ef58622..8d1ed7c 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -1046,8 +1046,8 @@ static int sil24_init_one(struct pci_dev
 	static int printed_version = 0;
 	unsigned int board_id = (unsigned int)ent->driver_data;
 	struct ata_port_info *pinfo = &sil24_port_info[board_id];
-	struct ata_host_set *host_set = NULL;
-	struct sil24_host_priv *hpriv = NULL;
+	struct ata_host_set *host_set;
+	struct sil24_host_priv *hpriv;
 	void __iomem *host_base = NULL;
 	void __iomem *port_base = NULL;
 	const char *reason;
@@ -1057,17 +1057,15 @@ static int sil24_init_one(struct pci_dev
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
 
-	/* alloc host_set and hpriv */
+	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev, pinfo,
-					SIL24_FLAG2NPORTS(pinfo->host_flags));
-	hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
-
-	if (!host_set || !hpriv) {
-		reason = "failed to allocate host_set and private_data";
+			SIL24_FLAG2NPORTS(pinfo->host_flags), sizeof(*hpriv));
+	if (!host_set) {
+		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
 		goto err;
 	}
-	host_set->private_data = hpriv;
+	hpriv = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
 	rc = ata_pci_acquire_resources(host_set, DMA_64BIT_MASK, &reason);
@@ -1136,7 +1134,6 @@ static int sil24_init_one(struct pci_dev
 	if (port_base)
 		pci_iounmap(pdev, port_base);
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
 
 	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
 	return rc;
@@ -1154,7 +1151,6 @@ static void sil24_remove_one(struct pci_
 	pci_iounmap(pdev, hpriv->port_base);
 
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
 }
 
 static int sil24_pci_device_resume(struct pci_dev *pdev)
diff --git a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
index 5c9758f..19818c3 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -247,7 +247,7 @@ static int sis_init_one (struct pci_dev 
 		dev_printk(KERN_INFO, &pdev->dev, "version " DRV_VERSION "\n");
 
 	/* alloc host_set */
-	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &sis_port_info, 2);
+	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &sis_port_info, 2, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index 56a74be..b6c7783 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -386,7 +386,8 @@ static int k2_sata_init_one (struct pci_
 	 * ent->driver_data and the same is done here.  Verify the bug
 	 * and fix.
 	 */
-	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &k2_sata_port_info, 4);
+	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &k2_sata_port_info,
+					    4, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index 170b341..36cf01d 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -1353,8 +1353,8 @@ static int pdc_sata_init_one(struct pci_
 {
 	static int printed_version;
 	unsigned int board_idx = (unsigned int) ent->driver_data;
-	struct ata_host_set *host_set = NULL;
-	struct pdc_host_priv *hpriv = NULL;
+	struct ata_host_set *host_set;
+	struct pdc_host_priv *hpriv;
 	void __iomem *mmio_base = NULL;
 	void __iomem *dimm_mmio = NULL;
 	unsigned long base;
@@ -1364,17 +1364,16 @@ static int pdc_sata_init_one(struct pci_
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
 
-	/* alloc host_set and hpriv */
+	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev,
-					    &pdc_port_info[board_idx], 4);
-	hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
-
-	if (!host_set || !hpriv) {
-		reason = "failed to allocate host_set and private_data";
+					    &pdc_port_info[board_idx],
+					    4, sizeof(*hpriv));
+	if (!host_set) {
+		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
 		goto err;
 	}
-	host_set->private_data = hpriv;
+	hpriv = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
 	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
@@ -1436,7 +1435,6 @@ static int pdc_sata_init_one(struct pci_
 	if (dimm_mmio)
 		pci_iounmap(pdev, dimm_mmio);
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
 
 	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
 	return rc;
@@ -1455,7 +1453,6 @@ static void pdc_sata_remove_one(struct p
 	pci_iounmap(pdev, hpriv->dimm_mmio);
 
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
 }
 
 
diff --git a/drivers/scsi/sata_uli.c b/drivers/scsi/sata_uli.c
index 97e5e7e..183d4e6 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -57,7 +57,6 @@ struct uli_priv {
 };
 
 static int uli_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
-static void uli_remove_one(struct pci_dev *pdev);
 static u32 uli_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void uli_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 
@@ -73,7 +72,7 @@ static struct pci_driver uli_pci_driver 
 	.name			= DRV_NAME,
 	.id_table		= uli_pci_tbl,
 	.probe			= uli_init_one,
-	.remove			= uli_remove_one,
+	.remove			= ata_pci_remove_one,
 };
 
 static struct scsi_host_template uli_sht = {
@@ -184,8 +183,8 @@ static int uli_init_one(struct pci_dev *
 {
 	static int printed_version;
 	unsigned int board_idx = (unsigned int) ent->driver_data;
-	struct ata_host_set *host_set = NULL;
-	struct uli_priv *hpriv = NULL;
+	struct ata_host_set *host_set;
+	struct uli_priv *hpriv;
 	struct ata_ioports *ioaddr;
 	const char *reason;
 	int rc;
@@ -193,17 +192,16 @@ static int uli_init_one(struct pci_dev *
 	if (!printed_version++)
 		dev_printk(KERN_INFO, &pdev->dev, "version " DRV_VERSION "\n");
 
-	/* alloc host_set and hpriv */
+	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &uli_port_info,
-					    board_idx == uli_5287 ? 4 : 2);
-	hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL);
-
-	if (!host_set || !hpriv) {
-		reason = "failed to allocate host_set and private_data";
+					    board_idx == uli_5287 ? 4 : 2,
+					    sizeof(*hpriv));
+	if (!host_set) {
+		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
 		goto err;
 	}
-	host_set->private_data = hpriv;
+	hpriv = host_set->private_data;
 
 	/* acquire generic ATA PCI resources */
 	rc = ata_pci_acquire_resources(host_set, ATA_DMA_MASK, &reason);
@@ -271,21 +269,10 @@ static int uli_init_one(struct pci_dev *
 
  err:
 	ata_pci_host_set_destroy(host_set);
-	kfree(hpriv);
-
 	dev_printk(KERN_ERR, &pdev->dev, "%s (error=%d)\n", reason, rc);
 	return rc;
 }
 
-static void uli_remove_one(struct pci_dev *pdev)
-{
-	struct ata_host_set *host_set = dev_get_drvdata(&pdev->dev);
-	struct uli_priv *hpriv = host_set->private_data;
-
-	ata_pci_remove_one(pdev);
-	kfree(hpriv);
-}
-
 static int __init uli_init(void)
 {
 	return pci_module_init(&uli_pci_driver);
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index 637682f..9920050 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -255,7 +255,7 @@ static int svia_init_one(struct pci_dev 
 
 	/* alloc host_set */
 	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &svia_port_info,
-					    N_PORTS);
+					    N_PORTS, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index 6806514..b99387d 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -357,7 +357,8 @@ static int __devinit vsc_sata_init_one (
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
 
 	/* alloc host_set */
-	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &vsc_sata_port_info, 4);
+	host_set = ata_host_set_alloc_pinfo(&pdev->dev, &vsc_sata_port_info,
+					    4, 0);
 	if (!host_set) {
 		reason = "failed to allocate host_set";
 		rc = -ENOMEM;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e2305a7..46e3c10 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -628,7 +628,6 @@ struct ata_port_info {
 	unsigned long		mwdma_mask;
 	unsigned long		udma_mask;
 	const struct ata_port_operations *port_ops;
-	void 			*private_data;
 	/* fields for BMDMA init */
 	irqreturn_t (*irq_handler)(int, void *, struct pt_regs *);
 };
@@ -682,8 +681,8 @@ extern int ata_dev_revalidate(struct ata
 extern void ata_port_disable(struct ata_port *);
 extern void ata_std_ports(struct ata_ioports *ioaddr);
 #ifdef CONFIG_PCI
-extern int ata_pci_init_one (struct pci_dev *pdev, struct ata_port_info **port_info,
-			     unsigned int n_ports);
+extern int ata_pci_init_one(struct pci_dev *pdev, struct ata_port_info **port_info,
+			    unsigned int n_ports, void *hpriv);
 extern void ata_pci_remove_one (struct pci_dev *pdev);
 extern void ata_pci_device_do_suspend(struct pci_dev *pdev, pm_message_t mesg);
 extern void ata_pci_device_do_resume(struct pci_dev *pdev);
@@ -692,13 +691,16 @@ extern int ata_pci_device_resume(struct 
 extern int ata_pci_clear_simplex(struct pci_dev *pdev);
 #endif /* CONFIG_PCI */
 extern struct ata_host_set *ata_host_set_alloc(struct device *dev,
-			struct scsi_host_template *sht, int n_ports);
+			struct scsi_host_template *sht, int n_ports,
+			size_t hpriv_sz);
 extern int ata_host_set_add_ports(struct ata_host_set *host_set,
 			struct scsi_host_template *sht, int n_ports);
 extern struct ata_host_set *ata_host_set_alloc_pinfo(struct device *dev,
-			const struct ata_port_info *pinfo, int n_ports);
+			const struct ata_port_info *pinfo, int n_ports,
+			size_t hpriv_sz);
 extern struct ata_host_set *ata_host_set_alloc_pinfo_ar(struct device *dev,
-			const struct ata_port_info **pinfo_ar, int n_ports);
+			const struct ata_port_info **pinfo_ar, int n_ports,
+			size_t hpriv_sz);
 extern int ata_host_set_add_ports_pinfo(struct ata_host_set *host_set,
 			const struct ata_port_info *pinfo, int n_ports);
 extern int ata_host_set_add_ports_pinfo_ar(struct ata_host_set *host_set,
@@ -850,6 +852,7 @@ extern void ata_pci_host_set_destroy(str
 extern int ata_pci_host_set_prepare(struct pci_dev *pdev,
 				    struct ata_port_info **pinfo_ar,
 				    unsigned int mask, unsigned int legacy_mask,
+				    size_t hpriv_sz,
 				    struct ata_host_set **r_host_set);
 extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits);
 extern unsigned long ata_pci_default_filter(const struct ata_port *, struct ata_device *, unsigned long);
-- 
1.3.2

[PATCH 17/17] libata: kill unused ATA_FLAG_MMIO

[Tejun Heo]

Kill unused ATA_FLAG_MMIO.

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

---

 drivers/scsi/ahci.c         |    6 ++----
 drivers/scsi/pdc_adma.c     |    2 +-
 drivers/scsi/sata_mv.c      |    4 ++--
 drivers/scsi/sata_promise.c |    3 +--
 drivers/scsi/sata_qstor.c   |    2 +-
 drivers/scsi/sata_sil.c     |    2 +-
 drivers/scsi/sata_sil24.c   |    4 ++--
 drivers/scsi/sata_svw.c     |    3 +--
 drivers/scsi/sata_sx4.c     |    4 ++--
 drivers/scsi/sata_vsc.c     |    3 +--
 include/linux/libata.h      |    1 -
 11 files changed, 14 insertions(+), 20 deletions(-)

ade4de8ced4e466dfe57410f8bb826f5d5f2d3a7
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index d639501..2c8fa32 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -271,8 +271,7 @@ static const struct ata_port_info ahci_p
 	{
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  ATA_FLAG_SKIP_D2H_BSY,
+				  ATA_FLAG_PIO_DMA | ATA_FLAG_SKIP_D2H_BSY,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
 		.port_ops	= &ahci_ops,
@@ -281,8 +280,7 @@ static const struct ata_port_info ahci_p
 	{
 		.sht		= &ahci_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  ATA_FLAG_SKIP_D2H_BSY |
+				  ATA_FLAG_PIO_DMA | ATA_FLAG_SKIP_D2H_BSY |
 				  AHCI_FLAG_RESET_NEEDS_CLO | AHCI_FLAG_NO_NCQ,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index 3e2c101..3c46da1 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -187,7 +187,7 @@ static struct ata_port_info adma_port_in
 	{
 		.sht		= &adma_ata_sht,
 		.host_flags	= ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST |
-				  ATA_FLAG_NO_LEGACY | ATA_FLAG_MMIO,
+				  ATA_FLAG_NO_LEGACY,
 		.pio_mask	= 0x10, /* pio4 */
 		.udma_mask	= 0x1f, /* udma0-4 */
 		.port_ops	= &adma_ata_ops,
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index 95e64ce..e46c840 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -92,8 +92,8 @@ enum {
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
 	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
 	MV_COMMON_FLAGS		= (ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				   ATA_FLAG_SATA_RESET | ATA_FLAG_MMIO |
-				   ATA_FLAG_NO_ATAPI | ATA_FLAG_PIO_POLLING),
+				   ATA_FLAG_SATA_RESET | ATA_FLAG_NO_ATAPI |
+				   ATA_FLAG_PIO_POLLING),
 	MV_6XXX_FLAGS		= MV_FLAG_IRQ_COALESCE,
 
 	CRQB_FLAG_READ		= (1 << 0),
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index 2495ef5..f70e902 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -83,8 +83,7 @@ enum {
 	PDC_HAS_PATA		= (1 << 1), /* PDC20375/20575 has PATA */
 
 	PDC_COMMON_FLAGS	= ATA_FLAG_NO_LEGACY | ATA_FLAG_SRST |
-				  ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI |
-				  ATA_FLAG_PIO_POLLING,
+				  ATA_FLAG_NO_ATAPI | ATA_FLAG_PIO_POLLING,
 };
 
 
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index d2ef344..fae3cf9 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -176,7 +176,7 @@ static const struct ata_port_info qs_por
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_SATA_RESET |
 				  //FIXME ATA_FLAG_SRST |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_POLLING,
+				  ATA_FLAG_PIO_POLLING,
 		.pio_mask	= 0x10, /* pio4 */
 		.udma_mask	= 0x7f, /* udma0-6 */
 		.port_ops	= &qs_ata_ops,
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index e35b4cd..7669892 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -59,7 +59,7 @@ enum {
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
 
 	SIL_DFL_HOST_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_HRST_TO_RESUME,
+				  ATA_FLAG_HRST_TO_RESUME,
 
 	/*
 	 * Controller IDs
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index f2f2b51..f0950c2 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -232,8 +232,8 @@ enum {
 
 	/* host flags */
 	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  ATA_FLAG_NCQ | ATA_FLAG_SKIP_D2H_BSY,
+				  ATA_FLAG_PIO_DMA | ATA_FLAG_NCQ |
+				  ATA_FLAG_SKIP_D2H_BSY,
 	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 24), /* IRQ loss errata on PCI-X */
 
 	IRQ_STAT_4PORTS		= 0xf,
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index 9184ca6..0510843 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -352,8 +352,7 @@ static void k2_sata_setup_port(struct at
 
 static const struct ata_port_info k2_sata_port_info = {
 	.sht			= &k2_sata_sht,
-	.host_flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO,
+	.host_flags		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	/* We don't care much about the PIO/UDMA masks, but the core
 	 * won't like us if we don't fill these
 	 */
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index 28e7615..bb9d9eb 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -217,8 +217,8 @@ static const struct ata_port_info pdc_po
 	{
 		.sht		= &pdc_sata_sht,
 		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_SRST | ATA_FLAG_MMIO |
-				  ATA_FLAG_NO_ATAPI | ATA_FLAG_PIO_POLLING,
+				  ATA_FLAG_SRST | ATA_FLAG_NO_ATAPI |
+				  ATA_FLAG_PIO_POLLING,
 		.pio_mask	= 0x1f, /* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= 0x7f, /* udma0-6 ; FIXME */
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index 7a845b0..2037f9a 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -314,8 +314,7 @@ static const struct ata_port_operations 
 
 static const struct ata_port_info vsc_sata_port_info = {
 	.sht		= &vsc_sata_sht,
-	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-			  ATA_FLAG_MMIO,
+	.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 	/* We don't care much about the PIO/UDMA masks, but the core
 	 * won't like us if we don't fill these.
 	 */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ce8f8ce..0431025 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -151,7 +151,6 @@ enum {
 					    /* (doesn't imply presence) */
 	ATA_FLAG_SATA		= (1 << 1),
 	ATA_FLAG_NO_LEGACY	= (1 << 2), /* no legacy mode check */
-	ATA_FLAG_MMIO		= (1 << 3), /* use MMIO, not PIO */
 	ATA_FLAG_SRST		= (1 << 4), /* (obsolete) use ATA SRST, not E.D.D. */
 	ATA_FLAG_SATA_RESET	= (1 << 5), /* (obsolete) use COMRESET */
 	ATA_FLAG_NO_ATAPI	= (1 << 6), /* No ATAPI support */
-- 
1.3.2

Re: [git-patches] libata: implement new initialization model w/ iomap support

[Tejun Heo]

The patchset is available in the following git tree.

http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=new-init
git://htj.dyndns.org/libata-tj new-init

-- 
tejun