* [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
* 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
* [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
* 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
* [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
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).