* [PATCH 2/3] ahci: convert to new reset mechanism
2006-02-11 7:26 [PATCHSET] ahci: convert to new reset mechanism Tejun Heo
2006-02-11 7:26 ` [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap Tejun Heo
@ 2006-02-11 7:26 ` Tejun Heo
2006-02-11 7:26 ` [PATCH 3/3] ahci: add softreset Tejun Heo
2 siblings, 0 replies; 10+ messages in thread
From: Tejun Heo @ 2006-02-11 7:26 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
Convert ahci ->phy_reset to new reset mechanism.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/ahci.c | 46 +++++++++++++++++++++++++++++-----------------
1 files changed, 29 insertions(+), 17 deletions(-)
6f3211bfdb66aa3a0b1e703736dc445b10ac4a2d
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 72bdd43..86bccb7 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -190,7 +190,7 @@ static void ahci_scr_write (struct ata_p
static int ahci_init_one (struct pci_dev *pdev, const struct pci_device_id *ent);
static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc);
static irqreturn_t ahci_interrupt (int irq, void *dev_instance, struct pt_regs *regs);
-static void ahci_phy_reset(struct ata_port *ap);
+static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes);
static void ahci_irq_clear(struct ata_port *ap);
static void ahci_eng_timeout(struct ata_port *ap);
static int ahci_port_start(struct ata_port *ap);
@@ -230,7 +230,7 @@ static const struct ata_port_operations
.tf_read = ahci_tf_read,
- .phy_reset = ahci_phy_reset,
+ .probe_reset = ahci_probe_reset,
.qc_prep = ahci_qc_prep,
.qc_issue = ahci_qc_issue,
@@ -252,8 +252,7 @@ static const struct ata_port_info ahci_p
{
.sht = &ahci_sht,
.host_flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
- ATA_FLAG_SATA_RESET | ATA_FLAG_MMIO |
- ATA_FLAG_PIO_DMA,
+ ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = 0x7f, /* udma0-6 ; FIXME */
.port_ops = &ahci_ops,
@@ -515,28 +514,35 @@ static void ahci_fill_cmd_slot(struct ah
pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
}
-static void ahci_phy_reset(struct ata_port *ap)
+static int ahci_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
{
- void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
- struct ata_device *dev = &ap->device[0];
- u32 new_tmp, tmp;
+ int rc;
+
+ DPRINTK("ENTER\n");
ahci_stop_engine(ap);
- __sata_phy_reset(ap);
+ rc = sata_std_hardreset(ap, verbose, class);
ahci_start_engine(ap);
- if (ap->flags & ATA_FLAG_PORT_DISABLED)
- return;
+ if (rc == 0)
+ *class = ahci_dev_classify(ap);
+ if (*class == ATA_DEV_UNKNOWN)
+ *class = ATA_DEV_NONE;
- dev->class = ahci_dev_classify(ap);
- if (!ata_dev_present(dev)) {
- ata_port_disable(ap);
- return;
- }
+ DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
+ return rc;
+}
+
+static void ahci_postreset(struct ata_port *ap, unsigned int *class)
+{
+ void __iomem *port_mmio = (void __iomem *) ap->ioaddr.cmd_addr;
+ u32 new_tmp, tmp;
+
+ ata_std_postreset(ap, class);
/* Make sure port's ATAPI bit is set appropriately */
new_tmp = tmp = readl(port_mmio + PORT_CMD);
- if (dev->class == ATA_DEV_ATAPI)
+ if (*class == ATA_DEV_ATAPI)
new_tmp |= PORT_CMD_ATAPI;
else
new_tmp &= ~PORT_CMD_ATAPI;
@@ -546,6 +552,12 @@ static void ahci_phy_reset(struct ata_po
}
}
+static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
+{
+ return ata_drive_probe_reset(ap, NULL, NULL, ahci_hardreset,
+ ahci_postreset, classes);
+}
+
static u8 ahci_check_status(struct ata_port *ap)
{
void __iomem *mmio = (void __iomem *) ap->ioaddr.cmd_addr;
--
1.1.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap
2006-02-11 7:26 [PATCHSET] ahci: convert to new reset mechanism Tejun Heo
@ 2006-02-11 7:26 ` Tejun Heo
2006-02-11 22:53 ` Jeff Garzik
2006-02-11 7:26 ` [PATCH 2/3] ahci: convert to new reset mechanism Tejun Heo
2006-02-11 7:26 ` [PATCH 3/3] ahci: add softreset Tejun Heo
2 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2006-02-11 7:26 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
Make ahci_fill_cmd_slot() take struct ahci_port_priv *pp instead of
struct ata_port *ap as suggested by Jeff Garzik.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/ahci.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
47c10126810dfdd091c2ff3f6316cab4ff504e5e
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 98ce6bb..72bdd43 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -507,9 +507,8 @@ static unsigned int ahci_dev_classify(st
return ata_dev_classify(&tf);
}
-static void ahci_fill_cmd_slot(struct ata_port *ap, u32 opts)
+static void ahci_fill_cmd_slot(struct ahci_port_priv *pp, u32 opts)
{
- struct ahci_port_priv *pp = ap->private_data;
pp->cmd_slot[0].opts = cpu_to_le32(opts);
pp->cmd_slot[0].status = 0;
pp->cmd_slot[0].tbl_addr = cpu_to_le32(pp->cmd_tbl_dma & 0xffffffff);
@@ -622,7 +621,7 @@ static void ahci_qc_prep(struct ata_queu
if (is_atapi)
opts |= AHCI_CMD_ATAPI;
- ahci_fill_cmd_slot(ap, opts);
+ ahci_fill_cmd_slot(pp, opts);
}
static void ahci_restart_port(struct ata_port *ap, u32 irq_stat)
--
1.1.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCHSET] ahci: convert to new reset mechanism
@ 2006-02-11 7:26 Tejun Heo
2006-02-11 7:26 ` [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap Tejun Heo
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Tejun Heo @ 2006-02-11 7:26 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: htejun
Hello, Jeff.
This is respin of ahci part of new reset mechanism patchset take #4[1].
I've prepended a patch to convert ahci_fill_cmd_slot() to take *pp
instead of *ap. This should have been done when posting the reset
patchset. Sorry about the mess. Was way too tired, I guess.
This patchset is against the current upstream[2], but will apply fine
over the inline-ata_qc_complete patch[3] I've just posted.
BTW, something wrong about ata_piix patches in the reset patchset?
Thanks.
--
tejun
[1] http://article.gmane.org/gmane.linux.ide/7935
[2] 489ff4c7d167be954f715128790bd80d3c888322
[3] http://article.gmane.org/gmane.linux.ide/7948
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] ahci: add softreset
2006-02-11 7:26 [PATCHSET] ahci: convert to new reset mechanism Tejun Heo
2006-02-11 7:26 ` [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap Tejun Heo
2006-02-11 7:26 ` [PATCH 2/3] ahci: convert to new reset mechanism Tejun Heo
@ 2006-02-11 7:26 ` Tejun Heo
2006-02-11 23:27 ` Jeff Garzik
2 siblings, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2006-02-11 7:26 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
Now that libata is smart enought to handle both soft and hard resets,
add softreset method.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/ahci.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 134 insertions(+), 1 deletions(-)
1edad0c8fc8e36457ac1f7c68ba4d86ff63986e3
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 86bccb7..a65895f 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -514,6 +514,138 @@ static void ahci_fill_cmd_slot(struct ah
pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
}
+static int ahci_wait_for_bit(void __iomem *reg, u32 mask, u32 val,
+ unsigned long interval_msec,
+ unsigned long timeout_msec)
+{
+ unsigned long timeout;
+ u32 tmp;
+
+ timeout = jiffies + (timeout_msec * HZ) / 1000;
+ do {
+ tmp = readl(reg);
+ if ((tmp & mask) != val)
+ return 0;
+ msleep(interval_msec);
+ } while (time_before(jiffies, timeout));
+
+ return -1;
+}
+
+static int ahci_softreset(struct ata_port *ap, int verbose, unsigned int *class)
+{
+ struct ahci_host_priv *hpriv = ap->host_set->private_data;
+ struct ahci_port_priv *pp = ap->private_data;
+ void __iomem *mmio = ap->host_set->mmio_base;
+ void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+ const u32 cmd_fis_len = 5; /* five dwords */
+ const char *reason = NULL;
+ struct ata_taskfile tf;
+ u8 *fis;
+ int rc;
+
+ DPRINTK("ENTER\n");
+
+ /* prepare for SRST (AHCI-1.1 10.4.1) */
+ rc = ahci_stop_engine(ap);
+ if (rc) {
+ reason = "failed to stop engine";
+ goto fail_restart;
+ }
+
+ /* check BUSY/DRQ, perform Command List Override if necessary */
+ ahci_tf_read(ap, &tf);
+ if (tf.command & (ATA_BUSY | ATA_DRQ)) {
+ u32 tmp;
+
+ if (!(hpriv->cap & HOST_CAP_CLO)) {
+ rc = -EIO;
+ reason = "port busy but no CLO";
+ goto fail_restart;
+ }
+
+ tmp = readl(port_mmio + PORT_CMD);
+ tmp |= PORT_CMD_CLO;
+ writel(tmp, port_mmio + PORT_CMD);
+ readl(port_mmio + PORT_CMD); /* flush */
+
+ if (ahci_wait_for_bit(port_mmio + PORT_CMD,
+ PORT_CMD_CLO, PORT_CMD_CLO, 1, 500)) {
+ rc = -EIO;
+ reason = "CLO failed";
+ goto fail_restart;
+ }
+ }
+
+ /* restart engine */
+ ahci_start_engine(ap);
+
+ ata_tf_init(ap, &tf, 0);
+ fis = pp->cmd_tbl;
+
+ /* issue the first D2H Register FIS */
+ ahci_fill_cmd_slot(pp, cmd_fis_len | AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY);
+
+ tf.ctl |= ATA_SRST;
+ ata_tf_to_fis(&tf, fis, 0);
+ fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
+
+ writel(1, port_mmio + PORT_CMD_ISSUE);
+ readl(port_mmio + PORT_CMD_ISSUE); /* flush */
+
+ if (ahci_wait_for_bit(port_mmio + PORT_CMD_ISSUE, 0x1, 0x1, 1, 500)) {
+ rc = -EIO;
+ reason = "1st FIS failed";
+ goto fail;
+ }
+
+ /* spec says at least 5us, but be generous and sleep for 1ms */
+ msleep(1);
+
+ /* issue the second D2H Register FIS */
+ ahci_fill_cmd_slot(pp, cmd_fis_len);
+
+ tf.ctl &= ~ATA_SRST;
+ ata_tf_to_fis(&tf, fis, 0);
+ fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
+
+ writel(1, port_mmio + PORT_CMD_ISSUE);
+ readl(port_mmio + PORT_CMD_ISSUE); /* flush */
+
+ /* spec mandates ">= 2ms" before checking status.
+ * We wait 150ms, because that was the magic delay used for
+ * ATAPI devices in Hale Landis's ATADRVR, for the period of time
+ * between when the ATA command register is written, and then
+ * status is checked. Because waiting for "a while" before
+ * checking status is fine, post SRST, we perform this magic
+ * delay here as well.
+ */
+ msleep(150);
+
+ *class = ATA_DEV_NONE;
+ if (sata_dev_present(ap)) {
+ if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+ rc = -EIO;
+ reason = "device not ready";
+ goto fail;
+ }
+ *class = ahci_dev_classify(ap);
+ }
+
+ DPRINTK("EXIT, class=%u\n", *class);
+ return 0;
+
+ fail_restart:
+ ahci_start_engine(ap);
+ fail:
+ if (verbose)
+ printk(KERN_ERR "ata%u: softreset failed (%s)\n",
+ ap->id, reason);
+ else
+ DPRINTK("EXIT, rc=%d reason=\"%s\"\n", rc, reason);
+ return rc;
+}
+
static int ahci_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
{
int rc;
@@ -554,7 +686,8 @@ static void ahci_postreset(struct ata_po
static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
{
- return ata_drive_probe_reset(ap, NULL, NULL, ahci_hardreset,
+ return ata_drive_probe_reset(ap, ata_std_probeinit,
+ ahci_softreset, ahci_hardreset,
ahci_postreset, classes);
}
--
1.1.5
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap
2006-02-11 7:26 ` [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap Tejun Heo
@ 2006-02-11 22:53 ` Jeff Garzik
0 siblings, 0 replies; 10+ messages in thread
From: Jeff Garzik @ 2006-02-11 22:53 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, linux-ide
Tejun Heo wrote:
> Make ahci_fill_cmd_slot() take struct ahci_port_priv *pp instead of
> struct ata_port *ap as suggested by Jeff Garzik.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
applied 1-2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ahci: add softreset
2006-02-11 7:26 ` [PATCH 3/3] ahci: add softreset Tejun Heo
@ 2006-02-11 23:27 ` Jeff Garzik
2006-02-12 4:58 ` Tejun
2006-02-15 6:36 ` [PATCH] " Tejun Heo
0 siblings, 2 replies; 10+ messages in thread
From: Jeff Garzik @ 2006-02-11 23:27 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, linux-ide
Tejun Heo wrote:
> Now that libata is smart enought to handle both soft and hard resets,
> add softreset method.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> ---
>
> drivers/scsi/ahci.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 134 insertions(+), 1 deletions(-)
>
> 1edad0c8fc8e36457ac1f7c68ba4d86ff63986e3
> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
> index 86bccb7..a65895f 100644
> --- a/drivers/scsi/ahci.c
> +++ b/drivers/scsi/ahci.c
> @@ -514,6 +514,138 @@ static void ahci_fill_cmd_slot(struct ah
> pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
> }
>
> +static int ahci_wait_for_bit(void __iomem *reg, u32 mask, u32 val,
> + unsigned long interval_msec,
> + unsigned long timeout_msec)
> +{
> + unsigned long timeout;
> + u32 tmp;
> +
> + timeout = jiffies + (timeout_msec * HZ) / 1000;
> + do {
> + tmp = readl(reg);
> + if ((tmp & mask) != val)
IMO the sense of this test should be reversed, as the above line of code
is the opposite of what the function name implies.
> + return 0;
> + msleep(interval_msec);
> + } while (time_before(jiffies, timeout));
> +
> + return -1;
> +}
> +
> +static int ahci_softreset(struct ata_port *ap, int verbose, unsigned int *class)
> +{
> + struct ahci_host_priv *hpriv = ap->host_set->private_data;
> + struct ahci_port_priv *pp = ap->private_data;
> + void __iomem *mmio = ap->host_set->mmio_base;
> + void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
> + const u32 cmd_fis_len = 5; /* five dwords */
> + const char *reason = NULL;
> + struct ata_taskfile tf;
> + u8 *fis;
> + int rc;
> +
> + DPRINTK("ENTER\n");
> +
> + /* prepare for SRST (AHCI-1.1 10.4.1) */
> + rc = ahci_stop_engine(ap);
> + if (rc) {
> + reason = "failed to stop engine";
> + goto fail_restart;
> + }
> +
> + /* check BUSY/DRQ, perform Command List Override if necessary */
> + ahci_tf_read(ap, &tf);
> + if (tf.command & (ATA_BUSY | ATA_DRQ)) {
> + u32 tmp;
> +
> + if (!(hpriv->cap & HOST_CAP_CLO)) {
> + rc = -EIO;
> + reason = "port busy but no CLO";
> + goto fail_restart;
> + }
> +
> + tmp = readl(port_mmio + PORT_CMD);
> + tmp |= PORT_CMD_CLO;
> + writel(tmp, port_mmio + PORT_CMD);
> + readl(port_mmio + PORT_CMD); /* flush */
> +
> + if (ahci_wait_for_bit(port_mmio + PORT_CMD,
> + PORT_CMD_CLO, PORT_CMD_CLO, 1, 500)) {
> + rc = -EIO;
> + reason = "CLO failed";
> + goto fail_restart;
> + }
> + }
> +
> + /* restart engine */
> + ahci_start_engine(ap);
> +
> + ata_tf_init(ap, &tf, 0);
> + fis = pp->cmd_tbl;
> +
> + /* issue the first D2H Register FIS */
> + ahci_fill_cmd_slot(pp, cmd_fis_len | AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY);
> +
> + tf.ctl |= ATA_SRST;
> + ata_tf_to_fis(&tf, fis, 0);
> + fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
> +
> + writel(1, port_mmio + PORT_CMD_ISSUE);
> + readl(port_mmio + PORT_CMD_ISSUE); /* flush */
> +
> + if (ahci_wait_for_bit(port_mmio + PORT_CMD_ISSUE, 0x1, 0x1, 1, 500)) {
> + rc = -EIO;
> + reason = "1st FIS failed";
> + goto fail;
> + }
> +
> + /* spec says at least 5us, but be generous and sleep for 1ms */
> + msleep(1);
> +
> + /* issue the second D2H Register FIS */
> + ahci_fill_cmd_slot(pp, cmd_fis_len);
> +
> + tf.ctl &= ~ATA_SRST;
> + ata_tf_to_fis(&tf, fis, 0);
> + fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
> +
> + writel(1, port_mmio + PORT_CMD_ISSUE);
> + readl(port_mmio + PORT_CMD_ISSUE); /* flush */
> +
> + /* spec mandates ">= 2ms" before checking status.
> + * We wait 150ms, because that was the magic delay used for
> + * ATAPI devices in Hale Landis's ATADRVR, for the period of time
> + * between when the ATA command register is written, and then
> + * status is checked. Because waiting for "a while" before
> + * checking status is fine, post SRST, we perform this magic
> + * delay here as well.
> + */
> + msleep(150);
> +
> + *class = ATA_DEV_NONE;
> + if (sata_dev_present(ap)) {
> + if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
> + rc = -EIO;
> + reason = "device not ready";
> + goto fail;
> + }
On AHCI, are you sure this busy_sleep is even necessary?
> + *class = ahci_dev_classify(ap);
> + }
> +
> + DPRINTK("EXIT, class=%u\n", *class);
> + return 0;
> +
> + fail_restart:
> + ahci_start_engine(ap);
> + fail:
> + if (verbose)
> + printk(KERN_ERR "ata%u: softreset failed (%s)\n",
> + ap->id, reason);
> + else
> + DPRINTK("EXIT, rc=%d reason=\"%s\"\n", rc, reason);
> + return rc;
> +}
> +
> static int ahci_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
> {
> int rc;
> @@ -554,7 +686,8 @@ static void ahci_postreset(struct ata_po
>
> static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
> {
> - return ata_drive_probe_reset(ap, NULL, NULL, ahci_hardreset,
> + return ata_drive_probe_reset(ap, ata_std_probeinit,
> + ahci_softreset, ahci_hardreset,
> ahci_postreset, classes);
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] ahci: add softreset
2006-02-11 23:27 ` Jeff Garzik
@ 2006-02-12 4:58 ` Tejun
2006-02-15 6:36 ` [PATCH] " Tejun Heo
1 sibling, 0 replies; 10+ messages in thread
From: Tejun @ 2006-02-12 4:58 UTC (permalink / raw)
To: Jeff Garzik; +Cc: albertcc, linux-ide
Jeff Garzik wrote:
>> +static int ahci_wait_for_bit(void __iomem *reg, u32 mask, u32 val,
>> + unsigned long interval_msec,
>> + unsigned long timeout_msec)
>> +{
>> + unsigned long timeout;
>> + u32 tmp;
>> +
>> + timeout = jiffies + (timeout_msec * HZ) / 1000;
>> + do {
>> + tmp = readl(reg);
>> + if ((tmp & mask) != val)
>
>
> IMO the sense of this test should be reversed, as the above line of code
> is the opposite of what the function name implies.
It didn't use to have @mask and just tested for a single bit to clear.
Some piece of currently dropped code needed @mask, @val stuff and the
changed function got left I think. I'll reverse the testing logic and
rename the function to something more appropriate - probably
ahci_poll_register().
>
>> + return 0;
>> + msleep(interval_msec);
>> + } while (time_before(jiffies, timeout));
>> +
>> + return -1;
>> +}
>> +
>> +static int ahci_softreset(struct ata_port *ap, int verbose, unsigned
>> int *class)
>> +{
[--snip--]
>> + /* issue the second D2H Register FIS */
>> + ahci_fill_cmd_slot(pp, cmd_fis_len);
>> +
>> + tf.ctl &= ~ATA_SRST;
>> + ata_tf_to_fis(&tf, fis, 0);
>> + fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
>> +
>> + writel(1, port_mmio + PORT_CMD_ISSUE);
>> + readl(port_mmio + PORT_CMD_ISSUE); /* flush */
>> +
>> + /* spec mandates ">= 2ms" before checking status.
>> + * We wait 150ms, because that was the magic delay used for
>> + * ATAPI devices in Hale Landis's ATADRVR, for the period of time
>> + * between when the ATA command register is written, and then
>> + * status is checked. Because waiting for "a while" before
>> + * checking status is fine, post SRST, we perform this magic
>> + * delay here as well.
>> + */
>> + msleep(150);
>> +
>> + *class = ATA_DEV_NONE;
>> + if (sata_dev_present(ap)) {
>> + if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
>> + rc = -EIO;
>> + reason = "device not ready";
>> + goto fail;
>> + }
>
>
> On AHCI, are you sure this busy_sleep is even necessary?
>
We should wait for something before proceeding with classfication. The
second FIS turns off SRST and the device will respond with class
signature when it gets out of reset status, which ends up clearing BUSY.
We can either wait for the issue bit to clear as in the the first FIS
or above. Do you think waiting for issue bit clearance is better?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] ahci: add softreset
2006-02-11 23:27 ` Jeff Garzik
2006-02-12 4:58 ` Tejun
@ 2006-02-15 6:36 ` Tejun Heo
2006-02-28 18:12 ` Jeff Garzik
1 sibling, 1 reply; 10+ messages in thread
From: Tejun Heo @ 2006-02-15 6:36 UTC (permalink / raw)
To: Jeff Garzik; +Cc: albertcc, linux-ide
Now that libata is smart enought to handle both soft and hard resets,
add softreset method.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Jeff, as I didn't get reply about the whether waiting is necessary or
not, I've just fixed the first part.
Thanks.
ahci.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 134 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 1c2ab3d..6b36345 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -513,6 +513,138 @@ static void ahci_fill_cmd_slot(struct ah
pp->cmd_slot[0].tbl_addr_hi = cpu_to_le32((pp->cmd_tbl_dma >> 16) >> 16);
}
+static int ahci_poll_register(void __iomem *reg, u32 mask, u32 val,
+ unsigned long interval_msec,
+ unsigned long timeout_msec)
+{
+ unsigned long timeout;
+ u32 tmp;
+
+ timeout = jiffies + (timeout_msec * HZ) / 1000;
+ do {
+ tmp = readl(reg);
+ if ((tmp & mask) == val)
+ return 0;
+ msleep(interval_msec);
+ } while (time_before(jiffies, timeout));
+
+ return -1;
+}
+
+static int ahci_softreset(struct ata_port *ap, int verbose, unsigned int *class)
+{
+ struct ahci_host_priv *hpriv = ap->host_set->private_data;
+ struct ahci_port_priv *pp = ap->private_data;
+ void __iomem *mmio = ap->host_set->mmio_base;
+ void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
+ const u32 cmd_fis_len = 5; /* five dwords */
+ const char *reason = NULL;
+ struct ata_taskfile tf;
+ u8 *fis;
+ int rc;
+
+ DPRINTK("ENTER\n");
+
+ /* prepare for SRST (AHCI-1.1 10.4.1) */
+ rc = ahci_stop_engine(ap);
+ if (rc) {
+ reason = "failed to stop engine";
+ goto fail_restart;
+ }
+
+ /* check BUSY/DRQ, perform Command List Override if necessary */
+ ahci_tf_read(ap, &tf);
+ if (tf.command & (ATA_BUSY | ATA_DRQ)) {
+ u32 tmp;
+
+ if (!(hpriv->cap & HOST_CAP_CLO)) {
+ rc = -EIO;
+ reason = "port busy but no CLO";
+ goto fail_restart;
+ }
+
+ tmp = readl(port_mmio + PORT_CMD);
+ tmp |= PORT_CMD_CLO;
+ writel(tmp, port_mmio + PORT_CMD);
+ readl(port_mmio + PORT_CMD); /* flush */
+
+ if (ahci_poll_register(port_mmio + PORT_CMD, PORT_CMD_CLO, 0x0,
+ 1, 500)) {
+ rc = -EIO;
+ reason = "CLO failed";
+ goto fail_restart;
+ }
+ }
+
+ /* restart engine */
+ ahci_start_engine(ap);
+
+ ata_tf_init(ap, &tf, 0);
+ fis = pp->cmd_tbl;
+
+ /* issue the first D2H Register FIS */
+ ahci_fill_cmd_slot(pp, cmd_fis_len | AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY);
+
+ tf.ctl |= ATA_SRST;
+ ata_tf_to_fis(&tf, fis, 0);
+ fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
+
+ writel(1, port_mmio + PORT_CMD_ISSUE);
+ readl(port_mmio + PORT_CMD_ISSUE); /* flush */
+
+ if (ahci_poll_register(port_mmio + PORT_CMD_ISSUE, 0x1, 0x0, 1, 500)) {
+ rc = -EIO;
+ reason = "1st FIS failed";
+ goto fail;
+ }
+
+ /* spec says at least 5us, but be generous and sleep for 1ms */
+ msleep(1);
+
+ /* issue the second D2H Register FIS */
+ ahci_fill_cmd_slot(pp, cmd_fis_len);
+
+ tf.ctl &= ~ATA_SRST;
+ ata_tf_to_fis(&tf, fis, 0);
+ fis[1] &= ~(1 << 7); /* turn off Command FIS bit */
+
+ writel(1, port_mmio + PORT_CMD_ISSUE);
+ readl(port_mmio + PORT_CMD_ISSUE); /* flush */
+
+ /* spec mandates ">= 2ms" before checking status.
+ * We wait 150ms, because that was the magic delay used for
+ * ATAPI devices in Hale Landis's ATADRVR, for the period of time
+ * between when the ATA command register is written, and then
+ * status is checked. Because waiting for "a while" before
+ * checking status is fine, post SRST, we perform this magic
+ * delay here as well.
+ */
+ msleep(150);
+
+ *class = ATA_DEV_NONE;
+ if (sata_dev_present(ap)) {
+ if (ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT)) {
+ rc = -EIO;
+ reason = "device not ready";
+ goto fail;
+ }
+ *class = ahci_dev_classify(ap);
+ }
+
+ DPRINTK("EXIT, class=%u\n", *class);
+ return 0;
+
+ fail_restart:
+ ahci_start_engine(ap);
+ fail:
+ if (verbose)
+ printk(KERN_ERR "ata%u: softreset failed (%s)\n",
+ ap->id, reason);
+ else
+ DPRINTK("EXIT, rc=%d reason=\"%s\"\n", rc, reason);
+ return rc;
+}
+
static int ahci_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
{
int rc;
@@ -553,7 +685,8 @@ static void ahci_postreset(struct ata_po
static int ahci_probe_reset(struct ata_port *ap, unsigned int *classes)
{
- return ata_drive_probe_reset(ap, NULL, NULL, ahci_hardreset,
+ return ata_drive_probe_reset(ap, ata_std_probeinit,
+ ahci_softreset, ahci_hardreset,
ahci_postreset, classes);
}
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ahci: add softreset
2006-02-15 6:36 ` [PATCH] " Tejun Heo
@ 2006-02-28 18:12 ` Jeff Garzik
2006-03-01 5:35 ` Tejun Heo
0 siblings, 1 reply; 10+ messages in thread
From: Jeff Garzik @ 2006-02-28 18:12 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, linux-ide
Tejun Heo wrote:
> Now that libata is smart enought to handle both soft and hard resets,
> add softreset method.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
ACK, finally
(though I am dropping all your other pending patches as requested)
> + /* spec says at least 5us, but be generous and sleep for 1ms */
> + msleep(1);
FWIW msleep simply guarantees a minimum delay. Depending on the
scheduler tick granularity, msleep(1) may delay 10-100ms.
This FWIW may or may not warrant updating the above comment, I leave it
up to you.
Jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ahci: add softreset
2006-02-28 18:12 ` Jeff Garzik
@ 2006-03-01 5:35 ` Tejun Heo
0 siblings, 0 replies; 10+ messages in thread
From: Tejun Heo @ 2006-03-01 5:35 UTC (permalink / raw)
To: Jeff Garzik; +Cc: albertcc, linux-ide
On Tue, Feb 28, 2006 at 01:12:38PM -0500, Jeff Garzik wrote:
> Tejun Heo wrote:
> >Now that libata is smart enought to handle both soft and hard resets,
> >add softreset method.
> >
> >Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> ACK, finally
>
> (though I am dropping all your other pending patches as requested)
>
Good, and I forgot to resend the regenerated patches after writing I
would do that ASAP. I was about to do that but then quilt2git didn't
work and, after fixing it, I completely forgot why I fixed quilt2git
in the first place. Duh... Will send soon.
>
> >+ /* spec says at least 5us, but be generous and sleep for 1ms */
> >+ msleep(1);
>
> FWIW msleep simply guarantees a minimum delay. Depending on the
> scheduler tick granularity, msleep(1) may delay 10-100ms.
>
> This FWIW may or may not warrant updating the above comment, I leave it
> up to you.
>
I think the comment is okay as it is and, more importantly, it
wouldn't fit in one line with any more detail. :-P
--
tejun
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2006-03-01 5:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-02-11 7:26 [PATCHSET] ahci: convert to new reset mechanism Tejun Heo
2006-02-11 7:26 ` [PATCH 1/3] ahci: make ahci_fill_cmd_slot() take *pp instead of *ap Tejun Heo
2006-02-11 22:53 ` Jeff Garzik
2006-02-11 7:26 ` [PATCH 2/3] ahci: convert to new reset mechanism Tejun Heo
2006-02-11 7:26 ` [PATCH 3/3] ahci: add softreset Tejun Heo
2006-02-11 23:27 ` Jeff Garzik
2006-02-12 4:58 ` Tejun
2006-02-15 6:36 ` [PATCH] " Tejun Heo
2006-02-28 18:12 ` Jeff Garzik
2006-03-01 5:35 ` Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).