[PATCHSET libata-dev#upstream] ata_piix: implement SIDPR SCR access

[PATCHSET libata-dev#upstream] ata_piix: implement SIDPR SCR access

[Tejun Heo]

This patchset implements SIDPR SCR access which is available for ICH8
and above.  With SIDPR SCR access enabled, ata_piix ports can access
PHY events and errors and can also be hardreset which is important as
in rare cases hardreset is necessary to recover from error conditions.

This patchset contains five patches.  The first two are misc clean up
patches.  3rd and 4th reorganize ata_piix initialization routine.  5th
implements SIDPR SCR access.

This patchset is against the current libat-dev#upstream (32a03e13).

Thanks.

--
tejun

[PATCH 1/5] ata_piix: kill unused constants and flags

[Tejun Heo]

Kill PIIX_FLAG_SCR, PIIX_PORT_ENABLED and PIIX_PORT_PRESENT.  These
are unused.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ata_piix.c |   27 +++++++--------------------
 1 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 3ddbf8b..017422c 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -102,19 +102,12 @@ enum {
 	ICH5_PCS		= 0x92,	/* port control and status */
 	PIIX_SCC		= 0x0A, /* sub-class code register */
 
-	PIIX_FLAG_SCR		= (1 << 26), /* SCR available */
 	PIIX_FLAG_AHCI		= (1 << 27), /* AHCI possible */
 	PIIX_FLAG_CHECKINTR	= (1 << 28), /* make sure PCI INTx enabled */
 
 	PIIX_PATA_FLAGS		= ATA_FLAG_SLAVE_POSS,
 	PIIX_SATA_FLAGS		= ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
 
-	/* combined mode.  if set, PATA is channel 0.
-	 * if clear, PATA is channel 1.
-	 */
-	PIIX_PORT_ENABLED	= (1 << 0),
-	PIIX_PORT_PRESENT	= (1 << 4),
-
 	PIIX_80C_PRI		= (1 << 5) | (1 << 4),
 	PIIX_80C_SEC		= (1 << 7) | (1 << 6),
 
@@ -552,7 +545,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich6_sata] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR,
+		.flags		= PIIX_SATA_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -562,8 +555,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich6_sata_ahci] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR |
-				  PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -573,8 +565,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich6m_sata_ahci] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR |
-				  PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -584,8 +575,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8_sata_ahci] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR |
-				  PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -595,8 +585,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8_2port_sata] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR |
-				  PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -606,8 +595,7 @@ static struct ata_port_info piix_port_info[] = {
 	[tolapai_sata_ahci] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR |
-				  PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -617,8 +605,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8m_apple_sata_ahci] =
 	{
 		.sht		= &piix_sht,
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SCR |
-				  PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
-- 
1.5.2.4

[PATCH 2/5] libata: use dev_driver_string() instead of "libata" in libata-sff.c

[Tejun Heo]

libata-sff code used DRV_NAME which is hardcoded to "libata" when
requesting resources.  Use dev_driver_string() such that low level
driver names are used in resource listing.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-sff.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 3b62479..16eb9e9 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -551,7 +551,7 @@ int ata_pci_init_bmdma(struct ata_host *host)
 		return rc;
 
 	/* request and iomap DMA region */
-	rc = pcim_iomap_regions(pdev, 1 << 4, DRV_NAME);
+	rc = pcim_iomap_regions(pdev, 1 << 4, dev_driver_string(gdev));
 	if (rc) {
 		dev_printk(KERN_ERR, gdev, "failed to request/iomap BAR4\n");
 		return -ENOMEM;
@@ -621,7 +621,8 @@ int ata_pci_init_sff_host(struct ata_host *host)
 			continue;
 		}
 
-		rc = pcim_iomap_regions(pdev, 0x3 << base, DRV_NAME);
+		rc = pcim_iomap_regions(pdev, 0x3 << base,
+					dev_driver_string(gdev));
 		if (rc) {
 			dev_printk(KERN_WARNING, gdev,
 				   "failed to request/iomap BARs for port %d "
@@ -741,6 +742,7 @@ int ata_pci_init_one(struct pci_dev *pdev,
 	struct device *dev = &pdev->dev;
 	const struct ata_port_info *pi = NULL;
 	struct ata_host *host = NULL;
+	const char *drv_name = dev_driver_string(&pdev->dev);
 	u8 mask;
 	int legacy_mode = 0;
 	int i, rc;
@@ -810,7 +812,7 @@ int ata_pci_init_one(struct pci_dev *pdev,
 
 	if (!legacy_mode) {
 		rc = devm_request_irq(dev, pdev->irq, pi->port_ops->irq_handler,
-				      IRQF_SHARED, DRV_NAME, host);
+				      IRQF_SHARED, drv_name, host);
 		if (rc)
 			goto err_out;
 
@@ -820,7 +822,7 @@ int ata_pci_init_one(struct pci_dev *pdev,
 		if (!ata_port_is_dummy(host->ports[0])) {
 			rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev),
 					      pi->port_ops->irq_handler,
-					      IRQF_SHARED, DRV_NAME, host);
+					      IRQF_SHARED, drv_name, host);
 			if (rc)
 				goto err_out;
 
@@ -831,7 +833,7 @@ int ata_pci_init_one(struct pci_dev *pdev,
 		if (!ata_port_is_dummy(host->ports[1])) {
 			rc = devm_request_irq(dev, ATA_SECONDARY_IRQ(pdev),
 					      pi->port_ops->irq_handler,
-					      IRQF_SHARED, DRV_NAME, host);
+					      IRQF_SHARED, drv_name, host);
 			if (rc)
 				goto err_out;
 
-- 
1.5.2.4

[PATCH 3/5] libata: factor out ata_pci_activate_sff_host() from ata_pci_one()

[Tejun Heo]

Factor out ata_pci_activate_sff_host() from ata_pci_one().  This does
about the same thing as ata_host_activate() but needs to be separate
because SFF controllers use different and multiple IRQs in legacy
mode.

This will be used to make SFF LLD initialization more flexible.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |    1 +
 drivers/ata/libata-sff.c  |  184 +++++++++++++++++++++++++--------------------
 include/linux/libata.h    |    3 +
 3 files changed, 107 insertions(+), 81 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 9d040a3..c731e4f 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -7642,6 +7642,7 @@ EXPORT_SYMBOL_GPL(pci_test_config_bits);
 EXPORT_SYMBOL_GPL(ata_pci_init_sff_host);
 EXPORT_SYMBOL_GPL(ata_pci_init_bmdma);
 EXPORT_SYMBOL_GPL(ata_pci_prepare_sff_host);
+EXPORT_SYMBOL_GPL(ata_pci_activate_sff_host);
 EXPORT_SYMBOL_GPL(ata_pci_init_one);
 EXPORT_SYMBOL_GPL(ata_pci_remove_one);
 #ifdef CONFIG_PM
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 16eb9e9..1be9364 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -714,6 +714,99 @@ int ata_pci_prepare_sff_host(struct pci_dev *pdev,
 }
 
 /**
+ *	ata_pci_activate_sff_host - start SFF host, request IRQ and register it
+ *	@host: target SFF ATA host
+ *	@irq_handler: irq_handler used when requesting IRQ(s)
+ *	@sht: scsi_host_template to use when registering the host
+ *
+ *	This is the counterpart of ata_host_activate() for SFF ATA
+ *	hosts.  This separate helper is necessary because SFF hosts
+ *	use two separate interrupts in legacy mode.
+ *
+ *	LOCKING:
+ *	Inherited from calling layer (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
+ */
+int ata_pci_activate_sff_host(struct ata_host *host,
+			      irq_handler_t irq_handler,
+			      struct scsi_host_template *sht)
+{
+	struct device *dev = host->dev;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	const char *drv_name = dev_driver_string(host->dev);
+	int legacy_mode = 0, rc;
+
+	rc = ata_host_start(host);
+	if (rc)
+		return rc;
+
+	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
+		u8 tmp8, mask;
+
+		/* 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;
+#if defined(CONFIG_NO_ATA_LEGACY)
+		/* Some platforms with PCI limits cannot address compat
+		   port space. In that case we punt if their firmware has
+		   left a device in compatibility mode */
+		if (legacy_mode) {
+			printk(KERN_ERR "ata: Compatibility mode ATA is not supported on this platform, skipping.\n");
+			return -EOPNOTSUPP;
+		}
+#endif
+	}
+
+	if (!devres_open_group(dev, NULL, GFP_KERNEL))
+		return -ENOMEM;
+
+	if (!legacy_mode) {
+		rc = devm_request_irq(dev, pdev->irq, irq_handler,
+				      IRQF_SHARED, drv_name, host);
+		if (rc)
+			goto out;
+
+		ata_port_desc(host->ports[0], "irq %d", pdev->irq);
+		ata_port_desc(host->ports[1], "irq %d", pdev->irq);
+	} else {
+		if (!ata_port_is_dummy(host->ports[0])) {
+			rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev),
+					      irq_handler, IRQF_SHARED,
+					      drv_name, host);
+			if (rc)
+				goto out;
+
+			ata_port_desc(host->ports[0], "irq %d",
+				      ATA_PRIMARY_IRQ(pdev));
+		}
+
+		if (!ata_port_is_dummy(host->ports[1])) {
+			rc = devm_request_irq(dev, ATA_SECONDARY_IRQ(pdev),
+					      irq_handler, IRQF_SHARED,
+					      drv_name, host);
+			if (rc)
+				goto out;
+
+			ata_port_desc(host->ports[1], "irq %d",
+				      ATA_SECONDARY_IRQ(pdev));
+		}
+	}
+
+	rc = ata_host_register(host, sht);
+ out:
+	if (rc == 0)
+		devres_remove_group(dev, NULL);
+	else
+		devres_release_group(dev, NULL);
+
+	return rc;
+}
+
+/**
  *	ata_pci_init_one - Initialize/register PCI IDE host controller
  *	@pdev: Controller to be initialized
  *	@ppi: array of port_info, must be enough for two ports
@@ -742,9 +835,6 @@ int ata_pci_init_one(struct pci_dev *pdev,
 	struct device *dev = &pdev->dev;
 	const struct ata_port_info *pi = NULL;
 	struct ata_host *host = NULL;
-	const char *drv_name = dev_driver_string(&pdev->dev);
-	u8 mask;
-	int legacy_mode = 0;
 	int i, rc;
 
 	DPRINTK("ENTER\n");
@@ -766,92 +856,24 @@ int ata_pci_init_one(struct pci_dev *pdev,
 	if (!devres_open_group(dev, NULL, GFP_KERNEL))
 		return -ENOMEM;
 
-	/* 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 = pcim_enable_device(pdev);
 	if (rc)
-		goto err_out;
+		goto out;
 
-	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
-		u8 tmp8;
-
-		/* 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;
-#if defined(CONFIG_NO_ATA_LEGACY)
-		/* Some platforms with PCI limits cannot address compat
-		   port space. In that case we punt if their firmware has
-		   left a device in compatibility mode */
-		if (legacy_mode) {
-			printk(KERN_ERR "ata: Compatibility mode ATA is not supported on this platform, skipping.\n");
-			rc = -EOPNOTSUPP;
-			goto err_out;
-		}
-#endif
-	}
-
-	/* prepare host */
+	/* prepare and activate SFF host */
 	rc = ata_pci_prepare_sff_host(pdev, ppi, &host);
 	if (rc)
-		goto err_out;
+		goto out;
 
 	pci_set_master(pdev);
+	rc = ata_pci_activate_sff_host(host, pi->port_ops->irq_handler,
+				       pi->sht);
+ out:
+	if (rc == 0)
+		devres_remove_group(&pdev->dev, NULL);
+	else
+		devres_release_group(&pdev->dev, NULL);
 
-	/* start host and request IRQ */
-	rc = ata_host_start(host);
-	if (rc)
-		goto err_out;
-
-	if (!legacy_mode) {
-		rc = devm_request_irq(dev, pdev->irq, pi->port_ops->irq_handler,
-				      IRQF_SHARED, drv_name, host);
-		if (rc)
-			goto err_out;
-
-		ata_port_desc(host->ports[0], "irq %d", pdev->irq);
-		ata_port_desc(host->ports[1], "irq %d", pdev->irq);
-	} else {
-		if (!ata_port_is_dummy(host->ports[0])) {
-			rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev),
-					      pi->port_ops->irq_handler,
-					      IRQF_SHARED, drv_name, host);
-			if (rc)
-				goto err_out;
-
-			ata_port_desc(host->ports[0], "irq %d",
-				      ATA_PRIMARY_IRQ(pdev));
-		}
-
-		if (!ata_port_is_dummy(host->ports[1])) {
-			rc = devm_request_irq(dev, ATA_SECONDARY_IRQ(pdev),
-					      pi->port_ops->irq_handler,
-					      IRQF_SHARED, drv_name, host);
-			if (rc)
-				goto err_out;
-
-			ata_port_desc(host->ports[1], "irq %d",
-				      ATA_SECONDARY_IRQ(pdev));
-		}
-	}
-
-	/* register */
-	rc = ata_host_register(host, pi->sht);
-	if (rc)
-		goto err_out;
-
-	devres_remove_group(dev, NULL);
-	return 0;
-
-err_out:
-	devres_release_group(dev, NULL);
 	return rc;
 }
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ccb0556..4374c42 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1033,6 +1033,9 @@ extern int ata_pci_init_bmdma(struct ata_host *host);
 extern int ata_pci_prepare_sff_host(struct pci_dev *pdev,
 				    const struct ata_port_info * const * ppi,
 				    struct ata_host **r_host);
+extern int ata_pci_activate_sff_host(struct ata_host *host,
+				     irq_handler_t irq_handler,
+				     struct scsi_host_template *sht);
 extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits);
 extern unsigned long ata_pci_default_filter(struct ata_device *dev,
 					    unsigned long xfer_mask);
-- 
1.5.2.4

[PATCH 4/5] ata_piix: convert to prepare - activate initialization

[Tejun Heo]

ata_piix requires more configuration during initialization than most
other SFF compliant controllers and one-go initialzation with
ata_pci_one() is too rigid.

This patch converts ata_piix to use two step prepare - activate
initialization used by other more advanced controllers.  This
conversion brings the happy side effect of enabling PCI device before
accessing PCI configuration registers.  Other than that, there
shouldn't be any behavior change.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ata_piix.c |   79 ++++++++++++++++++++++-------------------------
 1 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 017422c..9f512c9 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -312,7 +312,6 @@ static const struct ata_port_operations piix_pata_ops = {
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 	.cable_detect		= ata_cable_40wire,
 
-	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.irq_on			= ata_irq_on,
 
@@ -344,7 +343,6 @@ static const struct ata_port_operations ich_pata_ops = {
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 	.cable_detect		= ich_pata_cable_detect,
 
-	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.irq_on			= ata_irq_on,
 
@@ -371,7 +369,6 @@ static const struct ata_port_operations piix_sata_ops = {
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 
-	.irq_handler		= ata_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.irq_on			= ata_irq_on,
 
@@ -485,7 +482,6 @@ static const struct piix_map_db *piix_map_db_table[] = {
 static struct ata_port_info piix_port_info[] = {
 	[piix_pata_mwdma] = 	/* PIIX3 MWDMA only */
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_PATA_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x06, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
@@ -494,7 +490,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[piix_pata_33] =	/* PIIX4 at 33MHz */
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_PATA_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x06, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
@@ -504,7 +499,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich_pata_33] = 	/* ICH0 - ICH at 33Mhz*/
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_PATA_FLAGS,
 		.pio_mask 	= 0x1f,	/* pio 0-4 */
 		.mwdma_mask	= 0x06, /* Check: maybe 0x07  */
@@ -514,7 +508,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich_pata_66] = 	/* ICH controllers up to 66MHz */
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_PATA_FLAGS,
 		.pio_mask 	= 0x1f,	/* pio 0-4 */
 		.mwdma_mask	= 0x06, /* MWDMA0 is broken on chip */
@@ -524,7 +517,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich_pata_100] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x06, /* mwdma1-2 */
@@ -534,7 +526,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich5_sata] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -544,7 +535,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich6_sata] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -554,7 +544,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich6_sata_ahci] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -564,7 +553,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich6m_sata_ahci] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -574,7 +562,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich8_sata_ahci] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -584,7 +571,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich8_2port_sata] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -594,7 +580,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[tolapai_sata_ahci] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -604,7 +589,6 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich8m_apple_sata_ahci] =
 	{
-		.sht		= &piix_sht,
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
@@ -1191,10 +1175,10 @@ static int __devinit piix_check_450nx_errata(struct pci_dev *ata_dev)
 	return no_piix_dma;
 }
 
-static void __devinit piix_init_pcs(struct pci_dev *pdev,
-				    struct ata_port_info *pinfo,
+static void __devinit piix_init_pcs(struct ata_host *host,
 				    const struct piix_map_db *map_db)
 {
+	struct pci_dev *pdev = to_pci_dev(host->dev);
 	u16 pcs, new_pcs;
 
 	pci_read_config_word(pdev, ICH5_PCS, &pcs);
@@ -1208,11 +1192,10 @@ static void __devinit piix_init_pcs(struct pci_dev *pdev,
 	}
 }
 
-static void __devinit piix_init_sata_map(struct pci_dev *pdev,
-					 struct ata_port_info *pinfo,
-					 const struct piix_map_db *map_db)
+static const int *__devinit piix_init_sata_map(struct pci_dev *pdev,
+					       struct ata_port_info *pinfo,
+					       const struct piix_map_db *map_db)
 {
-	struct piix_host_priv *hpriv = pinfo[0].private_data;
 	const int *map;
 	int i, invalid_map = 0;
 	u8 map_value;
@@ -1236,7 +1219,6 @@ static void __devinit piix_init_sata_map(struct pci_dev *pdev,
 		case IDE:
 			WARN_ON((i & 1) || map[i + 1] != IDE);
 			pinfo[i / 2] = piix_port_info[ich_pata_100];
-			pinfo[i / 2].private_data = hpriv;
 			i++;
 			printk(" IDE IDE");
 			break;
@@ -1254,7 +1236,7 @@ static void __devinit piix_init_sata_map(struct pci_dev *pdev,
 		dev_printk(KERN_ERR, &pdev->dev,
 			   "invalid MAP value %u\n", map_value);
 
-	hpriv->map = map;
+	return map;
 }
 
 static void piix_iocfg_bit18_quirk(struct pci_dev *pdev)
@@ -1313,8 +1295,10 @@ static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct device *dev = &pdev->dev;
 	struct ata_port_info port_info[2];
 	const struct ata_port_info *ppi[] = { &port_info[0], &port_info[1] };
-	struct piix_host_priv *hpriv;
 	unsigned long port_flags;
+	struct ata_host *host;
+	struct piix_host_priv *hpriv;
+	int rc;
 
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev,
@@ -1324,17 +1308,31 @@ static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (!in_module_init)
 		return -ENODEV;
 
+	port_info[0] = piix_port_info[ent->driver_data];
+	port_info[1] = piix_port_info[ent->driver_data];
+
+	port_flags = port_info[0].flags;
+
+	/* enable device and prepare host */
+	rc = pcim_enable_device(pdev);
+	if (rc)
+		return rc;
+
+	/* SATA map init can change port_info, do it before prepping host */
 	hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
 	if (!hpriv)
 		return -ENOMEM;
 
-	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;
+	if (port_flags & ATA_FLAG_SATA)
+		hpriv->map = piix_init_sata_map(pdev, port_info,
+					piix_map_db_table[ent->driver_data]);
 
-	port_flags = port_info[0].flags;
+	rc = ata_pci_prepare_sff_host(pdev, ppi, &host);
+	if (rc)
+		return rc;
+	host->private_data = hpriv;
 
+	/* initialize controller */
 	if (port_flags & PIIX_FLAG_AHCI) {
 		u8 tmp;
 		pci_read_config_byte(pdev, PIIX_SCC, &tmp);
@@ -1345,13 +1343,8 @@ static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
-	/* Initialize SATA map */
-	if (port_flags & ATA_FLAG_SATA) {
-		piix_init_sata_map(pdev, port_info,
-				   piix_map_db_table[ent->driver_data]);
-		piix_init_pcs(pdev, port_info,
-			      piix_map_db_table[ent->driver_data]);
-	}
+	if (port_flags & ATA_FLAG_SATA)
+		piix_init_pcs(host, piix_map_db_table[ent->driver_data]);
 
 	/* apply IOCFG bit18 quirk */
 	piix_iocfg_bit18_quirk(pdev);
@@ -1369,12 +1362,14 @@ static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		/* This writes into the master table but it does not
 		   really matter for this errata as we will apply it to
 		   all the PIIX devices on the board */
-		port_info[0].mwdma_mask = 0;
-		port_info[0].udma_mask = 0;
-		port_info[1].mwdma_mask = 0;
-		port_info[1].udma_mask = 0;
+		host->ports[0]->mwdma_mask = 0;
+		host->ports[0]->udma_mask = 0;
+		host->ports[1]->mwdma_mask = 0;
+		host->ports[1]->udma_mask = 0;
 	}
-	return ata_pci_init_one(pdev, ppi);
+
+	pci_set_master(pdev);
+	return ata_pci_activate_sff_host(host, ata_interrupt, &piix_sht);
 }
 
 static int __init piix_init(void)
-- 
1.5.2.4

[PATCH 5/5] ata_piix: implement SIDPR SCR access

[Tejun Heo]

For ICH8, SCRs can be accessed using index and data register pair
located at BAR 5.  This patch implements support for it such that PHY
status, errors and hardreset are available for those controllers.

This is the only case where two devices on a PATA channel have access
to SCRs and creates a unique problem of mapping two SCRs to one link.
Note that this is different from PMP case in that they aren't quite
separate links - e.g. softreset resets both devices.

This problem is worked around by merging the SCR values.  To upper
layer, it looks like there is a single link with one set of SCRs but
with two devices.  This works well enough for PHY event, error
reporting and hardreset.

Supporting hardreset is important because in rare cases SATA devices
fail to recover without it after PHY errors.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ata_piix.c |  251 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 247 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index 9f512c9..612bee2 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -101,9 +101,14 @@ enum {
 	ICH5_PMR		= 0x90, /* port mapping register */
 	ICH5_PCS		= 0x92,	/* port control and status */
 	PIIX_SCC		= 0x0A, /* sub-class code register */
+	PIIX_SIDPR_BAR		= 5,
+	PIIX_SIDPR_LEN		= 16,
+	PIIX_SIDPR_IDX		= 0,
+	PIIX_SIDPR_DATA		= 4,
 
 	PIIX_FLAG_AHCI		= (1 << 27), /* AHCI possible */
 	PIIX_FLAG_CHECKINTR	= (1 << 28), /* make sure PCI INTx enabled */
+	PIIX_FLAG_SIDPR		= (1 << 29), /* SATA idx/data pair regs */
 
 	PIIX_PATA_FLAGS		= ATA_FLAG_SLAVE_POSS,
 	PIIX_SATA_FLAGS		= ATA_FLAG_SATA | PIIX_FLAG_CHECKINTR,
@@ -151,6 +156,7 @@ struct piix_map_db {
 
 struct piix_host_priv {
 	const int *map;
+	void __iomem *sidpr;
 };
 
 static int piix_init_one(struct pci_dev *pdev,
@@ -160,6 +166,9 @@ 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 ich_set_dmamode(struct ata_port *ap, struct ata_device *adev);
 static int ich_pata_cable_detect(struct ata_port *ap);
+static int piix_sidpr_scr_read(struct ata_port *ap, unsigned int reg, u32 *val);
+static int piix_sidpr_scr_write(struct ata_port *ap, unsigned int reg, u32 val);
+static void piix_sidpr_error_handler(struct ata_port *ap);
 #ifdef CONFIG_PM
 static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
 static int piix_pci_device_resume(struct pci_dev *pdev);
@@ -375,6 +384,35 @@ static const struct ata_port_operations piix_sata_ops = {
 	.port_start		= ata_port_start,
 };
 
+static const struct ata_port_operations piix_sidpr_sata_ops = {
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.check_status		= ata_check_status,
+	.exec_command		= ata_exec_command,
+	.dev_select		= ata_std_dev_select,
+
+	.bmdma_setup		= ata_bmdma_setup,
+	.bmdma_start		= ata_bmdma_start,
+	.bmdma_stop		= ata_bmdma_stop,
+	.bmdma_status		= ata_bmdma_status,
+	.qc_prep		= ata_qc_prep,
+	.qc_issue		= ata_qc_issue_prot,
+	.data_xfer		= ata_data_xfer,
+
+	.scr_read		= piix_sidpr_scr_read,
+	.scr_write		= piix_sidpr_scr_write,
+
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= piix_sidpr_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
+
+	.irq_clear		= ata_bmdma_irq_clear,
+	.irq_on			= ata_irq_on,
+
+	.port_start		= ata_port_start,
+};
+
 static const struct piix_map_db ich5_map_db = {
 	.mask = 0x7,
 	.port_enable = 0x3,
@@ -562,7 +600,8 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich8_sata_ahci] =
 	{
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI |
+				  PIIX_FLAG_SIDPR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -571,7 +610,8 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich8_2port_sata] =
 	{
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI |
+				  PIIX_FLAG_SIDPR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -589,7 +629,8 @@ static struct ata_port_info piix_port_info[] = {
 
 	[ich8m_apple_sata_ahci] =
 	{
-		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI,
+		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_AHCI |
+				  PIIX_FLAG_SIDPR,
 		.pio_mask	= 0x1f,	/* pio0-4 */
 		.mwdma_mask	= 0x07, /* mwdma0-2 */
 		.udma_mask	= ATA_UDMA6,
@@ -928,6 +969,180 @@ static void ich_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 	do_pata_set_dmamode(ap, adev, 1);
 }
 
+/*
+ * Serial ATA Index/Data Pair Superset Registers access
+ *
+ * Beginning from ICH8, there's a sane way to access SCRs using index
+ * and data register pair located at BAR5.  This creates an
+ * interesting problem of mapping two SCRs to one port.
+ *
+ * Although they have separate SCRs, the master and slave aren't
+ * independent enough to be treated as separate links - e.g. softreset
+ * resets both.  Also, there's no protocol defined for hard resetting
+ * singled device sharing the virtual port (no defined way to acquire
+ * device signature).  This is worked around by merging the SCR values
+ * into one sensible value and requesting follow-up SRST after
+ * hardreset.
+ *
+ * SCR merging is perfomed in nibbles which is the unit contents in
+ * SCRs are organized.  If two values are equal, the value is used.
+ * When they differ, merge table which lists precedence of possible
+ * values is consulted and the first match or the last entry when
+ * nothing matches is used.  When there's no merge table for the
+ * specific nibble, value from the first port is used.
+ */
+static const int piix_sidx_map[] = {
+	[SCR_STATUS]	= 0,
+	[SCR_ERROR]	= 2,
+	[SCR_CONTROL]	= 1,
+};
+
+static void piix_sidpr_sel(struct ata_device *dev, unsigned int reg)
+{
+	struct ata_port *ap = dev->link->ap;
+	struct piix_host_priv *hpriv = ap->host->private_data;
+
+	iowrite32(((ap->port_no * 2 + dev->devno) << 8) | piix_sidx_map[reg],
+		  hpriv->sidpr + PIIX_SIDPR_IDX);
+}
+
+static int piix_sidpr_read(struct ata_device *dev, unsigned int reg)
+{
+	struct piix_host_priv *hpriv = dev->link->ap->host->private_data;
+
+	piix_sidpr_sel(dev, reg);
+	return ioread32(hpriv->sidpr + PIIX_SIDPR_DATA);
+}
+
+static void piix_sidpr_write(struct ata_device *dev, unsigned int reg, u32 val)
+{
+	struct piix_host_priv *hpriv = dev->link->ap->host->private_data;
+
+	piix_sidpr_sel(dev, reg);
+	iowrite32(val, hpriv->sidpr + PIIX_SIDPR_DATA);
+}
+
+u32 piix_merge_scr(u32 val0, u32 val1, const int * const *merge_tbl)
+{
+	u32 val = 0;
+	int i, mi;
+
+	for (i = 0, mi = 0; i < 32 / 4; i++) {
+		u8 c0 = (val0 >> (i * 4)) & 0xf;
+		u8 c1 = (val1 >> (i * 4)) & 0xf;
+		u8 merged = c0;
+		const int *cur;
+
+		/* if no merge preference, assume the first value */
+		cur = merge_tbl[mi];
+		if (!cur)
+			goto done;
+		mi++;
+
+		/* if two values equal, use it */
+		if (c0 == c1)
+			goto done;
+
+		/* choose the first match or the last from the merge table */
+		while (*cur != -1) {
+			if (c0 == *cur || c1 == *cur)
+				break;
+			cur++;
+		}
+		if (*cur == -1)
+			cur--;
+		merged = *cur;
+	done:
+		val |= merged << (i * 4);
+	}
+
+	return val;
+}
+
+static int piix_sidpr_scr_read(struct ata_port *ap, unsigned int reg, u32 *val)
+{
+	const int * const sstatus_merge_tbl[] = {
+		/* DET */ (const int []){ 1, 3, 0, 4, 3, -1 },
+		/* SPD */ (const int []){ 2, 1, 0, -1 },
+		/* IPM */ (const int []){ 6, 2, 1, 0, -1 },
+		NULL,
+	};
+	const int * const scontrol_merge_tbl[] = {
+		/* DET */ (const int []){ 1, 0, 4, 0, -1 },
+		/* SPD */ (const int []){ 0, 2, 1, 0, -1 },
+		/* IPM */ (const int []){ 0, 1, 2, 3, 0, -1 },
+		NULL,
+	};
+	u32 v0, v1;
+
+	if (reg >= ARRAY_SIZE(piix_sidx_map))
+		return -EINVAL;
+
+	if (!(ap->flags & ATA_FLAG_SLAVE_POSS)) {
+		*val = piix_sidpr_read(&ap->link.device[0], reg);
+		return 0;
+	}
+
+	v0 = piix_sidpr_read(&ap->link.device[0], reg);
+	v1 = piix_sidpr_read(&ap->link.device[1], reg);
+
+	switch (reg) {
+	case SCR_STATUS:
+		*val = piix_merge_scr(v0, v1, sstatus_merge_tbl);
+		break;
+	case SCR_ERROR:
+		*val = v0 | v1;
+		break;
+	case SCR_CONTROL:
+		*val = piix_merge_scr(v0, v1, scontrol_merge_tbl);
+		break;
+	}
+
+	return 0;
+}
+
+static int piix_sidpr_scr_write(struct ata_port *ap, unsigned int reg, u32 val)
+{
+	if (reg >= ARRAY_SIZE(piix_sidx_map))
+		return -EINVAL;
+
+	piix_sidpr_write(&ap->link.device[0], reg, val);
+
+	if (ap->flags & ATA_FLAG_SLAVE_POSS)
+		piix_sidpr_write(&ap->link.device[1], reg, val);
+
+	return 0;
+}
+
+static int piix_sidpr_hardreset(struct ata_link *link, unsigned int *class,
+				unsigned long deadline)
+{
+	const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
+	int rc;
+
+	/* do hardreset */
+	rc = sata_link_hardreset(link, timing, deadline);
+	if (rc) {
+		ata_link_printk(link, KERN_ERR,
+				"COMRESET failed (errno=%d)\n", rc);
+		return rc;
+	}
+
+	/* TODO: phy layer with polling, timeouts, etc. */
+	if (ata_link_offline(link)) {
+		*class = ATA_DEV_NONE;
+		return 0;
+	}
+
+	return -EAGAIN;
+}
+
+static void piix_sidpr_error_handler(struct ata_port *ap)
+{
+	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
+			   piix_sidpr_hardreset, ata_std_postreset);
+}
+
 #ifdef CONFIG_PM
 static int piix_broken_suspend(void)
 {
@@ -1239,6 +1454,32 @@ static const int *__devinit piix_init_sata_map(struct pci_dev *pdev,
 	return map;
 }
 
+static void __devinit piix_init_sidpr(struct ata_host *host)
+{
+	struct pci_dev *pdev = to_pci_dev(host->dev);
+	struct piix_host_priv *hpriv = host->private_data;
+	int i;
+
+	/* check for availability */
+	for (i = 0; i < 4; i++)
+		if (hpriv->map[i] == IDE)
+			return;
+
+	if (!(host->ports[0]->flags & PIIX_FLAG_SIDPR))
+		return;
+
+	if (pci_resource_start(pdev, PIIX_SIDPR_BAR) == 0 ||
+	    pci_resource_len(pdev, PIIX_SIDPR_BAR) != PIIX_SIDPR_LEN)
+		return;
+
+	if (pcim_iomap_regions(pdev, 1 << PIIX_SIDPR_BAR, DRV_NAME))
+		return;
+
+	hpriv->sidpr = pcim_iomap_table(pdev)[PIIX_SIDPR_BAR];
+	host->ports[0]->ops = &piix_sidpr_sata_ops;
+	host->ports[1]->ops = &piix_sidpr_sata_ops;
+}
+
 static void piix_iocfg_bit18_quirk(struct pci_dev *pdev)
 {
 	static const struct dmi_system_id sysids[] = {
@@ -1343,8 +1584,10 @@ static int piix_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
-	if (port_flags & ATA_FLAG_SATA)
+	if (port_flags & ATA_FLAG_SATA) {
 		piix_init_pcs(host, piix_map_db_table[ent->driver_data]);
+		piix_init_sidpr(host);
+	}
 
 	/* apply IOCFG bit18 quirk */
 	piix_iocfg_bit18_quirk(pdev);
-- 
1.5.2.4

Re: [PATCH 1/5] ata_piix: kill unused constants and flags

[Jeff Garzik]

Tejun Heo wrote:
> Kill PIIX_FLAG_SCR, PIIX_PORT_ENABLED and PIIX_PORT_PRESENT.  These
> are unused.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/ata_piix.c |   27 +++++++--------------------
>  1 files changed, 7 insertions(+), 20 deletions(-)

applied 1-2

Re: [PATCH 3/5] libata: factor out ata_pci_activate_sff_host() from ata_pci_one()

[Jeff Garzik]

Tejun Heo wrote:
> Factor out ata_pci_activate_sff_host() from ata_pci_one().  This does
> about the same thing as ata_host_activate() but needs to be separate
> because SFF controllers use different and multiple IRQs in legacy
> mode.
> 
> This will be used to make SFF LLD initialization more flexible.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-core.c |    1 +
>  drivers/ata/libata-sff.c  |  184 +++++++++++++++++++++++++--------------------
>  include/linux/libata.h    |    3 +
>  3 files changed, 107 insertions(+), 81 deletions(-)

ACK patches 3-5, but they failed to apply to #upstream