linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4 #upstream-fixes] libata: cosmetic updates
@ 2009-08-04  8:52 Tejun Heo
  2009-08-04  8:52 ` [PATCH 2/4 #upstream-fixes] libata: implement more acpi filtering options Tejun Heo
  2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
  0 siblings, 2 replies; 11+ messages in thread
From: Tejun Heo @ 2009-08-04  8:52 UTC (permalink / raw)
  To: Jeff Garzik, ide, meissner

We're about to add more SATA_* and ATA_ACPI_FILTER_* constants.
Reformat them in preparation.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-acpi.c |   10 ----------
 include/linux/ata.h       |    4 ++--
 include/linux/libata.h    |    9 +++++++++
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index ac176da..b94fe5f 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -20,16 +20,6 @@
 
 #include <acpi/acpi_bus.h>
 
-enum {
-	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
-	ATA_ACPI_FILTER_LOCK	= 1 << 1,
-	ATA_ACPI_FILTER_DIPM	= 1 << 2,
-
-	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
-				  ATA_ACPI_FILTER_LOCK |
-				  ATA_ACPI_FILTER_DIPM,
-};
-
 static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
 module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644);
 MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM)");
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 9c75921..7d0769a 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -304,8 +304,8 @@ enum {
 	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
 
 	/* SETFEATURE Sector counts for SATA features */
-	SATA_AN			= 0x05,  /* Asynchronous Notification */
-	SATA_DIPM		= 0x03,  /* Device Initiated Power Management */
+	SATA_DIPM		= 0x03,	/* Device Initiated Power Management */
+	SATA_AN			= 0x05,	/* Asynchronous Notification */
 
 	/* feature values for SET_MAX */
 	ATA_SET_MAX_ADDR	= 0x00,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e5b6e33..8767adf 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -417,6 +417,15 @@ enum {
 				  ATA_TIMING_ACTIVE | ATA_TIMING_RECOVER |
 				  ATA_TIMING_DMACK_HOLD | ATA_TIMING_CYCLE |
 				  ATA_TIMING_UDMA,
+
+	/* ACPI constants */
+	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
+	ATA_ACPI_FILTER_LOCK	= 1 << 1,
+	ATA_ACPI_FILTER_DIPM	= 1 << 2,
+
+	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
+				  ATA_ACPI_FILTER_LOCK |
+				  ATA_ACPI_FILTER_DIPM,
 };
 
 enum ata_xfer_mask {
-- 
1.6.0.2



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/4 #upstream-fixes] libata: implement more acpi filtering options
  2009-08-04  8:52 [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Tejun Heo
@ 2009-08-04  8:52 ` Tejun Heo
  2009-08-04  8:53   ` [PATCH 3/4 #upstream-fixes] libata: make gtf_filter per-dev Tejun Heo
  2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
  1 sibling, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2009-08-04  8:52 UTC (permalink / raw)
  To: Jeff Garzik, ide, meissner

Currently libata-acpi can only filter DIPM among SATA feature enables
via _GTF.  This patch adds the capability to filter out FPDMA non-zero
offset, in-order guarantee and auto-activation.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-acpi.c |   19 +++++++++++++++----
 include/linux/ata.h       |    4 ++++
 include/linux/libata.h    |    2 ++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index b94fe5f..6ea9d02 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -22,7 +22,7 @@
 
 static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
 module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644);
-MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM)");
+MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM, 0x8=FPDMA non-zero offset, 0x10=FPDMA DMA Setup FIS auto-activate)");
 
 #define NO_PORT_MULT		0xffff
 #define SATA_ADR(root, pmp)	(((root) << 16) | (pmp))
@@ -637,12 +637,23 @@ static int ata_acpi_filter_tf(const struct ata_taskfile *tf,
 			return 1;
 	}
 
-	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_DIPM) {
+	if (tf->command == ATA_CMD_SET_FEATURES &&
+	    tf->feature == SETFEATURES_SATA_ENABLE) {
 		/* inhibit enabling DIPM */
-		if (tf->command == ATA_CMD_SET_FEATURES &&
-		    tf->feature == SETFEATURES_SATA_ENABLE &&
+		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_DIPM &&
 		    tf->nsect == SATA_DIPM)
 			return 1;
+
+		/* inhibit FPDMA non-zero offset */
+		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_OFFSET &&
+		    (tf->nsect == SATA_FPDMA_OFFSET ||
+		     tf->nsect == SATA_FPDMA_IN_ORDER))
+			return 1;
+
+		/* inhibit FPDMA auto activation */
+		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_AA &&
+		    tf->nsect == SATA_FPDMA_AA)
+			return 1;
 	}
 
 	return 0;
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 7d0769a..6066076 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -304,8 +304,12 @@ enum {
 	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
 
 	/* SETFEATURE Sector counts for SATA features */
+	SATA_FPDMA_OFFSET	= 0x01,	/* FPDMA non-zero buffer offsets */
+	SATA_FPDMA_AA		= 0x02,	/* FPDMA DMA setup FIS auto-activate */
 	SATA_DIPM		= 0x03,	/* Device Initiated Power Management */
+	SATA_FPDMA_IN_ORDER	= 0x04,	/* FPDMA in-order data delivery */
 	SATA_AN			= 0x05,	/* Asynchronous Notification */
+	SATA_SSP		= 0x06,	/* Software Settings Preservation */
 
 	/* feature values for SET_MAX */
 	ATA_SET_MAX_ADDR	= 0x00,
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 8767adf..c7938f7 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -422,6 +422,8 @@ enum {
 	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
 	ATA_ACPI_FILTER_LOCK	= 1 << 1,
 	ATA_ACPI_FILTER_DIPM	= 1 << 2,
+	ATA_ACPI_FILTER_FPDMA_OFFSET = 1 << 3,	/* FPDMA non-zero offset */
+	ATA_ACPI_FILTER_FPDMA_AA = 1 << 4,	/* FPDMA auto activate */
 
 	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
 				  ATA_ACPI_FILTER_LOCK |
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4 #upstream-fixes] libata: make gtf_filter per-dev
  2009-08-04  8:52 ` [PATCH 2/4 #upstream-fixes] libata: implement more acpi filtering options Tejun Heo
@ 2009-08-04  8:53   ` Tejun Heo
  2009-08-04  8:57     ` [PATCH 4/4 #upstream-fixes] ahci: filter FPDMA non-zero offset enable for Aspire 3810T Tejun Heo
  0 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2009-08-04  8:53 UTC (permalink / raw)
  To: Jeff Garzik, ide, meissner

Add ->gtf_filter to ata_device and set it to ata_acpi_gtf_filter when
initializing ata_link.  This is to allow quirks which apply different
gtf filters.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-acpi.c |   17 +++++++++--------
 drivers/ata/libata-core.c |    3 +++
 drivers/ata/libata.h      |    2 ++
 include/linux/libata.h    |    1 +
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 6ea9d02..bc6d32e 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -20,7 +20,7 @@
 
 #include <acpi/acpi_bus.h>
 
-static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
+unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
 module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644);
 MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM, 0x8=FPDMA non-zero offset, 0x10=FPDMA DMA Setup FIS auto-activate)");
 
@@ -603,10 +603,11 @@ static void ata_acpi_gtf_to_tf(struct ata_device *dev,
 	tf->command = gtf->tf[6];	/* 0x1f7 */
 }
 
-static int ata_acpi_filter_tf(const struct ata_taskfile *tf,
+static int ata_acpi_filter_tf(struct ata_device *dev,
+			      const struct ata_taskfile *tf,
 			      const struct ata_taskfile *ptf)
 {
-	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_SETXFER) {
+	if (dev->gtf_filter & ATA_ACPI_FILTER_SETXFER) {
 		/* libata doesn't use ACPI to configure transfer mode.
 		 * It will only confuse device configuration.  Skip.
 		 */
@@ -615,7 +616,7 @@ static int ata_acpi_filter_tf(const struct ata_taskfile *tf,
 			return 1;
 	}
 
-	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_LOCK) {
+	if (dev->gtf_filter & ATA_ACPI_FILTER_LOCK) {
 		/* BIOS writers, sorry but we don't wanna lock
 		 * features unless the user explicitly said so.
 		 */
@@ -640,18 +641,18 @@ static int ata_acpi_filter_tf(const struct ata_taskfile *tf,
 	if (tf->command == ATA_CMD_SET_FEATURES &&
 	    tf->feature == SETFEATURES_SATA_ENABLE) {
 		/* inhibit enabling DIPM */
-		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_DIPM &&
+		if (dev->gtf_filter & ATA_ACPI_FILTER_DIPM &&
 		    tf->nsect == SATA_DIPM)
 			return 1;
 
 		/* inhibit FPDMA non-zero offset */
-		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_OFFSET &&
+		if (dev->gtf_filter & ATA_ACPI_FILTER_FPDMA_OFFSET &&
 		    (tf->nsect == SATA_FPDMA_OFFSET ||
 		     tf->nsect == SATA_FPDMA_IN_ORDER))
 			return 1;
 
 		/* inhibit FPDMA auto activation */
-		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_AA &&
+		if (dev->gtf_filter & ATA_ACPI_FILTER_FPDMA_AA &&
 		    tf->nsect == SATA_FPDMA_AA)
 			return 1;
 	}
@@ -704,7 +705,7 @@ static int ata_acpi_run_tf(struct ata_device *dev,
 		pptf = &ptf;
 	}
 
-	if (!ata_acpi_filter_tf(&tf, pptf)) {
+	if (!ata_acpi_filter_tf(dev, &tf, pptf)) {
 		rtf = tf;
 		err_mask = ata_exec_internal(dev, &rtf, NULL,
 					     DMA_NONE, NULL, 0, 0);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8ac98ff..d3ba7bd 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5560,6 +5560,9 @@ void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
 
 		dev->link = link;
 		dev->devno = dev - link->device;
+#ifdef CONFIG_ATA_ACPI
+		dev->gtf_filter = ata_acpi_gtf_filter;
+#endif
 		ata_dev_init(dev);
 	}
 }
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 89a1e00..ab8ae14 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -118,6 +118,8 @@ extern void ata_lpm_schedule(struct ata_port *ap, enum link_pm);
 
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
+extern unsigned int ata_acpi_gtf_filter;
+
 extern void ata_acpi_associate_sata_port(struct ata_port *ap);
 extern void ata_acpi_associate(struct ata_host *host);
 extern void ata_acpi_dissociate(struct ata_host *host);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c7938f7..28b112d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -597,6 +597,7 @@ struct ata_device {
 #ifdef CONFIG_ATA_ACPI
 	acpi_handle		acpi_handle;
 	union acpi_object	*gtf_cache;
+	unsigned int		gtf_filter;
 #endif
 	/* n_sector is CLEAR_BEGIN, read comment above CLEAR_BEGIN */
 	u64			n_sectors;	/* size of device, if ATA */
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4 #upstream-fixes] ahci: filter FPDMA non-zero offset enable for Aspire 3810T
  2009-08-04  8:53   ` [PATCH 3/4 #upstream-fixes] libata: make gtf_filter per-dev Tejun Heo
@ 2009-08-04  8:57     ` Tejun Heo
  0 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2009-08-04  8:57 UTC (permalink / raw)
  To: Jeff Garzik, ide, meissner

Curiously, Aspire 3810T issues many SATA feature enable commands via
_GTF, of which one is invalid and another is not supported by the
drive.  In the process, it also enables FPDMA non-zero offset.
However, the feature also needs to be supported and enabled from the
controller and it's wrong to enable it from _GTF unless the controller
can do it by default.

Currently, this ends up enabling FPDMA non-zero offset only on the
drive side leading to NCQ command failures and eventual disabling of
NCQ.  This patch makes libata filter out FPDMA non-zero offset enable
for the machine.

This was reported by Marcus Meissner in bnc#522790.

  https://bugzilla.novell.com/show_bug.cgi?id=522790

Reported-by: Marcus Meissner <meissner@novell.com>
---
This concludes the four patches to work around a device configuration
issue caused by strange _GTF.  This looks a bit pervasive for
#upstream-fixes but the changes should only affect the problematic
machine and be fairly safe.

Thanks.

 drivers/ata/ahci.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 958c1fa..c84f9c9 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -2726,6 +2726,50 @@ static bool ahci_broken_suspend(struct pci_dev *pdev)
 	return !ver || strcmp(ver, dmi->driver_data) < 0;
 }
 
+static void ahci_gtf_filter_workaround(struct ata_host *host)
+{
+	static const struct dmi_system_id sysids[] = {
+		/*
+		 * Aspire 3810T issues a bunch of SATA enable commands
+		 * via _GTF including an invalid one and one which is
+		 * rejected by the device.  Among the successful ones
+		 * is FPDMA non-zero offset enable which when enabled
+		 * only on the drive side leads to NCQ command
+		 * failures.  Filter it out.
+		 */
+		{
+			.ident = "Aspire 3810T",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 3810T"),
+			},
+			.driver_data = (void *)ATA_ACPI_FILTER_FPDMA_OFFSET,
+		},
+		{ }
+	};
+	const struct dmi_system_id *dmi = dmi_first_match(sysids);
+	unsigned int filter;
+	int i;
+
+	if (!dmi)
+		return;
+
+	filter = (unsigned long)dmi->driver_data;
+	dev_printk(KERN_INFO, host->dev,
+		   "applying extra ACPI _GTF filter 0x%x for %s\n",
+		   filter, dmi->ident);
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		struct ata_link *link;
+		struct ata_device *dev;
+
+		ata_for_each_link(link, ap, EDGE)
+			ata_for_each_dev(dev, link, ALL)
+				dev->gtf_filter |= filter;
+	}
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version;
@@ -2885,6 +2929,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* apply workaround for ASUS P5W DH Deluxe mainboard */
 	ahci_p5wdh_workaround(host);
 
+	/* apply gtf filter quirk */
+	ahci_gtf_filter_workaround(host);
+
 	/* initialize adapter */
 	rc = ahci_configure_dma_masks(pdev, hpriv->cap & HOST_CAP_64);
 	if (rc)
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4 #upstream-fixes] libata: cosmetic updates
  2009-08-04  8:52 [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Tejun Heo
  2009-08-04  8:52 ` [PATCH 2/4 #upstream-fixes] libata: implement more acpi filtering options Tejun Heo
@ 2009-09-09  1:20 ` Jeff Garzik
  2009-09-15 19:16   ` [PATCH REDIFFED 1/4 #upstream] " Tejun Heo
                     ` (3 more replies)
  1 sibling, 4 replies; 11+ messages in thread
From: Jeff Garzik @ 2009-09-09  1:20 UTC (permalink / raw)
  To: Tejun Heo; +Cc: ide, meissner

On 08/04/2009 04:52 AM, Tejun Heo wrote:
> We're about to add more SATA_* and ATA_ACPI_FILTER_* constants.
> Reformat them in preparation.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> ---
>   drivers/ata/libata-acpi.c |   10 ----------
>   include/linux/ata.h       |    4 ++--
>   include/linux/libata.h    |    9 +++++++++

ok, but can you rediff + resend?



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH REDIFFED 1/4 #upstream] libata: cosmetic updates
  2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
@ 2009-09-15 19:16   ` Tejun Heo
  2009-10-06  4:25     ` Jeff Garzik
  2009-09-15 19:17   ` [PATCH REDIFFED 2/4 #upstream] libata: implement more acpi filtering options Tejun Heo
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2009-09-15 19:16 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: ide, meissner

We're about to add more SATA_* and ATA_ACPI_FILTER_* constants.
Reformat them in preparation.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
Almost forgot about this series.  Here's the refreshed version.

Thanks.

 drivers/ata/libata-acpi.c |   10 ----------
 include/linux/ata.h       |    6 +++---
 include/linux/libata.h    |    9 +++++++++
 3 files changed, 12 insertions(+), 13 deletions(-)

Index: ata/drivers/ata/libata-acpi.c
===================================================================
--- ata.orig/drivers/ata/libata-acpi.c
+++ ata/drivers/ata/libata-acpi.c
@@ -20,16 +20,6 @@

 #include <acpi/acpi_bus.h>

-enum {
-	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
-	ATA_ACPI_FILTER_LOCK	= 1 << 1,
-	ATA_ACPI_FILTER_DIPM	= 1 << 2,
-
-	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
-				  ATA_ACPI_FILTER_LOCK |
-				  ATA_ACPI_FILTER_DIPM,
-};
-
 static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
 module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644);
 MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM)");
Index: ata/include/linux/ata.h
===================================================================
--- ata.orig/include/linux/ata.h
+++ ata/include/linux/ata.h
@@ -334,9 +334,9 @@ enum {
 	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */

 	/* SETFEATURE Sector counts for SATA features */
-	SATA_AN			= 0x05,  /* Asynchronous Notification */
-	SATA_DIPM		= 0x03,  /* Device Initiated Power Management */
-	SATA_FPDMA_AA		= 0x02,  /* DMA Setup FIS Auto-Activate */
+	SATA_FPDMA_AA		= 0x02, /* FPDMA Setup FIS Auto-Activate */
+	SATA_DIPM		= 0x03,	/* Device Initiated Power Management */
+	SATA_AN			= 0x05,	/* Asynchronous Notification */

 	/* feature values for SET_MAX */
 	ATA_SET_MAX_ADDR	= 0x00,
Index: ata/include/linux/libata.h
===================================================================
--- ata.orig/include/linux/libata.h
+++ ata/include/linux/libata.h
@@ -418,6 +418,15 @@ enum {
 				  ATA_TIMING_ACTIVE | ATA_TIMING_RECOVER |
 				  ATA_TIMING_DMACK_HOLD | ATA_TIMING_CYCLE |
 				  ATA_TIMING_UDMA,
+
+	/* ACPI constants */
+	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
+	ATA_ACPI_FILTER_LOCK	= 1 << 1,
+	ATA_ACPI_FILTER_DIPM	= 1 << 2,
+
+	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
+				  ATA_ACPI_FILTER_LOCK |
+				  ATA_ACPI_FILTER_DIPM,
 };

 enum ata_xfer_mask {

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH REDIFFED 2/4 #upstream] libata: implement more acpi filtering options
  2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
  2009-09-15 19:16   ` [PATCH REDIFFED 1/4 #upstream] " Tejun Heo
@ 2009-09-15 19:17   ` Tejun Heo
  2009-10-06  4:56     ` Jeff Garzik
  2009-09-15 19:17   ` [PATCH REDIFFED 3/4 #upstream] libata: make gtf_filter per-dev Tejun Heo
  2009-09-15 19:18   ` [PATCH REDIFFED 4/4 #upstream] ahci: filter FPDMA non-zero offset enable for Aspire 3810T Tejun Heo
  3 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2009-09-15 19:17 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: ide, meissner

Currently libata-acpi can only filter DIPM among SATA feature enables
via _GTF.  This patch adds the capability to filter out FPDMA non-zero
offset, in-order guarantee and auto-activation.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-acpi.c |   19 +++++++++++++++----
 include/linux/ata.h       |    3 +++
 include/linux/libata.h    |    2 ++
 3 files changed, 20 insertions(+), 4 deletions(-)

Index: ata/drivers/ata/libata-acpi.c
===================================================================
--- ata.orig/drivers/ata/libata-acpi.c
+++ ata/drivers/ata/libata-acpi.c
@@ -22,7 +22,7 @@

 static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
 module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644);
-MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM)");
+MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM, 0x8=FPDMA non-zero offset, 0x10=FPDMA DMA Setup FIS auto-activate)");

 #define NO_PORT_MULT		0xffff
 #define SATA_ADR(root, pmp)	(((root) << 16) | (pmp))
@@ -637,12 +637,23 @@ static int ata_acpi_filter_tf(const stru
 			return 1;
 	}

-	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_DIPM) {
+	if (tf->command == ATA_CMD_SET_FEATURES &&
+	    tf->feature == SETFEATURES_SATA_ENABLE) {
 		/* inhibit enabling DIPM */
-		if (tf->command == ATA_CMD_SET_FEATURES &&
-		    tf->feature == SETFEATURES_SATA_ENABLE &&
+		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_DIPM &&
 		    tf->nsect == SATA_DIPM)
 			return 1;
+
+		/* inhibit FPDMA non-zero offset */
+		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_OFFSET &&
+		    (tf->nsect == SATA_FPDMA_OFFSET ||
+		     tf->nsect == SATA_FPDMA_IN_ORDER))
+			return 1;
+
+		/* inhibit FPDMA auto activation */
+		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_AA &&
+		    tf->nsect == SATA_FPDMA_AA)
+			return 1;
 	}

 	return 0;
Index: ata/include/linux/ata.h
===================================================================
--- ata.orig/include/linux/ata.h
+++ ata/include/linux/ata.h
@@ -334,9 +334,12 @@ enum {
 	SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */

 	/* SETFEATURE Sector counts for SATA features */
+	SATA_FPDMA_OFFSET	= 0x01,	/* FPDMA non-zero buffer offsets */
 	SATA_FPDMA_AA		= 0x02, /* FPDMA Setup FIS Auto-Activate */
 	SATA_DIPM		= 0x03,	/* Device Initiated Power Management */
+	SATA_FPDMA_IN_ORDER	= 0x04,	/* FPDMA in-order data delivery */
 	SATA_AN			= 0x05,	/* Asynchronous Notification */
+	SATA_SSP		= 0x06,	/* Software Settings Preservation */

 	/* feature values for SET_MAX */
 	ATA_SET_MAX_ADDR	= 0x00,
Index: ata/include/linux/libata.h
===================================================================
--- ata.orig/include/linux/libata.h
+++ ata/include/linux/libata.h
@@ -423,6 +423,8 @@ enum {
 	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
 	ATA_ACPI_FILTER_LOCK	= 1 << 1,
 	ATA_ACPI_FILTER_DIPM	= 1 << 2,
+	ATA_ACPI_FILTER_FPDMA_OFFSET = 1 << 3,	/* FPDMA non-zero offset */
+	ATA_ACPI_FILTER_FPDMA_AA = 1 << 4,	/* FPDMA auto activate */

 	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
 				  ATA_ACPI_FILTER_LOCK |

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH REDIFFED 3/4 #upstream] libata: make gtf_filter per-dev
  2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
  2009-09-15 19:16   ` [PATCH REDIFFED 1/4 #upstream] " Tejun Heo
  2009-09-15 19:17   ` [PATCH REDIFFED 2/4 #upstream] libata: implement more acpi filtering options Tejun Heo
@ 2009-09-15 19:17   ` Tejun Heo
  2009-09-15 19:18   ` [PATCH REDIFFED 4/4 #upstream] ahci: filter FPDMA non-zero offset enable for Aspire 3810T Tejun Heo
  3 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2009-09-15 19:17 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: ide, meissner

Add ->gtf_filter to ata_device and set it to ata_acpi_gtf_filter when
initializing ata_link.  This is to allow quirks which apply different
gtf filters.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 drivers/ata/libata-acpi.c |   17 +++++++++--------
 drivers/ata/libata-core.c |    3 +++
 drivers/ata/libata.h      |    2 ++
 include/linux/libata.h    |    1 +
 4 files changed, 15 insertions(+), 8 deletions(-)

Index: ata/drivers/ata/libata-acpi.c
===================================================================
--- ata.orig/drivers/ata/libata-acpi.c
+++ ata/drivers/ata/libata-acpi.c
@@ -20,7 +20,7 @@

 #include <acpi/acpi_bus.h>

-static unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
+unsigned int ata_acpi_gtf_filter = ATA_ACPI_FILTER_DEFAULT;
 module_param_named(acpi_gtf_filter, ata_acpi_gtf_filter, int, 0644);
 MODULE_PARM_DESC(acpi_gtf_filter, "filter mask for ACPI _GTF commands, set to filter out (0x1=set xfermode, 0x2=lock/freeze lock, 0x4=DIPM, 0x8=FPDMA non-zero offset, 0x10=FPDMA DMA Setup FIS auto-activate)");

@@ -603,10 +603,11 @@ static void ata_acpi_gtf_to_tf(struct at
 	tf->command = gtf->tf[6];	/* 0x1f7 */
 }

-static int ata_acpi_filter_tf(const struct ata_taskfile *tf,
+static int ata_acpi_filter_tf(struct ata_device *dev,
+			      const struct ata_taskfile *tf,
 			      const struct ata_taskfile *ptf)
 {
-	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_SETXFER) {
+	if (dev->gtf_filter & ATA_ACPI_FILTER_SETXFER) {
 		/* libata doesn't use ACPI to configure transfer mode.
 		 * It will only confuse device configuration.  Skip.
 		 */
@@ -615,7 +616,7 @@ static int ata_acpi_filter_tf(const stru
 			return 1;
 	}

-	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_LOCK) {
+	if (dev->gtf_filter & ATA_ACPI_FILTER_LOCK) {
 		/* BIOS writers, sorry but we don't wanna lock
 		 * features unless the user explicitly said so.
 		 */
@@ -640,18 +641,18 @@ static int ata_acpi_filter_tf(const stru
 	if (tf->command == ATA_CMD_SET_FEATURES &&
 	    tf->feature == SETFEATURES_SATA_ENABLE) {
 		/* inhibit enabling DIPM */
-		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_DIPM &&
+		if (dev->gtf_filter & ATA_ACPI_FILTER_DIPM &&
 		    tf->nsect == SATA_DIPM)
 			return 1;

 		/* inhibit FPDMA non-zero offset */
-		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_OFFSET &&
+		if (dev->gtf_filter & ATA_ACPI_FILTER_FPDMA_OFFSET &&
 		    (tf->nsect == SATA_FPDMA_OFFSET ||
 		     tf->nsect == SATA_FPDMA_IN_ORDER))
 			return 1;

 		/* inhibit FPDMA auto activation */
-		if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_FPDMA_AA &&
+		if (dev->gtf_filter & ATA_ACPI_FILTER_FPDMA_AA &&
 		    tf->nsect == SATA_FPDMA_AA)
 			return 1;
 	}
@@ -705,7 +706,7 @@ static int ata_acpi_run_tf(struct ata_de
 		pptf = &ptf;
 	}

-	if (!ata_acpi_filter_tf(&tf, pptf)) {
+	if (!ata_acpi_filter_tf(dev, &tf, pptf)) {
 		rtf = tf;
 		err_mask = ata_exec_internal(dev, &rtf, NULL,
 					     DMA_NONE, NULL, 0, 0);
Index: ata/drivers/ata/libata-core.c
===================================================================
--- ata.orig/drivers/ata/libata-core.c
+++ ata/drivers/ata/libata-core.c
@@ -5591,6 +5591,9 @@ void ata_link_init(struct ata_port *ap,

 		dev->link = link;
 		dev->devno = dev - link->device;
+#ifdef CONFIG_ATA_ACPI
+		dev->gtf_filter = ata_acpi_gtf_filter;
+#endif
 		ata_dev_init(dev);
 	}
 }
Index: ata/drivers/ata/libata.h
===================================================================
--- ata.orig/drivers/ata/libata.h
+++ ata/drivers/ata/libata.h
@@ -118,6 +118,8 @@ extern void ata_lpm_schedule(struct ata_

 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
+extern unsigned int ata_acpi_gtf_filter;
+
 extern void ata_acpi_associate_sata_port(struct ata_port *ap);
 extern void ata_acpi_associate(struct ata_host *host);
 extern void ata_acpi_dissociate(struct ata_host *host);
Index: ata/include/linux/libata.h
===================================================================
--- ata.orig/include/linux/libata.h
+++ ata/include/linux/libata.h
@@ -598,6 +598,7 @@ struct ata_device {
 #ifdef CONFIG_ATA_ACPI
 	acpi_handle		acpi_handle;
 	union acpi_object	*gtf_cache;
+	unsigned int		gtf_filter;
 #endif
 	/* n_sector is CLEAR_BEGIN, read comment above CLEAR_BEGIN */
 	u64			n_sectors;	/* size of device, if ATA */

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH REDIFFED 4/4 #upstream] ahci: filter FPDMA non-zero offset enable for Aspire 3810T
  2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
                     ` (2 preceding siblings ...)
  2009-09-15 19:17   ` [PATCH REDIFFED 3/4 #upstream] libata: make gtf_filter per-dev Tejun Heo
@ 2009-09-15 19:18   ` Tejun Heo
  3 siblings, 0 replies; 11+ messages in thread
From: Tejun Heo @ 2009-09-15 19:18 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: ide, meissner

Curiously, Aspire 3810T issues many SATA feature enable commands via
_GTF, of which one is invalid and another is not supported by the
drive.  In the process, it also enables FPDMA non-zero offset.
However, the feature also needs to be supported and enabled from the
controller and it's wrong to enable it from _GTF unless the controller
can do it by default.

Currently, this ends up enabling FPDMA non-zero offset only on the
drive side leading to NCQ command failures and eventual disabling of
NCQ.  This patch makes libata filter out FPDMA non-zero offset enable
for the machine.

This was reported by Marcus Meissner in bnc#522790.

  https://bugzilla.novell.com/show_bug.cgi?id=522790

Reported-by: Marcus Meissner <meissner@novell.com>
---
This concludes the four patches to work around a device configuration
issue caused by strange _GTF.  This looks a bit pervasive for
#upstream-fixes but the changes should only affect the problematic
machine and be fairly safe.

Thanks.

 drivers/ata/ahci.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Index: ata/drivers/ata/ahci.c
===================================================================
--- ata.orig/drivers/ata/ahci.c
+++ ata/drivers/ata/ahci.c
@@ -2858,6 +2858,50 @@ static bool ahci_broken_online(struct pc
 	return pdev->bus->number == (val >> 8) && pdev->devfn == (val & 0xff);
 }

+static void ahci_gtf_filter_workaround(struct ata_host *host)
+{
+	static const struct dmi_system_id sysids[] = {
+		/*
+		 * Aspire 3810T issues a bunch of SATA enable commands
+		 * via _GTF including an invalid one and one which is
+		 * rejected by the device.  Among the successful ones
+		 * is FPDMA non-zero offset enable which when enabled
+		 * only on the drive side leads to NCQ command
+		 * failures.  Filter it out.
+		 */
+		{
+			.ident = "Aspire 3810T",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "Aspire 3810T"),
+			},
+			.driver_data = (void *)ATA_ACPI_FILTER_FPDMA_OFFSET,
+		},
+		{ }
+	};
+	const struct dmi_system_id *dmi = dmi_first_match(sysids);
+	unsigned int filter;
+	int i;
+
+	if (!dmi)
+		return;
+
+	filter = (unsigned long)dmi->driver_data;
+	dev_printk(KERN_INFO, host->dev,
+		   "applying extra ACPI _GTF filter 0x%x for %s\n",
+		   filter, dmi->ident);
+
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		struct ata_link *link;
+		struct ata_device *dev;
+
+		ata_for_each_link(link, ap, EDGE)
+			ata_for_each_dev(dev, link, ALL)
+				dev->gtf_filter |= filter;
+	}
+}
+
 static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	static int printed_version;
@@ -3023,6 +3067,9 @@ static int ahci_init_one(struct pci_dev
 	/* apply workaround for ASUS P5W DH Deluxe mainboard */
 	ahci_p5wdh_workaround(host);

+	/* apply gtf filter quirk */
+	ahci_gtf_filter_workaround(host);
+
 	/* initialize adapter */
 	rc = ahci_configure_dma_masks(pdev, hpriv->cap & HOST_CAP_64);
 	if (rc)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH REDIFFED 1/4 #upstream] libata: cosmetic updates
  2009-09-15 19:16   ` [PATCH REDIFFED 1/4 #upstream] " Tejun Heo
@ 2009-10-06  4:25     ` Jeff Garzik
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2009-10-06  4:25 UTC (permalink / raw)
  To: Tejun Heo; +Cc: ide, meissner

On 09/15/2009 03:16 PM, Tejun Heo wrote:
> We're about to add more SATA_* and ATA_ACPI_FILTER_* constants.
> Reformat them in preparation.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> ---
> Almost forgot about this series.  Here's the refreshed version.
>
> Thanks.
>
>   drivers/ata/libata-acpi.c |   10 ----------
>   include/linux/ata.h       |    6 +++---
>   include/linux/libata.h    |    9 +++++++++
>   3 files changed, 12 insertions(+), 13 deletions(-)

applied



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH REDIFFED 2/4 #upstream] libata: implement more acpi filtering options
  2009-09-15 19:17   ` [PATCH REDIFFED 2/4 #upstream] libata: implement more acpi filtering options Tejun Heo
@ 2009-10-06  4:56     ` Jeff Garzik
  0 siblings, 0 replies; 11+ messages in thread
From: Jeff Garzik @ 2009-10-06  4:56 UTC (permalink / raw)
  To: Tejun Heo; +Cc: ide, meissner

On 09/15/2009 03:17 PM, Tejun Heo wrote:
> Currently libata-acpi can only filter DIPM among SATA feature enables
> via _GTF.  This patch adds the capability to filter out FPDMA non-zero
> offset, in-order guarantee and auto-activation.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> ---
>   drivers/ata/libata-acpi.c |   19 +++++++++++++++----
>   include/linux/ata.h       |    3 +++
>   include/linux/libata.h    |    2 ++
>   3 files changed, 20 insertions(+), 4 deletions(-)

applied this series



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2009-10-06  4:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-04  8:52 [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Tejun Heo
2009-08-04  8:52 ` [PATCH 2/4 #upstream-fixes] libata: implement more acpi filtering options Tejun Heo
2009-08-04  8:53   ` [PATCH 3/4 #upstream-fixes] libata: make gtf_filter per-dev Tejun Heo
2009-08-04  8:57     ` [PATCH 4/4 #upstream-fixes] ahci: filter FPDMA non-zero offset enable for Aspire 3810T Tejun Heo
2009-09-09  1:20 ` [PATCH 1/4 #upstream-fixes] libata: cosmetic updates Jeff Garzik
2009-09-15 19:16   ` [PATCH REDIFFED 1/4 #upstream] " Tejun Heo
2009-10-06  4:25     ` Jeff Garzik
2009-09-15 19:17   ` [PATCH REDIFFED 2/4 #upstream] libata: implement more acpi filtering options Tejun Heo
2009-10-06  4:56     ` Jeff Garzik
2009-09-15 19:17   ` [PATCH REDIFFED 3/4 #upstream] libata: make gtf_filter per-dev Tejun Heo
2009-09-15 19:18   ` [PATCH REDIFFED 4/4 #upstream] ahci: filter FPDMA non-zero offset enable for Aspire 3810T Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).