This series addresses regression problems with
commit 7faa33da9b7add01db9f1ad92c6a5d9145e940a7
ahci: start engine only during soft/hard resets
Because the fix provided by the above commit caused COMRESET failures and
10 second boot-time/resume-time hangs for certain DVD drives, we need to
make the fix conditional to certain devices/platforms. Add a flag for this.
Brian
v2: change flag name to AHCI_HFLAG_DELAY_ENGINE, write more detailed
comments
v1: flag named AHCI_HFLAG_STRICT_SPEC
Brian Norris (3):
ahci: add AHCI_HFLAG_DELAY_ENGINE host flag
ahci: move AHCI_HFLAGS() macro to ahci.h
ahci_platform: add STRICT_AHCI platform type
drivers/ata/ahci.c | 2 --
drivers/ata/ahci.h | 6 ++++++
drivers/ata/ahci_platform.c | 11 +++++++++++
drivers/ata/libahci.c | 5 +++++
4 files changed, 22 insertions(+), 2 deletions(-)
The following commit was intended to fix problems with specific AHCI
controller(s) that would become bricks if the AHCI specification was not
followed strictly (that is, if ahci_start_engine() was called while the
controller was in the wrong state):
commit 7faa33da9b7add01db9f1ad92c6a5d9145e940a7
ahci: start engine only during soft/hard resets
However, some devices currently have issues with that fix, so we must
implement a flag that delays the ahci_start_engine() call only for specific
controllers.
This commit simply introduces the flag, without enabling it in any driver.
Note that even when AHCI_HFLAG_DELAY_ENGINE is not enabled, this patch does
not constitue a full revert to commit 7faa33da; there is still a change in
behavior to the ahci_port_suspend() failure path.
Signed-off-by: Brian Norris <[email protected]>
---
drivers/ata/ahci.h | 3 +++
drivers/ata/libahci.c | 5 +++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index b175000..feb127e 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -210,6 +210,9 @@ enum {
AHCI_HFLAG_NO_SNTF = (1 << 12), /* no sntf */
AHCI_HFLAG_NO_FPDMA_AA = (1 << 13), /* no FPDMA AA */
AHCI_HFLAG_YES_FBS = (1 << 14), /* force FBS cap on */
+ AHCI_HFLAG_DELAY_ENGINE = (1 << 15), /* do not start engine on
+ port start (wait until
+ error-handling stage) */
/* ap->flags bits */
diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index a72bfd0..f9eaa82 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -737,6 +737,7 @@ static void ahci_power_down(struct ata_port *ap)
static void ahci_start_port(struct ata_port *ap)
{
+ struct ahci_host_priv *hpriv = ap->host->private_data;
struct ahci_port_priv *pp = ap->private_data;
struct ata_link *link;
struct ahci_em_priv *emp;
@@ -746,6 +747,10 @@ static void ahci_start_port(struct ata_port *ap)
/* enable FIS reception */
ahci_start_fis_rx(ap);
+ /* enable DMA */
+ if (!(hpriv->flags & AHCI_HFLAG_DELAY_ENGINE))
+ ahci_start_engine(ap);
+
/* turn on LEDs */
if (ap->flags & ATA_FLAG_EM) {
ata_for_each_link(link, ap, EDGE) {
Some platforms need to make use of the AHCI_HFLAG_DELAY_ENGINE flag.
Signed-off-by: Brian Norris <[email protected]>
---
drivers/ata/ahci_platform.c | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 48be4e1..0c86c77 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -26,6 +26,7 @@
enum ahci_type {
AHCI, /* standard platform ahci */
IMX53_AHCI, /* ahci on i.mx53 */
+ STRICT_AHCI, /* delayed DMA engine start */
};
static struct platform_device_id ahci_devtype[] = {
@@ -36,6 +37,9 @@ static struct platform_device_id ahci_devtype[] = {
.name = "imx53-ahci",
.driver_data = IMX53_AHCI,
}, {
+ .name = "strict-ahci",
+ .driver_data = STRICT_AHCI,
+ }, {
/* sentinel */
}
};
@@ -56,6 +60,13 @@ static const struct ata_port_info ahci_port_info[] = {
.udma_mask = ATA_UDMA6,
.port_ops = &ahci_pmp_retry_srst_ops,
},
+ [STRICT_AHCI] = {
+ AHCI_HFLAGS (AHCI_HFLAG_DELAY_ENGINE),
+ .flags = AHCI_FLAG_COMMON,
+ .pio_mask = ATA_PIO4,
+ .udma_mask = ATA_UDMA6,
+ .port_ops = &ahci_ops,
+ },
};
static struct scsi_host_template ahci_platform_sht = {
We will need this macro in both ahci.c and ahci_platform.c, so just move it
to the header.
Signed-off-by: Brian Norris <[email protected]>
---
drivers/ata/ahci.c | 2 --
drivers/ata/ahci.h | 3 +++
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index d07bf03..ddeb845 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -103,8 +103,6 @@ static struct ata_port_operations ahci_p5wdh_ops = {
.hardreset = ahci_p5wdh_hardreset,
};
-#define AHCI_HFLAGS(flags) .private_data = (void *)(flags)
-
static const struct ata_port_info ahci_port_info[] = {
/* by features */
[board_ahci] =
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index feb127e..c2594dd 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -195,6 +195,9 @@ enum {
PORT_FBS_EN = (1 << 0), /* Enable FBS */
/* hpriv->flags bits */
+
+#define AHCI_HFLAGS(flags) .private_data = (void *)(flags)
+
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 */
On Di, 21 Feb 2012, Brian Norris wrote:
> This series addresses regression problems with
>
> commit 7faa33da9b7add01db9f1ad92c6a5d9145e940a7
> ahci: start engine only during soft/hard resets
>
> Because the fix provided by the above commit caused COMRESET failures and
> 10 second boot-time/resume-time hangs for certain DVD drives, we need to
> make the fix conditional to certain devices/platforms. Add a flag for this.
I confirm that these patches without the previous fix also
fixes the regression I have seen. Thanks.
Best wishes
Norbert
------------------------------------------------------------------------
Norbert Preining preining@{jaist.ac.jp, logic.at, debian.org}
JAIST, Japan TeX Live & Debian Developer
DSA: 0x09C5B094 fp: 14DF 2E6C 0307 BE6D AD76 A9C0 D2BF 4AA3 09C5 B094
------------------------------------------------------------------------
SHIRMERS (pl.n.)
Tall young men who stand around smiling at weddings as if to suggest
that they know they bride reather well.
--- Douglas Adams, The Meaning of Liff