[PATCHSET 4/9] sata_sil24 fixes, errata workaround and reset updates, take 3

[PATCHSET 4/9] sata_sil24 fixes, errata workaround and reset updates, take 3

[Tejun Heo]

Hello,

This is the third take of sata_sil24-fixes-errata-workaround-and-
reset-updates patchset.  Changes from the last take[L] are...

* In patch #6 (errata fix), WoC configuration is moved below clearing
  port reset as required by the spec.

* reset updates are adjusted to removal of @verbose

This patchset is against

  upstream (c2a6585296009379e0f4eff39cdcb108b457ebf2)
  + [1] misc-reset-updates patchset (repost)
  + [2] implement-and-use-ata_wait_register patchset (repost)
  + [3] misc-ata_bus_probe-updates patchset

Thanks.

--
tejun

[L] http://article.gmane.org/gmane.linux.ide/9457
[1] http://article.gmane.org/gmane.linux.ide/9495
[2] http://article.gmane.org/gmane.linux.ide/9499
[3] http://article.gmane.org/gmane.linux.ide/9506

[PATCH 04/13] sata_sil24: add more constants

[Tejun Heo]

Add HOST_CTRL_* and more PORT_IRQ_* bits.

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

---

 drivers/scsi/sata_sil24.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

894c055f230f71befc0d0552b67fad2e3954b85c
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index c581d9f..4434cf2 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -86,6 +86,13 @@ enum {
 	/* HOST_SLOT_STAT bits */
 	HOST_SSTAT_ATTN		= (1 << 31),
 
+	/* HOST_CTRL bits */
+	HOST_CTRL_M66EN		= (1 << 16), /* M66EN PCI bus signal */
+	HOST_CTRL_TRDY		= (1 << 17), /* latched PCI TRDY */
+	HOST_CTRL_STOP		= (1 << 18), /* latched PCI STOP */
+	HOST_CTRL_DEVSEL	= (1 << 19), /* latched PCI DEVSEL */
+	HOST_CTRL_REQ64		= (1 << 20), /* latched PCI REQ64 */
+
 	/*
 	 * Port registers
 	 * (8192 bytes @ +0x0000, +0x2000, +0x4000 and +0x6000 @ BAR2)
@@ -142,7 +149,11 @@ enum {
 	PORT_IRQ_PWR_CHG	= (1 << 3), /* power management change */
 	PORT_IRQ_PHYRDY_CHG	= (1 << 4), /* PHY ready change */
 	PORT_IRQ_COMWAKE	= (1 << 5), /* COMWAKE received */
-	PORT_IRQ_UNK_FIS	= (1 << 6), /* Unknown FIS received */
+	PORT_IRQ_UNK_FIS	= (1 << 6), /* unknown FIS received */
+	PORT_IRQ_DEV_XCHG	= (1 << 7), /* device exchanged */
+	PORT_IRQ_8B10B		= (1 << 8), /* 8b/10b decode error threshold */
+	PORT_IRQ_CRC		= (1 << 9), /* CRC error threshold */
+	PORT_IRQ_HANDSHAKE	= (1 << 10), /* handshake error threshold */
 	PORT_IRQ_SDB_NOTIFY	= (1 << 11), /* SDB notify received */
 
 	/* bits[27:16] are unmasked (raw) */
-- 
1.2.4

[PATCH 01/13] libata: export ata_set_sata_spd()

[Tejun Heo]

This will be used by LLDD hardreset implementation.

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

---

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

a2f14a65e87adab732d92e4396a779344db4cf1d
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 808a978..8bf8cc6 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1738,7 +1738,7 @@ int ata_set_sata_spd_needed(struct ata_p
  *	0 if spd doesn't need to be changed, 1 if spd has been
  *	changed.  -EOPNOTSUPP if SCR registers are inaccessible.
  */
-static int ata_set_sata_spd(struct ata_port *ap)
+int ata_set_sata_spd(struct ata_port *ap)
 {
 	u32 scontrol;
 
@@ -5113,6 +5113,7 @@ EXPORT_SYMBOL_GPL(ata_bmdma_irq_clear);
 EXPORT_SYMBOL_GPL(ata_bmdma_status);
 EXPORT_SYMBOL_GPL(ata_bmdma_stop);
 EXPORT_SYMBOL_GPL(ata_port_probe);
+EXPORT_SYMBOL_GPL(ata_set_sata_spd);
 EXPORT_SYMBOL_GPL(sata_phy_reset);
 EXPORT_SYMBOL_GPL(__sata_phy_reset);
 EXPORT_SYMBOL_GPL(ata_bus_reset);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index bf2f334..1c75024 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -503,6 +503,7 @@ extern void ata_port_probe(struct ata_po
 extern void __sata_phy_reset(struct ata_port *ap);
 extern void sata_phy_reset(struct ata_port *ap);
 extern void ata_bus_reset(struct ata_port *ap);
+extern int ata_set_sata_spd(struct ata_port *ap);
 extern int ata_drive_probe_reset(struct ata_port *ap,
 			ata_probeinit_fn_t probeinit,
 			ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
-- 
1.2.4

Re: [PATCH 01/13] libata: export ata_set_sata_spd()

[Jeff Garzik]

Tejun Heo wrote:
> This will be used by LLDD hardreset implementation.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

applied 1-13

[PATCH 05/13] sata_sil24: consolidate host flags into SIL24_COMMON_FLAGS

[Tejun Heo]

All sil24 controllers share the same host flags except for NPORTS.
Consolidate them into SIL24_COMMON_FLAGS.

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

---

 drivers/scsi/sata_sil24.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

7935dc73039e8bfba9683ba2fdc8456d5e23261c
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 4434cf2..dd4db40 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -218,6 +218,10 @@ enum {
 	BID_SIL3132		= 1,
 	BID_SIL3131		= 2,
 
+	/* host flags */
+	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
+				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
+
 	IRQ_STAT_4PORTS		= 0xf,
 };
 
@@ -345,9 +349,7 @@ static struct ata_port_info sil24_port_i
 	/* sil_3124 */
 	{
 		.sht		= &sil24_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  SIL24_NPORTS2FLAG(4),
+		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(4),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -356,9 +358,7 @@ static struct ata_port_info sil24_port_i
 	/* sil_3132 */
 	{
 		.sht		= &sil24_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  SIL24_NPORTS2FLAG(2),
+		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(2),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -367,9 +367,7 @@ static struct ata_port_info sil24_port_i
 	/* sil_3131/sil_3531 */
 	{
 		.sht		= &sil24_sht,
-		.host_flags	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
-				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
-				  SIL24_NPORTS2FLAG(1),
+		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(1),
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
-- 
1.2.4

[PATCH 02/13] sata_sil24: typo fix

[Tejun Heo]

Type fix.

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

---

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

806b9b1ee825c73a863bff9b0663346b790b1d89
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index edb62c3..76b6ffc 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -174,7 +174,7 @@ enum {
 	PORT_CERR_CMD_PCIPERR	= 27, /* ctrl[15:13] 110 - PCI parity err while fetching PRB */
 	PORT_CERR_XFR_UNDEF	= 32, /* PSD ecode 00 - undefined */
 	PORT_CERR_XFR_TGTABRT	= 33, /* PSD ecode 01 - target abort */
-	PORT_CERR_XFR_MSGABRT	= 34, /* PSD ecode 10 - master abort */
+	PORT_CERR_XFR_MSTABRT	= 34, /* PSD ecode 10 - master abort */
 	PORT_CERR_XFR_PCIPERR	= 35, /* PSD ecode 11 - PCI prity err during transfer */
 	PORT_CERR_SENDSERVICE	= 36, /* FIS received while sending service */
 
-- 
1.2.4

[PATCH 03/13] sata_sil24: rename PORT_IRQ_SDB_FIS to PORT_IRQ_SDB_NOTIFY

[Tejun Heo]

Rename PORT_IRQ_SDB_FIS to more proper PORT_IRQ_SDB_NOTIFY.

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

---

 drivers/scsi/sata_sil24.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

3c3bdd9224c176613056d21a21ff5b01300eb386
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 76b6ffc..c581d9f 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -143,7 +143,7 @@ enum {
 	PORT_IRQ_PHYRDY_CHG	= (1 << 4), /* PHY ready change */
 	PORT_IRQ_COMWAKE	= (1 << 5), /* COMWAKE received */
 	PORT_IRQ_UNK_FIS	= (1 << 6), /* Unknown FIS received */
-	PORT_IRQ_SDB_FIS	= (1 << 11), /* SDB FIS received */
+	PORT_IRQ_SDB_NOTIFY	= (1 << 11), /* SDB notify received */
 
 	/* bits[27:16] are unmasked (raw) */
 	PORT_IRQ_RAW_SHIFT	= 16,
@@ -975,8 +975,8 @@ static int sil24_init_one(struct pci_dev
 
 		/* Configure interrupts */
 		writel(0xffff, port + PORT_IRQ_ENABLE_CLR);
-		writel(PORT_IRQ_COMPLETE | PORT_IRQ_ERROR | PORT_IRQ_SDB_FIS,
-		       port + PORT_IRQ_ENABLE_SET);
+		writel(PORT_IRQ_COMPLETE | PORT_IRQ_ERROR |
+		       PORT_IRQ_SDB_NOTIFY, port + PORT_IRQ_ENABLE_SET);
 
 		/* Clear interrupts */
 		writel(0x0fff0fff, port + PORT_IRQ_STAT);
-- 
1.2.4

[PATCH 10/13] sata_sil24: reimplement hardreset

[Tejun Heo]

Reimplement hardreset according to the datasheet.  The old hardreset
didn't reset controller status and the controller might not be ready
after reset.  Also, as SStatus is a bit flakey after hardreset,
sata_std_hardrset() didn't use to wait long enough before proceeding.

Note that as we're not depending on SStatus, DET==1 condition cannot
be used to wait for link, so use shorter timeout for no device case.

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

---

 drivers/scsi/sata_sil24.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 40 insertions(+), 3 deletions(-)

59f7b2f31fb7c35c9d5f966c06f965b356fe5886
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 933bc44..8fc2d31 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -522,10 +522,47 @@ static int sil24_softreset(struct ata_po
 
 static int sil24_hardreset(struct ata_port *ap, unsigned int *class)
 {
-	unsigned int dummy_class;
+	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
+	const char *reason;
+	int tout_msec;
+	u32 tmp;
+
+	/* sil24 does the right thing(tm) without any protection */
+	ata_set_sata_spd(ap);
+
+	tout_msec = 100;
+	if (sata_dev_present(ap))
+		tout_msec = 5000;
+
+	writel(PORT_CS_DEV_RST, port + PORT_CTRL_STAT);
+	tmp = ata_wait_register(port + PORT_CTRL_STAT,
+				PORT_CS_DEV_RST, PORT_CS_DEV_RST, 10, tout_msec);
+
+	/* SStatus oscillates between zero and valid status for short
+	 * duration after DEV_RST, give it time to settle.
+	 */
+	msleep(100);
+
+	if (tmp & PORT_CS_DEV_RST) {
+		if (!sata_dev_present(ap))
+			return 0;
+		reason = "link not ready";
+		goto err;
+	}
+
+	if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+		reason = "device not ready";
+		goto err;
+	}
+
+	/* sil24 doesn't report device class code after hardreset,
+	 * leave *class alone.
+	 */
+	return 0;
 
-	/* sil24 doesn't report device signature after hard reset */
-	return sata_std_hardreset(ap, &dummy_class);
+ err:
+	printk(KERN_ERR "ata%u: hardreset failed (%s)\n", ap->id, reason);
+	return -EIO;
 }
 
 static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes)
-- 
1.2.4

[PATCH 12/13] sata_sil24: fix on-memory structure byteorder

[Tejun Heo]

Data structures residing on memory and fetched by the controller
should have LE ordering.  Fix it.

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

---

 drivers/scsi/sata_sil24.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

08b6ae9dfa18d352e4ddb47dfb688cee7139702c
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 9d57198..40795e6 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -482,7 +482,7 @@ static int sil24_softreset(struct ata_po
 	}
 
 	/* do SRST */
-	prb->ctrl = PRB_CTRL_SRST;
+	prb->ctrl = cpu_to_le16(PRB_CTRL_SRST);
 	prb->fis[1] = 0; /* no PM yet */
 
 	writel((u32)paddr, port + PORT_CMD_ACTIVATE);
@@ -598,6 +598,7 @@ static void sil24_qc_prep(struct ata_que
 	union sil24_cmd_block *cb = pp->cmd_block + qc->tag;
 	struct sil24_prb *prb;
 	struct sil24_sge *sge;
+	u16 ctrl = 0;
 
 	switch (qc->tf.protocol) {
 	case ATA_PROT_PIO:
@@ -605,7 +606,6 @@ static void sil24_qc_prep(struct ata_que
 	case ATA_PROT_NODATA:
 		prb = &cb->ata.prb;
 		sge = cb->ata.sge;
-		prb->ctrl = 0;
 		break;
 
 	case ATA_PROT_ATAPI:
@@ -618,12 +618,10 @@ static void sil24_qc_prep(struct ata_que
 
 		if (qc->tf.protocol != ATA_PROT_ATAPI_NODATA) {
 			if (qc->tf.flags & ATA_TFLAG_WRITE)
-				prb->ctrl = PRB_CTRL_PACKET_WRITE;
+				ctrl = PRB_CTRL_PACKET_WRITE;
 			else
-				prb->ctrl = PRB_CTRL_PACKET_READ;
-		} else
-			prb->ctrl = 0;
-
+				ctrl = PRB_CTRL_PACKET_READ;
+		}
 		break;
 
 	default:
@@ -632,6 +630,7 @@ static void sil24_qc_prep(struct ata_que
 		BUG();
 	}
 
+	prb->ctrl = cpu_to_le16(ctrl);
 	ata_tf_to_fis(&qc->tf, prb->fis, 0);
 
 	if (qc->flags & ATA_QCFLAG_DMAMAP)
-- 
1.2.4

[PATCH 08/13] sata_sil24: put port into known state before softresetting

[Tejun Heo]

Make sure the controller has no pending commands and ready for command
before issuing SRST.

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

---

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

538d286b508e9d611eeb1acdfeb0bfde395a0027
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 722b287..fed47cf 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -475,6 +475,12 @@ static int sil24_softreset(struct ata_po
 	irq_enable = readl(port + PORT_IRQ_ENABLE_SET);
 	writel(irq_enable, port + PORT_IRQ_ENABLE_CLR);
 
+	/* put the port into known state */
+	if (sil24_init_port(ap)) {
+		reason ="port not ready";
+		goto err;
+	}
+
 	/*
 	 * XXX: Not sure whether the following sleep is needed or not.
 	 * The original driver had it.  So....
-- 
1.2.4

Re: [PATCH 08/13] sata_sil24: put port into known state before softresetting

[Jeff Garzik]

Tejun Heo wrote:
> Make sure the controller has no pending commands and ready for command
> before issuing SRST.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/sata_sil24.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> 538d286b508e9d611eeb1acdfeb0bfde395a0027
> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
> index 722b287..fed47cf 100644
> --- a/drivers/scsi/sata_sil24.c
> +++ b/drivers/scsi/sata_sil24.c
> @@ -475,6 +475,12 @@ static int sil24_softreset(struct ata_po
>  	irq_enable = readl(port + PORT_IRQ_ENABLE_SET);
>  	writel(irq_enable, port + PORT_IRQ_ENABLE_CLR);
>  
> +	/* put the port into known state */
> +	if (sil24_init_port(ap)) {
> +		reason ="port not ready";
> +		goto err;

This was actually split up a little too much.  Unless the implementation 
of a new function is rather large, or using the new function requires 
updating multiple files, better to keep the function addition and usage 
in the same patch.

Minor complaint though, it turns out OK in the end.

	Jeff

[PATCH 07/13] sata_sil24: implement sil24_init_port()

[Tejun Heo]

Implement sil24_init_port which performs port initialization via
PORT_CS_INIT.  To be used later by softreset and EH.

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

---

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

bf888c1ecf6e901aad754a1ba2128047c4e0ea6c
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index d2bfcac..722b287 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -438,6 +438,22 @@ static void sil24_tf_read(struct ata_por
 	*tf = pp->tf;
 }
 
+static int sil24_init_port(struct ata_port *ap)
+{
+	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
+	u32 tmp;
+
+	writel(PORT_CS_INIT, port + PORT_CTRL_STAT);
+	ata_wait_register(port + PORT_CTRL_STAT,
+			  PORT_CS_INIT, PORT_CS_INIT, 10, 100);
+	tmp = ata_wait_register(port + PORT_CTRL_STAT,
+				PORT_CS_RDY, 0, 10, 100);
+
+	if ((tmp & (PORT_CS_INIT | PORT_CS_RDY)) != PORT_CS_RDY)
+		return -EIO;
+	return 0;
+}
+
 static int sil24_softreset(struct ata_port *ap, unsigned int *class)
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
-- 
1.2.4

[PATCH 13/13] sata_sil24: enable 64bit

[Tejun Heo]

Enable 64bit.

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

---

 drivers/scsi/sata_sil24.c |   47 +++++++++++++++++++++++++++------------------
 1 files changed, 28 insertions(+), 19 deletions(-)

709a645787936306deaf99ec7e9a4cdf3629493c
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 40795e6..9320368 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -486,6 +486,7 @@ static int sil24_softreset(struct ata_po
 	prb->fis[1] = 0; /* no PM yet */
 
 	writel((u32)paddr, port + PORT_CMD_ACTIVATE);
+	writel((u64)paddr >> 32, port + PORT_CMD_ACTIVATE + 4);
 
 	mask = (PORT_IRQ_COMPLETE | PORT_IRQ_ERROR) << PORT_IRQ_RAW_SHIFT;
 	irq_stat = ata_wait_register(port + PORT_IRQ_STAT, mask, 0x0,
@@ -645,6 +646,8 @@ static unsigned int sil24_qc_issue(struc
 	dma_addr_t paddr = pp->cmd_block_dma + qc->tag * sizeof(*pp->cmd_block);
 
 	writel((u32)paddr, port + PORT_CMD_ACTIVATE);
+	writel((u64)paddr >> 32, port + PORT_CMD_ACTIVATE + 4);
+
 	return 0;
 }
 
@@ -981,22 +984,29 @@ static int sil24_init_one(struct pci_dev
 	/*
 	 * Configure the device
 	 */
-	/*
-	 * FIXME: This device is certainly 64-bit capable.  We just
-	 * don't know how to use it.  After fixing 32bit activation in
-	 * this function, enable 64bit masks here.
-	 */
-	rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
-	if (rc) {
-		dev_printk(KERN_ERR, &pdev->dev,
-			   "32-bit DMA enable failed\n");
-		goto out_free;
-	}
-	rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
-	if (rc) {
-		dev_printk(KERN_ERR, &pdev->dev,
-			   "32-bit consistent DMA enable failed\n");
-		goto out_free;
+	if (!pci_set_dma_mask(pdev, DMA_64BIT_MASK)) {
+		rc = pci_set_consistent_dma_mask(pdev, DMA_64BIT_MASK);
+		if (rc) {
+			rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+			if (rc) {
+				dev_printk(KERN_ERR, &pdev->dev,
+					   "64-bit DMA enable failed\n");
+				goto out_free;
+			}
+		}
+	} else {
+		rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit DMA enable failed\n");
+			goto out_free;
+		}
+		rc = pci_set_consistent_dma_mask(pdev, DMA_32BIT_MASK);
+		if (rc) {
+			dev_printk(KERN_ERR, &pdev->dev,
+				   "32-bit consistent DMA enable failed\n");
+			goto out_free;
+		}
 	}
 
 	/* GPIO off */
@@ -1054,9 +1064,8 @@ static int sil24_init_one(struct pci_dev
 		writel(0x0000, port + PORT_CRC_ERR_CNT);
 		writel(0x0000, port + PORT_HSHK_ERR_CNT);
 
-		/* FIXME: 32bit activation? */
-		writel(0, port + PORT_ACTIVATE_UPPER_ADDR);
-		writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_STAT);
+		/* Always use 64bit activation */
+		writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_CLR);
 
 		/* Configure interrupts */
 		writel(0xffff, port + PORT_IRQ_ENABLE_CLR);
-- 
1.2.4

[PATCH 09/13] sata_sil24: kill 10ms sleep in softreset

[Tejun Heo]

Nothing, not the datasheet nor the errats, says this delay is
necessary and with the previous PORT_CS_INIT change, we know the
controller is in good state.  Kill 10ms sleep.

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

---

 drivers/scsi/sata_sil24.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

3521764734fe58136495bd063c28f08e096f3b02
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index fed47cf..933bc44 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -481,12 +481,7 @@ static int sil24_softreset(struct ata_po
 		goto err;
 	}
 
-	/*
-	 * XXX: Not sure whether the following sleep is needed or not.
-	 * The original driver had it.  So....
-	 */
-	msleep(10);
-
+	/* do SRST */
 	prb->ctrl = PRB_CTRL_SRST;
 	prb->fis[1] = 0; /* no PM yet */
 
-- 
1.2.4

[PATCH 06/13] sata_sil24: implement loss of completion interrupt on PCI-X errta fix

[Tejun Heo]

SiI3124 might lose completion interrupt if completion interrupt occurs
shortly after SLOT_STAT register is read for the previous completion
interrupt if it is operating in PCI-X mode.

This currently doesn't trigger as libata never queues more than one
command, but it will with NCQ changes.  This patch implements the
workaround - turning on WoC and explicitly clearing interrupt.

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

---

 drivers/scsi/sata_sil24.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

b459821511f12a010742c89cb6cf1c6f776cedc9
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index dd4db40..d2bfcac 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -221,6 +221,7 @@ enum {
 	/* host flags */
 	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
+	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 24), /* IRQ loss errata on PCI-X */
 
 	IRQ_STAT_4PORTS		= 0xf,
 };
@@ -349,7 +350,8 @@ static struct ata_port_info sil24_port_i
 	/* sil_3124 */
 	{
 		.sht		= &sil24_sht,
-		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(4),
+		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(4) |
+				  SIL24_FLAG_PCIX_IRQ_WOC,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -738,6 +740,10 @@ static inline void sil24_host_intr(struc
 	slot_stat = readl(port + PORT_SLOT_STAT);
 	if (!(slot_stat & HOST_SSTAT_ATTN)) {
 		struct sil24_port_priv *pp = ap->private_data;
+
+		if (ap->flags & SIL24_FLAG_PCIX_IRQ_WOC)
+			writel(PORT_IRQ_COMPLETE, port + PORT_IRQ_STAT);
+
 		/*
 		 * !HOST_SSAT_ATTN guarantees successful completion,
 		 * so reading back tf registers is unnecessary for
@@ -869,6 +875,7 @@ static int sil24_init_one(struct pci_dev
 	void __iomem *host_base = NULL;
 	void __iomem *port_base = NULL;
 	int i, rc;
+	u32 tmp;
 
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
@@ -942,13 +949,23 @@ static int sil24_init_one(struct pci_dev
 	/* GPIO off */
 	writel(0, host_base + HOST_FLASH_CMD);
 
+	/* Apply workaround for completion IRQ loss on PCI-X errata */
+	if (probe_ent->host_flags & SIL24_FLAG_PCIX_IRQ_WOC) {
+		tmp = readl(host_base + HOST_CTRL);
+		if (tmp & (HOST_CTRL_TRDY | HOST_CTRL_STOP | HOST_CTRL_DEVSEL))
+			dev_printk(KERN_INFO, &pdev->dev,
+				   "Applying completion IRQ loss on PCI-X "
+				   "errata fix\n");
+		else
+			probe_ent->host_flags &= ~SIL24_FLAG_PCIX_IRQ_WOC;
+	}
+
 	/* clear global reset & mask interrupts during initialization */
 	writel(0, host_base + HOST_CTRL);
 
 	for (i = 0; i < probe_ent->n_ports; i++) {
 		void __iomem *port = port_base + i * PORT_REGS_SIZE;
 		unsigned long portu = (unsigned long)port;
-		u32 tmp;
 
 		probe_ent->port[i].cmd_addr = portu + PORT_PRB;
 		probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
@@ -970,6 +987,12 @@ static int sil24_init_one(struct pci_dev
 				           "failed to clear port RST\n");
 		}
 
+		/* Configure IRQ WoC */
+		if (probe_ent->host_flags & SIL24_FLAG_PCIX_IRQ_WOC)
+			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_STAT);
+		else
+			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_CLR);
+
 		/* Zero error counters. */
 		writel(0x8000, port + PORT_DECODE_ERR_THRESH);
 		writel(0x8000, port + PORT_CRC_ERR_THRESH);
-- 
1.2.4

Re: [PATCH 06/13] sata_sil24: implement loss of completion interrupt on PCI-X errta fix

[Jeff Garzik]

Tejun Heo wrote:
> @@ -738,6 +740,10 @@ static inline void sil24_host_intr(struc
>  	slot_stat = readl(port + PORT_SLOT_STAT);
>  	if (!(slot_stat & HOST_SSTAT_ATTN)) {
>  		struct sil24_port_priv *pp = ap->private_data;
> +
> +		if (ap->flags & SIL24_FLAG_PCIX_IRQ_WOC)
> +			writel(PORT_IRQ_COMPLETE, port + PORT_IRQ_STAT);
> +


This looks racy.  Since the interrupt is usually level-triggered, 
doesn't this introduce the possibility of losing interrupt events?

	Jeff

Re: [PATCH 06/13] sata_sil24: implement loss of completion interrupt on PCI-X errta fix

[Tejun Heo]

Jeff Garzik wrote:
> Tejun Heo wrote:
>> @@ -738,6 +740,10 @@ static inline void sil24_host_intr(struc
>>      slot_stat = readl(port + PORT_SLOT_STAT);
>>      if (!(slot_stat & HOST_SSTAT_ATTN)) {
>>          struct sil24_port_priv *pp = ap->private_data;
>> +
>> +        if (ap->flags & SIL24_FLAG_PCIX_IRQ_WOC)
>> +            writel(PORT_IRQ_COMPLETE, port + PORT_IRQ_STAT);
>> +
> 
> 
> This looks racy.  Since the interrupt is usually level-triggered, 
> doesn't this introduce the possibility of losing interrupt events?
> 

Above code without the WOC flag test is actually equivalent to...

irq_stat = readl(port + PORT_IRQ_STAT);
if (irq_stat & 0xffff == PORT_IRQ_COMPLETE) {
	writel(PORT_IRQ_COMPLETE, port + PORT_IRQ_STAT);
	/* finish commands */
	return;
}
/* error processing */

So, no race condition that I can think of.  It's just like any other 
interrupt clearing.  Read it, clear it, handle it.  If anything happens 
before the clearing we catch it while handling (in this case, sil24 NCQ 
support will read outstanding command mask after clearing the 
interrupt).  if anything happens after the clearing, we'll be called again.

-- 
tejun

[PATCH 11/13] sata_sil24: don't do hardreset during driver initialization

[Tejun Heo]

There's no need to perform hardreset during driver initialization.
It's already done during host reset and even if the controller is in
some wacky state, we now have proper hardreset to back us up.

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

---

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

7ed6ed6896c4f03fd7253611aec7e32c36c3ad1e
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 8fc2d31..9d57198 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -1070,11 +1070,6 @@ static int sil24_init_one(struct pci_dev
 
 		/* Clear port multiplier enable and resume bits */
 		writel(PORT_CS_PM_EN | PORT_CS_RESUME, port + PORT_CTRL_CLR);
-
-		/* Reset itself */
-		if (__sil24_reset_controller(port))
-			dev_printk(KERN_ERR, &pdev->dev,
-			           "failed to reset controller\n");
 	}
 
 	/* Turn on interrupts */
-- 
1.2.4

[PATCHSET] sata_sil24: fixes, errata workaround and reset updates, take 2

[Tejun Heo]

Hello, Jeff and Carlos.

This is the second take of sata_sil24-fixes-errata-workaround-and-
reset-updates patchset.  Changes from the last take[1] are...

* sil24_poll_register() is replaced by ata_wait_register()
* comment update as suggested by Jeff
* better constants
* byteorder fix and 64bit patches are pushed backward, as they're
  untested.
* hardreset routine updated to unconditionally wait 100ms after reset
* new patch #11 removes hardresetting in sil24_init_one() as it's
  already done during host reset and even if the port is in a funky
  state, we have hardreset to back us up.

This patchset contains 13 patches.

#01	prep
#02-06	fixes, updates, errata workaround
#07-09	update softreset
#10-11	update hardreset
#12-13	untested fixes

This patchset is against

  upstream[2]
  + libata-reset-updates patchset (3 patches) [3]
  + implement-and-use-ata_wait_register patchset [4]

After these updates, sil24 is ready for new EN and NCQ.  In my repo,
sil24 is working fine with both new EH and NCQ.

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/9406
[2] c2a6585296009379e0f4eff39cdcb108b457ebf2
[3] http://article.gmane.org/gmane.linux.ide/9414
[4] http://article.gmane.org/gmane.linux.ide/9450

[PATCH 06/13] sata_sil24: implement loss of completion interrupt on PCI-X errta fix

[Tejun Heo]

SiI3124 might lose completion interrupt if completion interrupt occurs
shortly after SLOT_STAT register is read for the previous completion
interrupt if it is operating in PCI-X mode.

This currently doesn't trigger as libata never queues more than one
command, but it will with NCQ changes.  This patch implements the
workaround - turning on WoC and explicitly clearing interrupt.

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

---

 drivers/scsi/sata_sil24.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

bfc72a2448cce3be32f0aae2927fb8643152ebcd
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index dd4db40..5f3a17d 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -221,6 +221,7 @@ enum {
 	/* host flags */
 	SIL24_COMMON_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
+	SIL24_FLAG_PCIX_IRQ_WOC	= (1 << 24), /* IRQ loss errata on PCI-X */
 
 	IRQ_STAT_4PORTS		= 0xf,
 };
@@ -349,7 +350,8 @@ static struct ata_port_info sil24_port_i
 	/* sil_3124 */
 	{
 		.sht		= &sil24_sht,
-		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(4),
+		.host_flags	= SIL24_COMMON_FLAGS | SIL24_NPORTS2FLAG(4) |
+				  SIL24_FLAG_PCIX_IRQ_WOC,
 		.pio_mask	= 0x1f,			/* pio0-4 */
 		.mwdma_mask	= 0x07,			/* mwdma0-2 */
 		.udma_mask	= 0x3f,			/* udma0-5 */
@@ -738,6 +740,10 @@ static inline void sil24_host_intr(struc
 	slot_stat = readl(port + PORT_SLOT_STAT);
 	if (!(slot_stat & HOST_SSTAT_ATTN)) {
 		struct sil24_port_priv *pp = ap->private_data;
+
+		if (ap->flags & SIL24_FLAG_PCIX_IRQ_WOC)
+			writel(PORT_IRQ_COMPLETE, port + PORT_IRQ_STAT);
+
 		/*
 		 * !HOST_SSAT_ATTN guarantees successful completion,
 		 * so reading back tf registers is unnecessary for
@@ -869,6 +875,7 @@ static int sil24_init_one(struct pci_dev
 	void __iomem *host_base = NULL;
 	void __iomem *port_base = NULL;
 	int i, rc;
+	u32 tmp;
 
 	if (!printed_version++)
 		dev_printk(KERN_DEBUG, &pdev->dev, "version " DRV_VERSION "\n");
@@ -942,13 +949,23 @@ static int sil24_init_one(struct pci_dev
 	/* GPIO off */
 	writel(0, host_base + HOST_FLASH_CMD);
 
+	/* Apply workaround for completion IRQ loss on PCI-X errata */
+	if (probe_ent->host_flags & SIL24_FLAG_PCIX_IRQ_WOC) {
+		tmp = readl(host_base + HOST_CTRL);
+		if (tmp & (HOST_CTRL_TRDY | HOST_CTRL_STOP | HOST_CTRL_DEVSEL))
+			dev_printk(KERN_INFO, &pdev->dev,
+				   "Applying completion IRQ loss on PCI-X "
+				   "errata fix\n");
+		else
+			probe_ent->host_flags &= ~SIL24_FLAG_PCIX_IRQ_WOC;
+	}
+
 	/* clear global reset & mask interrupts during initialization */
 	writel(0, host_base + HOST_CTRL);
 
 	for (i = 0; i < probe_ent->n_ports; i++) {
 		void __iomem *port = port_base + i * PORT_REGS_SIZE;
 		unsigned long portu = (unsigned long)port;
-		u32 tmp;
 
 		probe_ent->port[i].cmd_addr = portu + PORT_PRB;
 		probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
@@ -958,6 +975,12 @@ static int sil24_init_one(struct pci_dev
 		/* Initial PHY setting */
 		writel(0x20c, port + PORT_PHY_CFG);
 
+		/* Configure IRQ WoC */
+		if (probe_ent->host_flags & SIL24_FLAG_PCIX_IRQ_WOC)
+			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_STAT);
+		else
+			writel(PORT_CS_IRQ_WOC, port + PORT_CTRL_CLR);
+
 		/* Clear port RST */
 		tmp = readl(port + PORT_CTRL_STAT);
 		if (tmp & PORT_CS_PORT_RST) {
-- 
1.2.4