[PATCHSET] sata_nv: convert to new EH and add hotplug support, take 2

[PATCHSET] sata_nv: convert to new EH and add hotplug support, take 2

[Tejun Heo]

Hello,

This is the second take of convert-sata_nv-to-new-EH patchset.
Changes from the last take[1] are...

* controllers are differentiated by standard port_info/poert_ops
  instead of nv_host/host_desc

* thaw() fixed such that only the affected port's irq status is
  cleared.

I've tested it on ASUS A8N-E (nForce4, CK804, 10de:0054), and
everything works fine.  However, the controller doesn't have
protection against data transfer hang (IORDY hang) and the machine
completely locks up if a device is removed during active data
transfer.  I don't think it can be worked around without using the
better interface (ADMA).

This patchset contains 6 patches.

#01-03: prep sata_nv
#04   : improve irq handler
#05   : implement new EH
#06   : add hotplug support

This patchset is against

     upstream (db9ca5803566078aafe63cf364ef98b5097e4194)

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/11381

[PATCH 1/6] sata_nv: kill not-working hotplug code

[Tejun Heo]

sata_nv contained hotplug code which is mainly for demonstrating how
hotplug event is handled.  This patch kills the demo code in
prepration for real hotplug implementation.

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

---

 drivers/scsi/sata_nv.c |  169 ------------------------------------------------
 1 files changed, 0 insertions(+), 169 deletions(-)

bfecdd1697c29c0ca85c76a25e4666ec8a79e353
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 9055124..70f8194 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -64,12 +64,6 @@ enum {
 	NV_INT_STATUS_SDEV_PM		= 0x20,
 	NV_INT_STATUS_SDEV_ADDED	= 0x40,
 	NV_INT_STATUS_SDEV_REMOVED	= 0x80,
-	NV_INT_STATUS_PDEV_HOTPLUG	= (NV_INT_STATUS_PDEV_ADDED |
-					   NV_INT_STATUS_PDEV_REMOVED),
-	NV_INT_STATUS_SDEV_HOTPLUG	= (NV_INT_STATUS_SDEV_ADDED |
-					   NV_INT_STATUS_SDEV_REMOVED),
-	NV_INT_STATUS_HOTPLUG		= (NV_INT_STATUS_PDEV_HOTPLUG |
-					   NV_INT_STATUS_SDEV_HOTPLUG),
 
 	NV_INT_ENABLE			= 0x11,
 	NV_INT_ENABLE_CK804		= 0x441,
@@ -81,12 +75,6 @@ enum {
 	NV_INT_ENABLE_SDEV_PM		= 0x20,
 	NV_INT_ENABLE_SDEV_ADDED	= 0x40,
 	NV_INT_ENABLE_SDEV_REMOVED	= 0x80,
-	NV_INT_ENABLE_PDEV_HOTPLUG	= (NV_INT_ENABLE_PDEV_ADDED |
-					   NV_INT_ENABLE_PDEV_REMOVED),
-	NV_INT_ENABLE_SDEV_HOTPLUG	= (NV_INT_ENABLE_SDEV_ADDED |
-					   NV_INT_ENABLE_SDEV_REMOVED),
-	NV_INT_ENABLE_HOTPLUG		= (NV_INT_ENABLE_PDEV_HOTPLUG |
-					   NV_INT_ENABLE_SDEV_HOTPLUG),
 
 	NV_INT_CONFIG			= 0x12,
 	NV_INT_CONFIG_METHD		= 0x01, // 0 = INT, 1 = SMI
@@ -102,12 +90,6 @@ static irqreturn_t nv_interrupt (int irq
 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);
 static void nv_host_stop (struct ata_host_set *host_set);
-static void nv_enable_hotplug(struct ata_probe_ent *probe_ent);
-static void nv_disable_hotplug(struct ata_host_set *host_set);
-static int nv_check_hotplug(struct ata_host_set *host_set);
-static void nv_enable_hotplug_ck804(struct ata_probe_ent *probe_ent);
-static void nv_disable_hotplug_ck804(struct ata_host_set *host_set);
-static int nv_check_hotplug_ck804(struct ata_host_set *host_set);
 
 enum nv_host_type
 {
@@ -158,34 +140,18 @@ static const struct pci_device_id nv_pci
 struct nv_host_desc
 {
 	enum nv_host_type	host_type;
-	void			(*enable_hotplug)(struct ata_probe_ent *probe_ent);
-	void			(*disable_hotplug)(struct ata_host_set *host_set);
-	int			(*check_hotplug)(struct ata_host_set *host_set);
-
 };
 static struct nv_host_desc nv_device_tbl[] = {
 	{
 		.host_type	= GENERIC,
-		.enable_hotplug	= NULL,
-		.disable_hotplug= NULL,
-		.check_hotplug	= NULL,
 	},
 	{
 		.host_type	= NFORCE2,
-		.enable_hotplug	= nv_enable_hotplug,
-		.disable_hotplug= nv_disable_hotplug,
-		.check_hotplug	= nv_check_hotplug,
 	},
 	{
 		.host_type	= NFORCE3,
-		.enable_hotplug	= nv_enable_hotplug,
-		.disable_hotplug= nv_disable_hotplug,
-		.check_hotplug	= nv_check_hotplug,
 	},
 	{	.host_type	= CK804,
-		.enable_hotplug	= nv_enable_hotplug_ck804,
-		.disable_hotplug= nv_disable_hotplug_ck804,
-		.check_hotplug	= nv_check_hotplug_ck804,
 	},
 };
 
@@ -275,7 +241,6 @@ static irqreturn_t nv_interrupt (int irq
 				 struct pt_regs *regs)
 {
 	struct ata_host_set *host_set = dev_instance;
-	struct nv_host *host = host_set->private_data;
 	unsigned int i;
 	unsigned int handled = 0;
 	unsigned long flags;
@@ -301,9 +266,6 @@ static irqreturn_t nv_interrupt (int irq
 
 	}
 
-	if (host->host_desc->check_hotplug)
-		handled += host->host_desc->check_hotplug(host_set);
-
 	spin_unlock_irqrestore(&host_set->lock, flags);
 
 	return IRQ_RETVAL(handled);
@@ -329,10 +291,6 @@ static void nv_host_stop (struct ata_hos
 {
 	struct nv_host *host = host_set->private_data;
 
-	// Disable hotplug event interrupts.
-	if (host->host_desc->disable_hotplug)
-		host->host_desc->disable_hotplug(host_set);
-
 	kfree(host);
 
 	ata_pci_host_stop(host_set);
@@ -409,10 +367,6 @@ static int nv_init_one (struct pci_dev *
 	if (rc != NV_PORTS)
 		goto err_out_iounmap;
 
-	// Enable hotplug event interrupts.
-	if (host->host_desc->enable_hotplug)
-		host->host_desc->enable_hotplug(probe_ent);
-
 	kfree(probe_ent);
 
 	return 0;
@@ -432,129 +386,6 @@ err_out:
 	return rc;
 }
 
-static void nv_enable_hotplug(struct ata_probe_ent *probe_ent)
-{
-	u8 intr_mask;
-
-	outb(NV_INT_STATUS_HOTPLUG,
-		probe_ent->port[0].scr_addr + NV_INT_STATUS);
-
-	intr_mask = inb(probe_ent->port[0].scr_addr + NV_INT_ENABLE);
-	intr_mask |= NV_INT_ENABLE_HOTPLUG;
-
-	outb(intr_mask, probe_ent->port[0].scr_addr + NV_INT_ENABLE);
-}
-
-static void nv_disable_hotplug(struct ata_host_set *host_set)
-{
-	u8 intr_mask;
-
-	intr_mask = inb(host_set->ports[0]->ioaddr.scr_addr + NV_INT_ENABLE);
-
-	intr_mask &= ~(NV_INT_ENABLE_HOTPLUG);
-
-	outb(intr_mask, host_set->ports[0]->ioaddr.scr_addr + NV_INT_ENABLE);
-}
-
-static int nv_check_hotplug(struct ata_host_set *host_set)
-{
-	u8 intr_status;
-
-	intr_status = inb(host_set->ports[0]->ioaddr.scr_addr + NV_INT_STATUS);
-
-	// Clear interrupt status.
-	outb(0xff, host_set->ports[0]->ioaddr.scr_addr + NV_INT_STATUS);
-
-	if (intr_status & NV_INT_STATUS_HOTPLUG) {
-		if (intr_status & NV_INT_STATUS_PDEV_ADDED)
-			printk(KERN_WARNING "nv_sata: "
-				"Primary device added\n");
-
-		if (intr_status & NV_INT_STATUS_PDEV_REMOVED)
-			printk(KERN_WARNING "nv_sata: "
-				"Primary device removed\n");
-
-		if (intr_status & NV_INT_STATUS_SDEV_ADDED)
-			printk(KERN_WARNING "nv_sata: "
-				"Secondary device added\n");
-
-		if (intr_status & NV_INT_STATUS_SDEV_REMOVED)
-			printk(KERN_WARNING "nv_sata: "
-				"Secondary device removed\n");
-
-		return 1;
-	}
-
-	return 0;
-}
-
-static void nv_enable_hotplug_ck804(struct ata_probe_ent *probe_ent)
-{
-	struct pci_dev *pdev = to_pci_dev(probe_ent->dev);
-	u8 intr_mask;
-	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);
-
-	writeb(NV_INT_STATUS_HOTPLUG, probe_ent->mmio_base + NV_INT_STATUS_CK804);
-
-	intr_mask = readb(probe_ent->mmio_base + NV_INT_ENABLE_CK804);
-	intr_mask |= NV_INT_ENABLE_HOTPLUG;
-
-	writeb(intr_mask, probe_ent->mmio_base + NV_INT_ENABLE_CK804);
-}
-
-static void nv_disable_hotplug_ck804(struct ata_host_set *host_set)
-{
-	struct pci_dev *pdev = to_pci_dev(host_set->dev);
-	u8 intr_mask;
-	u8 regval;
-
-	intr_mask = readb(host_set->mmio_base + NV_INT_ENABLE_CK804);
-
-	intr_mask &= ~(NV_INT_ENABLE_HOTPLUG);
-
-	writeb(intr_mask, host_set->mmio_base + NV_INT_ENABLE_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);
-}
-
-static int nv_check_hotplug_ck804(struct ata_host_set *host_set)
-{
-	u8 intr_status;
-
-	intr_status = readb(host_set->mmio_base + NV_INT_STATUS_CK804);
-
-	// Clear interrupt status.
-	writeb(0xff, host_set->mmio_base + NV_INT_STATUS_CK804);
-
-	if (intr_status & NV_INT_STATUS_HOTPLUG) {
-		if (intr_status & NV_INT_STATUS_PDEV_ADDED)
-			printk(KERN_WARNING "nv_sata: "
-				"Primary device added\n");
-
-		if (intr_status & NV_INT_STATUS_PDEV_REMOVED)
-			printk(KERN_WARNING "nv_sata: "
-				"Primary device removed\n");
-
-		if (intr_status & NV_INT_STATUS_SDEV_ADDED)
-			printk(KERN_WARNING "nv_sata: "
-				"Secondary device added\n");
-
-		if (intr_status & NV_INT_STATUS_SDEV_REMOVED)
-			printk(KERN_WARNING "nv_sata: "
-				"Secondary device removed\n");
-
-		return 1;
-	}
-
-	return 0;
-}
-
 static int __init nv_init(void)
 {
 	return pci_module_init(&nv_pci_driver);
-- 
1.3.2

Re: [PATCH 1/6] sata_nv: kill not-working hotplug code

[Jeff Garzik]

applied 1-6, then applied a cset to s/spin_lock_irqsave/spin_lock/

Re: [PATCH 1/6] sata_nv: kill not-working hotplug code

[Tejun Heo]

Jeff Garzik wrote:
> applied 1-6, then applied a cset to s/spin_lock_irqsave/spin_lock/

Thanks.  I'm too accustomed to irqsave/restore.  I'll try to remember 
next time.  :-p

-- 
tejun

[PATCH 2/6] sata_nv: kill struct nv_host_desc and nv_host

[Tejun Heo]

nv_host_desc and nv_host are used to discern different generations of
nv controllers.  Kill those.  New EH/hotplug implementation will use
standard port_info/ata_port_operations for that.

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

---

 drivers/scsi/sata_nv.c |   50 ++----------------------------------------------
 1 files changed, 2 insertions(+), 48 deletions(-)

17f86185988378c641cac6a70a2a3244b9f69909
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 70f8194..e9c4547 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -89,7 +89,6 @@ static irqreturn_t nv_interrupt (int irq
 				 struct pt_regs *regs);
 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);
-static void nv_host_stop (struct ata_host_set *host_set);
 
 enum nv_host_type
 {
@@ -137,30 +136,6 @@ static const struct pci_device_id nv_pci
 	{ 0, } /* terminate list */
 };
 
-struct nv_host_desc
-{
-	enum nv_host_type	host_type;
-};
-static struct nv_host_desc nv_device_tbl[] = {
-	{
-		.host_type	= GENERIC,
-	},
-	{
-		.host_type	= NFORCE2,
-	},
-	{
-		.host_type	= NFORCE3,
-	},
-	{	.host_type	= CK804,
-	},
-};
-
-struct nv_host
-{
-	struct nv_host_desc	*host_desc;
-	unsigned long		host_flags;
-};
-
 static struct pci_driver nv_pci_driver = {
 	.name			= DRV_NAME,
 	.id_table		= nv_pci_tbl,
@@ -208,7 +183,7 @@ static const struct ata_port_operations 
 	.scr_write		= nv_scr_write,
 	.port_start		= ata_port_start,
 	.port_stop		= ata_port_stop,
-	.host_stop		= nv_host_stop,
+	.host_stop		= ata_pci_host_stop,
 };
 
 /* FIXME: The hardware provides the necessary SATA PHY controls
@@ -287,19 +262,9 @@ static void nv_scr_write (struct ata_por
 	iowrite32(val, (void __iomem *)ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
-static void nv_host_stop (struct ata_host_set *host_set)
-{
-	struct nv_host *host = host_set->private_data;
-
-	kfree(host);
-
-	ata_pci_host_stop(host_set);
-}
-
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version = 0;
-	struct nv_host *host;
 	struct ata_port_info *ppi;
 	struct ata_probe_ent *probe_ent;
 	int pci_dev_busy = 0;
@@ -341,19 +306,10 @@ static int nv_init_one (struct pci_dev *
 	if (!probe_ent)
 		goto err_out_regions;
 
-	host = kmalloc(sizeof(struct nv_host), GFP_KERNEL);
-	if (!host)
-		goto err_out_free_ent;
-
-	memset(host, 0, sizeof(struct nv_host));
-	host->host_desc = &nv_device_tbl[ent->driver_data];
-
-	probe_ent->private_data = host;
-
 	probe_ent->mmio_base = pci_iomap(pdev, 5, 0);
 	if (!probe_ent->mmio_base) {
 		rc = -EIO;
-		goto err_out_free_host;
+		goto err_out_free_ent;
 	}
 
 	base = (unsigned long)probe_ent->mmio_base;
@@ -373,8 +329,6 @@ static int nv_init_one (struct pci_dev *
 
 err_out_iounmap:
 	pci_iounmap(pdev, probe_ent->mmio_base);
-err_out_free_host:
-	kfree(host);
 err_out_free_ent:
 	kfree(probe_ent);
 err_out_regions:
-- 
1.3.2

[PATCH 3/6] sata_nv: simplify constants

[Tejun Heo]

Simplify interrupt constants and make NFORCE3 equal to NFORCE2.

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

---

 drivers/scsi/sata_nv.c |   31 ++++++++++++-------------------
 1 files changed, 12 insertions(+), 19 deletions(-)

412ebe5f623d51ffe81190a49223ceaf2b28836d
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index e9c4547..1cb7b44 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -54,28 +54,21 @@ enum {
 	NV_PORT0_SCR_REG_OFFSET		= 0x00,
 	NV_PORT1_SCR_REG_OFFSET		= 0x40,
 
+	/* INT_STATUS/ENABLE */
 	NV_INT_STATUS			= 0x10,
-	NV_INT_STATUS_CK804		= 0x440,
-	NV_INT_STATUS_PDEV_INT		= 0x01,
-	NV_INT_STATUS_PDEV_PM		= 0x02,
-	NV_INT_STATUS_PDEV_ADDED	= 0x04,
-	NV_INT_STATUS_PDEV_REMOVED	= 0x08,
-	NV_INT_STATUS_SDEV_INT		= 0x10,
-	NV_INT_STATUS_SDEV_PM		= 0x20,
-	NV_INT_STATUS_SDEV_ADDED	= 0x40,
-	NV_INT_STATUS_SDEV_REMOVED	= 0x80,
-
 	NV_INT_ENABLE			= 0x11,
+	NV_INT_STATUS_CK804		= 0x440,
 	NV_INT_ENABLE_CK804		= 0x441,
-	NV_INT_ENABLE_PDEV_MASK		= 0x01,
-	NV_INT_ENABLE_PDEV_PM		= 0x02,
-	NV_INT_ENABLE_PDEV_ADDED	= 0x04,
-	NV_INT_ENABLE_PDEV_REMOVED	= 0x08,
-	NV_INT_ENABLE_SDEV_MASK		= 0x10,
-	NV_INT_ENABLE_SDEV_PM		= 0x20,
-	NV_INT_ENABLE_SDEV_ADDED	= 0x40,
-	NV_INT_ENABLE_SDEV_REMOVED	= 0x80,
 
+	/* INT_STATUS/ENABLE bits */
+	NV_INT_DEV			= 0x01,
+	NV_INT_PM			= 0x02,
+	NV_INT_ADDED			= 0x04,
+	NV_INT_REMOVED			= 0x08,
+
+	NV_INT_PORT_SHIFT		= 4,	/* each port occupies 4 bits */
+
+	/* INT_CONFIG */
 	NV_INT_CONFIG			= 0x12,
 	NV_INT_CONFIG_METHD		= 0x01, // 0 = INT, 1 = SMI
 
@@ -94,7 +87,7 @@ enum nv_host_type
 {
 	GENERIC,
 	NFORCE2,
-	NFORCE3,
+	NFORCE3 = NFORCE2,	/* NF2 == NF3 as far as sata_nv is concerned */
 	CK804
 };
 
-- 
1.3.2

[PATCH 6/6] sata_nv: add hotplug support

[Tejun Heo]

Add hotplug support.

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

---

 drivers/scsi/sata_nv.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

9e6fcaab7c4457835c1417c45b702945c754bd4f
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 2a1bd85..19deaa4 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -69,7 +69,8 @@ enum {
 	NV_INT_PORT_SHIFT		= 4,	/* each port occupies 4 bits */
 
 	NV_INT_ALL			= 0x0f,
-	NV_INT_MASK			= NV_INT_DEV,
+	NV_INT_MASK			= NV_INT_DEV |
+					  NV_INT_ADDED | NV_INT_REMOVED,
 
 	/* INT_CONFIG */
 	NV_INT_CONFIG			= 0x12,
@@ -324,6 +325,12 @@ static int nv_host_intr(struct ata_port 
 	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
 	int handled;
 
+	/* freeze if hotplugged */
+	if (unlikely(irq_stat & (NV_INT_ADDED | NV_INT_REMOVED))) {
+		ata_port_freeze(ap);
+		return 1;
+	}
+
 	/* bail out if not our interrupt */
 	if (!(irq_stat & NV_INT_DEV))
 		return 0;
-- 
1.3.2

[PATCH 5/6] sata_nv: convert to new EH

[Tejun Heo]

Convert to new EH.

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

---

 drivers/scsi/sata_nv.c |  115 +++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 89 insertions(+), 26 deletions(-)

2f2aa6fb450c5bbd2cbf3f1aea1f66e6cd6c5f3e
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 94cb179..2a1bd85 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -68,6 +68,9 @@ enum {
 
 	NV_INT_PORT_SHIFT		= 4,	/* each port occupies 4 bits */
 
+	NV_INT_ALL			= 0x0f,
+	NV_INT_MASK			= NV_INT_DEV,
+
 	/* INT_CONFIG */
 	NV_INT_CONFIG			= 0x12,
 	NV_INT_CONFIG_METHD		= 0x01, // 0 = INT, 1 = SMI
@@ -88,6 +91,12 @@ static irqreturn_t nv_ck804_interrupt(in
 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);
 
+static void nv_nf2_freeze(struct ata_port *ap);
+static void nv_nf2_thaw(struct ata_port *ap);
+static void nv_ck804_freeze(struct ata_port *ap);
+static void nv_ck804_thaw(struct ata_port *ap);
+static void nv_error_handler(struct ata_port *ap);
+
 enum nv_host_type
 {
 	GENERIC,
@@ -166,14 +175,16 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.check_status		= ata_check_status,
 	.dev_select		= ata_std_dev_select,
-	.phy_reset		= sata_phy_reset,
 	.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,
-	.eng_timeout		= ata_eng_timeout,
+	.freeze			= ata_bmdma_freeze,
+	.thaw			= ata_bmdma_thaw,
+	.error_handler		= nv_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 	.data_xfer		= ata_pio_data_xfer,
 	.irq_handler		= nv_generic_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -191,14 +202,16 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.check_status		= ata_check_status,
 	.dev_select		= ata_std_dev_select,
-	.phy_reset		= sata_phy_reset,
 	.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,
-	.eng_timeout		= ata_eng_timeout,
+	.freeze			= nv_nf2_freeze,
+	.thaw			= nv_nf2_thaw,
+	.error_handler		= nv_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 	.data_xfer		= ata_pio_data_xfer,
 	.irq_handler		= nv_nf2_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -216,14 +229,16 @@ static const struct ata_port_operations 
 	.exec_command		= ata_exec_command,
 	.check_status		= ata_check_status,
 	.dev_select		= ata_std_dev_select,
-	.phy_reset		= sata_phy_reset,
 	.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,
-	.eng_timeout		= ata_eng_timeout,
+	.freeze			= nv_ck804_freeze,
+	.thaw			= nv_ck804_thaw,
+	.error_handler		= nv_error_handler,
+	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
 	.data_xfer		= ata_pio_data_xfer,
 	.irq_handler		= nv_ck804_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
@@ -234,22 +249,11 @@ static const struct ata_port_operations 
 	.host_stop		= nv_ck804_host_stop,
 };
 
-/* FIXME: The hardware provides the necessary SATA PHY controls
- * to support ATA_FLAG_SATA_RESET.  However, it is currently
- * necessary to disable that flag, to solve misdetection problems.
- * See http://bugme.osdl.org/show_bug.cgi?id=3352 for more info.
- *
- * This problem really needs to be investigated further.  But in the
- * meantime, we avoid ATA_FLAG_SATA_RESET to get people working.
- */
 static struct ata_port_info nv_port_info[] = {
 	/* generic */
 	{
 		.sht		= &nv_sht,
-		.host_flags	= ATA_FLAG_SATA |
-				  /* ATA_FLAG_SATA_RESET | */
-				  ATA_FLAG_SRST |
-				  ATA_FLAG_NO_LEGACY,
+		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -258,10 +262,7 @@ static struct ata_port_info nv_port_info
 	/* nforce2/3 */
 	{
 		.sht		= &nv_sht,
-		.host_flags	= ATA_FLAG_SATA |
-				  /* ATA_FLAG_SATA_RESET | */
-				  ATA_FLAG_SRST |
-				  ATA_FLAG_NO_LEGACY,
+		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -270,10 +271,7 @@ static struct ata_port_info nv_port_info
 	/* ck804 */
 	{
 		.sht		= &nv_sht,
-		.host_flags	= ATA_FLAG_SATA |
-				  /* ATA_FLAG_SATA_RESET | */
-				  ATA_FLAG_SRST |
-				  ATA_FLAG_NO_LEGACY,
+		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY,
 		.pio_mask	= NV_PIO_MASK,
 		.mwdma_mask	= NV_MWDMA_MASK,
 		.udma_mask	= NV_UDMA_MASK,
@@ -410,6 +408,71 @@ static void nv_scr_write (struct ata_por
 	iowrite32(val, (void __iomem *)ap->ioaddr.scr_addr + (sc_reg * 4));
 }
 
+static void nv_nf2_freeze(struct ata_port *ap)
+{
+	unsigned long scr_addr = ap->host_set->ports[0]->ioaddr.scr_addr;
+	int shift = ap->port_no * NV_INT_PORT_SHIFT;
+	u8 mask;
+
+	mask = inb(scr_addr + NV_INT_ENABLE);
+	mask &= ~(NV_INT_ALL << shift);
+	outb(mask, scr_addr + NV_INT_ENABLE);
+}
+
+static void nv_nf2_thaw(struct ata_port *ap)
+{
+	unsigned long scr_addr = ap->host_set->ports[0]->ioaddr.scr_addr;
+	int shift = ap->port_no * NV_INT_PORT_SHIFT;
+	u8 mask;
+
+	outb(NV_INT_ALL << shift, scr_addr + NV_INT_STATUS);
+
+	mask = inb(scr_addr + NV_INT_ENABLE);
+	mask |= (NV_INT_MASK << shift);
+	outb(mask, scr_addr + NV_INT_ENABLE);
+}
+
+static void nv_ck804_freeze(struct ata_port *ap)
+{
+	void __iomem *mmio_base = ap->host_set->mmio_base;
+	int shift = ap->port_no * NV_INT_PORT_SHIFT;
+	u8 mask;
+
+	mask = readb(mmio_base + NV_INT_ENABLE_CK804);
+	mask &= ~(NV_INT_ALL << shift);
+	writeb(mask, mmio_base + NV_INT_ENABLE_CK804);
+}
+
+static void nv_ck804_thaw(struct ata_port *ap)
+{
+	void __iomem *mmio_base = ap->host_set->mmio_base;
+	int shift = ap->port_no * NV_INT_PORT_SHIFT;
+	u8 mask;
+
+	writeb(NV_INT_ALL << shift, mmio_base + NV_INT_STATUS_CK804);
+
+	mask = readb(mmio_base + NV_INT_ENABLE_CK804);
+	mask |= (NV_INT_MASK << shift);
+	writeb(mask, mmio_base + NV_INT_ENABLE_CK804);
+}
+
+static int nv_hardreset(struct ata_port *ap, unsigned int *class)
+{
+	unsigned int dummy;
+
+	/* SATA hardreset fails to retrieve proper device signature on
+	 * some controllers.  Don't classify on hardreset.  For more
+	 * info, see http://bugme.osdl.org/show_bug.cgi?id=3352
+	 */
+	return sata_std_hardreset(ap, &dummy);
+}
+
+static void nv_error_handler(struct ata_port *ap)
+{
+	ata_bmdma_drive_eh(ap, ata_std_prereset, ata_std_softreset,
+			   nv_hardreset, ata_std_postreset);
+}
+
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version = 0;
-- 
1.3.2

[PATCH 4/6] sata_nv: better irq handlers

[Tejun Heo]

nf2/3 and ck804 have irq status register.  Implement better irq
handler for those flavors of nv.  This patch makes different flavors
of nv controllers use different irq handlers by using separate
port_info for each flavor.

This change also makes following EH and hotplug updates easier to
integrate.

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

---

 drivers/scsi/sata_nv.c |  211 ++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 194 insertions(+), 17 deletions(-)

7472c9c3ac075355e09ec44776040d633d7338df
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index 1cb7b44..94cb179 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -78,8 +78,13 @@ enum {
 };
 
 static int nv_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
-static irqreturn_t nv_interrupt (int irq, void *dev_instance,
-				 struct pt_regs *regs);
+static void nv_ck804_host_stop(struct ata_host_set *host_set);
+static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
+					struct pt_regs *regs);
+static irqreturn_t nv_nf2_interrupt(int irq, void *dev_instance,
+				    struct pt_regs *regs);
+static irqreturn_t nv_ck804_interrupt(int irq, void *dev_instance,
+				      struct pt_regs *regs);
 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);
 
@@ -154,7 +159,7 @@ static struct scsi_host_template nv_sht 
 	.bios_param		= ata_std_bios_param,
 };
 
-static const struct ata_port_operations nv_ops = {
+static const struct ata_port_operations nv_generic_ops = {
 	.port_disable		= ata_port_disable,
 	.tf_load		= ata_tf_load,
 	.tf_read		= ata_tf_read,
@@ -170,7 +175,7 @@ static const struct ata_port_operations 
 	.qc_issue		= ata_qc_issue_prot,
 	.eng_timeout		= ata_eng_timeout,
 	.data_xfer		= ata_pio_data_xfer,
-	.irq_handler		= nv_interrupt,
+	.irq_handler		= nv_generic_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= nv_scr_read,
 	.scr_write		= nv_scr_write,
@@ -179,6 +184,56 @@ static const struct ata_port_operations 
 	.host_stop		= ata_pci_host_stop,
 };
 
+static const struct ata_port_operations nv_nf2_ops = {
+	.port_disable		= ata_port_disable,
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.exec_command		= ata_exec_command,
+	.check_status		= ata_check_status,
+	.dev_select		= ata_std_dev_select,
+	.phy_reset		= sata_phy_reset,
+	.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,
+	.eng_timeout		= ata_eng_timeout,
+	.data_xfer		= ata_pio_data_xfer,
+	.irq_handler		= nv_nf2_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
+	.scr_read		= nv_scr_read,
+	.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 = {
+	.port_disable		= ata_port_disable,
+	.tf_load		= ata_tf_load,
+	.tf_read		= ata_tf_read,
+	.exec_command		= ata_exec_command,
+	.check_status		= ata_check_status,
+	.dev_select		= ata_std_dev_select,
+	.phy_reset		= sata_phy_reset,
+	.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,
+	.eng_timeout		= ata_eng_timeout,
+	.data_xfer		= ata_pio_data_xfer,
+	.irq_handler		= nv_ck804_interrupt,
+	.irq_clear		= ata_bmdma_irq_clear,
+	.scr_read		= nv_scr_read,
+	.scr_write		= nv_scr_write,
+	.port_start		= ata_port_start,
+	.port_stop		= ata_port_stop,
+	.host_stop		= nv_ck804_host_stop,
+};
+
 /* FIXME: The hardware provides the necessary SATA PHY controls
  * to support ATA_FLAG_SATA_RESET.  However, it is currently
  * necessary to disable that flag, to solve misdetection problems.
@@ -187,16 +242,43 @@ static const struct ata_port_operations 
  * This problem really needs to be investigated further.  But in the
  * meantime, we avoid ATA_FLAG_SATA_RESET to get people working.
  */
-static struct ata_port_info nv_port_info = {
-	.sht		= &nv_sht,
-	.host_flags	= ATA_FLAG_SATA |
-			  /* ATA_FLAG_SATA_RESET | */
-			  ATA_FLAG_SRST |
-			  ATA_FLAG_NO_LEGACY,
-	.pio_mask	= NV_PIO_MASK,
-	.mwdma_mask	= NV_MWDMA_MASK,
-	.udma_mask	= NV_UDMA_MASK,
-	.port_ops	= &nv_ops,
+static struct ata_port_info nv_port_info[] = {
+	/* generic */
+	{
+		.sht		= &nv_sht,
+		.host_flags	= ATA_FLAG_SATA |
+				  /* ATA_FLAG_SATA_RESET | */
+				  ATA_FLAG_SRST |
+				  ATA_FLAG_NO_LEGACY,
+		.pio_mask	= NV_PIO_MASK,
+		.mwdma_mask	= NV_MWDMA_MASK,
+		.udma_mask	= NV_UDMA_MASK,
+		.port_ops	= &nv_generic_ops,
+	},
+	/* nforce2/3 */
+	{
+		.sht		= &nv_sht,
+		.host_flags	= ATA_FLAG_SATA |
+				  /* ATA_FLAG_SATA_RESET | */
+				  ATA_FLAG_SRST |
+				  ATA_FLAG_NO_LEGACY,
+		.pio_mask	= NV_PIO_MASK,
+		.mwdma_mask	= NV_MWDMA_MASK,
+		.udma_mask	= NV_UDMA_MASK,
+		.port_ops	= &nv_nf2_ops,
+	},
+	/* ck804 */
+	{
+		.sht		= &nv_sht,
+		.host_flags	= ATA_FLAG_SATA |
+				  /* ATA_FLAG_SATA_RESET | */
+				  ATA_FLAG_SRST |
+				  ATA_FLAG_NO_LEGACY,
+		.pio_mask	= NV_PIO_MASK,
+		.mwdma_mask	= NV_MWDMA_MASK,
+		.udma_mask	= NV_UDMA_MASK,
+		.port_ops	= &nv_ck804_ops,
+	},
 };
 
 MODULE_AUTHOR("NVIDIA");
@@ -205,8 +287,8 @@ MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, nv_pci_tbl);
 MODULE_VERSION(DRV_VERSION);
 
-static irqreturn_t nv_interrupt (int irq, void *dev_instance,
-				 struct pt_regs *regs)
+static irqreturn_t nv_generic_interrupt(int irq, void *dev_instance,
+					struct pt_regs *regs)
 {
 	struct ata_host_set *host_set = dev_instance;
 	unsigned int i;
@@ -239,6 +321,79 @@ static irqreturn_t nv_interrupt (int irq
 	return IRQ_RETVAL(handled);
 }
 
+static int nv_host_intr(struct ata_port *ap, u8 irq_stat)
+{
+	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
+	int handled;
+
+	/* bail out if not our interrupt */
+	if (!(irq_stat & NV_INT_DEV))
+		return 0;
+
+	/* DEV interrupt w/ no active qc? */
+	if (unlikely(!qc || (qc->tf.flags & ATA_TFLAG_POLLING))) {
+		ata_check_status(ap);
+		return 1;
+	}
+
+	/* handle interrupt */
+	handled = ata_host_intr(ap, qc);
+	if (unlikely(!handled)) {
+		/* spurious, clear it */
+		ata_check_status(ap);
+	}
+
+	return 1;
+}
+
+static irqreturn_t nv_do_interrupt(struct ata_host_set *host_set, u8 irq_stat)
+{
+	int i, handled = 0;
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		if (ap && !(ap->flags & ATA_FLAG_DISABLED))
+			handled += nv_host_intr(ap, irq_stat);
+
+		irq_stat >>= NV_INT_PORT_SHIFT;
+	}
+
+	return IRQ_RETVAL(handled);
+}
+
+static irqreturn_t nv_nf2_interrupt(int irq, void *dev_instance,
+				    struct pt_regs *regs)
+{
+	struct ata_host_set *host_set = dev_instance;
+	unsigned long flags;
+	u8 irq_stat;
+	irqreturn_t ret;
+
+	spin_lock_irqsave(&host_set->lock, flags);
+	irq_stat = inb(host_set->ports[0]->ioaddr.scr_addr + NV_INT_STATUS);
+	ret = nv_do_interrupt(host_set, irq_stat);
+	spin_unlock_irqrestore(&host_set->lock, flags);
+
+	return ret;
+}
+
+static irqreturn_t nv_ck804_interrupt(int irq, void *dev_instance,
+				      struct pt_regs *regs)
+{
+	struct ata_host_set *host_set = dev_instance;
+	unsigned long flags;
+	u8 irq_stat;
+	irqreturn_t ret;
+
+	spin_lock_irqsave(&host_set->lock, flags);
+	irq_stat = readb(host_set->mmio_base + NV_INT_STATUS_CK804);
+	ret = nv_do_interrupt(host_set, irq_stat);
+	spin_unlock_irqrestore(&host_set->lock, flags);
+
+	return ret;
+}
+
 static u32 nv_scr_read (struct ata_port *ap, unsigned int sc_reg)
 {
 	if (sc_reg > SCR_CONTROL)
@@ -294,7 +449,7 @@ static int nv_init_one (struct pci_dev *
 
 	rc = -ENOMEM;
 
-	ppi = &nv_port_info;
+	ppi = &nv_port_info[ent->driver_data];
 	probe_ent = ata_pci_init_native_mode(pdev, &ppi, ATA_PORT_PRIMARY | ATA_PORT_SECONDARY);
 	if (!probe_ent)
 		goto err_out_regions;
@@ -310,6 +465,15 @@ static int nv_init_one (struct pci_dev *
 	probe_ent->port[0].scr_addr = base + NV_PORT0_SCR_REG_OFFSET;
 	probe_ent->port[1].scr_addr = base + NV_PORT1_SCR_REG_OFFSET;
 
+	/* enable SATA space for CK804 */
+	if (ent->driver_data == CK804) {
+		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);
+	}
+
 	pci_set_master(pdev);
 
 	rc = ata_device_add(probe_ent);
@@ -333,6 +497,19 @@ err_out:
 	return rc;
 }
 
+static void nv_ck804_host_stop(struct ata_host_set *host_set)
+{
+	struct pci_dev *pdev = to_pci_dev(host_set->dev);
+	u8 regval;
+
+	/* 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);
+
+	ata_pci_host_stop(host_set);
+}
+
 static int __init nv_init(void)
 {
 	return pci_module_init(&nv_pci_driver);
-- 
1.3.2