[PATCHSET #upstram-fixes] libata-acpi: improve ACPI corner case handling, take #2

[PATCHSET #upstram-fixes] libata-acpi: improve ACPI corner case handling, take #2

[Tejun Heo]

Hello,

This is the second take of improve-ACPI-corner-case-handling patchset
and contains the following ten patches.

  0001-libata-acpi-adjust-constness-in-ata_acpi_gtm-stm.patch
  0002-libata-update-ata_-_printk-macros-such-that-level.patch
  0003-libata-add-more-opcodes-to-ata.h.patch
  0004-libata-ata_dev_disable-should-be-called-from-EH-c.patch
  0005-libata-acpi-add-new-hooks-ata_acpi_dissociate-and.patch
  0006-libata-acpi-implement-and-use-ata_acpi_init_gtm.patch
  0007-libata-acpi-implement-dev-gtf_cache-and-evaluate-_.patch
  0008-libata-acpi-improve-ACPI-disabling.patch
  0009-libata-acpi-improve-_GTF-execution-error-handling-a.patch
  0010-libata-acpi-implement-_GTF-command-filtering.patch

Changes from the last take [L] are...

* @stm parameter is constified in 0001.

* 0005 is added.  This patch adds two ATA ACPI hooks for host
  dissociation and device disable.  The former is used to restore
  initial _GTM setting on driver detach.  The latter is used to
  implement _GTF caching in 0007.

* 0006 updated such that init_gtm completely replaces all internal
  _GTM data usage and init_gtm is restored using _STM on driver
  detach.  This change makes ignore-_GTM-failure-during-suspend patch
  unnecesary.

* 0007 is added.  This makes IDE _GTF evaluated right after _STM
  during resume.

* 0008 is updated such that ACPI evaulation failure doesn't trigger
  ACPI disabling unless it causes other failures.

* 0010 now also filters SET FEATURES - SET XFERMODE.  SETXFER from
  _GTF only disrupts device configuration by putting the device in
  possibly inconsistent mode from the controller.

The biggest change is that _GTM is now only performed on driver
attach.  The cached init_gtm value is used during resume.  This,
combined with moving _GTF evaluation right after _STM, solves all
currently known _GTM, _STM and _GTF evaluation failures.  By feeding
the initial values configured by the BIOS, it can avoid problems of
missing table entries or what not.

This makes ACPI mode configuration for both controller and device
meaningless but libata doesn't use them anyway.  In fact, all they do
is disrupting device configuration by feeding inconsistent mode to
devices via _GTF taskfiles, which are filtered by 0010 now.

Note that in many cases, _GTM/_STM implementations are very brittle
and can't be used as generic dynamic configuration method.

This patch fixes all known ATA ACPI related regressions including OSDL
bugs 9320 and 9530.

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.ide/26334

[PATCH 01/10] libata-acpi: adjust constness in ata_acpi_gtm/stm() parameters

[Tejun Heo]

* No internal function uses const ata_port.  Drop const from @ap.

* Make ata_acpi_stm() copy @stm before using it and change @stm to
  const.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |    7 ++++---
 include/linux/libata.h    |    4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 545ea86..8ae36ad 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -200,7 +200,7 @@ void ata_acpi_associate(struct ata_host *host)
  * RETURNS:
  * 0 on success, -ENOENT if _GTM doesn't exist, -errno on failure.
  */
-int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *gtm)
+int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *gtm)
 {
 	struct acpi_buffer output = { .length = ACPI_ALLOCATE_BUFFER };
 	union acpi_object *out_obj;
@@ -259,15 +259,16 @@ EXPORT_SYMBOL_GPL(ata_acpi_gtm);
  * RETURNS:
  * 0 on success, -ENOENT if _STM doesn't exist, -errno on failure.
  */
-int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm)
+int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm)
 {
 	acpi_status status;
+	struct ata_acpi_gtm		stm_buf = *stm;
 	struct acpi_object_list         input;
 	union acpi_object               in_params[3];
 
 	in_params[0].type = ACPI_TYPE_BUFFER;
 	in_params[0].buffer.length = sizeof(struct ata_acpi_gtm);
-	in_params[0].buffer.pointer = (u8 *)stm;
+	in_params[0].buffer.pointer = (u8 *)&stm_buf;
 	/* Buffers for id may need byteswapping ? */
 	in_params[1].type = ACPI_TYPE_BUFFER;
 	in_params[1].buffer.length = 512;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ef52a07..1ca9b89 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -940,8 +940,8 @@ enum {
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
 extern int ata_acpi_cbl_80wire(struct ata_port *ap);
-int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
-int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
+int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm);
+int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm);
 #else
 static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; }
 #endif
-- 
1.5.2.4

[PATCH 02/10] libata: update ata_*_printk() macros such that level can be a variable

[Tejun Heo]

Make prink helpers format @lv together rather than prepending to the
format string as constant.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 include/linux/libata.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1ca9b89..3784af3 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1013,18 +1013,18 @@ extern void ata_do_eh(struct ata_port *ap, ata_prereset_fn_t prereset,
  * printk helpers
  */
 #define ata_port_printk(ap, lv, fmt, args...) \
-	printk(lv"ata%u: "fmt, (ap)->print_id , ##args)
+	printk("%sata%u: "fmt, lv, (ap)->print_id , ##args)
 
 #define ata_link_printk(link, lv, fmt, args...) do { \
 	if ((link)->ap->nr_pmp_links) \
-		printk(lv"ata%u.%02u: "fmt, (link)->ap->print_id, \
+		printk("%sata%u.%02u: "fmt, lv, (link)->ap->print_id,	\
 		       (link)->pmp , ##args); \
 	else \
-		printk(lv"ata%u: "fmt, (link)->ap->print_id , ##args); \
+		printk("%sata%u: "fmt, lv, (link)->ap->print_id , ##args); \
 	} while(0)
 
 #define ata_dev_printk(dev, lv, fmt, args...) \
-	printk(lv"ata%u.%02u: "fmt, (dev)->link->ap->print_id, \
+	printk("%sata%u.%02u: "fmt, lv, (dev)->link->ap->print_id,	\
 	       (dev)->link->pmp + (dev)->devno , ##args)
 
 /*
-- 
1.5.2.4

[PATCH 03/10] libata: add more opcodes to ata.h

[Tejun Heo]

Add constants for DEVICE CONFIGURATION OVERLAY and SET_MAX to
include/linux/ata.h.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 include/linux/ata.h |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/include/linux/ata.h b/include/linux/ata.h
index 5c4e54a..72ab808 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -190,6 +190,8 @@ enum {
 	ATA_CMD_READ_LOG_EXT	= 0x2f,
 	ATA_CMD_PMP_READ	= 0xE4,
 	ATA_CMD_PMP_WRITE	= 0xE8,
+	ATA_CMD_CONF_OVERLAY	= 0xB1,
+	ATA_CMD_SEC_FREEZE_LOCK	= 0xF5,
 
 	/* READ_LOG_EXT pages */
 	ATA_LOG_SATA_NCQ	= 0x10,
@@ -239,6 +241,19 @@ enum {
 	SATA_AN			= 0x05,  /* Asynchronous Notification */
 	SATA_DIPM		= 0x03,  /* Device Initiated Power Management */
 
+	/* feature values for SET_MAX */
+	ATA_SET_MAX_ADDR	= 0x00,
+	ATA_SET_MAX_PASSWD	= 0x01,
+	ATA_SET_MAX_LOCK	= 0x02,
+	ATA_SET_MAX_UNLOCK	= 0x03,
+	ATA_SET_MAX_FREEZE_LOCK	= 0x04,
+
+	/* feature values for DEVICE CONFIGURATION OVERLAY */
+	ATA_DCO_RESTORE		= 0xC0,
+	ATA_DCO_FREEZE_LOCK	= 0xC1,
+	ATA_DCO_IDENTIFY	= 0xC2,
+	ATA_DCO_SET		= 0xC3,
+
 	/* ATAPI stuff */
 	ATAPI_PKT_DMA		= (1 << 0),
 	ATAPI_DMADIR		= (1 << 2),	/* ATAPI data dir:
-- 
1.5.2.4

[PATCH 04/10] libata: ata_dev_disable() should be called from EH context

[Tejun Heo]

ata_port_detach() calls ata_dev_disable() with host lock held but
ata_dev_disable() should be called from EH context.  ata_port_detach()
steals EH context by setting ATA_PFLAG_UNLOADAING and flushing EH.
Drop locking around ata_dev_disable() and note that ata_port_detach()
owns EH context at that point.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index e4dea86..a8131e5 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -7208,18 +7208,14 @@ static void ata_port_detach(struct ata_port *ap)
 
 	ata_port_wait_eh(ap);
 
-	/* EH is now guaranteed to see UNLOADING, so no new device
-	 * will be attached.  Disable all existing devices.
+	/* EH is now guaranteed to see UNLOADING - EH context belongs
+	 * to us.  Disable all existing devices.
 	 */
-	spin_lock_irqsave(ap->lock, flags);
-
 	ata_port_for_each_link(link, ap) {
 		ata_link_for_each_dev(dev, link)
 			ata_dev_disable(dev);
 	}
 
-	spin_unlock_irqrestore(ap->lock, flags);
-
 	/* Final freeze & EH.  All in-flight commands are aborted.  EH
 	 * will be skipped and retrials will be terminated with bad
 	 * target.
-- 
1.5.2.4

[PATCH 05/10] libata-acpi: add new hooks ata_acpi_dissociate() and ata_acpi_on_disable()

[Tejun Heo]

Add two hooks - ata_acpi_dissociate() which is called during driver
detach after the whole host is shutdown and ata_acpi_on_disable()
which is called when a device is disabled.

Signed-off-by: Tejun heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |   28 ++++++++++++++++++++++++++++
 drivers/ata/libata-core.c |    4 ++++
 drivers/ata/libata.h      |    8 ++++++--
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 8ae36ad..9e5fc5d 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -188,6 +188,21 @@ void ata_acpi_associate(struct ata_host *host)
 }
 
 /**
+ * ata_acpi_dissociate - dissociate ATA host from ACPI objects
+ * @host: target ATA host
+ *
+ * This function is called during driver detach after the whole host
+ * is shut down.
+ *
+ * LOCKING:
+ * EH context.
+ */
+void ata_acpi_dissociate(struct ata_host *host)
+{
+	/* nada */
+}
+
+/**
  * ata_acpi_gtm - execute _GTM
  * @ap: target ATA port
  * @gtm: out parameter for _GTM result
@@ -716,3 +731,16 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
 	dev->flags |= ATA_DFLAG_ACPI_FAILED;
 	return rc;
 }
+
+/**
+ * ata_acpi_on_disable - ATA ACPI hook called when a device is disabled
+ * @dev: target ATA device
+ *
+ * This function is called when @dev is about to be disabled.
+ *
+ * LOCKING:
+ * EH context.
+ */
+void ata_acpi_on_disable(struct ata_device *dev)
+{
+}
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index a8131e5..0e21322 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -622,6 +622,7 @@ void ata_dev_disable(struct ata_device *dev)
 	if (ata_dev_enabled(dev)) {
 		if (ata_msg_drv(dev->link->ap))
 			ata_dev_printk(dev, KERN_WARNING, "disabled\n");
+		ata_acpi_on_disable(dev);
 		ata_down_xfermask_limit(dev, ATA_DNXFER_FORCE_PIO0 |
 					     ATA_DNXFER_QUIET);
 		dev->class++;
@@ -7247,6 +7248,9 @@ void ata_host_detach(struct ata_host *host)
 
 	for (i = 0; i < host->n_ports; i++)
 		ata_port_detach(host->ports[i]);
+
+	/* the host is dead now, dissociate ACPI */
+	ata_acpi_dissociate(host);
 }
 
 /**
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 0e6cf3a..bbe59c2 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -108,15 +108,19 @@ extern void ata_lpm_schedule(struct ata_port *ap, enum link_pm);
 #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 void ata_acpi_dissociate(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);
+extern int ata_acpi_on_devcfg(struct ata_device *dev);
+extern void ata_acpi_on_disable(struct ata_device *dev);
 #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 void ata_acpi_dissociate(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) { }
-static inline int ata_acpi_on_devcfg(struct ata_device *adev) { return 0; }
+static inline int ata_acpi_on_devcfg(struct ata_device *dev) { return 0; }
+static inline void ata_acpi_on_disable(struct ata_device *dev) { }
 #endif
 
 /* libata-scsi.c */
-- 
1.5.2.4

[PATCH 06/10] libata-acpi: implement and use ata_acpi_init_gtm()

[Tejun Heo]

_GTM fetches currently configured transfer mode while _STM configures
controller according to _GTM parameter and prepares transfer mode
configuration TFs for _GTF.  In many cases _GTM and _STM
implementations are quite brittle and can't cope with configuration
changed by libata.

libata does not depend on ATA ACPI to configure devices.  The only
reason libata performs _GTM and _STM are to make _GTF evaluation
succeed and libata also doesn't care about how _GTF TFs configure
transfer mode.  It overrides that configuration anyway, so from
libata's POV, it doesn't matter what value is feeded to _STM as long
as evaluation succeeds for _STM and following _GTF.

This patch adds dev->__acpi_init_gtm and store initial _GTM values on
host initialization before modified by reset and mode configuration.
If the field is valid, ata_acpi_init_gtm() returns pointer to the
saved _GTM structure; otherwise, NULL.

This saved value is used for _STM during resume and peek at
BIOS/firmware programmed initial timing for later use.  The accessor
is there to make building w/o ACPI easy as dev->__acpi_init doesn't
exist if ACPI is not enabled.

On driver detach, the initial BIOS configuration is restored by
executing _STM with the initial _GTM values such that the next driver
can also use the initial BIOS configured values.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |   62 +++++++++++++++++++-------------------------
 include/linux/libata.h    |   14 ++++++++-
 2 files changed, 39 insertions(+), 37 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 9e5fc5d..b3aeca3 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -94,6 +94,9 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap)
 
 		dev->acpi_handle = acpi_get_child(ap->acpi_handle, i);
 	}
+
+	if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+		ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
 }
 
 static void ata_acpi_handle_hotplug(struct ata_port *ap, struct kobject *kobj,
@@ -199,7 +202,18 @@ void ata_acpi_associate(struct ata_host *host)
  */
 void ata_acpi_dissociate(struct ata_host *host)
 {
-	/* nada */
+	int i;
+
+	/* Restore initial _GTM values so that driver which attaches
+	 * afterward can use them too.
+	 */
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap);
+
+		if (ap->acpi_handle && gtm)
+			ata_acpi_stm(ap, gtm);
+	}
 }
 
 /**
@@ -409,22 +423,21 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
 
 int ata_acpi_cbl_80wire(struct ata_port *ap)
 {
-	struct ata_acpi_gtm gtm;
+	const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap);
 	int valid = 0;
 
-	/* No _GTM data, no information */
-	if (ata_acpi_gtm(ap, &gtm) < 0)
+	if (!gtm)
 		return 0;
 
 	/* Split timing, DMA enabled */
-	if ((gtm.flags & 0x11) == 0x11 && gtm.drive[0].dma < 55)
+	if ((gtm->flags & 0x11) == 0x11 && gtm->drive[0].dma < 55)
 		valid |= 1;
-	if ((gtm.flags & 0x14) == 0x14 && gtm.drive[1].dma < 55)
+	if ((gtm->flags & 0x14) == 0x14 && gtm->drive[1].dma < 55)
 		valid |= 2;
 	/* Shared timing, DMA enabled */
-	if ((gtm.flags & 0x11) == 0x01 && gtm.drive[0].dma < 55)
+	if ((gtm->flags & 0x11) == 0x01 && gtm->drive[0].dma < 55)
 		valid |= 1;
-	if ((gtm.flags & 0x14) == 0x04 && gtm.drive[0].dma < 55)
+	if ((gtm->flags & 0x14) == 0x04 && gtm->drive[0].dma < 55)
 		valid |= 2;
 
 	/* Drive check */
@@ -612,27 +625,8 @@ static int ata_acpi_push_id(struct ata_device *dev)
  */
 int ata_acpi_on_suspend(struct ata_port *ap)
 {
-	unsigned long flags;
-	int rc;
-
-	/* proceed iff per-port acpi_handle is valid */
-	if (!ap->acpi_handle)
-		return 0;
-	BUG_ON(ap->flags & ATA_FLAG_ACPI_SATA);
-
-	/* store timing parameters */
-	rc = ata_acpi_gtm(ap, &ap->acpi_gtm);
-
-	spin_lock_irqsave(ap->lock, flags);
-	if (rc == 0)
-		ap->pflags |= ATA_PFLAG_GTM_VALID;
-	else
-		ap->pflags &= ~ATA_PFLAG_GTM_VALID;
-	spin_unlock_irqrestore(ap->lock, flags);
-
-	if (rc == -ENOENT)
-		rc = 0;
-	return rc;
+	/* nada */
+	return 0;
 }
 
 /**
@@ -647,14 +641,12 @@ int ata_acpi_on_suspend(struct ata_port *ap)
  */
 void ata_acpi_on_resume(struct ata_port *ap)
 {
+	const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap);
 	struct ata_device *dev;
 
-	if (ap->acpi_handle && (ap->pflags & ATA_PFLAG_GTM_VALID)) {
-		BUG_ON(ap->flags & ATA_FLAG_ACPI_SATA);
-
-		/* restore timing parameters */
-		ata_acpi_stm(ap, &ap->acpi_gtm);
-	}
+	/* restore timing parameters */
+	if (ap->acpi_handle && gtm)
+		ata_acpi_stm(ap, gtm);
 
 	/* schedule _GTF */
 	ata_link_for_each_dev(dev, &ap->link)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3784af3..ba84d8a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -211,7 +211,7 @@ enum {
 
 	ATA_PFLAG_SUSPENDED	= (1 << 17), /* port is suspended (power) */
 	ATA_PFLAG_PM_PENDING	= (1 << 18), /* PM operation pending */
-	ATA_PFLAG_GTM_VALID	= (1 << 19), /* acpi_gtm data valid */
+	ATA_PFLAG_INIT_GTM_VALID = (1 << 19), /* initial gtm data valid */
 
 	/* struct ata_queued_cmd flags */
 	ATA_QCFLAG_ACTIVE	= (1 << 0), /* cmd not yet ack'd to scsi lyer */
@@ -653,7 +653,7 @@ struct ata_port {
 
 #ifdef CONFIG_ATA_ACPI
 	acpi_handle		acpi_handle;
-	struct ata_acpi_gtm	acpi_gtm;
+	struct ata_acpi_gtm	__acpi_init_gtm; /* use ata_acpi_init_gtm() */
 #endif
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
 };
@@ -939,10 +939,20 @@ enum {
 
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
+static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
+{
+	if (ap->pflags & ATA_PFLAG_INIT_GTM_VALID)
+		return &ap->__acpi_init_gtm;
+	return NULL;
+}
 extern int ata_acpi_cbl_80wire(struct ata_port *ap);
 int ata_acpi_stm(struct ata_port *ap, const struct ata_acpi_gtm *stm);
 int ata_acpi_gtm(struct ata_port *ap, struct ata_acpi_gtm *stm);
 #else
+static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
+{
+	return NULL;
+}
 static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; }
 #endif
 
-- 
1.5.2.4

[PATCH 07/10] libata-acpi: implement dev->gtf_cache and evaluate _GTF right after _STM during resume

[Tejun Heo]

On certain implementations, _GTF evaluation depends on preceding _STM
and both can be pretty picky about the configuration.  Using _GTM
result cached during controller initialization satisfies the most
neurotic _STM implementation.  However, libata evaluates _GTF after
reset during device configuration and the hardware state can be
different from what _GTF expects and can cause evaluation failure.

This patch adds dev->gtf_cache and updates ata_dev_get_GTF() such that
it uses the cached value if available.  Cache is cleared with a call
to ata_acpi_clear_gtf().

Because for SATA ACPI nodes _GTF must be evaluated after _SDD which
can't be done till IDENTIFY is complete, _GTF caching from
ata_acpi_on_resume() is used only for IDE ACPI nodes.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |   69 ++++++++++++++++++++++++++++++++-------------
 include/linux/libata.h    |    1 +
 2 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index b3aeca3..e0dd132 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -41,6 +41,12 @@ static int is_pci_dev(struct device *dev)
 	return (dev->bus == &pci_bus_type);
 }
 
+static void ata_acpi_clear_gtf(struct ata_device *dev)
+{
+	kfree(dev->gtf_cache);
+	dev->gtf_cache = NULL;
+}
+
 /**
  * ata_acpi_associate_sata_port - associate SATA port with ACPI objects
  * @ap: target SATA port
@@ -327,7 +333,6 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm);
  * ata_dev_get_GTF - get the drive bootup default taskfile settings
  * @dev: target ATA device
  * @gtf: output parameter for buffer containing _GTF taskfile arrays
- * @ptr_to_free: pointer which should be freed
  *
  * This applies to both PATA and SATA drives.
  *
@@ -344,8 +349,7 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm);
  * Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't
  * contain valid data.
  */
-static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
-			   void **ptr_to_free)
+static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
 {
 	struct ata_port *ap = dev->link->ap;
 	acpi_status status;
@@ -353,6 +357,12 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
 	union acpi_object *out_obj;
 	int rc = 0;
 
+	/* if _GTF is cached, use the cached value */
+	if (dev->gtf_cache) {
+		out_obj = dev->gtf_cache;
+		goto done;
+	}
+
 	/* set up output buffer */
 	output.length = ACPI_ALLOCATE_BUFFER;
 	output.pointer = NULL;	/* ACPI-CA sets this; save/free it later */
@@ -363,6 +373,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
 
 	/* _GTF has no input parameters */
 	status = acpi_evaluate_object(dev->acpi_handle, "_GTF", NULL, &output);
+	out_obj = dev->gtf_cache = output.pointer;
 
 	if (ACPI_FAILURE(status)) {
 		if (status != AE_NOT_FOUND) {
@@ -383,7 +394,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
 		goto out_free;
 	}
 
-	out_obj = output.pointer;
 	if (out_obj->type != ACPI_TYPE_BUFFER) {
 		ata_dev_printk(dev, KERN_WARNING,
 			       "_GTF unexpected object type 0x%x\n",
@@ -398,18 +408,19 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
 		goto out_free;
 	}
 
-	*ptr_to_free = out_obj;
-	*gtf = (void *)out_obj->buffer.pointer;
+ done:
 	rc = out_obj->buffer.length / REGS_PER_GTF;
-
-	if (ata_msg_probe(ap))
-		ata_dev_printk(dev, KERN_DEBUG, "%s: returning "
-			"gtf=%p, gtf_count=%d, ptr_to_free=%p\n",
-			__FUNCTION__, *gtf, rc, *ptr_to_free);
+	if (gtf) {
+		*gtf = (void *)out_obj->buffer.pointer;
+		if (ata_msg_probe(ap))
+			ata_dev_printk(dev, KERN_DEBUG,
+				       "%s: returning gtf=%p, gtf_count=%d\n",
+				       __FUNCTION__, *gtf, rc);
+	}
 	return rc;
 
  out_free:
-	kfree(output.pointer);
+	ata_acpi_clear_gtf(dev);
 	return rc;
 }
 
@@ -533,11 +544,10 @@ static int taskfile_load_raw(struct ata_device *dev,
 static int ata_acpi_exec_tfs(struct ata_device *dev)
 {
 	struct ata_acpi_gtf *gtf = NULL;
-	void *ptr_to_free = NULL;
 	int gtf_count, i, rc;
 
 	/* get taskfiles */
-	gtf_count = ata_dev_get_GTF(dev, &gtf, &ptr_to_free);
+	gtf_count = ata_dev_get_GTF(dev, &gtf);
 
 	/* execute them */
 	for (i = 0, rc = 0; i < gtf_count; i++) {
@@ -551,7 +561,7 @@ static int ata_acpi_exec_tfs(struct ata_device *dev)
 			rc = tmp;
 	}
 
-	kfree(ptr_to_free);
+	ata_acpi_clear_gtf(dev);
 
 	if (rc == 0)
 		return gtf_count;
@@ -644,13 +654,31 @@ void ata_acpi_on_resume(struct ata_port *ap)
 	const struct ata_acpi_gtm *gtm = ata_acpi_init_gtm(ap);
 	struct ata_device *dev;
 
-	/* restore timing parameters */
-	if (ap->acpi_handle && gtm)
+	if (ap->acpi_handle && gtm) {
+		/* _GTM valid */
+
+		/* restore timing parameters */
 		ata_acpi_stm(ap, gtm);
 
-	/* schedule _GTF */
-	ata_link_for_each_dev(dev, &ap->link)
-		dev->flags |= ATA_DFLAG_ACPI_PENDING;
+		/* _GTF should immediately follow _STM so that it can
+		 * use values set by _STM.  Cache _GTF result and
+		 * schedule _GTF.
+		 */
+		ata_link_for_each_dev(dev, &ap->link) {
+			ata_acpi_clear_gtf(dev);
+			if (ata_dev_get_GTF(dev, NULL) >= 0)
+				dev->flags |= ATA_DFLAG_ACPI_PENDING;
+		}
+	} else {
+		/* SATA _GTF needs to be evaulated after _SDD and
+		 * there's no reason to evaluate IDE _GTF early
+		 * without _STM.  Clear cache and schedule _GTF.
+		 */
+		ata_link_for_each_dev(dev, &ap->link) {
+			ata_acpi_clear_gtf(dev);
+			dev->flags |= ATA_DFLAG_ACPI_PENDING;
+		}
+	}
 }
 
 /**
@@ -735,4 +763,5 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
  */
 void ata_acpi_on_disable(struct ata_device *dev)
 {
+	ata_acpi_clear_gtf(dev);
 }
diff --git a/include/linux/libata.h b/include/linux/libata.h
index ba84d8a..cb91280 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -498,6 +498,7 @@ struct ata_device {
 	struct scsi_device	*sdev;		/* attached SCSI device */
 #ifdef CONFIG_ATA_ACPI
 	acpi_handle		acpi_handle;
+	union acpi_object	*gtf_cache;
 #endif
 	/* n_sector is used as CLEAR_OFFSET, read comment above CLEAR_OFFSET */
 	u64			n_sectors;	/* size of device, if ATA */
-- 
1.5.2.4

[PATCH 08/10] libata-acpi: improve ACPI disabling

[Tejun Heo]

* If _GTF evalution fails, it's pointless to retry.  If nothing else
  is wrong, just ignore the error.

* After disabling ACPI, return success iff the number of executed _GTF
  command equals zero.  Otherwise, tell EH to retry.  This change
  fixes bogus 1 return bug where ata_acpi_on_devcfg() expects the
  caller to reload IDENTIFY data and continue but the caller
  interprets it as an error.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |   59 +++++++++++++++++++++++++++++----------------
 1 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index e0dd132..5932ae2 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -346,8 +346,8 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm);
  * EH context.
  *
  * RETURNS:
- * Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't
- * contain valid data.
+ * Number of taskfiles on success, 0 if _GTF doesn't exist.  -EINVAL
+ * if _GTF is invalid.
  */
 static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
 {
@@ -380,6 +380,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
 			ata_dev_printk(dev, KERN_WARNING,
 				       "_GTF evaluation failed (AE 0x%x)\n",
 				       status);
+			rc = -EINVAL;
 		}
 		goto out_free;
 	}
@@ -391,6 +392,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
 				__FUNCTION__,
 				(unsigned long long)output.length,
 				output.pointer);
+		rc = -EINVAL;
 		goto out_free;
 	}
 
@@ -398,6 +400,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
 		ata_dev_printk(dev, KERN_WARNING,
 			       "_GTF unexpected object type 0x%x\n",
 			       out_obj->type);
+		rc = -EINVAL;
 		goto out_free;
 	}
 
@@ -405,6 +408,7 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf)
 		ata_dev_printk(dev, KERN_WARNING,
 			       "unexpected _GTF length (%d)\n",
 			       out_obj->buffer.length);
+		rc = -EINVAL;
 		goto out_free;
 	}
 
@@ -531,6 +535,7 @@ static int taskfile_load_raw(struct ata_device *dev,
 /**
  * ata_acpi_exec_tfs - get then write drive taskfile settings
  * @dev: target ATA device
+ * @nr_executed: out paramter for the number of executed commands
  *
  * Evaluate _GTF and excute returned taskfiles.
  *
@@ -538,16 +543,19 @@ static int taskfile_load_raw(struct ata_device *dev,
  * EH context.
  *
  * RETURNS:
- * Number of executed taskfiles on success, 0 if _GTF doesn't exist or
- * doesn't contain valid data.  -errno on other errors.
+ * Number of executed taskfiles on success, 0 if _GTF doesn't exist.
+ * -errno on other errors.
  */
-static int ata_acpi_exec_tfs(struct ata_device *dev)
+static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed)
 {
 	struct ata_acpi_gtf *gtf = NULL;
 	int gtf_count, i, rc;
 
 	/* get taskfiles */
-	gtf_count = ata_dev_get_GTF(dev, &gtf);
+	rc = ata_dev_get_GTF(dev, &gtf);
+	if (rc < 0)
+		return rc;
+	gtf_count = rc;
 
 	/* execute them */
 	for (i = 0, rc = 0; i < gtf_count; i++) {
@@ -559,12 +567,12 @@ static int ata_acpi_exec_tfs(struct ata_device *dev)
 		tmp = taskfile_load_raw(dev, gtf++);
 		if (!rc)
 			rc = tmp;
+
+		(*nr_executed)++;
 	}
 
 	ata_acpi_clear_gtf(dev);
 
-	if (rc == 0)
-		return gtf_count;
 	return rc;
 }
 
@@ -700,6 +708,7 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
 	struct ata_port *ap = dev->link->ap;
 	struct ata_eh_context *ehc = &ap->link.eh_context;
 	int acpi_sata = ap->flags & ATA_FLAG_ACPI_SATA;
+	int nr_executed = 0;
 	int rc;
 
 	if (!dev->acpi_handle)
@@ -718,14 +727,14 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
 	}
 
 	/* do _GTF */
-	rc = ata_acpi_exec_tfs(dev);
-	if (rc < 0)
+	rc = ata_acpi_exec_tfs(dev, &nr_executed);
+	if (rc)
 		goto acpi_err;
 
 	dev->flags &= ~ATA_DFLAG_ACPI_PENDING;
 
 	/* refresh IDENTIFY page if any _GTF command has been executed */
-	if (rc > 0) {
+	if (nr_executed) {
 		rc = ata_dev_reread_id(dev, 0);
 		if (rc < 0) {
 			ata_dev_printk(dev, KERN_ERR, "failed to IDENTIFY "
@@ -737,18 +746,26 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
 	return 0;
 
  acpi_err:
-	/* let EH retry on the first failure, disable ACPI on the second */
-	if (dev->flags & ATA_DFLAG_ACPI_FAILED) {
-		ata_dev_printk(dev, KERN_WARNING, "ACPI on devcfg failed the "
-			       "second time, disabling (errno=%d)\n", rc);
-
-		dev->acpi_handle = NULL;
+	/* ignore evaluation failure if we can continue safely */
+	if (rc == -EINVAL && !nr_executed && !(ap->pflags & ATA_PFLAG_FROZEN))
+		return 0;
 
-		/* if port is working, request IDENTIFY reload and continue */
-		if (!(ap->pflags & ATA_PFLAG_FROZEN))
-			rc = 1;
+	/* fail and let EH retry once more for unknown IO errors */
+	if (!(dev->flags & ATA_DFLAG_ACPI_FAILED)) {
+		dev->flags |= ATA_DFLAG_ACPI_FAILED;
+		return rc;
 	}
-	dev->flags |= ATA_DFLAG_ACPI_FAILED;
+
+	ata_dev_printk(dev, KERN_WARNING,
+		       "ACPI: failed the second time, disabled\n");
+	dev->acpi_handle = NULL;
+
+	/* We can safely continue if no _GTF command has been executed
+	 * and port is not frozen.
+	 */
+	if (!nr_executed && !(ap->pflags & ATA_PFLAG_FROZEN))
+		return 0;
+
 	return rc;
 }
 
-- 
1.5.2.4

[PATCH 09/10] libata-acpi: improve _GTF execution error handling and reporting

[Tejun Heo]

As _GTF commands can't transfer data, device error never signals
transfer error.  It indicates that the device vetoed the operation, so
it's meaningless to retry.

This patch makes libata-acpi to report and continue on device errors
when executing _GTF commands.  Also commands rejected by device don't
contribute to the number of _GTF commands executed.

While at it, update _GTF execution reporting such that all successful
commands are logged at KERN_DEBUG and rename taskfile_load_raw() to
ata_acpi_run_tf() for consistency.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |   77 +++++++++++++++++++++++++++------------------
 1 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 5932ae2..d4cb557 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -466,7 +466,7 @@ int ata_acpi_cbl_80wire(struct ata_port *ap)
 EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
 
 /**
- * taskfile_load_raw - send taskfile registers to host controller
+ * ata_acpi_run_tf - send taskfile registers to host controller
  * @dev: target ATA device
  * @gtf: raw ATA taskfile register set (0x1f1 - 0x1f7)
  *
@@ -485,14 +485,17 @@ EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
  * EH context.
  *
  * RETURNS:
- * 0 on success, -errno on failure.
+ * 1 if command is executed successfully.  0 if ignored or rejected,
+ * -errno on other errors.
  */
-static int taskfile_load_raw(struct ata_device *dev,
-			      const struct ata_acpi_gtf *gtf)
+static int ata_acpi_run_tf(struct ata_device *dev,
+			   const struct ata_acpi_gtf *gtf)
 {
-	struct ata_port *ap = dev->link->ap;
 	struct ata_taskfile tf, rtf;
 	unsigned int err_mask;
+	const char *level;
+	char msg[60];
+	int rc;
 
 	if ((gtf->tf[0] == 0) && (gtf->tf[1] == 0) && (gtf->tf[2] == 0)
 	    && (gtf->tf[3] == 0) && (gtf->tf[4] == 0) && (gtf->tf[5] == 0)
@@ -512,24 +515,39 @@ static int taskfile_load_raw(struct ata_device *dev,
 	tf.device  = gtf->tf[5];	/* 0x1f6 */
 	tf.command = gtf->tf[6];	/* 0x1f7 */
 
-	if (ata_msg_probe(ap))
-		ata_dev_printk(dev, KERN_DEBUG, "executing ACPI cmd "
-			       "%02x/%02x:%02x:%02x:%02x:%02x:%02x\n",
-			       tf.command, tf.feature, tf.nsect,
-			       tf.lbal, tf.lbam, tf.lbah, tf.device);
-
 	rtf = tf;
 	err_mask = ata_exec_internal(dev, &rtf, NULL, DMA_NONE, NULL, 0, 0);
-	if (err_mask) {
-		ata_dev_printk(dev, KERN_ERR,
-			"ACPI cmd %02x/%02x:%02x:%02x:%02x:%02x:%02x failed "
-			"(Emask=0x%x Stat=0x%02x Err=0x%02x)\n",
-			tf.command, tf.feature, tf.nsect, tf.lbal, tf.lbam,
-			tf.lbah, tf.device, err_mask, rtf.command, rtf.feature);
-		return -EIO;
+
+	switch (err_mask) {
+	case 0:
+		level = KERN_DEBUG;
+		snprintf(msg, sizeof(msg), "succeeded");
+		rc = 1;
+		break;
+
+	case AC_ERR_DEV:
+		level = KERN_INFO;
+		snprintf(msg, sizeof(msg),
+			 "rejected by device (Stat=0x%02x Err=0x%02x)",
+			 rtf.command, rtf.feature);
+		rc = 0;
+		break;
+
+	default:
+		level = KERN_ERR;
+		snprintf(msg, sizeof(msg),
+			 "failed (Emask=0x%x Stat=0x%02x Err=0x%02x)",
+			 err_mask, rtf.command, rtf.feature);
+		rc = -EIO;
+		break;
 	}
 
-	return 0;
+	ata_dev_printk(dev, level,
+		       "ACPI cmd %02x/%02x:%02x:%02x:%02x:%02x:%02x %s\n",
+		       tf.command, tf.feature, tf.nsect, tf.lbal,
+		       tf.lbam, tf.lbah, tf.device, msg);
+
+	return rc;
 }
 
 /**
@@ -558,22 +576,19 @@ static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed)
 	gtf_count = rc;
 
 	/* execute them */
-	for (i = 0, rc = 0; i < gtf_count; i++) {
-		int tmp;
-
-		/* ACPI errors are eventually ignored.  Run till the
-		 * end even after errors.
-		 */
-		tmp = taskfile_load_raw(dev, gtf++);
-		if (!rc)
-			rc = tmp;
-
-		(*nr_executed)++;
+	for (i = 0; i < gtf_count; i++) {
+		rc = ata_acpi_run_tf(dev, gtf++);
+		if (rc < 0)
+			break;
+		if (rc)
+			(*nr_executed)++;
 	}
 
 	ata_acpi_clear_gtf(dev);
 
-	return rc;
+	if (rc < 0)
+		return rc;
+	return 0;
 }
 
 /**
-- 
1.5.2.4

[PATCH 10/10] libata-acpi: implement _GTF command filtering

[Tejun Heo]

Implement _GTF command filtering which can be controlled by
libata.acpi_filter kernel parameter.  Currently SETXFER and LOCK
commands are filtered.

libata configures transfer mode by itself and _GTF SETXFER commands
can potentially disrupt device configuration.  _GTM/_STM mechanism
can't handle hotplugging too well and when _GTF is executed,
controller is in PIO0 rather than the mode _STM configured.

Note that detecting SET MAX LOCK requires looking at the previous
command.  This adds a bit to code complexity.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-acpi.c |  159 ++++++++++++++++++++++++++++++++------------
 1 files changed, 115 insertions(+), 44 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index d4cb557..7bf4bef 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2006 Randy Dunlap
  */
 
+#include <linux/module.h>
 #include <linux/ata.h>
 #include <linux/delay.h>
 #include <linux/device.h>
@@ -25,6 +26,18 @@
 #include <acpi/acmacros.h>
 #include <acpi/actypes.h>
 
+enum {
+	ATA_ACPI_FILTER_SETXFER	= 1 << 0,
+	ATA_ACPI_FILTER_LOCK	= 1 << 1,
+
+	ATA_ACPI_FILTER_DEFAULT	= ATA_ACPI_FILTER_SETXFER |
+				  ATA_ACPI_FILTER_LOCK,
+};
+
+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)");
+
 #define NO_PORT_MULT		0xffff
 #define SATA_ADR(root, pmp)	(((root) << 16) | (pmp))
 
@@ -465,6 +478,60 @@ int ata_acpi_cbl_80wire(struct ata_port *ap)
 
 EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
 
+static void ata_acpi_gtf_to_tf(struct ata_device *dev,
+			       const struct ata_acpi_gtf *gtf,
+			       struct ata_taskfile *tf)
+{
+	ata_tf_init(dev, tf);
+
+	tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+	tf->protocol = ATA_PROT_NODATA;
+	tf->feature = gtf->tf[0];	/* 0x1f1 */
+	tf->nsect   = gtf->tf[1];	/* 0x1f2 */
+	tf->lbal    = gtf->tf[2];	/* 0x1f3 */
+	tf->lbam    = gtf->tf[3];	/* 0x1f4 */
+	tf->lbah    = gtf->tf[4];	/* 0x1f5 */
+	tf->device  = gtf->tf[5];	/* 0x1f6 */
+	tf->command = gtf->tf[6];	/* 0x1f7 */
+}
+
+static int ata_acpi_filter_tf(const struct ata_taskfile *tf,
+			      const struct ata_taskfile *ptf)
+{
+	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_SETXFER) {
+		/* libata doesn't use ACPI to configure transfer mode.
+		 * It will only confuse device configuration.  Skip.
+		 */
+		if (tf->command == ATA_CMD_SET_FEATURES &&
+		    tf->feature == SETFEATURES_XFER)
+			return 1;
+	}
+
+	if (ata_acpi_gtf_filter & ATA_ACPI_FILTER_LOCK) {
+		/* BIOS writers, sorry but we don't wanna lock
+		 * features unless the user explicitly said so.
+		 */
+
+		/* DEVICE CONFIGURATION FREEZE LOCK */
+		if (tf->command == ATA_CMD_CONF_OVERLAY &&
+		    tf->feature == ATA_DCO_FREEZE_LOCK)
+			return 1;
+
+		/* SECURITY FREEZE LOCK */
+		if (tf->command == ATA_CMD_SEC_FREEZE_LOCK)
+			return 1;
+
+		/* SET MAX LOCK and SET MAX FREEZE LOCK */
+		if ((!ptf || ptf->command != ATA_CMD_READ_NATIVE_MAX) &&
+		    tf->command == ATA_CMD_SET_MAX &&
+		    (tf->feature == ATA_SET_MAX_LOCK ||
+		     tf->feature == ATA_SET_MAX_FREEZE_LOCK))
+			return 1;
+	}
+
+	return 0;
+}
+
 /**
  * ata_acpi_run_tf - send taskfile registers to host controller
  * @dev: target ATA device
@@ -485,13 +552,15 @@ EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
  * EH context.
  *
  * RETURNS:
- * 1 if command is executed successfully.  0 if ignored or rejected,
- * -errno on other errors.
+ * 1 if command is executed successfully.  0 if ignored, rejected or
+ * filtered out, -errno on other errors.
  */
 static int ata_acpi_run_tf(struct ata_device *dev,
-			   const struct ata_acpi_gtf *gtf)
+			   const struct ata_acpi_gtf *gtf,
+			   const struct ata_acpi_gtf *prev_gtf)
 {
-	struct ata_taskfile tf, rtf;
+	struct ata_taskfile *pptf = NULL;
+	struct ata_taskfile tf, ptf, rtf;
 	unsigned int err_mask;
 	const char *level;
 	char msg[60];
@@ -502,44 +571,44 @@ static int ata_acpi_run_tf(struct ata_device *dev,
 	    && (gtf->tf[6] == 0))
 		return 0;
 
-	ata_tf_init(dev, &tf);
-
-	/* convert gtf to tf */
-	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; /* TBD */
-	tf.protocol = ATA_PROT_NODATA;
-	tf.feature = gtf->tf[0];	/* 0x1f1 */
-	tf.nsect   = gtf->tf[1];	/* 0x1f2 */
-	tf.lbal    = gtf->tf[2];	/* 0x1f3 */
-	tf.lbam    = gtf->tf[3];	/* 0x1f4 */
-	tf.lbah    = gtf->tf[4];	/* 0x1f5 */
-	tf.device  = gtf->tf[5];	/* 0x1f6 */
-	tf.command = gtf->tf[6];	/* 0x1f7 */
-
-	rtf = tf;
-	err_mask = ata_exec_internal(dev, &rtf, NULL, DMA_NONE, NULL, 0, 0);
-
-	switch (err_mask) {
-	case 0:
-		level = KERN_DEBUG;
-		snprintf(msg, sizeof(msg), "succeeded");
-		rc = 1;
-		break;
-
-	case AC_ERR_DEV:
+	ata_acpi_gtf_to_tf(dev, gtf, &tf);
+	if (prev_gtf) {
+		ata_acpi_gtf_to_tf(dev, prev_gtf, &ptf);
+		pptf = &ptf;
+	}
+
+	if (!ata_acpi_filter_tf(&tf, pptf)) {
+		rtf = tf;
+		err_mask = ata_exec_internal(dev, &rtf, NULL,
+					     DMA_NONE, NULL, 0, 0);
+
+		switch (err_mask) {
+		case 0:
+			level = KERN_DEBUG;
+			snprintf(msg, sizeof(msg), "succeeded");
+			rc = 1;
+			break;
+
+		case AC_ERR_DEV:
+			level = KERN_INFO;
+			snprintf(msg, sizeof(msg),
+				 "rejected by device (Stat=0x%02x Err=0x%02x)",
+				 rtf.command, rtf.feature);
+			rc = 0;
+			break;
+
+		default:
+			level = KERN_ERR;
+			snprintf(msg, sizeof(msg),
+				 "failed (Emask=0x%x Stat=0x%02x Err=0x%02x)",
+				 err_mask, rtf.command, rtf.feature);
+			rc = -EIO;
+			break;
+		}
+	} else {
 		level = KERN_INFO;
-		snprintf(msg, sizeof(msg),
-			 "rejected by device (Stat=0x%02x Err=0x%02x)",
-			 rtf.command, rtf.feature);
+		snprintf(msg, sizeof(msg), "filtered out");
 		rc = 0;
-		break;
-
-	default:
-		level = KERN_ERR;
-		snprintf(msg, sizeof(msg),
-			 "failed (Emask=0x%x Stat=0x%02x Err=0x%02x)",
-			 err_mask, rtf.command, rtf.feature);
-		rc = -EIO;
-		break;
 	}
 
 	ata_dev_printk(dev, level,
@@ -566,7 +635,7 @@ static int ata_acpi_run_tf(struct ata_device *dev,
  */
 static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed)
 {
-	struct ata_acpi_gtf *gtf = NULL;
+	struct ata_acpi_gtf *gtf = NULL, *pgtf = NULL;
 	int gtf_count, i, rc;
 
 	/* get taskfiles */
@@ -576,12 +645,14 @@ static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed)
 	gtf_count = rc;
 
 	/* execute them */
-	for (i = 0; i < gtf_count; i++) {
-		rc = ata_acpi_run_tf(dev, gtf++);
+	for (i = 0; i < gtf_count; i++, gtf++) {
+		rc = ata_acpi_run_tf(dev, gtf, pgtf);
 		if (rc < 0)
 			break;
-		if (rc)
+		if (rc) {
 			(*nr_executed)++;
+			pgtf = gtf;
+		}
 	}
 
 	ata_acpi_clear_gtf(dev);
-- 
1.5.2.4

Re: git repo and #upstream merging

[Tejun Heo]

Tejun Heo wrote:
> This is the second take of improve-ACPI-corner-case-handling patchset
> and contains the following ten patches.

Jeff, this patchset can also be pulled from the following git tree.

  master.kernel.org:/pub/scm/linux/kernel/git/tj/libata-dev.git acpi-fixes

Also, merging this into #upstream can cause some interesting conflicts
w/ ACPI timing handling update patches.  If you ACK the patchset, I'll
prep merged git HEAD for you.

Thanks.

-- 
tejun

Re: [PATCH 01/10] libata-acpi: adjust constness in ata_acpi_gtm/stm() parameters

[Jeff Garzik]

Tejun Heo wrote:
> * No internal function uses const ata_port.  Drop const from @ap.
> 
> * Make ata_acpi_stm() copy @stm before using it and change @stm to
>   const.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
>  drivers/ata/libata-acpi.c |    7 ++++---
>  include/linux/libata.h    |    4 ++--
>  2 files changed, 6 insertions(+), 5 deletions(-)

applied 1-10 to #upstream-fixes

Re: [PATCH 01/10] libata-acpi: adjust constness in ata_acpi_gtm/stm() parameters

[Jeff Garzik]

Tejun Heo wrote:
> * No internal function uses const ata_port.  Drop const from @ap.
> 
> * Make ata_acpi_stm() copy @stm before using it and change @stm to
>   const.

for the record, I -would- use const ata_port where feasible; its just so 
rarely feasible, since members of that struct are often being modified. 
  However in a small query-style function, I could see some const-ness 
perhaps...

	Jeff

Re: git repo and #upstream merging

[Jeff Garzik]

Tejun Heo wrote:
> Tejun Heo wrote:
>> This is the second take of improve-ACPI-corner-case-handling patchset
>> and contains the following ten patches.
> 
> Jeff, this patchset can also be pulled from the following git tree.
> 
>   master.kernel.org:/pub/scm/linux/kernel/git/tj/libata-dev.git acpi-fixes
> 
> Also, merging this into #upstream can cause some interesting conflicts
> w/ ACPI timing handling update patches.  If you ACK the patchset, I'll
> prep merged git HEAD for you.

It was easier for me to

* merge the patches into #upstream-fixes

* rebase #upstream on top of #upstream-fixes,
	while dropping the conflicting ACPI patches

And IMO that gives you a bit more freedom to do the merged git -- though 
that obviously implies you will need to resend the ACPI patchset that 
you had previously submitted.

	Jeff