[PATCH 5/6] sata_sil24: use sata_phy_debounce() in sil24_hardreset()

[PATCH 5/6] sata_sil24: use sata_phy_debounce() in sil24_hardreset()

[Tejun Heo]

Do phy debouncing instead of unconditional wait after DEV_RST.

---

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

b8c4776dbd446dd2087689d23c1a6ca153abd293
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 629679e..72219fa 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -591,7 +591,7 @@ static int sil24_hardreset(struct ata_po
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
 	const char *reason;
-	int tout_msec;
+	int tout_msec, rc;
 	u32 tmp;
 
 	/* sil24 does the right thing(tm) without any protection */
@@ -605,10 +605,15 @@ static int sil24_hardreset(struct ata_po
 	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.
+	/* SStatus oscillates between zero and valid status after
+	 * DEV_RST, debounce it.
 	 */
-	msleep(100);
+	rc = sata_phy_debounce(ap, ATA_DEBOUNCE_INTERVAL,
+			       ATA_DEBOUNCE_DURATION, ATA_DEBOUNCE_TIMEOUT);
+	if (rc) {
+		reason = "PHY debouncing failed";
+		goto err;
+	}
 
 	if (tmp & PORT_CS_DEV_RST) {
 		if (ata_port_offline(ap))
-- 
1.2.4

[PATCH 3/6] sata_sil: new interrupt handler

[Tejun Heo]

The DMA complete bit of these controllers reflects ATA IRQ status
while no DMA command is in progress.  So, we can tell whether the
controller is raising an interrupt or not in deterministic manner.
This patch gives sata_sil its own interrupt handler which behaves much
better than the original one in terms of error detection and handling.
This change is also necessary for later hotplug support.

Further improvements are possible, in both 2 and 4 ports versions, we
can get all status with only one readl and using custom bmdma
operations can further cut down register accesses.

---

 drivers/scsi/sata_sil.c |   94 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 93 insertions(+), 1 deletions(-)

4cd6609bcdca30ea40436a695497ab35217ad4b5
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 1c0a751..94fdbf0 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -111,6 +111,8 @@ static void sil_dev_config(struct ata_po
 static u32 sil_scr_read (struct ata_port *ap, unsigned int sc_reg);
 static void sil_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void sil_post_set_mode (struct ata_port *ap);
+static irqreturn_t sil_interrupt(int irq, void *dev_instance,
+				 struct pt_regs *regs);
 static void sil_freeze(struct ata_port *ap);
 static void sil_thaw(struct ata_port *ap);
 
@@ -195,7 +197,7 @@ static const struct ata_port_operations 
 	.thaw			= sil_thaw,
 	.error_handler		= ata_bmdma_error_handler,
 	.post_internal_cmd	= ata_bmdma_post_internal_cmd,
-	.irq_handler		= ata_interrupt,
+	.irq_handler		= sil_interrupt,
 	.irq_clear		= ata_bmdma_irq_clear,
 	.scr_read		= sil_scr_read,
 	.scr_write		= sil_scr_write,
@@ -335,6 +337,96 @@ static void sil_scr_write (struct ata_po
 		writel(val, mmio);
 }
 
+static void sil_host_intr(struct ata_port *ap, u32 bmdma2)
+{
+	struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag);
+	u8 status;
+
+	if (unlikely(!qc || qc->tf.ctl & ATA_NIEN))
+		goto freeze;
+
+	/* Check whether we are expecting interrupt in this state */
+	switch (ap->hsm_task_state) {
+	case HSM_ST_FIRST:
+		/* Some pre-ATAPI-4 devices assert INTRQ
+		 * at this state when ready to receive CDB.
+		 */
+
+		/* Check the ATA_DFLAG_CDB_INTR flag is enough here.
+		 * The flag was turned on only for atapi devices.
+		 * No need to check is_atapi_taskfile(&qc->tf) again.
+		 */
+		if (!(qc->dev->flags & ATA_DFLAG_CDB_INTR))
+			goto err_hsm;
+		break;
+	case HSM_ST_LAST:
+		if (qc->tf.protocol == ATA_PROT_DMA ||
+		    qc->tf.protocol == ATA_PROT_ATAPI_DMA) {
+			/* clear DMA-Start bit */
+			ap->ops->bmdma_stop(qc);
+
+			if (bmdma2 & SIL_DMA_ERROR) {
+				qc->err_mask |= AC_ERR_HOST_BUS;
+				ap->hsm_task_state = HSM_ST_ERR;
+			}
+		}
+		break;
+	case HSM_ST:
+		break;
+	default:
+		goto err_hsm;
+	}
+
+	/* check main status, clearing INTRQ */
+	status = ata_chk_status(ap);
+	if (unlikely(status & ATA_BUSY))
+		goto err_hsm;
+
+	/* ack bmdma irq events */
+	ap->ops->irq_clear(ap);
+
+	/* kick HSM in the ass */
+	ata_hsm_move(ap, qc, status, 0);
+
+	return;
+
+ err_hsm:
+	qc->err_mask |= AC_ERR_HSM;
+ freeze:
+	ata_port_freeze(ap);
+}
+
+static irqreturn_t sil_interrupt(int irq, void *dev_instance,
+				 struct pt_regs *regs)
+{
+	struct ata_host_set *host_set = dev_instance;
+	void __iomem *mmio_base = host_set->mmio_base;
+	int handled = 0;
+	unsigned long flags;
+	int i;
+
+	/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */
+	spin_lock_irqsave(&host_set->lock, flags);
+
+	for (i = 0; i < host_set->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+		u32 bmdma2 = readl(mmio_base + sil_port[ap->port_no].bmdma2);
+
+		if (unlikely(!ap || ap->flags & ATA_FLAG_DISABLED))
+			continue;
+
+		if (!(bmdma2 & SIL_DMA_COMPLETE))
+			continue;
+
+		sil_host_intr(ap, bmdma2);
+		handled = 1;
+	}
+
+	spin_unlock_irqrestore(&host_set->lock, flags);
+
+	return IRQ_RETVAL(handled);
+}
+
 static void sil_freeze(struct ata_port *ap)
 {
 	void __iomem *mmio_base = ap->host_set->mmio_base;
-- 
1.2.4

[PATCH 2/6] sata_sil: add new constants in preparation for new interrupt handler

[Tejun Heo]

sata_sil is about to get a brand new interrupt handler.  Add relevant
constants.

---

 drivers/scsi/sata_sil.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

3d5ca9ae81dfd7cc0220feb7da6b148dad866c0e
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 3e998b7..1c0a751 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -54,6 +54,7 @@ enum {
 	 */
 	SIL_FLAG_RERR_ON_DMA_ACT = (1 << 29),
 	SIL_FLAG_MOD15WRITE	= (1 << 30),
+
 	SIL_DFL_HOST_FLAGS	= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO,
 
@@ -84,6 +85,20 @@ enum {
 	/* BMDMA/BMDMA2 */
 	SIL_INTR_STEERING	= (1 << 1),
 
+	SIL_DMA_ENABLE		= (1 << 0),  /* DMA run switch */
+	SIL_DMA_RDWR		= (1 << 3),  /* DMA Rd-Wr */
+	SIL_DMA_SATA_IRQ	= (1 << 4),  /* OR of all SATA IRQs */
+	SIL_DMA_ACTIVE		= (1 << 16), /* DMA running */
+	SIL_DMA_ERROR		= (1 << 17), /* PCI bus error */
+	SIL_DMA_COMPLETE	= (1 << 18), /* cmd complete / IRQ pending */
+	SIL_DMA_N_SATA_IRQ	= (1 << 6),  /* SATA_IRQ for the next channel */
+	SIL_DMA_N_ACTIVE	= (1 << 24), /* ACTIVE for the next channel */
+	SIL_DMA_N_ERROR		= (1 << 25), /* ERROR for the next channel */
+	SIL_DMA_N_COMPLETE	= (1 << 26), /* COMPLETE for the next channel */
+
+	/* SIEN */
+	SIL_SIEN_N		= (1 << 16), /* triggered by SError.N */
+
 	/*
 	 * Others
 	 */
@@ -225,6 +240,7 @@ static const struct {
 	unsigned long tf;	/* ATA taskfile register block */
 	unsigned long ctl;	/* ATA control/altstatus register block */
 	unsigned long bmdma;	/* DMA register block */
+	unsigned long bmdma2;	/* DMA register block #2 */
 	unsigned long fifo_cfg;	/* FIFO Valid Byte Count and Control */
 	unsigned long scr;	/* SATA control register block */
 	unsigned long sien;	/* SATA Interrupt Enable register */
@@ -232,10 +248,10 @@ static const struct {
 	unsigned long sfis_cfg;	/* SATA FIS reception config register */
 } sil_port[] = {
 	/* port 0 ... */
-	{ 0x80, 0x8A, 0x00, 0x40, 0x100, 0x148, 0xb4, 0x14c },
-	{ 0xC0, 0xCA, 0x08, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc },
-	{ 0x280, 0x28A, 0x200, 0x240, 0x300, 0x348, 0x2b4, 0x34c },
-	{ 0x2C0, 0x2CA, 0x208, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc },
+	{ 0x80, 0x8A, 0x00, 0x10, 0x40, 0x100, 0x148, 0xb4, 0x14c },
+	{ 0xC0, 0xCA, 0x08, 0x18, 0x44, 0x180, 0x1c8, 0xf4, 0x1cc },
+	{ 0x280, 0x28A, 0x200, 0x210, 0x240, 0x300, 0x348, 0x2b4, 0x34c },
+	{ 0x2C0, 0x2CA, 0x208, 0x218, 0x244, 0x380, 0x3c8, 0x2f4, 0x3cc },
 	/* ... port 3 */
 };
 
-- 
1.2.4

[PATCH 4/6] sata_sil24: rename PORT_PRB to PORT_LRAM and add PORT_LRAM_SLOT_SZ

[Tejun Heo]

PORT_PRB is a misnomer as the area also contains other stuff.  Rename
it to PORT_LRAM and add PORT_LRAM_SLOT_SZ.

---

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

7b6eafd6c25c22b66aa7dd610099569596606a1c
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 4747047..629679e 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -98,7 +98,9 @@ enum {
 	 * (8192 bytes @ +0x0000, +0x2000, +0x4000 and +0x6000 @ BAR2)
 	 */
 	PORT_REGS_SIZE		= 0x2000,
-	PORT_PRB		= 0x0000, /* (32 bytes PRB + 16 bytes SGEs * 6) * 31 (3968 bytes) */
+
+	PORT_LRAM		= 0x0000, /* 31 LRAM slots and PM regs */
+	PORT_LRAM_SLOT_SZ	= 0x0080, /* 32 bytes PRB + 2 SGE, ACT... */
 
 	PORT_PM			= 0x0f80, /* 8 bytes PM * 16 (128 bytes) */
 		/* 32 bit regs */
@@ -1103,7 +1105,7 @@ static int sil24_init_one(struct pci_dev
 		void __iomem *port = port_base + i * PORT_REGS_SIZE;
 		unsigned long portu = (unsigned long)port;
 
-		probe_ent->port[i].cmd_addr = portu + PORT_PRB;
+		probe_ent->port[i].cmd_addr = portu;
 		probe_ent->port[i].scr_addr = portu + PORT_SCONTROL;
 
 		ata_std_ports(&probe_ent->port[i]);
-- 
1.2.4

[PATCHSET 07/11] prep LLDDs for hotplug support, take 1 (REPOST)

[Tejun Heo]

Hairu.

This is part of patchset series described in [T].

This is the first take of prep-LLDDs-for-hotplug-support patchset.
Some of patches in this patchset are from the first take of
add-hotplug-support patchset.

Things worth noting are

* Simply exporting ata_hsm_move() and calling the function with
  appropriate status is good enough for driving HSM for LLDDs
  implementing their own irq handlers.  sata_sil's new irq handler
  now uses it.

* TF faking is removed from sata_sil24.

This patchset is against

  upstream (acc696d93dcf993dec123d69d599979e1456ffec)
  + [1] prep-for-new-EH patchset
  + [2] new-EH-framework patchset, take 3
  + [3] new-EH-implementation patchset, take 3
  + [4] merge-irq-pio patchset
  + [5] add-NCQ-support patchset, take 3
  + [6] prep for hotplug support, take 2

--
tejun

[T] http://article.gmane.org/gmane.linux.ide/9957
[1] http://article.gmane.org/gmane.linux.ide/9959
[2] http://article.gmane.org/gmane.linux.ide/9984
[3] http://article.gmane.org/gmane.linux.ide/9995
[4] http://article.gmane.org/gmane.linux.ide/10005
[5] http://article.gmane.org/gmane.linux.ide/10011
[6] http://article.gmane.org/gmane.linux.ide/10028

[PATCH 5/6] sata_sil24: use sata_phy_debounce() in sil24_hardreset()

[Tejun Heo]

Do phy debouncing instead of unconditional wait after DEV_RST.

---

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

b8c4776dbd446dd2087689d23c1a6ca153abd293
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 629679e..72219fa 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -591,7 +591,7 @@ static int sil24_hardreset(struct ata_po
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
 	const char *reason;
-	int tout_msec;
+	int tout_msec, rc;
 	u32 tmp;
 
 	/* sil24 does the right thing(tm) without any protection */
@@ -605,10 +605,15 @@ static int sil24_hardreset(struct ata_po
 	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.
+	/* SStatus oscillates between zero and valid status after
+	 * DEV_RST, debounce it.
 	 */
-	msleep(100);
+	rc = sata_phy_debounce(ap, ATA_DEBOUNCE_INTERVAL,
+			       ATA_DEBOUNCE_DURATION, ATA_DEBOUNCE_TIMEOUT);
+	if (rc) {
+		reason = "PHY debouncing failed";
+		goto err;
+	}
 
 	if (tmp & PORT_CS_DEV_RST) {
 		if (ata_port_offline(ap))
-- 
1.2.4

[PATCH 1/6] libata: export ata_hsm_move()

[Tejun Heo]

ata_hsm_move() will be used by LLDDs which depend on standard PIO HSM
but implement their own interrupt handlers.

---

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

80a810231596efc4c208307250cfea7c2b55cb15
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 7259646..9acfdc7 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4099,9 +4099,8 @@ static void ata_hsm_qc_complete(struct a
  *	RETURNS:
  *	1 when poll next status needed, 0 otherwise.
  */
-
-static int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
-			 u8 status, int in_wq)
+int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
+		 u8 status, int in_wq)
 {
 	unsigned long flags = 0;
 	int poll_next;
@@ -5821,6 +5820,7 @@ EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_set_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
+EXPORT_SYMBOL_GPL(ata_hsm_move);
 EXPORT_SYMBOL_GPL(ata_qc_complete);
 EXPORT_SYMBOL_GPL(ata_qc_complete_multiple);
 EXPORT_SYMBOL_GPL(ata_qc_issue_prot);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 9e06d82..815087f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -704,6 +704,8 @@ extern void ata_bmdma_drive_eh(struct at
 			       ata_postreset_fn_t postreset);
 extern void ata_bmdma_error_handler(struct ata_port *ap);
 extern void ata_bmdma_post_internal_cmd(struct ata_queued_cmd *qc);
+extern int ata_hsm_move(struct ata_port *ap, struct ata_queued_cmd *qc,
+			u8 status, int in_wq);
 extern void ata_qc_complete(struct ata_queued_cmd *qc);
 extern int ata_qc_complete_multiple(struct ata_port *ap, u32 qc_active,
 				    void (*finish_qc)(struct ata_queued_cmd *));
-- 
1.2.4

[PATCH 6/6] sata_sil24: kill ops->tf_read() and use ata_std_noop_check_status()

[Tejun Heo]

sil24 doesn't have single TF image.  Result TF's are bound to each
command.  As libata now allows TF-less implementation, kill
ops->tf_read and use ata_std_noop_check_status() for check_status
callbacks.  Result TF is loaded directly from LRAM into qc->result_tf
when needed.

---

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

10b600147b7dc8b2c086281a39d7f23be428e931
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 72219fa..ef41d5f 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -311,7 +311,6 @@ static struct sil24_cerr_info {
 struct sil24_port_priv {
 	union sil24_cmd_block *cmd_block;	/* 32 cmd blocks */
 	dma_addr_t cmd_block_dma;		/* DMA base addr for them */
-	struct ata_taskfile tf;			/* Cached taskfile registers */
 };
 
 /* ap->host_set->private_data */
@@ -321,10 +320,8 @@ struct sil24_host_priv {
 };
 
 static void sil24_dev_config(struct ata_port *ap, struct ata_device *dev);
-static u8 sil24_check_status(struct ata_port *ap);
 static u32 sil24_scr_read(struct ata_port *ap, unsigned sc_reg);
 static void sil24_scr_write(struct ata_port *ap, unsigned sc_reg, u32 val);
-static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
 static int sil24_probe_reset(struct ata_port *ap, unsigned int *classes);
 static void sil24_qc_prep(struct ata_queued_cmd *qc);
 static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc);
@@ -378,12 +375,10 @@ static const struct ata_port_operations 
 
 	.dev_config		= sil24_dev_config,
 
-	.check_status		= sil24_check_status,
-	.check_altstatus	= sil24_check_status,
+	.check_status		= ata_noop_check_status,
+	.check_altstatus	= ata_noop_check_status,
 	.dev_select		= ata_noop_dev_select,
 
-	.tf_read		= sil24_tf_read,
-
 	.probe_reset		= sil24_probe_reset,
 
 	.qc_prep		= sil24_qc_prep,
@@ -460,21 +455,15 @@ static void sil24_dev_config(struct ata_
 		writel(PORT_CS_CDB16, port + PORT_CTRL_CLR);
 }
 
-static inline void sil24_update_tf(struct ata_port *ap)
+static void sil24_read_tf(struct ata_port *ap, int tag, struct ata_taskfile *tf)
 {
-	struct sil24_port_priv *pp = ap->private_data;
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
-	struct sil24_prb __iomem *prb = port;
+	struct sil24_prb __iomem *prb;
 	u8 fis[6 * 4];
 
-	memcpy_fromio(fis, prb->fis, 6 * 4);
-	ata_tf_from_fis(fis, &pp->tf);
-}
-
-static u8 sil24_check_status(struct ata_port *ap)
-{
-	struct sil24_port_priv *pp = ap->private_data;
-	return pp->tf.command;
+	prb = port + PORT_LRAM + tag * PORT_LRAM_SLOT_SZ;
+	memcpy_fromio(fis, prb->fis, sizeof(fis));
+	ata_tf_from_fis(fis, tf);
 }
 
 static int sil24_scr_map[] = {
@@ -505,12 +494,6 @@ static void sil24_scr_write(struct ata_p
 	}
 }
 
-static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
-{
-	struct sil24_port_priv *pp = ap->private_data;
-	*tf = pp->tf;
-}
-
 static int sil24_init_port(struct ata_port *ap)
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
@@ -533,6 +516,7 @@ static int sil24_softreset(struct ata_po
 	struct sil24_port_priv *pp = ap->private_data;
 	struct sil24_prb *prb = &pp->cmd_block[0].ata.prb;
 	dma_addr_t paddr = pp->cmd_block_dma;
+	struct ata_taskfile tf;
 	u32 mask, irq_stat;
 	const char *reason;
 
@@ -572,8 +556,8 @@ static int sil24_softreset(struct ata_po
 		goto err;
 	}
 
-	sil24_update_tf(ap);
-	*class = ata_dev_classify(&pp->tf);
+	sil24_read_tf(ap, 0, &tf);
+	*class = ata_dev_classify(&tf);
 
 	if (*class == ATA_DEV_UNKNOWN)
 		*class = ATA_DEV_NONE;
@@ -622,15 +606,13 @@ static int sil24_hardreset(struct ata_po
 		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.
+	/* Sil24 doesn't store signature FIS after hardreset, so we
+	 * can't wait for BSY to clear.  Some devices take a long time
+	 * to get ready and those devices will choke if we don't wait
+	 * for BSY clearance here.  Tell libata to perform follow-up
+	 * softreset.
 	 */
-	return 0;
+	return -EAGAIN;
 
  err:
 	ata_port_printk(ap, KERN_ERR, "hardreset failed (%s)\n", reason);
@@ -817,7 +799,7 @@ static void sil24_error_intr(struct ata_
 		/* record error info */
 		qc = ata_qc_from_tag(ap, ap->active_tag);
 		if (qc) {
-			sil24_update_tf(ap);
+			sil24_read_tf(ap, sil24_tag(qc->tag), &qc->result_tf);
 			qc->err_mask |= err_mask;
 		} else
 			ehi->err_mask |= err_mask;
@@ -835,7 +817,7 @@ static void sil24_error_intr(struct ata_
 static void sil24_finish_qc(struct ata_queued_cmd *qc)
 {
 	if (qc->flags & ATA_QCFLAG_RESULT_TF)
-		sil24_update_tf(qc->ap);
+		sil24_read_tf(qc->ap, sil24_tag(qc->tag), &qc->result_tf);
 }
 
 static inline void sil24_host_intr(struct ata_port *ap)
@@ -955,8 +937,6 @@ static int sil24_port_start(struct ata_p
 	if (!pp)
 		goto err_out;
 
-	pp->tf.command = ATA_DRDY;
-
 	cb = dma_alloc_coherent(dev, cb_size, &cb_dma, GFP_KERNEL);
 	if (!cb)
 		goto err_out_pp;
-- 
1.2.4

Re: [PATCH 3/6] sata_sil: new interrupt handler

[Alan Cox]

On Gwe, 2006-05-12 at 00:14 +0900, Tejun Heo wrote:
> The DMA complete bit of these controllers reflects ATA IRQ status
> while no DMA command is in progress.  So, we can tell whether the
> controller is raising an interrupt or not in deterministic manner.
> This patch gives sata_sil its own interrupt handler which behaves much
> better than the original one in terms of error detection and handling.
> This change is also necessary for later hotplug support.
> 

Is it not possible to avoid duplicating so much code. If the
sil_host_intr and a few other _intr routines end up with their own
host_intr functions closely based on the core one and intimate state
machine knowledge then fixes are going to get missed in one or the other
over time 

Re: [PATCH 3/6] sata_sil: new interrupt handler

[Tejun Heo]

Alan Cox wrote:
> On Gwe, 2006-05-12 at 00:14 +0900, Tejun Heo wrote:
>> The DMA complete bit of these controllers reflects ATA IRQ status
>> while no DMA command is in progress.  So, we can tell whether the
>> controller is raising an interrupt or not in deterministic manner.
>> This patch gives sata_sil its own interrupt handler which behaves much
>> better than the original one in terms of error detection and handling.
>> This change is also necessary for later hotplug support.
>>
> 
> Is it not possible to avoid duplicating so much code. If the
> sil_host_intr and a few other _intr routines end up with their own
> host_intr functions closely based on the core one and intimate state
> machine knowledge then fixes are going to get missed in one or the other
> over time 
> 

Yes, it's a bit worrying.  The current sata_sil irq handler is a little
bit too different to factor codes with the stock handler while a bit too
similar to justify the copying.  I think it's okay to leave it as it is
for the time being.  sata_sil will probably deviate more from the stock
handler as more BMDMA2 stuff gets implemented and controllers which
resemble the traditional BMDMA interface more than sata_sil should be
able to use ata_host_intr() with some wrapping.

-- 
tejun

[PATCH 5/6] sata_sil24: use sata_phy_debounce() in sil24_hardreset()

[Tejun Heo]

Do phy debouncing instead of unconditional wait after DEV_RST.

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

---

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

1f6218924d908e924488ea1ce855bc1ba62f6396
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index a39e8d0..89d5d70 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -591,7 +591,7 @@ static int sil24_hardreset(struct ata_po
 {
 	void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
 	const char *reason;
-	int tout_msec;
+	int tout_msec, rc;
 	u32 tmp;
 
 	/* sil24 does the right thing(tm) without any protection */
@@ -605,10 +605,14 @@ static int sil24_hardreset(struct ata_po
 	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.
+	/* SStatus oscillates between zero and valid status after
+	 * DEV_RST, debounce it.
 	 */
-	msleep(100);
+	rc = sata_phy_debounce(ap, sata_deb_timing_eh);
+	if (rc) {
+		reason = "PHY debouncing failed";
+		goto err;
+	}
 
 	if (tmp & PORT_CS_DEV_RST) {
 		if (ata_port_offline(ap))
-- 
1.3.2

Re: [PATCH 5/6] sata_sil24: use sata_phy_debounce() in sil24_hardreset()

[Jeff Garzik]

Tejun Heo wrote:
> Do phy debouncing instead of unconditional wait after DEV_RST.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK patches 4 - 5