* [PATCH 01/10] libata-pmp: update ata_eh_reset() for PMP
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 06/10] sata_sil24: implement PMP support Tejun Heo
` (9 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
PMP always requires SRST to be enabled. Also, hardreset reports
classification code from the first device when PMP is attached, not
from the PMP. Update ata_eh_reset() such that followup softreset is
performed if the controller is PMP capable and the host link is being
reset.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/libata-eh.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 60186f8..687419b 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2003,6 +2003,8 @@ static int ata_eh_followup_srst_needed(struct ata_link *link,
return 1;
if (rc != 0)
return 0;
+ if ((link->ap->flags & ATA_FLAG_PMP) && ata_is_host_link(link))
+ return 1;
if (classify && !(link->flags & ATA_LFLAG_ASSUME_CLASS) &&
classes[0] == ATA_DEV_UNKNOWN)
return 1;
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 06/10] sata_sil24: implement PMP support
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
2007-09-23 4:19 ` [PATCH 01/10] libata-pmp: update ata_eh_reset() for PMP Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 07/10] sata_sil24: implement PORT_RST Tejun Heo
` (8 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Implement PMP support. sil24 supports full FIS-switching. However,
it has a PMP DMA CS errata which requires port-wide resetting if
commands are outstanding to three or more devices when an error occurs
on one of them.
ATAPI commands often result in CHECK SENSE and it's crucial to not
reset them before fetching sense data. Unfortunately, ATAPI CHECK
SENSE causes a lot of problem if command is outstanding to any other
device usually resulting in port-wide reset. So, sata_sil24
implements sil24_qc_defer() which guarantees ATAPI command is run by
itself.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/sata_sil24.c | 227 +++++++++++++++++++++++++++++++++++++++++----
1 files changed, 206 insertions(+), 21 deletions(-)
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index b4f81eb..03bfbb6 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -30,7 +30,7 @@
#include <linux/libata.h>
#define DRV_NAME "sata_sil24"
-#define DRV_VERSION "1.0"
+#define DRV_VERSION "1.1"
/*
* Port request block (PRB) 32 bytes
@@ -238,7 +238,7 @@ enum {
SIL24_COMMON_FLAGS = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
ATA_FLAG_NCQ | ATA_FLAG_ACPI_SATA |
- ATA_FLAG_AN,
+ ATA_FLAG_AN | ATA_FLAG_PMP,
SIL24_COMMON_LFLAGS = ATA_LFLAG_SKIP_D2H_BSY,
SIL24_FLAG_PCIX_IRQ_WOC = (1 << 24), /* IRQ loss errata on PCI-X */
@@ -330,9 +330,14 @@ static u8 sil24_check_status(struct ata_port *ap);
static int sil24_scr_read(struct ata_port *ap, unsigned sc_reg, u32 *val);
static int 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_qc_defer(struct ata_queued_cmd *qc);
static void sil24_qc_prep(struct ata_queued_cmd *qc);
static unsigned int sil24_qc_issue(struct ata_queued_cmd *qc);
static void sil24_irq_clear(struct ata_port *ap);
+static void sil24_pmp_attach(struct ata_port *ap);
+static void sil24_pmp_detach(struct ata_port *ap);
+static int sil24_pmp_read(struct ata_device *dev, int pmp, int reg, u32 *r_val);
+static int sil24_pmp_write(struct ata_device *dev, int pmp, int reg, u32 val);
static void sil24_freeze(struct ata_port *ap);
static void sil24_thaw(struct ata_port *ap);
static void sil24_error_handler(struct ata_port *ap);
@@ -341,6 +346,7 @@ static int sil24_port_start(struct ata_port *ap);
static int sil24_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
#ifdef CONFIG_PM
static int sil24_pci_device_resume(struct pci_dev *pdev);
+static int sil24_port_resume(struct ata_port *ap);
#endif
static const struct pci_device_id sil24_pci_tbl[] = {
@@ -393,7 +399,7 @@ static const struct ata_port_operations sil24_ops = {
.tf_read = sil24_tf_read,
- .qc_defer = ata_std_qc_defer,
+ .qc_defer = sil24_qc_defer,
.qc_prep = sil24_qc_prep,
.qc_issue = sil24_qc_issue,
@@ -402,12 +408,21 @@ static const struct ata_port_operations sil24_ops = {
.scr_read = sil24_scr_read,
.scr_write = sil24_scr_write,
+ .pmp_attach = sil24_pmp_attach,
+ .pmp_detach = sil24_pmp_detach,
+ .pmp_read = sil24_pmp_read,
+ .pmp_write = sil24_pmp_write,
+
.freeze = sil24_freeze,
.thaw = sil24_thaw,
.error_handler = sil24_error_handler,
.post_internal_cmd = sil24_post_internal_cmd,
.port_start = sil24_port_start,
+
+#ifdef CONFIG_PM
+ .port_resume = sil24_port_resume,
+#endif
};
/*
@@ -521,11 +536,40 @@ static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
*tf = pp->tf;
}
+static void sil24_config_pmp(struct ata_port *ap, int attached)
+{
+ void __iomem *port = ap->ioaddr.cmd_addr;
+
+ if (attached)
+ writel(PORT_CS_PMP_EN, port + PORT_CTRL_STAT);
+ else
+ writel(PORT_CS_PMP_EN, port + PORT_CTRL_CLR);
+}
+
+static void sil24_clear_pmp(struct ata_port *ap)
+{
+ void __iomem *port = ap->ioaddr.cmd_addr;
+ int i;
+
+ writel(PORT_CS_PMP_RESUME, port + PORT_CTRL_CLR);
+
+ for (i = 0; i < SATA_PMP_MAX_PORTS; i++) {
+ void __iomem *pmp_base = port + PORT_PMP + i * PORT_PMP_SIZE;
+
+ writel(0, pmp_base + PORT_PMP_STATUS);
+ writel(0, pmp_base + PORT_PMP_QACTIVE);
+ }
+}
+
static int sil24_init_port(struct ata_port *ap)
{
void __iomem *port = ap->ioaddr.cmd_addr;
u32 tmp;
+ /* clear PMP error status */
+ if (ap->nr_pmp_links)
+ sil24_clear_pmp(ap);
+
writel(PORT_CS_INIT, port + PORT_CTRL_STAT);
ata_wait_register(port + PORT_CTRL_STAT,
PORT_CS_INIT, PORT_CS_INIT, 10, 100);
@@ -640,7 +684,7 @@ static int sil24_do_softreset(struct ata_link *link, unsigned int *class,
static int sil24_softreset(struct ata_link *link, unsigned int *class,
unsigned long deadline)
{
- return sil24_do_softreset(link, class, 0, deadline);
+ return sil24_do_softreset(link, class, SATA_PMP_CTRL_PORT, deadline);
}
static int sil24_hardreset(struct ata_link *link, unsigned int *class,
@@ -708,6 +752,38 @@ static inline void sil24_fill_sg(struct ata_queued_cmd *qc,
}
}
+static int sil24_qc_defer(struct ata_queued_cmd *qc)
+{
+ struct ata_link *link = qc->dev->link;
+ struct ata_port *ap = link->ap;
+ u8 prot = qc->tf.protocol;
+ int is_atapi = (prot == ATA_PROT_ATAPI ||
+ prot == ATA_PROT_ATAPI_NODATA ||
+ prot == ATA_PROT_ATAPI_DMA);
+
+ /* ATAPI commands completing with CHECK_SENSE cause various
+ * weird problems if other commands are active. PMP DMA CS
+ * errata doesn't cover all and HSM violation occurs even with
+ * only one other device active. Always run an ATAPI command
+ * by itself.
+ */
+ if (unlikely(ap->excl_link)) {
+ if (link == ap->excl_link) {
+ if (ap->nr_active_links)
+ return ATA_DEFER_PORT;
+ qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
+ } else
+ return ATA_DEFER_PORT;
+ } else if (unlikely(is_atapi)) {
+ ap->excl_link = link;
+ if (ap->nr_active_links)
+ return ATA_DEFER_PORT;
+ qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
+ }
+
+ return ata_std_qc_defer(qc);
+}
+
static void sil24_qc_prep(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc->ap;
@@ -751,7 +827,7 @@ static void sil24_qc_prep(struct ata_queued_cmd *qc)
}
prb->ctrl = cpu_to_le16(ctrl);
- ata_tf_to_fis(&qc->tf, 0, 1, prb->fis);
+ ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, prb->fis);
if (qc->flags & ATA_QCFLAG_DMAMAP)
sil24_fill_sg(qc, sge);
@@ -780,6 +856,65 @@ static void sil24_irq_clear(struct ata_port *ap)
/* unused */
}
+static void sil24_pmp_attach(struct ata_port *ap)
+{
+ sil24_config_pmp(ap, 1);
+ sil24_init_port(ap);
+}
+
+static void sil24_pmp_detach(struct ata_port *ap)
+{
+ sil24_init_port(ap);
+ sil24_config_pmp(ap, 0);
+}
+
+static int sil24_pmp_read(struct ata_device *dev, int pmp, int reg, u32 *r_val)
+{
+ struct ata_port *ap = dev->link->ap;
+ struct ata_taskfile tf;
+ int rc;
+
+ sata_pmp_read_init_tf(&tf, dev, pmp, reg);
+ rc = sil24_exec_polled_cmd(ap, SATA_PMP_CTRL_PORT, &tf, 1, 0,
+ SATA_PMP_SCR_TIMEOUT);
+ if (rc == 0) {
+ sil24_read_tf(ap, 0, &tf);
+ *r_val = sata_pmp_read_val(&tf);
+ }
+ return rc;
+}
+
+static int sil24_pmp_write(struct ata_device *dev, int pmp, int reg, u32 val)
+{
+ struct ata_port *ap = dev->link->ap;
+ struct ata_taskfile tf;
+
+ sata_pmp_write_init_tf(&tf, dev, pmp, reg, val);
+ return sil24_exec_polled_cmd(ap, SATA_PMP_CTRL_PORT, &tf, 1, 0,
+ SATA_PMP_SCR_TIMEOUT);
+}
+
+static int sil24_pmp_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ return sil24_do_softreset(link, class, link->pmp, deadline);
+}
+
+static int sil24_pmp_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ int rc;
+
+ rc = sil24_init_port(link->ap);
+ if (rc) {
+ ata_link_printk(link, KERN_ERR,
+ "hardreset failed (port not ready)\n");
+ return rc;
+ }
+
+ return sata_pmp_std_hardreset(link, class, deadline);
+}
+
static void sil24_freeze(struct ata_port *ap)
{
void __iomem *port = ap->ioaddr.cmd_addr;
@@ -807,8 +942,10 @@ static void sil24_error_intr(struct ata_port *ap)
{
void __iomem *port = ap->ioaddr.cmd_addr;
struct sil24_port_priv *pp = ap->private_data;
- struct ata_eh_info *ehi = &ap->link.eh_info;
- int freeze = 0;
+ struct ata_queued_cmd *qc = NULL;
+ struct ata_link *link;
+ struct ata_eh_info *ehi;
+ int abort = 0, freeze = 0;
u32 irq_stat;
/* on error, we need to clear IRQ explicitly */
@@ -816,6 +953,8 @@ static void sil24_error_intr(struct ata_port *ap)
writel(irq_stat, port + PORT_IRQ_STAT);
/* first, analyze and record host port events */
+ link = &ap->link;
+ ehi = &link->eh_info;
ata_ehi_clear_desc(ehi);
ata_ehi_push_desc(ehi, "irq_stat 0x%08x", irq_stat);
@@ -844,8 +983,43 @@ static void sil24_error_intr(struct ata_port *ap)
if (irq_stat & PORT_IRQ_ERROR) {
struct sil24_cerr_info *ci = NULL;
unsigned int err_mask = 0, action = 0;
- struct ata_queued_cmd *qc;
- u32 cerr;
+ u32 context, cerr;
+ int pmp;
+
+ abort = 1;
+
+ /* DMA Context Switch Failure in Port Multiplier Mode
+ * errata. If we have active commands to 3 or more
+ * devices, any error condition on active devices can
+ * corrupt DMA context switching.
+ */
+ if (ap->nr_active_links >= 3) {
+ ehi->err_mask |= AC_ERR_OTHER;
+ ehi->action |= ATA_EH_HARDRESET;
+ ata_ehi_push_desc(ehi, "PMP DMA CS errata");
+ freeze = 1;
+ }
+
+ /* find out the offending link and qc */
+ if (ap->nr_pmp_links) {
+ context = readl(port + PORT_CONTEXT);
+ pmp = (context >> 5) & 0xf;
+
+ if (pmp < ap->nr_pmp_links) {
+ link = &ap->pmp_link[pmp];
+ ehi = &link->eh_info;
+ qc = ata_qc_from_tag(ap, link->active_tag);
+
+ ata_ehi_clear_desc(ehi);
+ ata_ehi_push_desc(ehi, "irq_stat 0x%08x",
+ irq_stat);
+ } else {
+ err_mask |= AC_ERR_HSM;
+ action |= ATA_EH_HARDRESET;
+ freeze = 1;
+ }
+ } else
+ qc = ata_qc_from_tag(ap, link->active_tag);
/* analyze CMD_ERR */
cerr = readl(port + PORT_CMD_ERR);
@@ -864,7 +1038,6 @@ static void sil24_error_intr(struct ata_port *ap)
}
/* record error info */
- qc = ata_qc_from_tag(ap, ap->link.active_tag);
if (qc) {
sil24_read_tf(ap, qc->tag, &pp->tf);
qc->err_mask |= err_mask;
@@ -872,13 +1045,21 @@ static void sil24_error_intr(struct ata_port *ap)
ehi->err_mask |= err_mask;
ehi->action |= action;
+
+ /* if PMP, resume */
+ if (ap->nr_pmp_links)
+ writel(PORT_CS_PMP_RESUME, port + PORT_CTRL_STAT);
}
/* freeze or abort */
if (freeze)
ata_port_freeze(ap);
- else
- ata_port_abort(ap);
+ else if (abort) {
+ if (qc)
+ ata_link_abort(qc->dev->link);
+ else
+ ata_port_abort(ap);
+ }
}
static void sil24_finish_qc(struct ata_queued_cmd *qc)
@@ -971,16 +1152,14 @@ static irqreturn_t sil24_interrupt(int irq, void *dev_instance)
static void sil24_error_handler(struct ata_port *ap)
{
- struct ata_eh_context *ehc = &ap->link.eh_context;
-
- if (sil24_init_port(ap)) {
+ if (sil24_init_port(ap))
ata_eh_freeze_port(ap);
- ehc->i.action |= ATA_EH_HARDRESET;
- }
/* perform recovery */
- ata_do_eh(ap, ata_std_prereset, sil24_softreset, sil24_hardreset,
- ata_std_postreset);
+ sata_pmp_do_eh(ap, ata_std_prereset, sil24_softreset, sil24_hardreset,
+ ata_std_postreset, sata_pmp_std_prereset,
+ sil24_pmp_softreset, sil24_pmp_hardreset,
+ sata_pmp_std_postreset);
}
static void sil24_post_internal_cmd(struct ata_queued_cmd *qc)
@@ -988,8 +1167,8 @@ static void sil24_post_internal_cmd(struct ata_queued_cmd *qc)
struct ata_port *ap = qc->ap;
/* make DMA engine forget about the failed command */
- if (qc->flags & ATA_QCFLAG_FAILED)
- sil24_init_port(ap);
+ if ((qc->flags & ATA_QCFLAG_FAILED) && sil24_init_port(ap))
+ ata_eh_freeze_port(ap);
}
static int sil24_port_start(struct ata_port *ap)
@@ -1190,6 +1369,12 @@ static int sil24_pci_device_resume(struct pci_dev *pdev)
return 0;
}
+
+static int sil24_port_resume(struct ata_port *ap)
+{
+ sil24_config_pmp(ap, ap->nr_pmp_links);
+ return 0;
+}
#endif
static int __init sil24_init(void)
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 07/10] sata_sil24: implement PORT_RST
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
2007-09-23 4:19 ` [PATCH 01/10] libata-pmp: update ata_eh_reset() for PMP Tejun Heo
2007-09-23 4:19 ` [PATCH 06/10] sata_sil24: implement PMP support Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 02/10] libata-pmp: implement Port Multiplier support Tejun Heo
` (7 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
As DEV_RST (hardreset) sometimes fail to recover the controller
(especially after PMP DMA CS errata). In such cases, perform PORT_RST
prior to DEV_RST.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/sata_sil24.c | 93 ++++++++++++++++++++++++++++++++++-----------
1 files changed, 70 insertions(+), 23 deletions(-)
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 03bfbb6..15b9a80 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -323,6 +323,7 @@ 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 */
+ int do_port_rst;
};
static void sil24_dev_config(struct ata_device *dev);
@@ -536,6 +537,31 @@ static void sil24_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
*tf = pp->tf;
}
+static void sil24_config_port(struct ata_port *ap)
+{
+ void __iomem *port = ap->ioaddr.cmd_addr;
+
+ /* configure IRQ WoC */
+ if (ap->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);
+ writel(0x8000, port + PORT_HSHK_ERR_THRESH);
+ writel(0x0000, port + PORT_DECODE_ERR_CNT);
+ writel(0x0000, port + PORT_CRC_ERR_CNT);
+ writel(0x0000, port + PORT_HSHK_ERR_CNT);
+
+ /* always use 64bit activation */
+ writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_CLR);
+
+ /* clear port multiplier enable and resume bits */
+ writel(PORT_CS_PMP_EN | PORT_CS_PMP_RESUME, port + PORT_CTRL_CLR);
+}
+
static void sil24_config_pmp(struct ata_port *ap, int attached)
{
void __iomem *port = ap->ioaddr.cmd_addr;
@@ -564,6 +590,7 @@ static void sil24_clear_pmp(struct ata_port *ap)
static int sil24_init_port(struct ata_port *ap)
{
void __iomem *port = ap->ioaddr.cmd_addr;
+ struct sil24_port_priv *pp = ap->private_data;
u32 tmp;
/* clear PMP error status */
@@ -576,8 +603,12 @@ static int sil24_init_port(struct ata_port *ap)
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)
+ if ((tmp & (PORT_CS_INIT | PORT_CS_RDY)) != PORT_CS_RDY) {
+ pp->do_port_rst = 1;
+ ap->link.eh_context.i.action |= ATA_EH_HARDRESET;
return -EIO;
+ }
+
return 0;
}
@@ -692,10 +723,34 @@ static int sil24_hardreset(struct ata_link *link, unsigned int *class,
{
struct ata_port *ap = link->ap;
void __iomem *port = ap->ioaddr.cmd_addr;
+ struct sil24_port_priv *pp = ap->private_data;
+ int did_port_rst = 0;
const char *reason;
int tout_msec, rc;
u32 tmp;
+ retry:
+ /* Sometimes, DEV_RST is not enough to recover the controller.
+ * This happens often after PM DMA CS errata.
+ */
+ if (pp->do_port_rst) {
+ ata_port_printk(ap, KERN_WARNING, "controller in dubious "
+ "state, performing PORT_RST\n");
+
+ writel(PORT_CS_PORT_RST, port + PORT_CTRL_STAT);
+ msleep(10);
+ writel(PORT_CS_PORT_RST, port + PORT_CTRL_CLR);
+ ata_wait_register(port + PORT_CTRL_STAT, PORT_CS_RDY, 0,
+ 10, 5000);
+
+ /* restore port configuration */
+ sil24_config_port(ap);
+ sil24_config_pmp(ap, ap->nr_pmp_links);
+
+ pp->do_port_rst = 0;
+ did_port_rst = 1;
+ }
+
/* sil24 does the right thing(tm) without any protection */
sata_set_spd(link);
@@ -732,6 +787,11 @@ static int sil24_hardreset(struct ata_link *link, unsigned int *class,
return -EAGAIN;
err:
+ if (!did_port_rst) {
+ pp->do_port_rst = 1;
+ goto retry;
+ }
+
ata_link_printk(link, KERN_ERR, "hardreset failed (%s)\n", reason);
return -EIO;
}
@@ -997,6 +1057,7 @@ static void sil24_error_intr(struct ata_port *ap)
ehi->err_mask |= AC_ERR_OTHER;
ehi->action |= ATA_EH_HARDRESET;
ata_ehi_push_desc(ehi, "PMP DMA CS errata");
+ pp->do_port_rst = 1;
freeze = 1;
}
@@ -1152,6 +1213,8 @@ static irqreturn_t sil24_interrupt(int irq, void *dev_instance)
static void sil24_error_handler(struct ata_port *ap)
{
+ struct sil24_port_priv *pp = ap->private_data;
+
if (sil24_init_port(ap))
ata_eh_freeze_port(ap);
@@ -1160,6 +1223,8 @@ static void sil24_error_handler(struct ata_port *ap)
ata_std_postreset, sata_pmp_std_prereset,
sil24_pmp_softreset, sil24_pmp_hardreset,
sata_pmp_std_postreset);
+
+ pp->do_port_rst = 0;
}
static void sil24_post_internal_cmd(struct ata_queued_cmd *qc)
@@ -1206,7 +1271,6 @@ static int sil24_port_start(struct ata_port *ap)
static void sil24_init_controller(struct ata_host *host)
{
void __iomem *host_base = host->iomap[SIL24_HOST_BAR];
- void __iomem *port_base = host->iomap[SIL24_PORT_BAR];
u32 tmp;
int i;
@@ -1218,7 +1282,8 @@ static void sil24_init_controller(struct ata_host *host)
/* init ports */
for (i = 0; i < host->n_ports; i++) {
- void __iomem *port = port_base + i * PORT_REGS_SIZE;
+ struct ata_port *ap = host->ports[i];
+ void __iomem *port = ap->ioaddr.cmd_addr;
/* Initial PHY setting */
writel(0x20c, port + PORT_PHY_CFG);
@@ -1235,26 +1300,8 @@ static void sil24_init_controller(struct ata_host *host)
"failed to clear port RST\n");
}
- /* Configure IRQ WoC */
- if (host->ports[0]->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);
- writel(0x8000, port + PORT_HSHK_ERR_THRESH);
- writel(0x0000, port + PORT_DECODE_ERR_CNT);
- writel(0x0000, port + PORT_CRC_ERR_CNT);
- writel(0x0000, port + PORT_HSHK_ERR_CNT);
-
- /* Always use 64bit activation */
- writel(PORT_CS_32BIT_ACTV, port + PORT_CTRL_CLR);
-
- /* Clear port multiplier enable and resume bits */
- writel(PORT_CS_PMP_EN | PORT_CS_PMP_RESUME,
- port + PORT_CTRL_CLR);
+ /* configure port */
+ sil24_config_port(ap);
}
/* Turn on interrupts */
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 02/10] libata-pmp: implement Port Multiplier support
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (2 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 07/10] sata_sil24: implement PORT_RST Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 03/10] libata-pmp: hook PMP support and enable it Tejun Heo
` (6 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Implement Port Multiplier support. To support PMP, a LLDD has to
supply ops->pmp_read() and pmp_write(). If non-null, ->pmp_attach and
->pmp_detach are called on PMP attach and detach, respectively.
->pmp_read/write() can be called while the port is frozen, so they
must be implemented by polling. This patch supplies several helpers
to ease ->pmp_read/write() implementation.
Also, irq_handler and error_handler must be PMP aware. Most of PMP
aware EH can be done by calling ata_pmp_do_eh() with appropriate
methods. PMP EH uses separate set of reset methods and this patch
implements standard prereset, hardreset and postreset methods.
This patch only implements PMP support. The next patch will integrate
PMP into the reset of libata and thus enable PMP support.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/Makefile | 3 +-
drivers/ata/libata-core.c | 11 +
drivers/ata/libata-pmp.c | 1182 +++++++++++++++++++++++++++++++++++++++++++++
drivers/ata/libata.h | 5 +
include/linux/libata.h | 19 +
5 files changed, 1219 insertions(+), 1 deletions(-)
create mode 100644 drivers/ata/libata-pmp.c
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 4eb7fb4..9b0c38e 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -70,5 +70,6 @@ obj-$(CONFIG_ATA_GENERIC) += ata_generic.o
# Should be last libata driver
obj-$(CONFIG_PATA_LEGACY) += pata_legacy.o
-libata-objs := libata-core.o libata-scsi.o libata-sff.o libata-eh.o
+libata-objs := libata-core.o libata-scsi.o libata-sff.o libata-eh.o \
+ libata-pmp.o
libata-$(CONFIG_ATA_ACPI) += libata-acpi.o
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e650fb9..23441e1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3863,6 +3863,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
{ "IOMEGA ZIP 250 ATAPI", NULL, ATA_HORKAGE_NODMA }, /* temporary fix */
{ "IOMEGA ZIP 250 ATAPI Floppy",
NULL, ATA_HORKAGE_NODMA },
+ /* Odd clown on sil3726/4726 PMPs */
+ { "Config Disk", NULL, ATA_HORKAGE_NODMA |
+ ATA_HORKAGE_SKIP_PM },
/* Weird ATAPI devices */
{ "TORiSAN DVD-ROM DRD-N216", NULL, ATA_HORKAGE_MAX_SEC_128 },
@@ -7242,6 +7245,14 @@ EXPORT_SYMBOL_GPL(ata_pci_default_filter);
EXPORT_SYMBOL_GPL(ata_pci_clear_simplex);
#endif /* CONFIG_PCI */
+EXPORT_SYMBOL_GPL(sata_pmp_read_init_tf);
+EXPORT_SYMBOL_GPL(sata_pmp_read_val);
+EXPORT_SYMBOL_GPL(sata_pmp_write_init_tf);
+EXPORT_SYMBOL_GPL(sata_pmp_std_prereset);
+EXPORT_SYMBOL_GPL(sata_pmp_std_hardreset);
+EXPORT_SYMBOL_GPL(sata_pmp_std_postreset);
+EXPORT_SYMBOL_GPL(sata_pmp_do_eh);
+
EXPORT_SYMBOL_GPL(__ata_ehi_push_desc);
EXPORT_SYMBOL_GPL(ata_ehi_push_desc);
EXPORT_SYMBOL_GPL(ata_ehi_clear_desc);
diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
new file mode 100644
index 0000000..671d171
--- /dev/null
+++ b/drivers/ata/libata-pmp.c
@@ -0,0 +1,1182 @@
+/*
+ * libata-pmp.c - libata port multiplier support
+ *
+ * Copyright (c) 2007 SUSE Linux Products GmbH
+ * Copyright (c) 2007 Tejun Heo <teheo@suse.de>
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/kernel.h>
+#include <linux/libata.h>
+#include "libata.h"
+
+/**
+ * sata_pmp_read - read PMP register
+ * @link: link to read PMP register for
+ * @reg: register to read
+ * @r_val: resulting value
+ *
+ * Wrapper around ap->ops->pmp_read to make it easier to call and
+ * nomarlize error return value.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+static int sata_pmp_read(struct ata_link *link, int reg, u32 *r_val)
+{
+ struct ata_port *ap = link->ap;
+ struct ata_device *pmp_dev = ap->link.device;
+ int rc;
+
+ might_sleep();
+
+ rc = ap->ops->pmp_read(pmp_dev, link->pmp, reg, r_val);
+ if (rc)
+ rc = -EIO;
+ return rc;
+}
+
+/**
+ * sata_pmp_write - write PMP register
+ * @link: link to write PMP register for
+ * @reg: register to write
+ * @r_val: value to write
+ *
+ * Wrapper around ap->ops->pmp_write to make it easier to call
+ * and nomarlize error return value.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+static int sata_pmp_write(struct ata_link *link, int reg, u32 val)
+{
+ struct ata_port *ap = link->ap;
+ struct ata_device *pmp_dev = ap->link.device;
+ int rc;
+
+ might_sleep();
+
+ rc = ap->ops->pmp_write(pmp_dev, link->pmp, reg, val);
+ if (rc)
+ rc = -EIO;
+ return rc;
+}
+
+/**
+ * sata_pmp_read_init_tf - initialize TF for PMP read
+ * @tf: taskfile to initialize
+ * @dev: PMP dev
+ * @pmp: port multiplier port number
+ * @reg: register to read
+ *
+ * Initialize @tf for PMP read command.
+ *
+ * LOCKING:
+ * None.
+ */
+void sata_pmp_read_init_tf(struct ata_taskfile *tf,
+ struct ata_device *dev, int pmp, int reg)
+{
+ ata_tf_init(dev, tf);
+ tf->command = ATA_CMD_PMP_READ;
+ tf->protocol = ATA_PROT_NODATA;
+ tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+ tf->feature = reg;
+ tf->device = pmp;
+}
+
+/**
+ * sata_pmp_read_val - extract PMP read result from TF
+ * @tf: target TF
+ *
+ * Determine PMP read result from @tf.
+ *
+ * LOCKING:
+ * None.
+ */
+u32 sata_pmp_read_val(const struct ata_taskfile *tf)
+{
+ return tf->nsect | tf->lbal << 8 | tf->lbam << 16 | tf->lbah << 24;
+}
+
+/**
+ * sata_pmp_read_init_tf - initialize TF for PMP write
+ * @tf: taskfile to initialize
+ * @dev: PMP dev
+ * @pmp: port multiplier port number
+ * @reg: register to read
+ * @val: value to write
+ *
+ * Initialize @tf for PMP write command.
+ *
+ * LOCKING:
+ * None.
+ */
+void sata_pmp_write_init_tf(struct ata_taskfile *tf,
+ struct ata_device *dev, int pmp, int reg, u32 val)
+{
+ ata_tf_init(dev, tf);
+ tf->command = ATA_CMD_PMP_WRITE;
+ tf->protocol = ATA_PROT_NODATA;
+ tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+ tf->feature = reg;
+ tf->device = pmp;
+ tf->nsect = val & 0xff;
+ tf->lbal = (val >> 8) & 0xff;
+ tf->lbam = (val >> 16) & 0xff;
+ tf->lbah = (val >> 24) & 0xff;
+}
+
+/**
+ * sata_pmp_scr_read - read PSCR
+ * @link: ATA link to read PSCR for
+ * @reg: PSCR to read
+ * @r_val: resulting value
+ *
+ * Read PSCR @reg into @r_val for @link, to be called from
+ * ata_scr_read().
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+int sata_pmp_scr_read(struct ata_link *link, int reg, u32 *r_val)
+{
+ if (reg > SATA_PMP_PSCR_CONTROL)
+ return -EINVAL;
+
+ return sata_pmp_read(link, reg, r_val);
+}
+
+/**
+ * sata_pmp_scr_write - write PSCR
+ * @link: ATA link to write PSCR for
+ * @reg: PSCR to write
+ * @val: value to be written
+ *
+ * Write @val to PSCR @reg for @link, to be called from
+ * ata_scr_write() and ata_scr_write_flush().
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+int sata_pmp_scr_write(struct ata_link *link, int reg, u32 val)
+{
+ if (reg > SATA_PMP_PSCR_CONTROL)
+ return -EINVAL;
+
+ return sata_pmp_write(link, reg, val);
+}
+
+/**
+ * sata_pmp_std_prereset - prepare PMP link for reset
+ * @link: link to be reset
+ * @deadline: deadline jiffies for the operation
+ *
+ * @link is about to be reset. Initialize it.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep)
+ *
+ * RETURNS:
+ * 0 on success, -errno otherwise.
+ */
+int sata_pmp_std_prereset(struct ata_link *link, unsigned long deadline)
+{
+ struct ata_eh_context *ehc = &link->eh_context;
+ const unsigned long *timing = sata_ehc_deb_timing(ehc);
+ int rc;
+
+ /* force HRST? */
+ if (link->flags & ATA_LFLAG_NO_SRST)
+ ehc->i.action |= ATA_EH_HARDRESET;
+
+ /* handle link resume */
+ if ((ehc->i.flags & ATA_EHI_RESUME_LINK) &&
+ (link->flags & ATA_LFLAG_HRST_TO_RESUME))
+ ehc->i.action |= ATA_EH_HARDRESET;
+
+ /* if we're about to do hardreset, nothing more to do */
+ if (ehc->i.action & ATA_EH_HARDRESET)
+ return 0;
+
+ /* resume link */
+ rc = sata_link_resume(link, timing, deadline);
+ if (rc) {
+ /* phy resume failed */
+ ata_link_printk(link, KERN_WARNING, "failed to resume link "
+ "for reset (errno=%d)\n", rc);
+ return rc;
+ }
+
+ /* clear SError bits including .X which blocks the port when set */
+ rc = sata_scr_write(link, SCR_ERROR, 0xffffffff);
+ if (rc) {
+ ata_link_printk(link, KERN_ERR,
+ "failed to clear SError (errno=%d)\n", rc);
+ return rc;
+ }
+
+ return 0;
+}
+
+/**
+ * sata_pmp_std_hardreset - standard hardreset method for PMP link
+ * @link: link to be reset
+ * @class: resulting class of attached device
+ * @deadline: deadline jiffies for the operation
+ *
+ * Hardreset PMP port @link. Note that this function doesn't
+ * wait for BSY clearance. There simply isn't a generic way to
+ * wait the event. Instead, this function return -EAGAIN thus
+ * telling libata-EH to followup with softreset.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep)
+ *
+ * RETURNS:
+ * 0 on success, -errno otherwise.
+ */
+int sata_pmp_std_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ const unsigned long *timing = sata_ehc_deb_timing(&link->eh_context);
+ u32 tmp;
+ int rc;
+
+ DPRINTK("ENTER\n");
+
+ /* do hardreset */
+ rc = sata_link_hardreset(link, timing, deadline);
+ if (rc) {
+ ata_link_printk(link, KERN_ERR,
+ "COMRESET failed (errno=%d)\n", rc);
+ goto out;
+ }
+
+ /* clear SError bits including .X which blocks the port when set */
+ rc = sata_scr_write(link, SCR_ERROR, 0xffffffff);
+ if (rc) {
+ ata_link_printk(link, KERN_ERR, "failed to clear SError "
+ "during hardreset (errno=%d)\n", rc);
+ goto out;
+ }
+
+ /* if device is present, follow up with srst to wait for !BSY */
+ if (ata_link_online(link))
+ rc = -EAGAIN;
+ out:
+ /* if SCR isn't accessible, we need to reset the PMP */
+ if (rc && rc != -EAGAIN && sata_scr_read(link, SCR_STATUS, &tmp))
+ rc = -ERESTART;
+
+ DPRINTK("EXIT, rc=%d\n", rc);
+ return rc;
+}
+
+/**
+ * ata_std_postreset - standard postreset method for PMP link
+ * @link: the target ata_link
+ * @classes: classes of attached devices
+ *
+ * This function is invoked after a successful reset. Note that
+ * the device might have been reset more than once using
+ * different reset methods before postreset is invoked.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep)
+ */
+void sata_pmp_std_postreset(struct ata_link *link, unsigned int *class)
+{
+ u32 serror;
+
+ DPRINTK("ENTER\n");
+
+ /* clear SError */
+ if (sata_scr_read(link, SCR_ERROR, &serror) == 0)
+ sata_scr_write(link, SCR_ERROR, serror);
+
+ /* print link status */
+ sata_print_link_status(link);
+
+ DPRINTK("EXIT\n");
+}
+
+/**
+ * sata_pmp_read_gscr - read GSCR block of SATA PMP
+ * @dev: PMP device
+ * @gscr: buffer to read GSCR block into
+ *
+ * Read selected PMP GSCRs from the PMP at @dev. This will serve
+ * as configuration and identification info for the PMP.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+static int sata_pmp_read_gscr(struct ata_device *dev, u32 *gscr)
+{
+ static const int gscr_to_read[] = { 0, 1, 2, 32, 33, 64, 96 };
+ int i, rc;
+
+ for (i = 0; i < ARRAY_SIZE(gscr_to_read); i++) {
+ int reg = gscr_to_read[i];
+
+ rc = sata_pmp_read(dev->link, reg, &gscr[reg]);
+ if (rc) {
+ ata_dev_printk(dev, KERN_ERR, "failed to read "
+ "PMP GSCR[%d] (errno=%d)\n", reg, rc);
+ return rc;
+ }
+ }
+
+ return 0;
+}
+
+static const char *sata_pmp_spec_rev_str(const u32 *gscr)
+{
+ u32 rev = gscr[SATA_PMP_GSCR_REV];
+
+ if (rev & (1 << 2))
+ return "1.1";
+ if (rev & (1 << 1))
+ return "1.0";
+ return "<unknown>";
+}
+
+static int sata_pmp_configure(struct ata_device *dev, int print_info)
+{
+ struct ata_port *ap = dev->link->ap;
+ u32 *gscr = dev->gscr;
+ const char *reason;
+ int nr_ports, rc;
+
+ nr_ports = sata_pmp_gscr_ports(gscr);
+
+ if (nr_ports <= 0 || nr_ports > SATA_PMP_MAX_PORTS) {
+ rc = -EINVAL;
+ reason = "invalid nr_ports";
+ goto fail;
+ }
+
+ if ((ap->flags & ATA_FLAG_AN) &&
+ (gscr[SATA_PMP_GSCR_FEAT] & SATA_PMP_FEAT_NOTIFY))
+ dev->flags |= ATA_DFLAG_AN;
+
+ /* monitor SERR_PHYRDY_CHG on fan-out ports */
+ rc = sata_pmp_write(dev->link, SATA_PMP_GSCR_ERROR_EN, SERR_PHYRDY_CHG);
+ if (rc) {
+ reason = "failed to write GSCR_ERROR_EN";
+ goto fail;
+ }
+
+ /* turn off notification till fan-out ports are reset and configured */
+ if (gscr[SATA_PMP_GSCR_FEAT_EN] & SATA_PMP_FEAT_NOTIFY) {
+ gscr[SATA_PMP_GSCR_FEAT_EN] &= ~SATA_PMP_FEAT_NOTIFY;
+
+ rc = sata_pmp_write(dev->link, SATA_PMP_GSCR_FEAT_EN,
+ gscr[SATA_PMP_GSCR_FEAT_EN]);
+ if (rc) {
+ reason = "failed to write GSCR_FEAT_EN";
+ goto fail;
+ }
+ }
+
+ if (print_info) {
+ ata_dev_printk(dev, KERN_INFO, "Port Multiplier %s, "
+ "0x%04x:0x%04x r%d, %d ports, feat 0x%x/0x%x\n",
+ sata_pmp_spec_rev_str(gscr),
+ sata_pmp_gscr_vendor(gscr),
+ sata_pmp_gscr_devid(gscr),
+ sata_pmp_gscr_rev(gscr),
+ nr_ports, gscr[SATA_PMP_GSCR_FEAT_EN],
+ gscr[SATA_PMP_GSCR_FEAT]);
+
+ if (!(dev->flags & ATA_DFLAG_AN))
+ ata_dev_printk(dev, KERN_INFO,
+ "Asynchronous notification not supported, "
+ "hotplug won't\n work on fan-out "
+ "ports. Use warm-plug instead.\n");
+ }
+
+ return 0;
+
+ fail:
+ ata_dev_printk(dev, KERN_ERR,
+ "failed to configure Port Multiplier (%s)\n", reason);
+ return rc;
+}
+
+static int sata_pmp_init_links(struct ata_port *ap, int nr_ports)
+{
+ struct ata_link *pmp_link = ap->pmp_link;
+ int i;
+
+ if (!pmp_link) {
+ pmp_link = kzalloc(sizeof(pmp_link[0]) * SATA_PMP_MAX_PORTS,
+ GFP_NOIO);
+ if (!pmp_link)
+ return -ENOMEM;
+
+ for (i = 0; i < SATA_PMP_MAX_PORTS; i++)
+ ata_link_init(ap, &pmp_link[i], i);
+
+ ap->pmp_link = pmp_link;
+ }
+
+ for (i = 0; i < nr_ports; i++) {
+ struct ata_link *link = &pmp_link[i];
+ struct ata_eh_context *ehc = &link->eh_context;
+
+ link->flags = 0;
+ ehc->i.probe_mask |= 1;
+ ehc->i.action |= ATA_EH_SOFTRESET;
+ ehc->i.flags |= ATA_EHI_RESUME_LINK;
+ }
+
+ return 0;
+}
+
+static void sata_pmp_quirks(struct ata_port *ap)
+{
+ u32 *gscr = ap->link.device->gscr;
+ u16 vendor = sata_pmp_gscr_vendor(gscr);
+ u16 devid = sata_pmp_gscr_devid(gscr);
+ struct ata_link *link;
+
+ if (vendor == 0x1095 && devid == 0x3726) {
+ /* sil3726 quirks */
+ ata_port_for_each_link(link, ap) {
+ /* SError.N need a kick in the ass to get working */
+ link->flags |= ATA_LFLAG_HRST_TO_RESUME;
+
+ /* class code report is unreliable */
+ if (link->pmp < 5)
+ link->flags |= ATA_LFLAG_ASSUME_ATA;
+
+ /* port 5 is for SEMB device and it doesn't like SRST */
+ if (link->pmp == 5)
+ link->flags |= ATA_LFLAG_NO_SRST |
+ ATA_LFLAG_ASSUME_SEMB;
+ }
+ } else if (vendor == 0x1095 && devid == 0x4723) {
+ /* sil4723 quirks */
+ ata_port_for_each_link(link, ap) {
+ /* SError.N need a kick in the ass to get working */
+ link->flags |= ATA_LFLAG_HRST_TO_RESUME;
+
+ /* class code report is unreliable */
+ if (link->pmp < 2)
+ link->flags |= ATA_LFLAG_ASSUME_ATA;
+
+ /* the config device at port 2 locks up on SRST */
+ if (link->pmp == 2)
+ link->flags |= ATA_LFLAG_NO_SRST |
+ ATA_LFLAG_ASSUME_ATA;
+ }
+ } else if (vendor == 0x1095 && devid == 0x4726) {
+ /* sil4726 quirks */
+ ata_port_for_each_link(link, ap) {
+ /* SError.N need a kick in the ass to get working */
+ link->flags |= ATA_LFLAG_HRST_TO_RESUME;
+
+ /* class code report is unreliable */
+ if (link->pmp < 5)
+ link->flags |= ATA_LFLAG_ASSUME_ATA;
+
+ /* The config device, which can be either at
+ * port 0 or 5, locks up on SRST.
+ */
+ if (link->pmp == 0 || link->pmp == 5)
+ link->flags |= ATA_LFLAG_NO_SRST |
+ ATA_LFLAG_ASSUME_ATA;
+
+ /* Port 6 is for SEMB device which doesn't
+ * like SRST either.
+ */
+ if (link->pmp == 6)
+ link->flags |= ATA_LFLAG_NO_SRST |
+ ATA_LFLAG_ASSUME_SEMB;
+ }
+ } else if (vendor == 0x1095 && (devid == 0x5723 || devid == 0x5733 ||
+ devid == 0x5734 || devid == 0x5744)) {
+ /* sil5723/5744 quirks */
+
+ /* sil5723/5744 has either two or three downstream
+ * ports depending on operation mode. The last port
+ * is empty if any actual IO device is available or
+ * occupied by a pseudo configuration device
+ * otherwise. Don't try hard to recover it.
+ */
+ ap->pmp_link[ap->nr_pmp_links - 1].flags |= ATA_LFLAG_NO_RETRY;
+ } else if (vendor == 0x11ab && devid == 0x4140) {
+ /* Marvell 88SM4140 quirks. Fan-out ports require PHY
+ * reset to work; other than that, it behaves very
+ * nicely.
+ */
+ ata_port_for_each_link(link, ap)
+ link->flags |= ATA_LFLAG_HRST_TO_RESUME;
+ }
+}
+
+/**
+ * sata_pmp_attach - attach a SATA PMP device
+ * @dev: SATA PMP device to attach
+ *
+ * Configure and attach SATA PMP device @dev. This function is
+ * also responsible for allocating and initializing PMP links.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+int sata_pmp_attach(struct ata_device *dev)
+{
+ struct ata_link *link = dev->link;
+ struct ata_port *ap = link->ap;
+ unsigned long flags;
+ struct ata_link *tlink;
+ int rc;
+
+ /* is it hanging off the right place? */
+ if (!(ap->flags & ATA_FLAG_PMP)) {
+ ata_dev_printk(dev, KERN_ERR,
+ "host does not support Port Multiplier\n");
+ return -EINVAL;
+ }
+
+ if (!ata_is_host_link(link)) {
+ ata_dev_printk(dev, KERN_ERR,
+ "Port Multipliers cannot be nested\n");
+ return -EINVAL;
+ }
+
+ if (dev->devno) {
+ ata_dev_printk(dev, KERN_ERR,
+ "Port Multiplier must be the first device\n");
+ return -EINVAL;
+ }
+
+ WARN_ON(link->pmp != 0);
+ link->pmp = SATA_PMP_CTRL_PORT;
+
+ /* read GSCR block */
+ rc = sata_pmp_read_gscr(dev, dev->gscr);
+ if (rc)
+ goto fail;
+
+ /* config PMP */
+ rc = sata_pmp_configure(dev, 1);
+ if (rc)
+ goto fail;
+
+ rc = sata_pmp_init_links(ap, sata_pmp_gscr_ports(dev->gscr));
+ if (rc) {
+ ata_dev_printk(dev, KERN_INFO,
+ "failed to initialize PMP links\n");
+ goto fail;
+ }
+
+ /* attach it */
+ spin_lock_irqsave(ap->lock, flags);
+ WARN_ON(ap->nr_pmp_links);
+ ap->nr_pmp_links = sata_pmp_gscr_ports(dev->gscr);
+ spin_unlock_irqrestore(ap->lock, flags);
+
+ sata_pmp_quirks(ap);
+
+ if (ap->ops->pmp_attach)
+ ap->ops->pmp_attach(ap);
+
+ ata_port_for_each_link(tlink, ap)
+ sata_link_init_spd(tlink);
+
+ return 0;
+
+ fail:
+ link->pmp = 0;
+ return rc;
+}
+
+/**
+ * sata_pmp_detach - detach a SATA PMP device
+ * @dev: SATA PMP device to detach
+ *
+ * Detach SATA PMP device @dev. This function is also
+ * responsible for deconfiguring PMP links.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ */
+static void sata_pmp_detach(struct ata_device *dev)
+{
+ struct ata_link *link = dev->link;
+ struct ata_port *ap = link->ap;
+ struct ata_link *tlink;
+ unsigned long flags;
+
+ ata_dev_printk(dev, KERN_INFO, "Port Multiplier detaching\n");
+
+ WARN_ON(!ata_is_host_link(link) || dev->devno ||
+ link->pmp != SATA_PMP_CTRL_PORT);
+
+ if (ap->ops->pmp_detach)
+ ap->ops->pmp_detach(ap);
+
+ ata_port_for_each_link(tlink, ap)
+ ata_eh_detach_dev(tlink->device);
+
+ spin_lock_irqsave(ap->lock, flags);
+ ap->nr_pmp_links = 0;
+ link->pmp = 0;
+ spin_unlock_irqrestore(ap->lock, flags);
+}
+
+/**
+ * sata_pmp_same_pmp - does new GSCR matches the configured PMP?
+ * @dev: PMP device to compare against
+ * @new_gscr: GSCR block of the new device
+ *
+ * Compare @new_gscr against @dev and determine whether @dev is
+ * the PMP described by @new_gscr.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * 1 if @dev matches @new_gscr, 0 otherwise.
+ */
+static int sata_pmp_same_pmp(struct ata_device *dev, const u32 *new_gscr)
+{
+ const u32 *old_gscr = dev->gscr;
+ u16 old_vendor, new_vendor, old_devid, new_devid;
+ int old_nr_ports, new_nr_ports;
+
+ old_vendor = sata_pmp_gscr_vendor(old_gscr);
+ new_vendor = sata_pmp_gscr_vendor(new_gscr);
+ old_devid = sata_pmp_gscr_devid(old_gscr);
+ new_devid = sata_pmp_gscr_devid(new_gscr);
+ old_nr_ports = sata_pmp_gscr_ports(old_gscr);
+ new_nr_ports = sata_pmp_gscr_ports(new_gscr);
+
+ if (old_vendor != new_vendor) {
+ ata_dev_printk(dev, KERN_INFO, "Port Multiplier "
+ "vendor mismatch '0x%x' != '0x%x'\n",
+ old_vendor, new_vendor);
+ return 0;
+ }
+
+ if (old_devid != new_devid) {
+ ata_dev_printk(dev, KERN_INFO, "Port Multiplier "
+ "device ID mismatch '0x%x' != '0x%x'\n",
+ old_devid, new_devid);
+ return 0;
+ }
+
+ if (old_nr_ports != new_nr_ports) {
+ ata_dev_printk(dev, KERN_INFO, "Port Multiplier "
+ "nr_ports mismatch '0x%x' != '0x%x'\n",
+ old_nr_ports, new_nr_ports);
+ return 0;
+ }
+
+ return 1;
+}
+
+/**
+ * sata_pmp_revalidate - revalidate SATA PMP
+ * @dev: PMP device to revalidate
+ * @new_class: new class code
+ *
+ * Re-read GSCR block and make sure @dev is still attached to the
+ * port and properly configured.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno otherwise.
+ */
+static int sata_pmp_revalidate(struct ata_device *dev, unsigned int new_class)
+{
+ struct ata_link *link = dev->link;
+ struct ata_port *ap = link->ap;
+ u32 *gscr = (void *)ap->sector_buf;
+ int rc;
+
+ DPRINTK("ENTER\n");
+
+ ata_eh_about_to_do(link, NULL, ATA_EH_REVALIDATE);
+
+ if (!ata_dev_enabled(dev)) {
+ rc = -ENODEV;
+ goto fail;
+ }
+
+ /* wrong class? */
+ if (ata_class_enabled(new_class) && new_class != ATA_DEV_PMP) {
+ rc = -ENODEV;
+ goto fail;
+ }
+
+ /* read GSCR */
+ rc = sata_pmp_read_gscr(dev, gscr);
+ if (rc)
+ goto fail;
+
+ /* is the pmp still there? */
+ if (!sata_pmp_same_pmp(dev, gscr)) {
+ rc = -ENODEV;
+ goto fail;
+ }
+
+ memcpy(dev->gscr, gscr, sizeof(gscr[0]) * SATA_PMP_GSCR_DWORDS);
+
+ rc = sata_pmp_configure(dev, 0);
+ if (rc)
+ goto fail;
+
+ ata_eh_done(link, NULL, ATA_EH_REVALIDATE);
+
+ DPRINTK("EXIT, rc=0\n");
+ return 0;
+
+ fail:
+ ata_dev_printk(dev, KERN_ERR,
+ "PMP revalidation failed (errno=%d)\n", rc);
+ DPRINTK("EXIT, rc=%d\n", rc);
+ return rc;
+}
+
+/**
+ * sata_pmp_revalidate_quick - revalidate SATA PMP quickly
+ * @dev: PMP device to revalidate
+ *
+ * Make sure the attached PMP is accessible.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno otherwise.
+ */
+static int sata_pmp_revalidate_quick(struct ata_device *dev)
+{
+ u32 prod_id;
+ int rc;
+
+ rc = sata_pmp_read(dev->link, SATA_PMP_GSCR_PROD_ID, &prod_id);
+ if (rc) {
+ ata_dev_printk(dev, KERN_ERR, "failed to read PMP product ID\n");
+ return rc;
+ }
+
+ if (prod_id != dev->gscr[SATA_PMP_GSCR_PROD_ID]) {
+ ata_dev_printk(dev, KERN_ERR, "PMP product ID mismatch\n");
+ /* something weird is going on, request full PMP recovery */
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * sata_pmp_eh_recover_pmp - recover PMP
+ * @ap: ATA port PMP is attached to
+ * @prereset: prereset method (can be NULL)
+ * @softreset: softreset method
+ * @hardreset: hardreset method
+ * @postreset: postreset method (can be NULL)
+ *
+ * Recover PMP attached to @ap. Recovery procedure is somewhat
+ * similar to that of ata_eh_recover() except that reset should
+ * always be performed in hard->soft sequence and recovery
+ * failure results in PMP detachment.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+static int sata_pmp_eh_recover_pmp(struct ata_port *ap,
+ ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
+ ata_reset_fn_t hardreset, ata_postreset_fn_t postreset)
+{
+ struct ata_link *link = &ap->link;
+ struct ata_eh_context *ehc = &link->eh_context;
+ struct ata_device *dev = link->device;
+ int tries = ATA_EH_PMP_TRIES;
+ int detach = 0, rc = 0;
+ int reval_failed = 0;
+
+ DPRINTK("ENTER\n");
+
+ if (dev->flags & ATA_DFLAG_DETACH) {
+ detach = 1;
+ goto fail;
+ }
+
+ retry:
+ ehc->classes[0] = ATA_DEV_UNKNOWN;
+
+ if (ehc->i.action & ATA_EH_RESET_MASK) {
+ struct ata_link *tlink;
+
+ ata_eh_freeze_port(ap);
+
+ /* reset */
+ ehc->i.action = ATA_EH_HARDRESET;
+ rc = ata_eh_reset(link, 0, prereset, softreset, hardreset,
+ postreset);
+ if (rc) {
+ ata_link_printk(link, KERN_ERR,
+ "failed to reset PMP, giving up\n");
+ goto fail;
+ }
+
+ ata_eh_thaw_port(ap);
+
+ /* PMP is reset, SErrors cannot be trusted, scan all */
+ ata_port_for_each_link(tlink, ap)
+ ata_ehi_schedule_probe(&tlink->eh_context.i);
+ }
+
+ /* If revalidation is requested, revalidate and reconfigure;
+ * otherwise, do quick revalidation.
+ */
+ if (ehc->i.action & ATA_EH_REVALIDATE)
+ rc = sata_pmp_revalidate(dev, ehc->classes[0]);
+ else
+ rc = sata_pmp_revalidate_quick(dev);
+
+ if (rc) {
+ tries--;
+
+ if (rc == -ENODEV) {
+ ehc->i.probe_mask |= 1;
+ detach = 1;
+ /* give it just two more chances */
+ tries = min(tries, 2);
+ }
+
+ if (tries) {
+ int sleep = ehc->i.flags & ATA_EHI_DID_RESET;
+
+ /* consecutive revalidation failures? speed down */
+ if (reval_failed)
+ sata_down_spd_limit(link);
+ else
+ reval_failed = 1;
+
+ ata_dev_printk(dev, KERN_WARNING,
+ "retrying hardreset%s\n",
+ sleep ? " in 5 secs" : "");
+ if (sleep)
+ ssleep(5);
+ ehc->i.action |= ATA_EH_HARDRESET;
+ goto retry;
+ } else {
+ ata_dev_printk(dev, KERN_ERR, "failed to recover PMP "
+ "after %d tries, giving up\n",
+ ATA_EH_PMP_TRIES);
+ goto fail;
+ }
+ }
+
+ /* okay, PMP resurrected */
+ ehc->i.flags = 0;
+
+ DPRINTK("EXIT, rc=0\n");
+ return 0;
+
+ fail:
+ sata_pmp_detach(dev);
+ if (detach)
+ ata_eh_detach_dev(dev);
+ else
+ ata_dev_disable(dev);
+
+ DPRINTK("EXIT, rc=%d\n", rc);
+ return rc;
+}
+
+static int sata_pmp_eh_handle_disabled_links(struct ata_port *ap)
+{
+ struct ata_link *link;
+ unsigned long flags;
+ int rc;
+
+ spin_lock_irqsave(ap->lock, flags);
+
+ ata_port_for_each_link(link, ap) {
+ if (!(link->flags & ATA_LFLAG_DISABLED))
+ continue;
+
+ spin_unlock_irqrestore(ap->lock, flags);
+
+ /* Some PMPs require hardreset sequence to get
+ * SError.N working.
+ */
+ if ((link->flags & ATA_LFLAG_HRST_TO_RESUME) &&
+ (link->eh_context.i.flags & ATA_EHI_RESUME_LINK))
+ sata_link_hardreset(link, sata_deb_timing_normal,
+ jiffies + ATA_TMOUT_INTERNAL_QUICK);
+
+ /* unconditionally clear SError.N */
+ rc = sata_scr_write(link, SCR_ERROR, SERR_PHYRDY_CHG);
+ if (rc) {
+ ata_link_printk(link, KERN_ERR, "failed to clear "
+ "SError.N (errno=%d)\n", rc);
+ return rc;
+ }
+
+ spin_lock_irqsave(ap->lock, flags);
+ }
+
+ spin_unlock_irqrestore(ap->lock, flags);
+
+ return 0;
+}
+
+static int sata_pmp_handle_link_fail(struct ata_link *link, int *link_tries)
+{
+ struct ata_port *ap = link->ap;
+ unsigned long flags;
+
+ if (link_tries[link->pmp] && --link_tries[link->pmp])
+ return 1;
+
+ /* disable this link */
+ if (!(link->flags & ATA_LFLAG_DISABLED)) {
+ ata_link_printk(link, KERN_WARNING,
+ "failed to recover link after %d tries, disabling\n",
+ ATA_EH_PMP_LINK_TRIES);
+
+ spin_lock_irqsave(ap->lock, flags);
+ link->flags |= ATA_LFLAG_DISABLED;
+ spin_unlock_irqrestore(ap->lock, flags);
+ }
+
+ ata_dev_disable(link->device);
+ link->eh_context.i.action = 0;
+
+ return 0;
+}
+
+/**
+ * sata_pmp_eh_recover - recover PMP-enabled port
+ * @ap: ATA port to recover
+ * @prereset: prereset method (can be NULL)
+ * @softreset: softreset method
+ * @hardreset: hardreset method
+ * @postreset: postreset method (can be NULL)
+ * @pmp_prereset: PMP prereset method (can be NULL)
+ * @pmp_softreset: PMP softreset method (can be NULL)
+ * @pmp_hardreset: PMP hardreset method (can be NULL)
+ * @pmp_postreset: PMP postreset method (can be NULL)
+ *
+ * Drive EH recovery operation for PMP enabled port @ap. This
+ * function recovers host and PMP ports with proper retrials and
+ * fallbacks. Actual recovery operations are performed using
+ * ata_eh_recover() and sata_pmp_eh_recover_pmp().
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+static int sata_pmp_eh_recover(struct ata_port *ap,
+ ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
+ ata_reset_fn_t hardreset, ata_postreset_fn_t postreset,
+ ata_prereset_fn_t pmp_prereset, ata_reset_fn_t pmp_softreset,
+ ata_reset_fn_t pmp_hardreset, ata_postreset_fn_t pmp_postreset)
+{
+ int pmp_tries, link_tries[SATA_PMP_MAX_PORTS];
+ struct ata_link *pmp_link = &ap->link;
+ struct ata_device *pmp_dev = pmp_link->device;
+ struct ata_eh_context *pmp_ehc = &pmp_link->eh_context;
+ struct ata_link *link;
+ struct ata_device *dev;
+ u32 gscr_error, sntf;
+ int cnt, rc;
+
+ pmp_tries = ATA_EH_PMP_TRIES;
+ ata_port_for_each_link(link, ap)
+ link_tries[link->pmp] = ATA_EH_PMP_LINK_TRIES;
+
+ retry:
+ /* PMP attached? */
+ if (!ap->nr_pmp_links) {
+ rc = ata_eh_recover(ap, prereset, softreset, hardreset,
+ postreset, NULL);
+ if (rc) {
+ ata_link_for_each_dev(dev, &ap->link)
+ ata_dev_disable(dev);
+ return rc;
+ }
+
+ if (pmp_dev->class != ATA_DEV_PMP)
+ return 0;
+
+ /* new PMP online */
+ ata_port_for_each_link(link, ap)
+ link_tries[link->pmp] = ATA_EH_PMP_LINK_TRIES;
+
+ /* fall through */
+ }
+
+ /* recover pmp */
+ rc = sata_pmp_eh_recover_pmp(ap, prereset, softreset, hardreset,
+ postreset);
+ if (rc)
+ goto pmp_fail;
+
+ /* handle disabled links */
+ rc = sata_pmp_eh_handle_disabled_links(ap);
+ if (rc)
+ goto pmp_fail;
+
+ /* recover links */
+ rc = ata_eh_recover(ap, pmp_prereset, pmp_softreset, pmp_hardreset,
+ pmp_postreset, &link);
+ if (rc)
+ goto link_fail;
+
+ /* Connection status might have changed while resetting other
+ * links, check SATA_PMP_GSCR_ERROR before returning.
+ */
+
+ /* clear SNotification */
+ rc = sata_scr_read(&ap->link, SCR_NOTIFICATION, &sntf);
+ if (rc == 0)
+ sata_scr_write(&ap->link, SCR_NOTIFICATION, sntf);
+
+ /* enable notification */
+ if (pmp_dev->flags & ATA_DFLAG_AN) {
+ pmp_dev->gscr[SATA_PMP_GSCR_FEAT_EN] |= SATA_PMP_FEAT_NOTIFY;
+
+ rc = sata_pmp_write(pmp_dev->link, SATA_PMP_GSCR_FEAT_EN,
+ pmp_dev->gscr[SATA_PMP_GSCR_FEAT_EN]);
+ if (rc) {
+ ata_dev_printk(pmp_dev, KERN_ERR,
+ "failed to write PMP_FEAT_EN\n");
+ goto pmp_fail;
+ }
+ }
+
+ /* check GSCR_ERROR */
+ rc = sata_pmp_read(pmp_link, SATA_PMP_GSCR_ERROR, &gscr_error);
+ if (rc) {
+ ata_dev_printk(pmp_dev, KERN_ERR,
+ "failed to read PMP_GSCR_ERROR\n");
+ goto pmp_fail;
+ }
+
+ cnt = 0;
+ ata_port_for_each_link(link, ap) {
+ if (!(gscr_error & (1 << link->pmp)))
+ continue;
+
+ if (sata_pmp_handle_link_fail(link, link_tries)) {
+ ata_ehi_hotplugged(&link->eh_context.i);
+ cnt++;
+ } else {
+ ata_link_printk(link, KERN_WARNING,
+ "PHY status changed but maxed out on retries, "
+ "giving up\n");
+ ata_link_printk(link, KERN_WARNING,
+ "Manully issue scan to resume this link\n");
+ }
+ }
+
+ if (cnt) {
+ ata_port_printk(ap, KERN_INFO, "PMP SError.N set for some "
+ "ports, repeating recovery\n");
+ goto retry;
+ }
+
+ return 0;
+
+ link_fail:
+ if (sata_pmp_handle_link_fail(link, link_tries)) {
+ pmp_ehc->i.action |= ATA_EH_HARDRESET;
+ goto retry;
+ }
+
+ /* fall through */
+ pmp_fail:
+ /* Control always ends up here after detaching PMP. Shut up
+ * and return if we're unloading.
+ */
+ if (ap->pflags & ATA_PFLAG_UNLOADING)
+ return rc;
+
+ if (!ap->nr_pmp_links)
+ goto retry;
+
+ if (--pmp_tries) {
+ ata_port_printk(ap, KERN_WARNING,
+ "failed to recover PMP, retrying in 5 secs\n");
+ pmp_ehc->i.action |= ATA_EH_HARDRESET;
+ ssleep(5);
+ goto retry;
+ }
+
+ ata_port_printk(ap, KERN_ERR,
+ "failed to recover PMP after %d tries, giving up\n",
+ ATA_EH_PMP_TRIES);
+ sata_pmp_detach(pmp_dev);
+ ata_dev_disable(pmp_dev);
+
+ return rc;
+}
+
+/**
+ * sata_pmp_do_eh - do standard error handling for PMP-enabled host
+ * @ap: host port to handle error for
+ * @prereset: prereset method (can be NULL)
+ * @softreset: softreset method
+ * @hardreset: hardreset method
+ * @postreset: postreset method (can be NULL)
+ * @pmp_prereset: PMP prereset method (can be NULL)
+ * @pmp_softreset: PMP softreset method (can be NULL)
+ * @pmp_hardreset: PMP hardreset method (can be NULL)
+ * @pmp_postreset: PMP postreset method (can be NULL)
+ *
+ * Perform standard error handling sequence for PMP-enabled host
+ * @ap.
+ *
+ * LOCKING:
+ * Kernel thread context (may sleep).
+ */
+void sata_pmp_do_eh(struct ata_port *ap,
+ ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
+ ata_reset_fn_t hardreset, ata_postreset_fn_t postreset,
+ ata_prereset_fn_t pmp_prereset, ata_reset_fn_t pmp_softreset,
+ ata_reset_fn_t pmp_hardreset, ata_postreset_fn_t pmp_postreset)
+{
+ ata_eh_autopsy(ap);
+ ata_eh_report(ap);
+ sata_pmp_eh_recover(ap, prereset, softreset, hardreset, postreset,
+ pmp_prereset, pmp_softreset, pmp_hardreset,
+ pmp_postreset);
+ ata_eh_finish(ap);
+}
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index fc8a786..2ae92bd 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -152,6 +152,11 @@ extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);
extern void ata_scsi_dev_rescan(struct work_struct *work);
extern int ata_bus_probe(struct ata_port *ap);
+/* libata-pmp.c */
+extern int sata_pmp_scr_read(struct ata_link *link, int reg, u32 *val);
+extern int sata_pmp_scr_write(struct ata_link *link, int reg, u32 val);
+extern int sata_pmp_attach(struct ata_device *dev);
+
/* libata-eh.c */
extern enum scsi_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd);
extern void ata_scsi_error(struct Scsi_Host *host);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index cd9c2a2..19a962c 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -944,6 +944,25 @@ extern unsigned long ata_pci_default_filter(struct ata_device *, unsigned long);
#endif /* CONFIG_PCI */
/*
+ * PMP
+ */
+extern void sata_pmp_read_init_tf(struct ata_taskfile *tf,
+ struct ata_device *dev, int pmp, int reg);
+extern u32 sata_pmp_read_val(const struct ata_taskfile *tf);
+extern void sata_pmp_write_init_tf(struct ata_taskfile *tf,
+ struct ata_device *dev,
+ int pmp, int reg, u32 val);
+extern int sata_pmp_std_prereset(struct ata_link *link, unsigned long deadline);
+extern int sata_pmp_std_hardreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline);
+extern void sata_pmp_std_postreset(struct ata_link *link, unsigned int *class);
+extern void sata_pmp_do_eh(struct ata_port *ap,
+ ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
+ ata_reset_fn_t hardreset, ata_postreset_fn_t postreset,
+ ata_prereset_fn_t pmp_prereset, ata_reset_fn_t pmp_softreset,
+ ata_reset_fn_t pmp_hardreset, ata_postreset_fn_t pmp_postreset);
+
+/*
* EH
*/
extern void ata_eng_timeout(struct ata_port *ap);
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 03/10] libata-pmp: hook PMP support and enable it
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (3 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 02/10] libata-pmp: implement Port Multiplier support Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 04/10] libata-pmp: extend ACPI support to cover PMP Tejun Heo
` (5 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Hook PMP support into libata and enable it. Connect SCR and probing
functions, and update ata_dev_classify() to detect PMP.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/libata-core.c | 113 ++++++++++++++++++++++++++++++++-------------
drivers/ata/libata-eh.c | 18 ++++++-
2 files changed, 96 insertions(+), 35 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 23441e1..3b46678 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -59,7 +59,7 @@
#include "libata.h"
-#define DRV_VERSION "2.21" /* must be exactly four chars */
+#define DRV_VERSION "3.00" /* must be exactly four chars */
/* debounce timing parameters in msecs { interval, duration, timeout } */
@@ -672,29 +672,49 @@ static unsigned int ata_devchk(struct ata_port *ap, unsigned int device)
* None.
*
* RETURNS:
- * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, or %ATA_DEV_UNKNOWN
- * the event of failure.
+ * Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
+ * %ATA_DEV_UNKNOWN the event of failure.
*/
-
unsigned int ata_dev_classify(const struct ata_taskfile *tf)
{
/* Apple's open source Darwin code hints that some devices only
* put a proper signature into the LBA mid/high registers,
* So, we only check those. It's sufficient for uniqueness.
+ *
+ * ATA/ATAPI-7 (d1532v1r1: Feb. 19, 2003) specified separate
+ * signatures for ATA and ATAPI devices attached on SerialATA,
+ * 0x3c/0xc3 and 0x69/0x96 respectively. However, SerialATA
+ * spec has never mentioned about using different signatures
+ * for ATA/ATAPI devices. Then, Serial ATA II: Port
+ * Multiplier specification began to use 0x69/0x96 to identify
+ * port multpliers and 0x3c/0xc3 to identify SEMB device.
+ * ATA/ATAPI-7 dropped descriptions about 0x3c/0xc3 and
+ * 0x69/0x96 shortly and described them as reserved for
+ * SerialATA.
+ *
+ * We follow the current spec and consider that 0x69/0x96
+ * identifies a port multiplier and 0x3c/0xc3 a SEMB device.
*/
-
- if (((tf->lbam == 0) && (tf->lbah == 0)) ||
- ((tf->lbam == 0x3c) && (tf->lbah == 0xc3))) {
+ if ((tf->lbam == 0) && (tf->lbah == 0)) {
DPRINTK("found ATA device by sig\n");
return ATA_DEV_ATA;
}
- if (((tf->lbam == 0x14) && (tf->lbah == 0xeb)) ||
- ((tf->lbam == 0x69) && (tf->lbah == 0x96))) {
+ if ((tf->lbam == 0x14) && (tf->lbah == 0xeb)) {
DPRINTK("found ATAPI device by sig\n");
return ATA_DEV_ATAPI;
}
+ if ((tf->lbam == 0x69) && (tf->lbah == 0x96)) {
+ DPRINTK("found PMP device by sig\n");
+ return ATA_DEV_PMP;
+ }
+
+ if ((tf->lbam == 0x3c) && (tf->lbah == 0xc3)) {
+ printk("ata: SEMB device ignored\n");
+ return ATA_DEV_SEMB_UNSUP; /* not yet */
+ }
+
DPRINTK("unknown device\n");
return ATA_DEV_UNKNOWN;
}
@@ -3424,6 +3444,12 @@ int ata_std_prereset(struct ata_link *link, unsigned long deadline)
(link->flags & ATA_LFLAG_HRST_TO_RESUME))
ehc->i.action |= ATA_EH_HARDRESET;
+ /* Some PMPs don't work with only SRST, force hardreset if PMP
+ * is supported.
+ */
+ if (ap->flags & ATA_FLAG_PMP)
+ ehc->i.action |= ATA_EH_HARDRESET;
+
/* if we're about to do hardreset, nothing more to do */
if (ehc->i.action & ATA_EH_HARDRESET)
return 0;
@@ -3614,6 +3640,16 @@ int sata_std_hardreset(struct ata_link *link, unsigned int *class,
/* wait a while before checking status, see SRST for more info */
msleep(150);
+ /* If PMP is supported, we have to do follow-up SRST. Note
+ * that some PMPs don't send D2H Reg FIS after hardreset at
+ * all if the first port is empty. Wait for it just for a
+ * second and request follow-up SRST.
+ */
+ if (ap->flags & ATA_FLAG_PMP) {
+ ata_wait_ready(ap, jiffies + HZ);
+ return -EAGAIN;
+ }
+
rc = ata_wait_ready(ap, deadline);
/* link occupied, -ENODEV too is an error */
if (rc) {
@@ -5957,22 +5993,26 @@ int sata_scr_valid(struct ata_link *link)
* @val: Place to store read value
*
* Read SCR register @reg of @link into *@val. This function is
- * guaranteed to succeed if the cable type of the port is SATA
- * and the port implements ->scr_read.
+ * guaranteed to succeed if @link is ap->link, the cable type of
+ * the port is SATA and the port implements ->scr_read.
*
* LOCKING:
- * None.
+ * None if @link is ap->link. Kernel thread context otherwise.
*
* RETURNS:
* 0 on success, negative errno on failure.
*/
int sata_scr_read(struct ata_link *link, int reg, u32 *val)
{
- struct ata_port *ap = link->ap;
+ if (ata_is_host_link(link)) {
+ struct ata_port *ap = link->ap;
- if (sata_scr_valid(link))
- return ap->ops->scr_read(ap, reg, val);
- return -EOPNOTSUPP;
+ if (sata_scr_valid(link))
+ return ap->ops->scr_read(ap, reg, val);
+ return -EOPNOTSUPP;
+ }
+
+ return sata_pmp_scr_read(link, reg, val);
}
/**
@@ -5982,22 +6022,26 @@ int sata_scr_read(struct ata_link *link, int reg, u32 *val)
* @val: value to write
*
* Write @val to SCR register @reg of @link. This function is
- * guaranteed to succeed if the cable type of the port is SATA
- * and the port implements ->scr_read.
+ * guaranteed to succeed if @link is ap->link, the cable type of
+ * the port is SATA and the port implements ->scr_read.
*
* LOCKING:
- * None.
+ * None if @link is ap->link. Kernel thread context otherwise.
*
* RETURNS:
* 0 on success, negative errno on failure.
*/
int sata_scr_write(struct ata_link *link, int reg, u32 val)
{
- struct ata_port *ap = link->ap;
+ if (ata_is_host_link(link)) {
+ struct ata_port *ap = link->ap;
- if (sata_scr_valid(link))
- return ap->ops->scr_write(ap, reg, val);
- return -EOPNOTSUPP;
+ if (sata_scr_valid(link))
+ return ap->ops->scr_write(ap, reg, val);
+ return -EOPNOTSUPP;
+ }
+
+ return sata_pmp_scr_write(link, reg, val);
}
/**
@@ -6010,23 +6054,27 @@ int sata_scr_write(struct ata_link *link, int reg, u32 val)
* function performs flush after writing to the register.
*
* LOCKING:
- * None.
+ * None if @link is ap->link. Kernel thread context otherwise.
*
* RETURNS:
* 0 on success, negative errno on failure.
*/
int sata_scr_write_flush(struct ata_link *link, int reg, u32 val)
{
- struct ata_port *ap = link->ap;
- int rc;
+ if (ata_is_host_link(link)) {
+ struct ata_port *ap = link->ap;
+ int rc;
- if (sata_scr_valid(link)) {
- rc = ap->ops->scr_write(ap, reg, val);
- if (rc == 0)
- rc = ap->ops->scr_read(ap, reg, &val);
- return rc;
+ if (sata_scr_valid(link)) {
+ rc = ap->ops->scr_write(ap, reg, val);
+ if (rc == 0)
+ rc = ap->ops->scr_read(ap, reg, &val);
+ return rc;
+ }
+ return -EOPNOTSUPP;
}
- return -EOPNOTSUPP;
+
+ return sata_pmp_scr_write(link, reg, val);
}
/**
@@ -6415,6 +6463,7 @@ static void ata_host_release(struct device *gendev, void *res)
if (ap->scsi_host)
scsi_host_put(ap->scsi_host);
+ kfree(ap->pmp_link);
kfree(ap);
host->ports[i] = NULL;
}
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 687419b..5f2c0f3 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2209,6 +2209,8 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
readid_flags |= ATA_READID_POSTRESET;
if ((action & ATA_EH_REVALIDATE) && ata_dev_enabled(dev)) {
+ WARN_ON(dev->class == ATA_DEV_PMP);
+
if (ata_link_offline(link)) {
rc = -EIO;
goto err;
@@ -2234,8 +2236,11 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
ata_class_enabled(ehc->classes[dev->devno])) {
dev->class = ehc->classes[dev->devno];
- rc = ata_dev_read_id(dev, &dev->class, readid_flags,
- dev->id);
+ if (dev->class == ATA_DEV_PMP)
+ rc = sata_pmp_attach(dev);
+ else
+ rc = ata_dev_read_id(dev, &dev->class,
+ readid_flags, dev->id);
switch (rc) {
case 0:
new_mask |= 1 << dev->devno;
@@ -2264,7 +2269,8 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
* device detection messages backwards.
*/
ata_link_for_each_dev(dev, link) {
- if (!(new_mask & (1 << dev->devno)))
+ if (!(new_mask & (1 << dev->devno)) ||
+ dev->class == ATA_DEV_PMP)
continue;
ehc->i.flags |= ATA_EHI_PRINTINFO;
@@ -2521,6 +2527,12 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
if (rc)
goto dev_fail;
+ /* if PMP got attached, return, pmp EH will take care of it */
+ if (link->device->class == ATA_DEV_PMP) {
+ ehc->i.action = 0;
+ return 0;
+ }
+
/* configure transfer mode if necessary */
if (ehc->i.flags & ATA_EHI_SETMODE) {
rc = ata_set_mode(link, &dev);
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 04/10] libata-pmp: extend ACPI support to cover PMP
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (4 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 03/10] libata-pmp: hook PMP support and enable it Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 08/10] ahci: implement PMP support Tejun Heo
` (4 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Extend ata_acpi_associate_sata_port() such that it can handle PMP and
call it when PMP is attached and detached.
Build breakage when !CONFIG_ATA_ACPI was spotted and fixed by Petr
Vandrovec.
Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Petr Vandrovec <petr@vandrovec.name>
---
drivers/ata/libata-acpi.c | 36 ++++++++++++++++++++++++++++++++----
drivers/ata/libata-pmp.c | 4 ++++
drivers/ata/libata.h | 2 ++
3 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index dc9842e..a276c06 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -40,12 +40,40 @@ static int is_pci_dev(struct device *dev)
return (dev->bus == &pci_bus_type);
}
-static void ata_acpi_associate_sata_port(struct ata_port *ap)
+/**
+ * ata_acpi_associate_sata_port - associate SATA port with ACPI objects
+ * @ap: target SATA port
+ *
+ * Look up ACPI objects associated with @ap and initialize acpi_handle
+ * fields of @ap, the port and devices accordingly.
+ *
+ * LOCKING:
+ * EH context.
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+void ata_acpi_associate_sata_port(struct ata_port *ap)
{
- acpi_integer adr = SATA_ADR(ap->port_no, NO_PORT_MULT);
+ WARN_ON(!(ap->flags & ATA_FLAG_ACPI_SATA));
+
+ if (!ap->nr_pmp_links) {
+ acpi_integer adr = SATA_ADR(ap->port_no, NO_PORT_MULT);
+
+ ap->link.device->acpi_handle =
+ acpi_get_child(ap->host->acpi_handle, adr);
+ } else {
+ struct ata_link *link;
+
+ ap->link.device->acpi_handle = NULL;
- ap->link.device->acpi_handle =
- acpi_get_child(ap->host->acpi_handle, adr);
+ ata_port_for_each_link(link, ap) {
+ acpi_integer adr = SATA_ADR(ap->port_no, link->pmp);
+
+ link->device->acpi_handle =
+ acpi_get_child(ap->host->acpi_handle, adr);
+ }
+ }
}
static void ata_acpi_associate_ide_port(struct ata_port *ap)
diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index 671d171..eeffce6 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -607,6 +607,8 @@ int sata_pmp_attach(struct ata_device *dev)
ata_port_for_each_link(tlink, ap)
sata_link_init_spd(tlink);
+ ata_acpi_associate_sata_port(ap);
+
return 0;
fail:
@@ -646,6 +648,8 @@ static void sata_pmp_detach(struct ata_device *dev)
ap->nr_pmp_links = 0;
link->pmp = 0;
spin_unlock_irqrestore(ap->lock, flags);
+
+ ata_acpi_associate_sata_port(ap);
}
/**
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 2ae92bd..b57f8c0 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -102,11 +102,13 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);
/* libata-acpi.c */
#ifdef CONFIG_ATA_ACPI
+extern void ata_acpi_associate_sata_port(struct ata_port *ap);
extern void ata_acpi_associate(struct ata_host *host);
extern int ata_acpi_on_suspend(struct ata_port *ap);
extern void ata_acpi_on_resume(struct ata_port *ap);
extern int ata_acpi_on_devcfg(struct ata_device *adev);
#else
+static inline void ata_acpi_associate_sata_port(struct ata_port *ap) { }
static inline void ata_acpi_associate(struct ata_host *host) { }
static inline int ata_acpi_on_suspend(struct ata_port *ap) { return 0; }
static inline void ata_acpi_on_resume(struct ata_port *ap) { }
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 08/10] ahci: implement PMP support
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (5 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 04/10] libata-pmp: extend ACPI support to cover PMP Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 05/10] libata-pmp: implement qc_defer for command switching " Tejun Heo
` (3 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Implement AHCI PMP support. ahci only supports command based
switching. Also, for some reason, NCQ over PMP doesn't work now.
Other than that, everything works.
Tested on ICH9R, JMB360/363 + SIMG3726, 4726 and 5744.
Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Forrest Zhao <forrest.zhao@gmail.com>
---
drivers/ata/ahci.c | 220 +++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 174 insertions(+), 46 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 9f3c591..d06a8b3 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -46,7 +46,7 @@
#include <linux/libata.h>
#define DRV_NAME "ahci"
-#define DRV_VERSION "2.3"
+#define DRV_VERSION "3.0"
enum {
@@ -96,6 +96,7 @@ enum {
/* HOST_CAP bits */
HOST_CAP_SSC = (1 << 14), /* Slumber capable */
+ HOST_CAP_PMP = (1 << 17), /* Port Multiplier support */
HOST_CAP_CLO = (1 << 24), /* Command List Override support */
HOST_CAP_SSS = (1 << 27), /* Staggered Spin-up */
HOST_CAP_SNTF = (1 << 29), /* SNotification register */
@@ -143,7 +144,8 @@ enum {
PORT_IRQ_IF_ERR |
PORT_IRQ_CONNECT |
PORT_IRQ_PHYRDY |
- PORT_IRQ_UNK_FIS,
+ PORT_IRQ_UNK_FIS |
+ PORT_IRQ_BAD_PMP,
PORT_IRQ_ERROR = PORT_IRQ_FREEZE |
PORT_IRQ_TF_ERR |
PORT_IRQ_HBUS_DATA_ERR,
@@ -153,6 +155,7 @@ enum {
/* PORT_CMD bits */
PORT_CMD_ATAPI = (1 << 24), /* Device is ATAPI */
+ PORT_CMD_PMP = (1 << 17), /* PMP attached */
PORT_CMD_LIST_ON = (1 << 15), /* cmd list DMA engine running */
PORT_CMD_FIS_ON = (1 << 14), /* FIS DMA engine running */
PORT_CMD_FIS_RX = (1 << 4), /* Enable FIS receive DMA engine */
@@ -204,6 +207,7 @@ struct ahci_host_priv {
};
struct ahci_port_priv {
+ struct ata_link *active_link;
struct ahci_cmd_hdr *cmd_slot;
dma_addr_t cmd_slot_dma;
void *cmd_tbl;
@@ -229,6 +233,10 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc);
static u8 ahci_check_status(struct ata_port *ap);
static void ahci_freeze(struct ata_port *ap);
static void ahci_thaw(struct ata_port *ap);
+static void ahci_pmp_attach(struct ata_port *ap);
+static void ahci_pmp_detach(struct ata_port *ap);
+static int ahci_pmp_read(struct ata_device *dev, int pmp, int reg, u32 *r_val);
+static int ahci_pmp_write(struct ata_device *dev, int pmp, int reg, u32 val);
static void ahci_error_handler(struct ata_port *ap);
static void ahci_vt8251_error_handler(struct ata_port *ap);
static void ahci_post_internal_cmd(struct ata_queued_cmd *qc);
@@ -268,7 +276,7 @@ static const struct ata_port_operations ahci_ops = {
.tf_read = ahci_tf_read,
- .qc_defer = ata_std_qc_defer,
+ .qc_defer = sata_pmp_qc_defer_cmd_switch,
.qc_prep = ahci_qc_prep,
.qc_issue = ahci_qc_issue,
@@ -283,6 +291,11 @@ static const struct ata_port_operations ahci_ops = {
.error_handler = ahci_error_handler,
.post_internal_cmd = ahci_post_internal_cmd,
+ .pmp_attach = ahci_pmp_attach,
+ .pmp_detach = ahci_pmp_detach,
+ .pmp_read = ahci_pmp_read,
+ .pmp_write = ahci_pmp_write,
+
#ifdef CONFIG_PM
.port_suspend = ahci_port_suspend,
.port_resume = ahci_port_resume,
@@ -299,7 +312,7 @@ static const struct ata_port_operations ahci_vt8251_ops = {
.tf_read = ahci_tf_read,
- .qc_defer = ata_std_qc_defer,
+ .qc_defer = sata_pmp_qc_defer_cmd_switch,
.qc_prep = ahci_qc_prep,
.qc_issue = ahci_qc_issue,
@@ -314,6 +327,11 @@ static const struct ata_port_operations ahci_vt8251_ops = {
.error_handler = ahci_vt8251_error_handler,
.post_internal_cmd = ahci_post_internal_cmd,
+ .pmp_attach = ahci_pmp_attach,
+ .pmp_detach = ahci_pmp_detach,
+ .pmp_read = ahci_pmp_read,
+ .pmp_write = ahci_pmp_write,
+
#ifdef CONFIG_PM
.port_suspend = ahci_port_suspend,
.port_resume = ahci_port_resume,
@@ -1108,7 +1126,12 @@ static int ahci_do_softreset(struct ata_link *link, unsigned int *class,
static int ahci_softreset(struct ata_link *link, unsigned int *class,
unsigned long deadline)
{
- return ahci_do_softreset(link, class, 0, deadline);
+ int pmp = 0;
+
+ if (link->ap->flags & ATA_FLAG_PMP)
+ pmp = SATA_PMP_CTRL_PORT;
+
+ return ahci_do_softreset(link, class, pmp, deadline);
}
static int ahci_hardreset(struct ata_link *link, unsigned int *class,
@@ -1135,7 +1158,7 @@ static int ahci_hardreset(struct ata_link *link, unsigned int *class,
if (rc == 0 && ata_link_online(link))
*class = ahci_dev_classify(ap);
- if (*class == ATA_DEV_UNKNOWN)
+ if (rc != -EAGAIN && *class == ATA_DEV_UNKNOWN)
*class = ATA_DEV_NONE;
DPRINTK("EXIT, rc=%d, class=%u\n", rc, *class);
@@ -1190,6 +1213,12 @@ static void ahci_postreset(struct ata_link *link, unsigned int *class)
}
}
+static int ahci_pmp_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ return ahci_do_softreset(link, class, link->pmp, deadline);
+}
+
static u8 ahci_check_status(struct ata_port *ap)
{
void __iomem *mmio = ap->ioaddr.cmd_addr;
@@ -1248,7 +1277,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)
*/
cmd_tbl = pp->cmd_tbl + qc->tag * AHCI_CMD_TBL_SZ;
- ata_tf_to_fis(&qc->tf, 0, 1, cmd_tbl);
+ ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, cmd_tbl);
if (is_atapi) {
memset(cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32);
memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len);
@@ -1261,7 +1290,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)
/*
* Fill in command slot information.
*/
- opts = cmd_fis_len | n_elem << 16;
+ opts = cmd_fis_len | n_elem << 16 | (qc->dev->link->pmp << 12);
if (qc->tf.flags & ATA_TFLAG_WRITE)
opts |= AHCI_CMD_WRITE;
if (is_atapi)
@@ -1273,65 +1302,85 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)
static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
{
struct ahci_port_priv *pp = ap->private_data;
- struct ata_eh_info *ehi = &ap->link.eh_info;
- unsigned int err_mask = 0, action = 0;
- struct ata_queued_cmd *qc;
+ struct ata_eh_info *host_ehi = &ap->link.eh_info;
+ struct ata_link *link = NULL;
+ struct ata_queued_cmd *active_qc;
+ struct ata_eh_info *active_ehi;
u32 serror;
- ata_ehi_clear_desc(ehi);
+ /* determine active link */
+ ata_port_for_each_link(link, ap)
+ if (ata_link_active(link))
+ break;
+ if (!link)
+ link = &ap->link;
+
+ active_qc = ata_qc_from_tag(ap, link->active_tag);
+ active_ehi = &link->eh_info;
+
+ /* record irq stat */
+ ata_ehi_clear_desc(host_ehi);
+ ata_ehi_push_desc(host_ehi, "irq_stat 0x%08x", irq_stat);
/* AHCI needs SError cleared; otherwise, it might lock up */
ahci_scr_read(ap, SCR_ERROR, &serror);
ahci_scr_write(ap, SCR_ERROR, serror);
-
- /* analyze @irq_stat */
- ata_ehi_push_desc(ehi, "irq_stat 0x%08x", irq_stat);
+ host_ehi->serror |= serror;
/* some controllers set IRQ_IF_ERR on device errors, ignore it */
if (ap->flags & AHCI_FLAG_IGN_IRQ_IF_ERR)
irq_stat &= ~PORT_IRQ_IF_ERR;
if (irq_stat & PORT_IRQ_TF_ERR) {
- err_mask |= AC_ERR_DEV;
+ /* If qc is active, charge it; otherwise, the active
+ * link. There's no active qc on NCQ errors. It will
+ * be determined by EH by reading log page 10h.
+ */
+ if (active_qc)
+ active_qc->err_mask |= AC_ERR_DEV;
+ else
+ active_ehi->err_mask |= AC_ERR_DEV;
+
if (ap->flags & AHCI_FLAG_IGN_SERR_INTERNAL)
- serror &= ~SERR_INTERNAL;
+ host_ehi->serror &= ~SERR_INTERNAL;
+ }
+
+ if (irq_stat & PORT_IRQ_UNK_FIS) {
+ u32 *unk = (u32 *)(pp->rx_fis + RX_FIS_UNK);
+
+ active_ehi->err_mask |= AC_ERR_HSM;
+ active_ehi->action |= ATA_EH_SOFTRESET;
+ ata_ehi_push_desc(active_ehi,
+ "unknown FIS %08x %08x %08x %08x" ,
+ unk[0], unk[1], unk[2], unk[3]);
+ }
+
+ if (ap->nr_pmp_links && (irq_stat & PORT_IRQ_BAD_PMP)) {
+ active_ehi->err_mask |= AC_ERR_HSM;
+ active_ehi->action |= ATA_EH_SOFTRESET;
+ ata_ehi_push_desc(active_ehi, "incorrect PMP");
}
if (irq_stat & (PORT_IRQ_HBUS_ERR | PORT_IRQ_HBUS_DATA_ERR)) {
- err_mask |= AC_ERR_HOST_BUS;
- action |= ATA_EH_SOFTRESET;
+ host_ehi->err_mask |= AC_ERR_HOST_BUS;
+ host_ehi->action |= ATA_EH_SOFTRESET;
+ ata_ehi_push_desc(host_ehi, "host bus error");
}
if (irq_stat & PORT_IRQ_IF_ERR) {
- err_mask |= AC_ERR_ATA_BUS;
- action |= ATA_EH_SOFTRESET;
- ata_ehi_push_desc(ehi, "interface fatal error");
+ host_ehi->err_mask |= AC_ERR_ATA_BUS;
+ host_ehi->action |= ATA_EH_SOFTRESET;
+ ata_ehi_push_desc(host_ehi, "interface fatal error");
}
if (irq_stat & (PORT_IRQ_CONNECT | PORT_IRQ_PHYRDY)) {
- ata_ehi_hotplugged(ehi);
- ata_ehi_push_desc(ehi, "%s", irq_stat & PORT_IRQ_CONNECT ?
+ ata_ehi_hotplugged(host_ehi);
+ ata_ehi_push_desc(host_ehi, "%s",
+ irq_stat & PORT_IRQ_CONNECT ?
"connection status changed" : "PHY RDY changed");
}
- if (irq_stat & PORT_IRQ_UNK_FIS) {
- u32 *unk = (u32 *)(pp->rx_fis + RX_FIS_UNK);
-
- err_mask |= AC_ERR_HSM;
- action |= ATA_EH_SOFTRESET;
- ata_ehi_push_desc(ehi, "unknown FIS %08x %08x %08x %08x",
- unk[0], unk[1], unk[2], unk[3]);
- }
-
/* okay, let's hand over to EH */
- ehi->serror |= serror;
- ehi->action |= action;
-
- qc = ata_qc_from_tag(ap, ap->link.active_tag);
- if (qc)
- qc->err_mask |= err_mask;
- else
- ehi->err_mask |= err_mask;
if (irq_stat & PORT_IRQ_FREEZE)
ata_port_freeze(ap);
@@ -1369,7 +1418,8 @@ static void ahci_port_intr(struct ata_port *ap)
sata_async_notification(ap);
}
- if (ap->link.sactive)
+ /* pp->active_link is valid iff any command is in flight */
+ if (ap->qc_active && pp->active_link->sactive)
qc_active = readl(port_mmio + PORT_SCR_ACT);
else
qc_active = readl(port_mmio + PORT_CMD_ISSUE);
@@ -1507,6 +1557,13 @@ static unsigned int ahci_qc_issue(struct ata_queued_cmd *qc)
{
struct ata_port *ap = qc->ap;
void __iomem *port_mmio = ahci_port_base(ap);
+ struct ahci_port_priv *pp = ap->private_data;
+
+ /* Keep track of the currently active link. It will be used
+ * in completion path to determine whether NCQ phase is in
+ * progress.
+ */
+ pp->active_link = qc->dev->link;
if (qc->tf.protocol == ATA_PROT_NCQ)
writel(1 << qc->tag, port_mmio + PORT_SCR_ACT);
@@ -1536,8 +1593,11 @@ static void ahci_thaw(struct ata_port *ap)
writel(tmp, port_mmio + PORT_IRQ_STAT);
writel(1 << ap->port_no, mmio + HOST_IRQ_STAT);
- /* turn IRQ back on */
- writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
+ /* turn IRQ back on, ignore BAD_PMP if PMP isn't attached */
+ tmp = pp->intr_mask;
+ if (!ap->nr_pmp_links)
+ tmp &= ~PORT_IRQ_BAD_PMP;
+ writel(tmp, port_mmio + PORT_IRQ_MASK);
}
static void ahci_error_handler(struct ata_port *ap)
@@ -1549,8 +1609,10 @@ static void ahci_error_handler(struct ata_port *ap)
}
/* perform recovery */
- ata_do_eh(ap, ata_std_prereset, ahci_softreset, ahci_hardreset,
- ahci_postreset);
+ sata_pmp_do_eh(ap, ata_std_prereset, ahci_softreset,
+ ahci_hardreset, ahci_postreset,
+ sata_pmp_std_prereset, ahci_pmp_softreset,
+ sata_pmp_std_hardreset, sata_pmp_std_postreset);
}
static void ahci_vt8251_error_handler(struct ata_port *ap)
@@ -1575,11 +1637,74 @@ static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
ahci_kick_engine(ap, 1);
}
+static void ahci_pmp_attach(struct ata_port *ap)
+{
+ void __iomem *port_mmio = ahci_port_base(ap);
+ u32 cmd;
+
+ cmd = readl(port_mmio + PORT_CMD);
+ cmd |= PORT_CMD_PMP;
+ writel(cmd, port_mmio + PORT_CMD);
+}
+
+static void ahci_pmp_detach(struct ata_port *ap)
+{
+ void __iomem *port_mmio = ahci_port_base(ap);
+ struct ahci_host_priv *hpriv = ap->host->private_data;
+ unsigned long flags;
+ u32 cmd;
+
+ cmd = readl(port_mmio + PORT_CMD);
+ cmd &= ~PORT_CMD_PMP;
+ writel(cmd, port_mmio + PORT_CMD);
+
+ if (hpriv->cap & HOST_CAP_NCQ) {
+ spin_lock_irqsave(ap->lock, flags);
+ ap->flags |= ATA_FLAG_NCQ;
+ spin_unlock_irqrestore(ap->lock, flags);
+ }
+}
+
+static int ahci_pmp_read(struct ata_device *dev, int pmp, int reg, u32 *r_val)
+{
+ struct ata_port *ap = dev->link->ap;
+ struct ata_taskfile tf;
+ int rc;
+
+ ahci_kick_engine(ap, 0);
+
+ sata_pmp_read_init_tf(&tf, dev, pmp, reg);
+ rc = ahci_exec_polled_cmd(ap, SATA_PMP_CTRL_PORT, &tf, 1, 0,
+ SATA_PMP_SCR_TIMEOUT);
+ if (rc == 0) {
+ ahci_tf_read(ap, &tf);
+ *r_val = sata_pmp_read_val(&tf);
+ }
+ return rc;
+}
+
+static int ahci_pmp_write(struct ata_device *dev, int pmp, int reg, u32 val)
+{
+ struct ata_port *ap = dev->link->ap;
+ struct ata_taskfile tf;
+
+ ahci_kick_engine(ap, 0);
+
+ sata_pmp_write_init_tf(&tf, dev, pmp, reg, val);
+ return ahci_exec_polled_cmd(ap, SATA_PMP_CTRL_PORT, &tf, 1, 0,
+ SATA_PMP_SCR_TIMEOUT);
+}
+
static int ahci_port_resume(struct ata_port *ap)
{
ahci_power_up(ap);
ahci_start_port(ap);
+ if (ap->nr_pmp_links)
+ ahci_pmp_attach(ap);
+ else
+ ahci_pmp_detach(ap);
+
return 0;
}
@@ -1860,6 +1985,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (hpriv->cap & HOST_CAP_NCQ)
pi.flags |= ATA_FLAG_NCQ;
+ if (hpriv->cap & HOST_CAP_PMP)
+ pi.flags |= ATA_FLAG_PMP;
+
host = ata_host_alloc_pinfo(&pdev->dev, ppi, fls(hpriv->port_map));
if (!host)
return -ENOMEM;
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 05/10] libata-pmp: implement qc_defer for command switching PMP support
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (6 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 08/10] ahci: implement PMP support Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 09/10] ahci: move host flags over to pi.private_data Tejun Heo
` (2 subsequent siblings)
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Implement sata_pmp_qc_defer_cmd_switch() - standard qc_defer for
command switching PMP support.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/libata-core.c | 1 +
drivers/ata/libata-pmp.c | 30 ++++++++++++++++++++++++++++++
include/linux/libata.h | 1 +
3 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 3b46678..6fae92c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -7294,6 +7294,7 @@ EXPORT_SYMBOL_GPL(ata_pci_default_filter);
EXPORT_SYMBOL_GPL(ata_pci_clear_simplex);
#endif /* CONFIG_PCI */
+EXPORT_SYMBOL_GPL(sata_pmp_qc_defer_cmd_switch);
EXPORT_SYMBOL_GPL(sata_pmp_read_init_tf);
EXPORT_SYMBOL_GPL(sata_pmp_read_val);
EXPORT_SYMBOL_GPL(sata_pmp_write_init_tf);
diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index eeffce6..f6c4b11 100644
--- a/drivers/ata/libata-pmp.c
+++ b/drivers/ata/libata-pmp.c
@@ -70,6 +70,36 @@ static int sata_pmp_write(struct ata_link *link, int reg, u32 val)
}
/**
+ * sata_pmp_qc_defer_cmd_switch - qc_defer for command switching PMP
+ * @qc: ATA command in question
+ *
+ * A host which has command switching PMP support cannot issue
+ * commands to multiple links simultaneously.
+ *
+ * LOCKING:
+ * spin_lock_irqsave(host lock)
+ *
+ * RETURNS:
+ * ATA_DEFER_* if deferring is needed, 0 otherwise.
+ */
+int sata_pmp_qc_defer_cmd_switch(struct ata_queued_cmd *qc)
+{
+ struct ata_link *link = qc->dev->link;
+ struct ata_port *ap = link->ap;
+
+ if (ap->excl_link == NULL || ap->excl_link == link) {
+ if (ap->nr_active_links == 0 || ata_link_active(link)) {
+ qc->flags |= ATA_QCFLAG_CLEAR_EXCL;
+ return ata_std_qc_defer(qc);
+ }
+
+ ap->excl_link = link;
+ }
+
+ return ATA_DEFER_PORT;
+}
+
+/**
* sata_pmp_read_init_tf - initialize TF for PMP read
* @tf: taskfile to initialize
* @dev: PMP dev
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 19a962c..1467fa9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -946,6 +946,7 @@ extern unsigned long ata_pci_default_filter(struct ata_device *, unsigned long);
/*
* PMP
*/
+extern int sata_pmp_qc_defer_cmd_switch(struct ata_queued_cmd *qc);
extern void sata_pmp_read_init_tf(struct ata_taskfile *tf,
struct ata_device *dev, int pmp, int reg);
extern u32 sata_pmp_read_val(const struct ata_taskfile *tf);
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 09/10] ahci: move host flags over to pi.private_data
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (7 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 05/10] libata-pmp: implement qc_defer for command switching " Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-23 4:19 ` [PATCH 10/10] ahci: implement AHCI_HFLAG_NO_PMP Tejun Heo
2007-09-26 2:09 ` Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6) Jeff Garzik
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Private pi.flags area is full and we need more private flags. Move
host private flags over to pi.private_data. During initialization,
these flags are copied to hpriv->flags.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/ahci.c | 63 ++++++++++++++++++++++++++++-----------------------
1 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index d06a8b3..1111dc2 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -169,14 +169,16 @@ enum {
PORT_CMD_ICC_PARTIAL = (0x2 << 28), /* Put i/f in partial state */
PORT_CMD_ICC_SLUMBER = (0x6 << 28), /* Put i/f in slumber state */
+ /* hpriv->flags bits */
+ AHCI_HFLAG_NO_NCQ = (1 << 0),
+ AHCI_HFLAG_IGN_IRQ_IF_ERR = (1 << 1), /* ignore IRQ_IF_ERR */
+ AHCI_HFLAG_IGN_SERR_INTERNAL = (1 << 2), /* ignore SERR_INTERNAL */
+ AHCI_HFLAG_32BIT_ONLY = (1 << 3), /* force 32bit */
+ AHCI_HFLAG_MV_PATA = (1 << 4), /* PATA port */
+ AHCI_HFLAG_NO_MSI = (1 << 5), /* no PCI MSI */
+
/* ap->flags bits */
- AHCI_FLAG_NO_NCQ = (1 << 24),
- AHCI_FLAG_IGN_IRQ_IF_ERR = (1 << 25), /* ignore IRQ_IF_ERR */
- AHCI_FLAG_IGN_SERR_INTERNAL = (1 << 27), /* ignore SERR_INTERNAL */
- AHCI_FLAG_32BIT_ONLY = (1 << 28), /* force 32bit */
- AHCI_FLAG_MV_PATA = (1 << 29), /* PATA port */
- AHCI_FLAG_NO_MSI = (1 << 30), /* no PCI MSI */
- AHCI_FLAG_NO_HOTPLUG = (1 << 31), /* ignore PxSERR.DIAG.N */
+ AHCI_FLAG_NO_HOTPLUG = (1 << 24), /* ignore PxSERR.DIAG.N */
AHCI_FLAG_COMMON = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
@@ -200,6 +202,7 @@ struct ahci_sg {
};
struct ahci_host_priv {
+ unsigned int flags; /* AHCI_HFLAG_* */
u32 cap; /* cap to use */
u32 port_map; /* port map to use */
u32 saved_cap; /* saved initial cap */
@@ -341,6 +344,8 @@ static const struct ata_port_operations ahci_vt8251_ops = {
.port_stop = ahci_port_stop,
};
+#define AHCI_HFLAGS(flags) .private_data = (void *)(flags)
+
static const struct ata_port_info ahci_port_info[] = {
/* board_ahci */
{
@@ -352,7 +357,8 @@ static const struct ata_port_info ahci_port_info[] = {
},
/* board_ahci_vt8251 */
{
- .flags = AHCI_FLAG_COMMON | AHCI_FLAG_NO_NCQ,
+ AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ),
+ .flags = AHCI_FLAG_COMMON,
.link_flags = AHCI_LFLAG_COMMON | ATA_LFLAG_HRST_TO_RESUME,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
@@ -360,7 +366,8 @@ static const struct ata_port_info ahci_port_info[] = {
},
/* board_ahci_ign_iferr */
{
- .flags = AHCI_FLAG_COMMON | AHCI_FLAG_IGN_IRQ_IF_ERR,
+ AHCI_HFLAGS (AHCI_HFLAG_IGN_IRQ_IF_ERR),
+ .flags = AHCI_FLAG_COMMON,
.link_flags = AHCI_LFLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
@@ -368,9 +375,9 @@ static const struct ata_port_info ahci_port_info[] = {
},
/* board_ahci_sb600 */
{
- .flags = AHCI_FLAG_COMMON |
- AHCI_FLAG_IGN_SERR_INTERNAL |
- AHCI_FLAG_32BIT_ONLY,
+ AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL |
+ AHCI_HFLAG_32BIT_ONLY),
+ .flags = AHCI_FLAG_COMMON,
.link_flags = AHCI_LFLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
@@ -378,11 +385,10 @@ static const struct ata_port_info ahci_port_info[] = {
},
/* board_ahci_mv */
{
- .sht = &ahci_sht,
+ AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ | AHCI_HFLAG_NO_MSI |
+ AHCI_HFLAG_MV_PATA),
.flags = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
- ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA |
- AHCI_FLAG_NO_NCQ | AHCI_FLAG_NO_MSI |
- AHCI_FLAG_MV_PATA,
+ ATA_FLAG_MMIO | ATA_FLAG_PIO_DMA,
.link_flags = AHCI_LFLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
@@ -534,7 +540,6 @@ static inline void __iomem *ahci_port_base(struct ata_port *ap)
/**
* ahci_save_initial_config - Save and fixup initial config values
* @pdev: target PCI device
- * @pi: associated ATA port info
* @hpriv: host private area to store config values
*
* Some registers containing configuration info might be setup by
@@ -548,7 +553,6 @@ static inline void __iomem *ahci_port_base(struct ata_port *ap)
* None.
*/
static void ahci_save_initial_config(struct pci_dev *pdev,
- const struct ata_port_info *pi,
struct ahci_host_priv *hpriv)
{
void __iomem *mmio = pcim_iomap_table(pdev)[AHCI_PCI_BAR];
@@ -562,13 +566,13 @@ static void ahci_save_initial_config(struct pci_dev *pdev,
hpriv->saved_port_map = port_map = readl(mmio + HOST_PORTS_IMPL);
/* some chips have errata preventing 64bit use */
- if ((cap & HOST_CAP_64) && (pi->flags & AHCI_FLAG_32BIT_ONLY)) {
+ if ((cap & HOST_CAP_64) && (hpriv->flags & AHCI_HFLAG_32BIT_ONLY)) {
dev_printk(KERN_INFO, &pdev->dev,
"controller can't do 64bit DMA, forcing 32bit\n");
cap &= ~HOST_CAP_64;
}
- if ((cap & HOST_CAP_NCQ) && (pi->flags & AHCI_FLAG_NO_NCQ)) {
+ if ((cap & HOST_CAP_NCQ) && (hpriv->flags & AHCI_HFLAG_NO_NCQ)) {
dev_printk(KERN_INFO, &pdev->dev,
"controller can't do NCQ, turning off CAP_NCQ\n");
cap &= ~HOST_CAP_NCQ;
@@ -579,7 +583,7 @@ static void ahci_save_initial_config(struct pci_dev *pdev,
* is asserted through the standard AHCI port
* presence register, as bit 4 (counting from 0)
*/
- if (pi->flags & AHCI_FLAG_MV_PATA) {
+ if (hpriv->flags & AHCI_HFLAG_MV_PATA) {
dev_printk(KERN_ERR, &pdev->dev,
"MV_AHCI HACK: port_map %x -> %x\n",
hpriv->port_map,
@@ -913,13 +917,14 @@ static void ahci_port_init(struct pci_dev *pdev, struct ata_port *ap,
static void ahci_init_controller(struct ata_host *host)
{
+ struct ahci_host_priv *hpriv = host->private_data;
struct pci_dev *pdev = to_pci_dev(host->dev);
void __iomem *mmio = host->iomap[AHCI_PCI_BAR];
int i;
void __iomem *port_mmio;
u32 tmp;
- if (host->ports[0]->flags & AHCI_FLAG_MV_PATA) {
+ if (hpriv->flags & AHCI_HFLAG_MV_PATA) {
port_mmio = __ahci_port_base(host, 4);
writel(0, port_mmio + PORT_IRQ_MASK);
@@ -1301,6 +1306,7 @@ static void ahci_qc_prep(struct ata_queued_cmd *qc)
static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
struct ahci_port_priv *pp = ap->private_data;
struct ata_eh_info *host_ehi = &ap->link.eh_info;
struct ata_link *link = NULL;
@@ -1328,7 +1334,7 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
host_ehi->serror |= serror;
/* some controllers set IRQ_IF_ERR on device errors, ignore it */
- if (ap->flags & AHCI_FLAG_IGN_IRQ_IF_ERR)
+ if (hpriv->flags & AHCI_HFLAG_IGN_IRQ_IF_ERR)
irq_stat &= ~PORT_IRQ_IF_ERR;
if (irq_stat & PORT_IRQ_TF_ERR) {
@@ -1341,7 +1347,7 @@ static void ahci_error_intr(struct ata_port *ap, u32 irq_stat)
else
active_ehi->err_mask |= AC_ERR_DEV;
- if (ap->flags & AHCI_FLAG_IGN_SERR_INTERNAL)
+ if (hpriv->flags & AHCI_HFLAG_IGN_SERR_INTERNAL)
host_ehi->serror &= ~SERR_INTERNAL;
}
@@ -1971,15 +1977,16 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (rc)
return rc;
- if ((pi.flags & AHCI_FLAG_NO_MSI) || pci_enable_msi(pdev))
- pci_intx(pdev, 1);
-
hpriv = devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL);
if (!hpriv)
return -ENOMEM;
+ hpriv->flags |= (unsigned long)pi.private_data;
+
+ if ((hpriv->flags & AHCI_HFLAG_NO_MSI) || pci_enable_msi(pdev))
+ pci_intx(pdev, 1);
/* save initial config */
- ahci_save_initial_config(pdev, &pi, hpriv);
+ ahci_save_initial_config(pdev, hpriv);
/* prepare host */
if (hpriv->cap & HOST_CAP_NCQ)
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* [PATCH 10/10] ahci: implement AHCI_HFLAG_NO_PMP
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (8 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 09/10] ahci: move host flags over to pi.private_data Tejun Heo
@ 2007-09-23 4:19 ` Tejun Heo
2007-09-26 2:09 ` Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6) Jeff Garzik
10 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-23 4:19 UTC (permalink / raw)
To: jeff, alan, linux-ide; +Cc: Tejun Heo
Of course some controllers lie about PMP support. Black list them.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/ahci.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 1111dc2..ad3ff29 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -176,6 +176,7 @@ enum {
AHCI_HFLAG_32BIT_ONLY = (1 << 3), /* force 32bit */
AHCI_HFLAG_MV_PATA = (1 << 4), /* PATA port */
AHCI_HFLAG_NO_MSI = (1 << 5), /* no PCI MSI */
+ AHCI_HFLAG_NO_PMP = (1 << 6), /* no PMP */
/* ap->flags bits */
AHCI_FLAG_NO_HOTPLUG = (1 << 24), /* ignore PxSERR.DIAG.N */
@@ -357,7 +358,7 @@ static const struct ata_port_info ahci_port_info[] = {
},
/* board_ahci_vt8251 */
{
- AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ),
+ AHCI_HFLAGS (AHCI_HFLAG_NO_NCQ | AHCI_HFLAG_NO_PMP),
.flags = AHCI_FLAG_COMMON,
.link_flags = AHCI_LFLAG_COMMON | ATA_LFLAG_HRST_TO_RESUME,
.pio_mask = 0x1f, /* pio0-4 */
@@ -376,7 +377,7 @@ static const struct ata_port_info ahci_port_info[] = {
/* board_ahci_sb600 */
{
AHCI_HFLAGS (AHCI_HFLAG_IGN_SERR_INTERNAL |
- AHCI_HFLAG_32BIT_ONLY),
+ AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_PMP),
.flags = AHCI_FLAG_COMMON,
.link_flags = AHCI_LFLAG_COMMON,
.pio_mask = 0x1f, /* pio0-4 */
@@ -578,6 +579,12 @@ static void ahci_save_initial_config(struct pci_dev *pdev,
cap &= ~HOST_CAP_NCQ;
}
+ if ((cap && HOST_CAP_PMP) && (hpriv->flags & AHCI_HFLAG_NO_PMP)) {
+ dev_printk(KERN_INFO, &pdev->dev,
+ "controller can't do PMP, turning off CAP_PMP\n");
+ cap &= ~HOST_CAP_PMP;
+ }
+
/*
* Temporary Marvell 6145 hack: PATA port presence
* is asserted through the standard AHCI port
--
1.5.0.3
^ permalink raw reply related [flat|nested] 52+ messages in thread* Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-23 4:19 [PATCHSET 2/2] implement PMP support, take 6 Tejun Heo
` (9 preceding siblings ...)
2007-09-23 4:19 ` [PATCH 10/10] ahci: implement AHCI_HFLAG_NO_PMP Tejun Heo
@ 2007-09-26 2:09 ` Jeff Garzik
2007-09-26 2:12 ` Jeff Garzik
` (2 more replies)
10 siblings, 3 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-09-26 2:09 UTC (permalink / raw)
To: Tejun Heo; +Cc: alan, linux-ide
Here's one thing that jumps out at me, reviewing the PMP patchset:
PMP reads and writes require polling, which is not something we should
impose upon the design. Conversations with a port multiplier are
fundamentally packetized, and modern controllers treat these just like
the bazillion other types of packets they must deal with: just another
entry on a DMA ring, with an interrupt to signal completion/reception.
For situations like this, libata EH needs to the use normal, natural
delivery method: initiate an action, and wait asynchronously for completion.
Certainly the flow of control might be synchronous inside libata EH;
that requirement is normal and usable with an issue+wait setup.
More and more I am convinced this "mission creep", the slow expansion of
polling even into modern controllers, is going in the wrong direction.
Consider the mvsas driver (Marvell 6440 SAS/SATA), whose rough draft I
posted yesterday: in order to support SAS wide ports -- multiple phys
aggregated at runtime into a single "SAS port" -- and remote SATA device
attachment behind expanders then PMPs, their hardware design became
- single command queue, for all ports/phys
- single response queue, for all ports/phys
- events from all ports, SAS and SATA, aggregate
onto those queues
- a single MSI interrupt signals "go look for new work on
DMA ring"
There, even the concept of "port" is fluid, and the libata EH model of
freezing and thawing a port (with the desired irq-off result) just
doesn't fit the hardware.
The model under SAS+SATA is even closer to that of a network driver:
you just have a very dumb driver, that sends and receives frames, as
signalled via interrupt.
As such, polling is simply an outmoded concept. It does not make sense
on new hardware, and forcing design decisions down that path only lead
to a cascade of similar design decisions -- pmp_read polling being just
one example of such a result.
Just like the Linux kernel MM platform API presents 3 levels of page
table entries, even when the hardware may only have 2, libata high level
API _must_ be implemented as 100% asynchronous event driven API.
If the default implementation chooses to use polling -- i.e. all SFF
controllers -- that's fine. But in the new SAS/SATA world its clear
that we have far too many polling-related assumptions as it is.
Polling just flat out doesn't make sense on modern SAS/SATA -- and even
a couple modern SATA controllers. On such controllers, we are notified
immediately via interrupt even in the event of errors.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-26 2:09 ` Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6) Jeff Garzik
@ 2007-09-26 2:12 ` Jeff Garzik
2007-09-26 8:41 ` Tejun Heo
2007-09-28 14:20 ` Mark Lord
2 siblings, 0 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-09-26 2:12 UTC (permalink / raw)
To: Tejun Heo; +Cc: alan, linux-ide
Jeff Garzik wrote:
> Just like the Linux kernel MM platform API presents 3 levels of page
> table entries, even when the hardware may only have 2, libata high level
> API _must_ be implemented as 100% asynchronous event driven API.
To be more clear, non-taskfile packets/commands should be delivered via
->qc_issue, just like taskfiles.
This is similar to sas_task in libsas, which can be a SATA packet, a SAS
packet, a management packet, ...
In the long run, we should probably look at integrating ata_queued_cmd
and sas_task. Once ata_queued_cmd becomes multi-protocol (tf, pmp,
sgpio, ...), it becomes more generic. sas_task is also multi-protocol.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-26 2:09 ` Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6) Jeff Garzik
2007-09-26 2:12 ` Jeff Garzik
@ 2007-09-26 8:41 ` Tejun Heo
2007-09-28 12:10 ` Tejun Heo
2007-09-28 13:54 ` Jeff Garzik
2007-09-28 14:20 ` Mark Lord
2 siblings, 2 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-26 8:41 UTC (permalink / raw)
To: Jeff Garzik; +Cc: alan, linux-ide
Jeff Garzik wrote:
[--snip--]
> There, even the concept of "port" is fluid, and the libata EH model of
> freezing and thawing a port (with the desired irq-off result) just
> doesn't fit the hardware.
Well, the current model was developed while struggling with the first
generation PMP hardware which had a lot of quirks. More focus was on
making the system run and keeping it that way. Anyways, now that the
quirks are better understood, I think we can make PMP register access
irq-driven safely.
[--snip--]
> As such, polling is simply an outmoded concept. It does not make sense
> on new hardware, and forcing design decisions down that path only lead
> to a cascade of similar design decisions -- pmp_read polling being just
> one example of such a result.
For pmp read/write, I agree that IRQ driven access would be better but
even for fairly advanced controllers like ahci or sil24, I think
resetting-by-polling is a good idea.
> Just like the Linux kernel MM platform API presents 3 levels of page
> table entries, even when the hardware may only have 2, libata high level
> API _must_ be implemented as 100% asynchronous event driven API.
Or allow both?
> If the default implementation chooses to use polling -- i.e. all SFF
> controllers -- that's fine. But in the new SAS/SATA world its clear
> that we have far too many polling-related assumptions as it is.
>
> Polling just flat out doesn't make sense on modern SAS/SATA -- and even
> a couple modern SATA controllers. On such controllers, we are notified
> immediately via interrupt even in the event of errors.
For SAS, I don't have any strong opinion. For SATA, I think we
definitely need to allow or even prefer polling for host port resets.
Is this NACK on the patchset or can we update PMP access later?
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-26 8:41 ` Tejun Heo
@ 2007-09-28 12:10 ` Tejun Heo
2007-09-28 13:54 ` Jeff Garzik
1 sibling, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-28 12:10 UTC (permalink / raw)
To: Jeff Garzik; +Cc: alan, linux-ide
Tejun Heo wrote:
> Is this NACK on the patchset or can we update PMP access later?
Ping?
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-26 8:41 ` Tejun Heo
2007-09-28 12:10 ` Tejun Heo
@ 2007-09-28 13:54 ` Jeff Garzik
2007-09-28 14:18 ` Tejun Heo
1 sibling, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-09-28 13:54 UTC (permalink / raw)
To: Tejun Heo; +Cc: alan, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
> [--snip--]
>> There, even the concept of "port" is fluid, and the libata EH model of
>> freezing and thawing a port (with the desired irq-off result) just
>> doesn't fit the hardware.
>
> Well, the current model was developed while struggling with the first
> generation PMP hardware which had a lot of quirks. More focus was on
> making the system run and keeping it that way. Anyways, now that the
> quirks are better understood, I think we can make PMP register access
> irq-driven safely.
>
> [--snip--]
>> As such, polling is simply an outmoded concept. It does not make sense
>> on new hardware, and forcing design decisions down that path only lead
>> to a cascade of similar design decisions -- pmp_read polling being just
>> one example of such a result.
>
> For pmp read/write, I agree that IRQ driven access would be better but
> even for fairly advanced controllers like ahci or sil24, I think
> resetting-by-polling is a good idea.
Not for SAS/SATA controllers. These SAS+SATA controllers allow you set
a bit initiating phy reset, and then receive an interrupt when something
interesting happens.
The reason why I (we?) am just finding out about this is: both
drivers/scsi/libsas and drivers/scsi/ipr are still using the old-EH :(
But overall, anything sufficiently "high level" is not friendly to
polling -- if a polling model is even possible.
>> Just like the Linux kernel MM platform API presents 3 levels of page
>> table entries, even when the hardware may only have 2, libata high level
>> API _must_ be implemented as 100% asynchronous event driven API.
>
> Or allow both?
>
>> If the default implementation chooses to use polling -- i.e. all SFF
Note the line quoted above, immediately following your response... such
a setup does permit both: an asynchronous submit/complete API can be
implementing under the hood via polling or irq, as the driver chooses.
The main point is to not /require/ polling.
>> controllers -- that's fine. But in the new SAS/SATA world its clear
>> that we have far too many polling-related assumptions as it is.
>>
>> Polling just flat out doesn't make sense on modern SAS/SATA -- and even
>> a couple modern SATA controllers. On such controllers, we are notified
>> immediately via interrupt even in the event of errors.
>
> For SAS, I don't have any strong opinion. For SATA, I think we
> definitely need to allow or even prefer polling for host port resets.
>
> Is this NACK on the patchset or can we update PMP access later?
Sorry, yes, it is a NAK: polling should not be requirement.
I considered making multi-protocol ->qc_issue() a requirement too, but
that seemed like it might delay things too much. But consider this a
strong to-do item... ->pmp_read and ->pmp_write hooks should be folded
into ->qc_issue and ata_qc_complete(), because quite often a PMP
read/write packet can be delivered just like any other packet. We want
a single "submit packet to hardware" interface, not multiple ones.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 13:54 ` Jeff Garzik
@ 2007-09-28 14:18 ` Tejun Heo
2007-09-28 14:57 ` Alan Cox
2007-09-28 15:22 ` Jeff Garzik
0 siblings, 2 replies; 52+ messages in thread
From: Tejun Heo @ 2007-09-28 14:18 UTC (permalink / raw)
To: Jeff Garzik; +Cc: alan, linux-ide
Jeff Garzik wrote:
>> Is this NACK on the patchset or can we update PMP access later?
>
> Sorry, yes, it is a NAK: polling should not be requirement.
>
> I considered making multi-protocol ->qc_issue() a requirement too, but
> that seemed like it might delay things too much. But consider this a
> strong to-do item... ->pmp_read and ->pmp_write hooks should be folded
> into ->qc_issue and ata_qc_complete(), because quite often a PMP
> read/write packet can be delivered just like any other packet. We want
> a single "submit packet to hardware" interface, not multiple ones.
Aieee... Another merge delay. I wish the review process proceeded a bit
swifter. The patchset has been around literally for years now and
submitted for review six times if I have the take number right. :-(
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 14:18 ` Tejun Heo
@ 2007-09-28 14:57 ` Alan Cox
2007-09-28 15:20 ` Jeff Garzik
2007-09-28 15:22 ` Jeff Garzik
1 sibling, 1 reply; 52+ messages in thread
From: Alan Cox @ 2007-09-28 14:57 UTC (permalink / raw)
To: Tejun Heo; +Cc: Jeff Garzik, linux-ide
> Aieee... Another merge delay. I wish the review process proceeded a bit
> swifter. The patchset has been around literally for years now and
> submitted for review six times if I have the take number right. :-(
Agreed - thats one reason I stick everything in -mm. Its taking several
months for a known fix to get from me to the outside. I'm not sure its
entirely a Jeff problem however.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 14:57 ` Alan Cox
@ 2007-09-28 15:20 ` Jeff Garzik
2007-09-28 15:43 ` Alan Cox
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-09-28 15:20 UTC (permalink / raw)
To: Alan Cox; +Cc: Tejun Heo, linux-ide, Andrew Morton
Alan Cox wrote:
>> Aieee... Another merge delay. I wish the review process proceeded a bit
>> swifter. The patchset has been around literally for years now and
>> submitted for review six times if I have the take number right. :-(
>
> Agreed - thats one reason I stick everything in -mm. Its taking several
> months for a known fix to get from me to the outside. I'm not sure its
> entirely a Jeff problem however.
This doesn't affect Tejun or anyone else really, but going through -mm
tends to gum up the works. Your patches wind up not applying to
libata#upstream, in which case they get dropped and I assume Andrew or
someone will resend usable versions.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 15:20 ` Jeff Garzik
@ 2007-09-28 15:43 ` Alan Cox
2007-09-28 15:40 ` Jeff Garzik
0 siblings, 1 reply; 52+ messages in thread
From: Alan Cox @ 2007-09-28 15:43 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, linux-ide, Andrew Morton
> This doesn't affect Tejun or anyone else really, but going through -mm
> tends to gum up the works. Your patches wind up not applying to
> libata#upstream, in which case they get dropped and I assume Andrew or
> someone will resend usable versions.
Its the only way to get testing done and get them into distributions in
time.
Alan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 15:43 ` Alan Cox
@ 2007-09-28 15:40 ` Jeff Garzik
2007-09-28 20:00 ` Mark Lord
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-09-28 15:40 UTC (permalink / raw)
To: Alan Cox; +Cc: Tejun Heo, linux-ide, Andrew Morton
Alan Cox wrote:
>> This doesn't affect Tejun or anyone else really, but going through -mm
>> tends to gum up the works. Your patches wind up not applying to
>> libata#upstream, in which case they get dropped and I assume Andrew or
>> someone will resend usable versions.
>
> Its the only way to get testing done and get them into distributions in
> time.
Not true... libata gets auto-propagated into -mm, and Andrew pulls
regularly.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 15:40 ` Jeff Garzik
@ 2007-09-28 20:00 ` Mark Lord
2007-09-29 1:49 ` Jeff Garzik
2007-09-29 3:29 ` Jeff Garzik
0 siblings, 2 replies; 52+ messages in thread
From: Mark Lord @ 2007-09-28 20:00 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Alan Cox, Tejun Heo, linux-ide, Andrew Morton
Jeff Garzik wrote:
> Alan Cox wrote:
>>> This doesn't affect Tejun or anyone else really, but going through
>>> -mm tends to gum up the works. Your patches wind up not applying to
>>> libata#upstream, in which case they get dropped and I assume Andrew
>>> or someone will resend usable versions.
>>
>> Its the only way to get testing done and get them into distributions in
>> time.
>
> Not true... libata gets auto-propagated into -mm, and Andrew pulls
> regularly.
I believe the point was that getting things into libata is glacial
compared with getting them into -mm. Witness the PMP patchset.
Cheers
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 20:00 ` Mark Lord
@ 2007-09-29 1:49 ` Jeff Garzik
2007-09-29 3:29 ` Jeff Garzik
1 sibling, 0 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-09-29 1:49 UTC (permalink / raw)
To: Mark Lord; +Cc: Alan Cox, Tejun Heo, linux-ide, Andrew Morton
Mark Lord wrote:
> Jeff Garzik wrote:
>> Alan Cox wrote:
>>>> This doesn't affect Tejun or anyone else really, but going through
>>>> -mm tends to gum up the works. Your patches wind up not applying to
>>>> libata#upstream, in which case they get dropped and I assume Andrew
>>>> or someone will resend usable versions.
>>>
>>> Its the only way to get testing done and get them into distributions in
>>> time.
>>
>> Not true... libata gets auto-propagated into -mm, and Andrew pulls
>> regularly.
>
> I believe the point was that getting things into libata is glacial
> compared with getting them into -mm. Witness the PMP patchset.
I don't think PMP was ever in -mm?
But regardless, people can send me "for test only" or WIP stuff that I
can auto-propagate to -mm via libata-dev.git. I keep many
libata-dev.git branches alive with testing and WIP code, and it doesn't
consume much time at all.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 20:00 ` Mark Lord
2007-09-29 1:49 ` Jeff Garzik
@ 2007-09-29 3:29 ` Jeff Garzik
2007-09-29 4:58 ` Andrew Morton
` (2 more replies)
1 sibling, 3 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-09-29 3:29 UTC (permalink / raw)
To: Mark Lord; +Cc: Alan Cox, Tejun Heo, linux-ide, Andrew Morton
(my last response only addressed -mm)
Mark Lord wrote:
> I believe the point was that getting things into libata is glacial
IMHO would say that there are two causes of that:
1) I am sometimes slow in merging, part of which is my own fault, and I
can only promise to try and do better there. Part of which is a result
of stuff being dependent on -mm (which requires plenty of time
hand-merging) rather than libata-dev.git#upstream.
2) I have been intentionally staging major libata behavior changes
between Linux releases. Luckily most of these are behind us, but, in
several cases with things like ACPI on/off (hopefully 'on' in 2.6.24),
probing changes (switchover to new EH for probing via hotplug, etc.),
interrupt handling changes.
I dislike getting "too much" into a single release, because of the
difficulty of getting large scale feedback without a major kernel release.
So far I think the kernel releases have been pretty darn successful in
"not breaking everybody" but that clearly conflicts with desired
development speed, given the glacial pace of each kernel release. If
each kernel release were 1-2 months apart, we would have many more
testing points, and I think you guys and I would both be happy. But
that's not the reality today, with 3+ month kernel release cycles (ugh!!).
I'm very much interested in hearing suggestions and comments.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 3:29 ` Jeff Garzik
@ 2007-09-29 4:58 ` Andrew Morton
2007-09-29 5:09 ` Jeff Garzik
2007-09-29 16:51 ` Greg Freemyer
2007-09-29 20:56 ` Alan Cox
2 siblings, 1 reply; 52+ messages in thread
From: Andrew Morton @ 2007-09-29 4:58 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Alan Cox, Tejun Heo, linux-ide
On Fri, 28 Sep 2007 23:29:44 -0400 Jeff Garzik <jeff@garzik.org> wrote:
> (my last response only addressed -mm)
>
> Mark Lord wrote:
> > I believe the point was that getting things into libata is glacial
>
> IMHO would say that there are two causes of that:
> 1) I am sometimes slow in merging, part of which is my own fault, and I
> can only promise to try and do better there. Part of which is a result
> of stuff being dependent on -mm (which requires plenty of time
> hand-merging) rather than libata-dev.git#upstream.
>
> 2) I have been intentionally staging major libata behavior changes
> between Linux releases. Luckily most of these are behind us, but, in
> several cases with things like ACPI on/off (hopefully 'on' in 2.6.24),
> probing changes (switchover to new EH for probing via hotplug, etc.),
> interrupt handling changes.
>
> I dislike getting "too much" into a single release, because of the
> difficulty of getting large scale feedback without a major kernel release.
>
> So far I think the kernel releases have been pretty darn successful in
> "not breaking everybody" but that clearly conflicts with desired
> development speed, given the glacial pace of each kernel release. If
> each kernel release were 1-2 months apart, we would have many more
> testing points, and I think you guys and I would both be happy. But
> that's not the reality today, with 3+ month kernel release cycles (ugh!!).
>
> I'm very much interested in hearing suggestions and comments.
>
There's an easy fix...
The releases are slow because a) there's so much stuff in them and b) it
takes so long to stabilise it all.
If all developers were to be more careful in their work, and take more time
to review and test others' work, both problems get fixed: less code, higher
quality.
We don't know how to make this happen. We haven't even tried.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 4:58 ` Andrew Morton
@ 2007-09-29 5:09 ` Jeff Garzik
0 siblings, 0 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-09-29 5:09 UTC (permalink / raw)
To: Andrew Morton; +Cc: Mark Lord, Alan Cox, Tejun Heo, linux-ide
Andrew Morton wrote:
> On Fri, 28 Sep 2007 23:29:44 -0400 Jeff Garzik <jeff@garzik.org> wrote:
>
>> (my last response only addressed -mm)
>>
>> Mark Lord wrote:
>>> I believe the point was that getting things into libata is glacial
>> IMHO would say that there are two causes of that:
>> 1) I am sometimes slow in merging, part of which is my own fault, and I
>> can only promise to try and do better there. Part of which is a result
>> of stuff being dependent on -mm (which requires plenty of time
>> hand-merging) rather than libata-dev.git#upstream.
>>
>> 2) I have been intentionally staging major libata behavior changes
>> between Linux releases. Luckily most of these are behind us, but, in
>> several cases with things like ACPI on/off (hopefully 'on' in 2.6.24),
>> probing changes (switchover to new EH for probing via hotplug, etc.),
>> interrupt handling changes.
>>
>> I dislike getting "too much" into a single release, because of the
>> difficulty of getting large scale feedback without a major kernel release.
>>
>> So far I think the kernel releases have been pretty darn successful in
>> "not breaking everybody" but that clearly conflicts with desired
>> development speed, given the glacial pace of each kernel release. If
>> each kernel release were 1-2 months apart, we would have many more
>> testing points, and I think you guys and I would both be happy. But
>> that's not the reality today, with 3+ month kernel release cycles (ugh!!).
>>
>> I'm very much interested in hearing suggestions and comments.
>>
>
> There's an easy fix...
>
> The releases are slow because a) there's so much stuff in them and b) it
> takes so long to stabilise it all.
>
> If all developers were to be more careful in their work, and take more time
> to review and test others' work, both problems get fixed: less code, higher
> quality.
>
> We don't know how to make this happen. We haven't even tried.
I think you missed part of my point, with regards to libata:
Even though stuff gets tested in -mm and in mainline -rc releases, we
simply do not have the test audience that a major kernel release does,
when it comes to determining whether new libata probe behavior will
break millions of boxes (or not).
Unlike CPUs and other hardware, our attached devices (disks, cd-roms)
are entirely black boxes of [mis]behavior, with plenty of libata
behavior based _entirely_ on observations in the field, not stuff
documented in a specification somewhere.
3-4 true "trial and error" periods per year (kernel releases) give
libata a similar number of test points per years. Not a whole lot of
room when you consider that we must actively (but carefully!)
experimental with new kernel features in the each kernel release.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 3:29 ` Jeff Garzik
2007-09-29 4:58 ` Andrew Morton
@ 2007-09-29 16:51 ` Greg Freemyer
2007-09-29 20:56 ` Alan Cox
2 siblings, 0 replies; 52+ messages in thread
From: Greg Freemyer @ 2007-09-29 16:51 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Alan Cox, Tejun Heo, linux-ide, Andrew Morton
On 9/28/07, Jeff Garzik <jeff@garzik.org> wrote:
> (my last response only addressed -mm)
>
> Mark Lord wrote:
> > I believe the point was that getting things into libata is glacial
>
> IMHO would say that there are two causes of that:
> 1) I am sometimes slow in merging, part of which is my own fault, and I
> can only promise to try and do better there. Part of which is a result
> of stuff being dependent on -mm (which requires plenty of time
> hand-merging) rather than libata-dev.git#upstream.
>
> 2) I have been intentionally staging major libata behavior changes
> between Linux releases. Luckily most of these are behind us, but, in
> several cases with things like ACPI on/off (hopefully 'on' in 2.6.24),
> probing changes (switchover to new EH for probing via hotplug, etc.),
> interrupt handling changes.
>
> I dislike getting "too much" into a single release, because of the
> difficulty of getting large scale feedback without a major kernel release.
>
> So far I think the kernel releases have been pretty darn successful in
> "not breaking everybody" but that clearly conflicts with desired
> development speed, given the glacial pace of each kernel release. If
> each kernel release were 1-2 months apart, we would have many more
> testing points, and I think you guys and I would both be happy. But
> that's not the reality today, with 3+ month kernel release cycles (ugh!!).
>
> I'm very much interested in hearing suggestions and comments.
>
> Jeff
Perhaps you could more fully leverage the various distro
alpha/beta/rcs/release kernels.
Specific to the PMP patches, they went into the openSUSE beta/rc
kernels (2.6.23 based) in early Aug. I think. So they have already
been through a significant set of Novell internal and community
testing. Likely similar testing to what it would get in -mm.
I believe they are part of the OpenSUSE 10.3 (2.6.23 based) full
release that is set for Thursday (Oct. 4) and thus will see a huge
amount of public testing.
Greg
--
Greg Freemyer
Litigation Triage Solutions Specialist
http://www.linkedin.com/in/gregfreemyer
The Norcross Group
The Intersection of Evidence & Technology
http://www.norcrossgroup.com
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 3:29 ` Jeff Garzik
2007-09-29 4:58 ` Andrew Morton
2007-09-29 16:51 ` Greg Freemyer
@ 2007-09-29 20:56 ` Alan Cox
2007-10-01 12:28 ` Jeff Garzik
2 siblings, 1 reply; 52+ messages in thread
From: Alan Cox @ 2007-09-29 20:56 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Tejun Heo, linux-ide, Andrew Morton
ty today, with 3+ month kernel release cycles (ugh!!).
>
> I'm very much interested in hearing suggestions and comments.
I think PMP should go in for 2.6.24 and then get revised over time to not
poll and to go via qc_issue. Which seems to be the consensus of everyone
but you on this one.
In general I have no big problems with the way its going - need to get
stuff through -mm into libata faster but thats something I need to sort
out with you specifically and I can accept your view its more my problem.
But PMP is a definite candidate for in ASAP
Alan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 20:56 ` Alan Cox
@ 2007-10-01 12:28 ` Jeff Garzik
0 siblings, 0 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-10-01 12:28 UTC (permalink / raw)
To: Alan Cox; +Cc: Mark Lord, Tejun Heo, linux-ide, Andrew Morton
Alan Cox wrote:
> ty today, with 3+ month kernel release cycles (ugh!!).
>> I'm very much interested in hearing suggestions and comments.
>
> I think PMP should go in for 2.6.24 and then get revised over time to not
> poll and to go via qc_issue. Which seems to be the consensus of everyone
> but you on this one.
It's not an "over time" issue. SAS drivers that will be ready for
2.6.24 (broadsas, mvsas) have both been coded to support PMP
transparently -- but the current libata PMP code requires that PMP
support in SAS be TURNED OFF, because it is fundamentally incompatible
by its design.
This is an issue that has been present and known for many, many months
-- as long as drivers/scsi/libsas/sas_ata.c has been in the tree.
The more we walk down the polling path, the more incompatible we become
with SATA-capable controllers that are in users' hands today.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 14:18 ` Tejun Heo
2007-09-28 14:57 ` Alan Cox
@ 2007-09-28 15:22 ` Jeff Garzik
2007-09-28 16:48 ` Tejun Heo
1 sibling, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-09-28 15:22 UTC (permalink / raw)
To: Tejun Heo; +Cc: alan, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
>>> Is this NACK on the patchset or can we update PMP access later?
>> Sorry, yes, it is a NAK: polling should not be requirement.
>>
>> I considered making multi-protocol ->qc_issue() a requirement too, but
>> that seemed like it might delay things too much. But consider this a
>> strong to-do item... ->pmp_read and ->pmp_write hooks should be folded
>> into ->qc_issue and ata_qc_complete(), because quite often a PMP
>> read/write packet can be delivered just like any other packet. We want
>> a single "submit packet to hardware" interface, not multiple ones.
>
> Aieee... Another merge delay. I wish the review process proceeded a bit
> swifter. The patchset has been around literally for years now and
> submitted for review six times if I have the take number right. :-(
Well the vast majority of the patches are in, what five out of six
original patchsets?
Sorry I didn't catch the polling requirement beforehand, it was not
really clear from a quick read.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 15:22 ` Jeff Garzik
@ 2007-09-28 16:48 ` Tejun Heo
2007-09-28 20:02 ` Mark Lord
0 siblings, 1 reply; 52+ messages in thread
From: Tejun Heo @ 2007-09-28 16:48 UTC (permalink / raw)
To: Jeff Garzik; +Cc: alan, linux-ide
Jeff Garzik wrote:
>> Aieee... Another merge delay. I wish the review process proceeded a bit
>> swifter. The patchset has been around literally for years now and
>> submitted for review six times if I have the take number right. :-(
>
> Well the vast majority of the patches are in, what five out of six
> original patchsets?
Yeah, I'm frustrated mainly because I've been telling people that
mainline will probably have PMP support when 2.6.24 comes out and it
seems we'll miss the merge window again. Oh, well...
> Sorry I didn't catch the polling requirement beforehand, it was not
> really clear from a quick read.
->pmp_read/write stuff is something which I've been meaning to change
anyway. When developing the PMP code, PMP register access while frozen
seemed necessary but now I think we can be just as safe without it. I
was thinking about changing it after merge because the current code
received a lot of testing and I didn't want to destabilize it right
before merging.
I'll be back home mid next week. I'll try to re-test and re-submit the
changes ASAP.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 16:48 ` Tejun Heo
@ 2007-09-28 20:02 ` Mark Lord
2007-09-28 20:25 ` Bartlomiej Zolnierkiewicz
2007-09-28 21:03 ` Alan Cox
0 siblings, 2 replies; 52+ messages in thread
From: Mark Lord @ 2007-09-28 20:02 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, alan, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
>>> Aieee... Another merge delay. I wish the review process proceeded a bit
>>> swifter. The patchset has been around literally for years now and
>>> submitted for review six times if I have the take number right. :-(
>> Well the vast majority of the patches are in, what five out of six
>> original patchsets?
>
> Yeah, I'm frustrated mainly because I've been telling people that
> mainline will probably have PMP support when 2.6.24 comes out and it
> seems we'll miss the merge window again. Oh, well...
>
>> Sorry I didn't catch the polling requirement beforehand, it was not
>> really clear from a quick read.
>
> ->pmp_read/write stuff is something which I've been meaning to change
> anyway. When developing the PMP code, PMP register access while frozen
> seemed necessary but now I think we can be just as safe without it. I
> was thinking about changing it after merge because the current code
> received a lot of testing and I didn't want to destabilize it right
> before merging.
>
> I'll be back home mid next week. I'll try to re-test and re-submit the
> changes ASAP.
Jeff, seeing as Tejun's commitment is never in doubt here,
I really believe we should go with the existing PMP patchset
for 2.6.24 (unless the respin happens quickly enough).
This functionality is way overdue, and we shouldn't be impeding it
as long as we have been.
Tejun will definitely continue to rework the changes you've asked for
in time for the next release, but let's not hold things up unreasonably here.
Cheers
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 20:02 ` Mark Lord
@ 2007-09-28 20:25 ` Bartlomiej Zolnierkiewicz
2007-09-28 21:03 ` Alan Cox
1 sibling, 0 replies; 52+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-09-28 20:25 UTC (permalink / raw)
To: Mark Lord; +Cc: Jeff Garzik, Tejun Heo, alan, linux-ide
On Friday 28 September 2007, Mark Lord wrote:
> Tejun Heo wrote:
> > Jeff Garzik wrote:
> >>> Aieee... Another merge delay. I wish the review process proceeded a bit
> >>> swifter. The patchset has been around literally for years now and
> >>> submitted for review six times if I have the take number right. :-(
> >> Well the vast majority of the patches are in, what five out of six
> >> original patchsets?
> >
> > Yeah, I'm frustrated mainly because I've been telling people that
> > mainline will probably have PMP support when 2.6.24 comes out and it
> > seems we'll miss the merge window again. Oh, well...
> >
> >> Sorry I didn't catch the polling requirement beforehand, it was not
> >> really clear from a quick read.
> >
> > ->pmp_read/write stuff is something which I've been meaning to change
> > anyway. When developing the PMP code, PMP register access while frozen
> > seemed necessary but now I think we can be just as safe without it. I
> > was thinking about changing it after merge because the current code
> > received a lot of testing and I didn't want to destabilize it right
> > before merging.
This is an excellent point for merging the PMP code as it is currently
and doing revamp later. PMP patchset in the current form has got quite
a lot of testing in -mm and "last minute" changes have a tendency to
bring up some nasty surprises.
> > I'll be back home mid next week. I'll try to re-test and re-submit the
> > changes ASAP.
>
> Jeff, seeing as Tejun's commitment is never in doubt here,
> I really believe we should go with the existing PMP patchset
> for 2.6.24 (unless the respin happens quickly enough).
>
> This functionality is way overdue, and we shouldn't be impeding it
> as long as we have been.
It is way, way overdue...
> Tejun will definitely continue to rework the changes you've asked for
> in time for the next release, but let's not hold things up unreasonably here.
Seconded.
Bart
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 20:02 ` Mark Lord
2007-09-28 20:25 ` Bartlomiej Zolnierkiewicz
@ 2007-09-28 21:03 ` Alan Cox
2007-09-29 1:43 ` Jeff Garzik
1 sibling, 1 reply; 52+ messages in thread
From: Alan Cox @ 2007-09-28 21:03 UTC (permalink / raw)
To: Mark Lord; +Cc: Jeff Garzik, Tejun Heo, linux-ide
> Jeff, seeing as Tejun's commitment is never in doubt here,
> I really believe we should go with the existing PMP patchset
> for 2.6.24 (unless the respin happens quickly enough).
>
> This functionality is way overdue, and we shouldn't be impeding it
> as long as we have been.
I would second this. Its far too important to not get this stuff upstream
and usable NOW. Yes the model will have to change a bit but the entire
libata today has almost no resemblence to the one a year ago.
It can evolve and then the old one can die off just as we did with EH
(except for libsas anyway)
Alan
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 21:03 ` Alan Cox
@ 2007-09-29 1:43 ` Jeff Garzik
2007-09-29 5:24 ` Tejun Heo
2007-09-29 12:32 ` Mark Lord
0 siblings, 2 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-09-29 1:43 UTC (permalink / raw)
To: Alan Cox, Mark Lord; +Cc: Tejun Heo, linux-ide
Alan Cox wrote:
>> Jeff, seeing as Tejun's commitment is never in doubt here,
>> I really believe we should go with the existing PMP patchset
>> for 2.6.24 (unless the respin happens quickly enough).
>>
>> This functionality is way overdue, and we shouldn't be impeding it
>> as long as we have been.
>
> I would second this. Its far too important to not get this stuff upstream
> and usable NOW. Yes the model will have to change a bit but the entire
> libata today has almost no resemblence to the one a year ago.
>
> It can evolve and then the old one can die off just as we did with EH
> (except for libsas anyway)
I certainly deserve plenty of blame for not catching this fact earlier,
much to my chagrin. But there are real technical issues at hand:
Polling ALREADY makes the job of fixing SAS/SATA exception handling
difficult. Expanding polling to something SAS/SATA controllers treat as
fundamentally irq-driven and integrated with the rest of the command
flow is moving in the wrong direction.
To re-re-re-summarize, polling in PMP is fundamentally broken for an
ENTIRE CLASS OF HARDWARE that we actively support today. And
jgarzik/misc-2.6.git#sas is adding two more controllers to that list.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 1:43 ` Jeff Garzik
@ 2007-09-29 5:24 ` Tejun Heo
2007-10-01 13:31 ` Jeff Garzik
2007-09-29 12:32 ` Mark Lord
1 sibling, 1 reply; 52+ messages in thread
From: Tejun Heo @ 2007-09-29 5:24 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Alan Cox, Mark Lord, linux-ide
Jeff Garzik wrote:
> Polling ALREADY makes the job of fixing SAS/SATA exception handling
> difficult. Expanding polling to something SAS/SATA controllers treat as
> fundamentally irq-driven and integrated with the rest of the command
> flow is moving in the wrong direction.
>
> To re-re-re-summarize, polling in PMP is fundamentally broken for an
> ENTIRE CLASS OF HARDWARE that we actively support today. And
> jgarzik/misc-2.6.git#sas is adding two more controllers to that list.
As an interim solution, it doesn't make anything worse tho. Those
drivers don't support PMP anyway. After rc1 merge, polling PMP access
can be replaced with new qc_issue (probably ata_exec_internal) based code.
The question here is whether it's worth to include PMP support with
polling PMP register access as an interim solution for 2.6.24. I think
it will be beneficial for both user convenience and testing as long as
the said change is made soon after -rc1.
My vote is yes but this kind of decision ultimately falls on the
subsystem maintainer, so it's your call, Jeff.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 5:24 ` Tejun Heo
@ 2007-10-01 13:31 ` Jeff Garzik
2007-10-02 0:11 ` Tejun Heo
2007-10-02 14:25 ` Alan Cox
0 siblings, 2 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-10-01 13:31 UTC (permalink / raw)
To: Tejun Heo; +Cc: Alan Cox, Mark Lord, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
>> Polling ALREADY makes the job of fixing SAS/SATA exception handling
>> difficult. Expanding polling to something SAS/SATA controllers treat as
>> fundamentally irq-driven and integrated with the rest of the command
>> flow is moving in the wrong direction.
>>
>> To re-re-re-summarize, polling in PMP is fundamentally broken for an
>> ENTIRE CLASS OF HARDWARE that we actively support today. And
>> jgarzik/misc-2.6.git#sas is adding two more controllers to that list.
>
> As an interim solution, it doesn't make anything worse tho. Those
> drivers don't support PMP anyway. After rc1 merge, polling PMP access
> can be replaced with new qc_issue (probably ata_exec_internal) based code.
>
> The question here is whether it's worth to include PMP support with
> polling PMP register access as an interim solution for 2.6.24. I think
> it will be beneficial for both user convenience and testing as long as
> the said change is made soon after -rc1.
Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS
driver releases out of PMP.
I pulled your last PMP patchset, and will now endeavor to fix the API
prior to 2.6.24 merge window opening.
Linux high level message-submit / message-complete APIs should never
_require_ polling, even if its 100% polling under the hood. There are
far too many cases in the field where you don't have direct access to
hardware registers to poll. Or such polling would interfere with the
operation of other ports. Or any of a myriad of other reasons.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-01 13:31 ` Jeff Garzik
@ 2007-10-02 0:11 ` Tejun Heo
2007-10-02 14:25 ` Alan Cox
1 sibling, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-10-02 0:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Alan Cox, Mark Lord, linux-ide
Jeff Garzik wrote:
> Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS
> driver releases out of PMP.
Not merging PMP has about the same effect for SAS, right?
> I pulled your last PMP patchset, and will now endeavor to fix the API
> prior to 2.6.24 merge window opening.
I have a preliminary patch. The patch is pretty simple too. I'll
submit after some testing but I'm not sure whether doing this right
before the merge window is a good idea. I'd prefer not to support PMP
on SAS for 2.6.24.
> Linux high level message-submit / message-complete APIs should never
> _require_ polling, even if its 100% polling under the hood. There are
> far too many cases in the field where you don't have direct access to
> hardware registers to poll. Or such polling would interfere with the
> operation of other ports. Or any of a myriad of other reasons.
I don't think that's true for actual ATA controllers but, for SAS,
probably true. Is the frozen reset mechanism a headache too for SAS?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-01 13:31 ` Jeff Garzik
2007-10-02 0:11 ` Tejun Heo
@ 2007-10-02 14:25 ` Alan Cox
2007-10-02 14:30 ` Jeff Garzik
1 sibling, 1 reply; 52+ messages in thread
From: Alan Cox @ 2007-10-02 14:25 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, Mark Lord, linux-ide
> Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS
> driver releases out of PMP.
No PMP in 2.6.24 screws PMP users, who outnumber SAS users by a few
thousand to one I suspect.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-02 14:25 ` Alan Cox
@ 2007-10-02 14:30 ` Jeff Garzik
0 siblings, 0 replies; 52+ messages in thread
From: Jeff Garzik @ 2007-10-02 14:30 UTC (permalink / raw)
To: Alan Cox; +Cc: Tejun Heo, Mark Lord, linux-ide
Alan Cox wrote:
>> Polling PMP 2.6.24 is completely unacceptable. It screws the 2.6.24 SAS
>> driver releases out of PMP.
>
> No PMP in 2.6.24 screws PMP users, who outnumber SAS users by a few
> thousand to one I suspect.
no-PMP-in-2.6.24 is not happening, so that's rather irrelevant.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 1:43 ` Jeff Garzik
2007-09-29 5:24 ` Tejun Heo
@ 2007-09-29 12:32 ` Mark Lord
2007-10-01 12:38 ` Jeff Garzik
1 sibling, 1 reply; 52+ messages in thread
From: Mark Lord @ 2007-09-29 12:32 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Alan Cox, Tejun Heo, linux-ide
Jeff Garzik wrote:
>
> I certainly deserve plenty of blame for not catching this fact earlier,
> much to my chagrin. But there are real technical issues at hand:
>
> Polling ALREADY makes the job of fixing SAS/SATA exception handling
> difficult. Expanding polling to something SAS/SATA controllers treat as
> fundamentally irq-driven and integrated with the rest of the command
> flow is moving in the wrong direction.
Fine. But the exising PMP patchset has already taken close to a year
to become mature and safe enough to pass "Jeff review". Tons of us are
awaiting it in mainline, and to rework it to meet your latest concerns
will delay it until 2.6.25 at least --> mid 2008. Not Acceptable.
It doesn't *break* anything (we know of).
It doesn't affect existing SAS functionality.
It *does* add working, critical functionality to libata.
And.. Tejun isn't just going to lay down and be happy
with it in 2.6.24. He's fully committed to the rework you're demanding,
but in a more timely and correct fashion.
Linux kernel development is supposed to happen incrementally nowadays.
Get a nice working solution in place, and then enhance/tune it.
The current implementation is certainly kernel-worthy,
and will be reworked over time as is the rest of libata.
The barrier to entry for new libata functionality is way too high.
Yes, we want readable, *reliable* code, because screwing people's
filesystems is not an option. Fine.
Tejun's current code is more than "good enough",
and "perfect" code doesn't exist.
But we're all striving for it anyway.
Cheers
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-29 12:32 ` Mark Lord
@ 2007-10-01 12:38 ` Jeff Garzik
2007-10-02 0:12 ` Tejun Heo
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-10-01 12:38 UTC (permalink / raw)
To: Mark Lord; +Cc: Alan Cox, Tejun Heo, linux-ide
Mark Lord wrote:
> Linux kernel development is supposed to happen incrementally nowadays.
> Get a nice working solution in place, and then enhance/tune it.
It's not about enhancing and tuning.
It's about me (and/or James B) having to __undo__ the current code, just
to get things working on an entire class of SATA-capable controllers out
in the field.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-01 12:38 ` Jeff Garzik
@ 2007-10-02 0:12 ` Tejun Heo
2007-10-02 12:56 ` Jeff Garzik
0 siblings, 1 reply; 52+ messages in thread
From: Tejun Heo @ 2007-10-02 0:12 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Alan Cox, linux-ide
Jeff Garzik wrote:
> Mark Lord wrote:
>> Linux kernel development is supposed to happen incrementally nowadays.
>> Get a nice working solution in place, and then enhance/tune it.
>
> It's not about enhancing and tuning.
>
> It's about me (and/or James B) having to __undo__ the current code, just
> to get things working on an entire class of SATA-capable controllers out
> in the field.
Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough?
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-02 0:12 ` Tejun Heo
@ 2007-10-02 12:56 ` Jeff Garzik
2007-10-02 13:06 ` Mark Lord
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-10-02 12:56 UTC (permalink / raw)
To: Tejun Heo; +Cc: Mark Lord, Alan Cox, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
>> Mark Lord wrote:
>>> Linux kernel development is supposed to happen incrementally nowadays.
>>> Get a nice working solution in place, and then enhance/tune it.
>> It's not about enhancing and tuning.
>>
>> It's about me (and/or James B) having to __undo__ the current code, just
>> to get things working on an entire class of SATA-capable controllers out
>> in the field.
>
> Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough?
The point was that I was going to turn on PMP support for SAS
controllers in 2.6.24 to coincide with the merge, but now cannot without
fixing things.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-02 12:56 ` Jeff Garzik
@ 2007-10-02 13:06 ` Mark Lord
2007-10-02 13:30 ` Jeff Garzik
0 siblings, 1 reply; 52+ messages in thread
From: Mark Lord @ 2007-10-02 13:06 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, Alan Cox, linux-ide
Jeff Garzik wrote:
> Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> Mark Lord wrote:
>>>> Linux kernel development is supposed to happen incrementally nowadays.
>>>> Get a nice working solution in place, and then enhance/tune it.
>>> It's not about enhancing and tuning.
>>>
>>> It's about me (and/or James B) having to __undo__ the current code, just
>>> to get things working on an entire class of SATA-capable controllers out
>>> in the field.
>>
>> Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough?
>
> The point was that I was going to turn on PMP support for SAS
> controllers in 2.6.24 to coincide with the merge, but now cannot without
> fixing things.
So, don't.
Just a few days ago PMP was slated to not be enabled for *any* controllers
until 2.6.25 or so. Now we have it a release early, for all but SAS.
Cheers
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-02 13:06 ` Mark Lord
@ 2007-10-02 13:30 ` Jeff Garzik
2007-10-06 22:02 ` Tejun Heo
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-10-02 13:30 UTC (permalink / raw)
To: Mark Lord; +Cc: Tejun Heo, Alan Cox, linux-ide
Mark Lord wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> Jeff Garzik wrote:
>>>> Mark Lord wrote:
>>>>> Linux kernel development is supposed to happen incrementally nowadays.
>>>>> Get a nice working solution in place, and then enhance/tune it.
>>>> It's not about enhancing and tuning.
>>>>
>>>> It's about me (and/or James B) having to __undo__ the current code,
>>>> just
>>>> to get things working on an entire class of SATA-capable controllers
>>>> out
>>>> in the field.
>>>
>>> Hmmm... Simpy not setting ATA_FLAG_PMP isn't enough?
>>
>> The point was that I was going to turn on PMP support for SAS
>> controllers in 2.6.24 to coincide with the merge, but now cannot
>> without fixing things.
>
> So, don't.
>
> Just a few days ago PMP was slated to not be enabled for *any* controllers
> until 2.6.25 or so. Now we have it a release early, for all but SAS.
A few days before that, both PMP and SAS /were/ slated for 2.6.24, and
after I fix the design problems, they will be again.
One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-02 13:30 ` Jeff Garzik
@ 2007-10-06 22:02 ` Tejun Heo
2007-10-09 2:09 ` Jeff Garzik
0 siblings, 1 reply; 52+ messages in thread
From: Tejun Heo @ 2007-10-06 22:02 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Alan Cox, linux-ide
Jeff Garzik wrote:
> A few days before that, both PMP and SAS /were/ slated for 2.6.24, and
> after I fix the design problems, they will be again.
>
> One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
Just an update to let you know that I've been working on it. sata_sil24
works okay but ahci still craps itself after resetting downstream ports.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-06 22:02 ` Tejun Heo
@ 2007-10-09 2:09 ` Jeff Garzik
2007-10-09 6:54 ` Tejun Heo
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-10-09 2:09 UTC (permalink / raw)
To: Tejun Heo; +Cc: Mark Lord, Alan Cox, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
>> A few days before that, both PMP and SAS /were/ slated for 2.6.24, and
>> after I fix the design problems, they will be again.
>>
>> One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
>
> Just an update to let you know that I've been working on it. sata_sil24
> works okay but ahci still craps itself after resetting downstream ports.
Thanks for your patience and perseverance.
It looks like it would be too difficult to get SAS PMP working for
2.6.24 merge window open, so I think it is only fair to rescind my
assertion of "polling PMP not in 2.6.24 release."
Removing the polling remains a design requirement for SAS, but the more
I look at old-EH-encrusted libsas, the more work I feel it needs before
its ready for PMP. Sorry.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-10-09 2:09 ` Jeff Garzik
@ 2007-10-09 6:54 ` Tejun Heo
0 siblings, 0 replies; 52+ messages in thread
From: Tejun Heo @ 2007-10-09 6:54 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Alan Cox, linux-ide
Jeff Garzik wrote:
> Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> A few days before that, both PMP and SAS /were/ slated for 2.6.24, and
>>> after I fix the design problems, they will be again.
>>>
>>> One way or another, upstream will /not/ be doing polling PMP in 2.6.24.
>>
>> Just an update to let you know that I've been working on it. sata_sil24
>> works okay but ahci still craps itself after resetting downstream ports.
This turns out to be a different issue. SIMG 5723/5744 doesn't like
being driven by ahci controllers (both ICH9 and JMB) and the problem
isn't related to whether polling is used or not. It's very weird
because SATA tracer doesn't show much difference on the host side wire
between being driven by ahci and sil24/32, but on the fan-out side,
things are seriously broken (repeated COMRESET/COMWAKE/COMINIT and no
FIS successfully being relayed from the host side).
Other than that, I've tested things on various combinations and am
fairly confident with it. I think we can go for 2.6.24 merge. Even
things turn out to be bad, we at least have pretty good bisection point.
> Thanks for your patience and perseverance.
Thanks. :-)
> It looks like it would be too difficult to get SAS PMP working for
> 2.6.24 merge window open, so I think it is only fair to rescind my
> assertion of "polling PMP not in 2.6.24 release."
>
> Removing the polling remains a design requirement for SAS, but the more
> I look at old-EH-encrusted libsas, the more work I feel it needs before
> its ready for PMP. Sorry.
I wish libata EH was easier to deal with from SAS side. I think being
able to present SAS end point as an independent ATA host to libata EH
would probably make life easier for both sides, but I don't have any
actual experience with SAS (yet).
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-26 2:09 ` Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6) Jeff Garzik
2007-09-26 2:12 ` Jeff Garzik
2007-09-26 8:41 ` Tejun Heo
@ 2007-09-28 14:20 ` Mark Lord
2007-09-28 15:36 ` Jeff Garzik
2 siblings, 1 reply; 52+ messages in thread
From: Mark Lord @ 2007-09-28 14:20 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, alan, linux-ide
Jeff Garzik wrote:
..
> As such, polling is simply an outmoded concept. It does not make sense
> on new hardware, and forcing design decisions down that path only lead
> to a cascade of similar design decisions -- pmp_read polling being just
> one example of such a result.
A related tangent here, but I wonder if someday we might consider
something like NAPI for libata. Our I/O rates can far exceed those of the
network stack, and there polling (NAPI) has helped quite a bit.
But our data behaviour is mostly synchronous, whereas network stuff is more
asynchronous. But still, with multiple drives all using NCQ on a port,
a polled interface option might give higher throughput and lower CPU loading.
Just like with the networking code.
Not that this specifically relates to concerns with the current PMP patches,
but I'm curious where Jeff sees this all going, as he works on both sides.
Cheers
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 14:20 ` Mark Lord
@ 2007-09-28 15:36 ` Jeff Garzik
2007-09-28 15:55 ` Alan Cox
0 siblings, 1 reply; 52+ messages in thread
From: Jeff Garzik @ 2007-09-28 15:36 UTC (permalink / raw)
To: Mark Lord; +Cc: Tejun Heo, alan, linux-ide
Mark Lord wrote:
> Jeff Garzik wrote:
> ..
>> As such, polling is simply an outmoded concept. It does not make
>> sense on new hardware, and forcing design decisions down that path
>> only lead to a cascade of similar design decisions -- pmp_read polling
>> being just one example of such a result.
>
> A related tangent here, but I wonder if someday we might consider
> something like NAPI for libata. Our I/O rates can far exceed those of the
> network stack, and there polling (NAPI) has helped quite a bit.
I certainly think about these issues each time I write a new storage
driver, given that most new hardware has interrupt mitigation/coalescing
capability built into it -- in AHCI's case, at my urging.
Storage is a different profile, with quite a bit few interrupts.
Storage interrupt rates are way, way below that of networking. Ethernet
is basically forced to deal with an interrupt event every 1K. By
design, storage gathers up as much I/O as possible, such that each
interrupt could be completing 128K, 512K, 1MB or more.
Finally, for storage I/O workloads with lots of smaller I/Os, those
workloads are often seek-heavy, where any interrupt overhead is few and
far between, while you're waiting on your slow disk to seek. You
/don't/ want to be polling for long milliseconds, waiting for a seek.
That's a good way to increase your CPU usage, rather than decrease it.
So, there just hasn't been a real need so far. If interrupt overhead
becomes an issue, my first step would be to start programming the
interrupt coalescing hardware that comes in modern SATA controllers.
PCI MSI, found on all new controllers, tends to mitigate interrupt
overhead even further.
> But our data behaviour is mostly synchronous, whereas network stuff is more
> asynchronous. But still, with multiple drives all using NCQ on a port,
> a polled interface option might give higher throughput and lower CPU
> loading.
There are definitely options worth exploring. And with full ports all
doing NCQ, you certainly have a lot of events to deal with.
Jeff
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: Polling (was Re: [PATCHSET 2/2] implement PMP support, take 6)
2007-09-28 15:36 ` Jeff Garzik
@ 2007-09-28 15:55 ` Alan Cox
0 siblings, 0 replies; 52+ messages in thread
From: Alan Cox @ 2007-09-28 15:55 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Mark Lord, Tejun Heo, linux-ide
> So, there just hasn't been a real need so far. If interrupt overhead
> becomes an issue, my first step would be to start programming the
> interrupt coalescing hardware that comes in modern SATA controllers.
Another factor is queue sizes. You don't get 1024 entry queues on your
typical disk controller and that makes NAPI type stuff less viable, and
sensible IRQ colaescing on the card more sane.
Alan
PS: Jeff - if you want stuff from -mm and it's got cross dependancies,
clashes just ask me for "patch xyz versus libata head" and I'll sort it
out
^ permalink raw reply [flat|nested] 52+ messages in thread