[PATCHSET 02/03] prep LLDDs for hotplug support, take 3

[PATCHSET 02/03] prep LLDDs for hotplug support, take 3

[Tejun Heo]

Hello,

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

This is the third take of prep-LLDDs-for-hotplug-support patchset.
Changes from the last take[L] are.

* TF access is mandatory, sata_sil24()-tf-optional patch has been
  dropped.  sil24_hardreset() update is separated into
  update-sil24_hardreset() patch.

* sata_sil now calls ata_bmdma_irq_clear() directly instead of calling
  ops->irq_clear()

* sil_interrupt() now uses spin_lock/unlock_irq()

This patchset is against

  upstream (ef2824073fba9def3cf122e89cc485f66dd71f70)
  + [1] set-PIO-0-after-successful-EH-reset
  + [2] prep-for-hotplug-support patchset

Thanks.

--
tejun

[T] http://article.gmane.org/gmane.linux.ide/10891
[L] http://article.gmane.org/gmane.linux.ide/10573
[1] http://article.gmane.org/gmane.linux.ide/10890
[2] http://article.gmane.org/gmane.linux.ide/10892

[PATCH 4/5] 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.

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

---

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

11848689bbd06985a0ae2417f659eb4ea0f37e47
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 26d7c54..a39e8d0 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.3.2

[PATCH 2/5] 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.

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

---

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

818c7f93a025c8f5766260212250c69b7b25c0ff
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 3bd8077..f926883 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
 	 */
@@ -226,6 +241,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 */
@@ -233,10 +249,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.3.2

[PATCH 5/5] sata_sil24: update sil24_hardreset()

[Tejun Heo]

Use phy debouncing instead of unconditional wait after DEV_RST and
make sil24_hardreset() to request followup SRST as that's the only way
to wait for !BSY.  Note that the original implementation never worked
- if the cached status was !BSY, ata_busy_sleep() finished
immediately; otherwise, it timed out regardless of the actual device
status.

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

---

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

caf7d6bb9000c574682983765d2c6c06c0273a5e
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index a39e8d0..4a83090 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_before_fsrst);
+	if (rc) {
+		reason = "PHY debouncing failed";
+		goto err;
+	}
 
 	if (tmp & PORT_CS_DEV_RST) {
 		if (ata_port_offline(ap))
@@ -617,15 +621,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);
-- 
1.3.2

[PATCH 1/5] 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.

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

---

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

082a1eadbc745b02f08aece3875bcab0b9a6c27c
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index bbdc1ed..261fcd4 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4135,9 +4135,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;
@@ -5879,6 +5878,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 8822340..a506b2f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -721,6 +721,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.3.2

[PATCH 3/5] 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.

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

---

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

bf8f8bae070d0b05bb8f0d577f1751afcc19a163
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index f926883..695c06c 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);
 
@@ -196,7 +198,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,
@@ -336,6 +338,95 @@ 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 */
+	ata_bmdma_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;
+	int i;
+
+	/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */
+	spin_lock_irq(&host_set->lock);
+
+	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_irq(&host_set->lock);
+
+	return IRQ_RETVAL(handled);
+}
+
 static void sil_freeze(struct ata_port *ap)
 {
 	void __iomem *mmio_base = ap->host_set->mmio_base;
-- 
1.3.2

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

[Jeff Garzik]

Tejun Heo wrote:
> sata_sil is about to get a brand new interrupt handler.  Add relevant
> constants.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK patches 1-2

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

[Jeff Garzik]

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.
> 
> 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.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/sata_sil.c |   93 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 92 insertions(+), 1 deletions(-)
> 
> bf8f8bae070d0b05bb8f0d577f1751afcc19a163
> diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
> index f926883..695c06c 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);
>  
> @@ -196,7 +198,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,
> @@ -336,6 +338,95 @@ 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 */
> +	ata_bmdma_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;
> +	int i;
> +
> +	/* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */
> +	spin_lock_irq(&host_set->lock);
> +
> +	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;

Is this bit asserted even on error?


> +		sil_host_intr(ap, bmdma2);
> +		handled = 1;
> +	}
> +
> +	spin_unlock_irq(&host_set->lock);

use spin_lock() and spin_unlock()

Re: [PATCH 5/5] sata_sil24: update sil24_hardreset()

[Jeff Garzik]

Tejun Heo wrote:
> Use phy debouncing instead of unconditional wait after DEV_RST and
> make sil24_hardreset() to request followup SRST as that's the only way
> to wait for !BSY.  Note that the original implementation never worked
> - if the cached status was !BSY, ata_busy_sleep() finished
> immediately; otherwise, it timed out regardless of the actual device
> status.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

ACK 4-5

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

[Tejun Heo]

>> +        if (!(bmdma2 & SIL_DMA_COMPLETE))
>> +            continue;
> 
> Is this bit asserted even on error?
> 

Yeap.

> 
>> +        sil_host_intr(ap, bmdma2);
>> +        handled = 1;
>> +    }
>> +
>> +    spin_unlock_irq(&host_set->lock);
> 
> use spin_lock() and spin_unlock()

Of course.  What was I on?  :-(

-- 
tejun

[PATCH 5/5] sata_sil24: update sil24_hardreset()

[Tejun Heo]

Use phy debouncing instead of unconditional wait after DEV_RST and
make sil24_hardreset() to request followup SRST as that's the only way
to wait for !BSY.  Note that the original implementation never worked
- if the cached status was !BSY, ata_busy_sleep() finished
immediately; otherwise, it timed out regardless of the actual device
status.

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

---

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

e8e008e7b5ed8c65675cc9b3e778b8bb909f65ab
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index a39e8d0..4a83090 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_before_fsrst);
+	if (rc) {
+		reason = "PHY debouncing failed";
+		goto err;
+	}
 
 	if (tmp & PORT_CS_DEV_RST) {
 		if (ata_port_offline(ap))
@@ -617,15 +621,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);
-- 
1.3.2