[PATCH 1/13] ATA ACPI: Makefile/Kconfig/doc.

[PATCH 1/13] ATA ACPI: Makefile/Kconfig/doc.

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Add ata_acpi in Makefile and Kconfig.
Add ACPI obj_handle.
Add ata_acpi.c to libata kernel-doc template file.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 Documentation/DocBook/libata.tmpl |    6 ++++++
 drivers/scsi/Kconfig              |    5 +++++
 drivers/scsi/Makefile             |    3 +++
 include/linux/libata.h            |    6 ++++++
 4 files changed, 20 insertions(+)

--- linux-2616-rc4-ata.orig/drivers/scsi/Makefile
+++ linux-2616-rc4-ata/drivers/scsi/Makefile
@@ -164,6 +164,9 @@ CFLAGS_ncr53c8xx.o	:= $(ncr53c8xx-flags-
 zalon7xx-objs	:= zalon.o ncr53c8xx.o
 NCR_Q720_mod-objs	:= NCR_Q720.o ncr53c8xx.o
 libata-objs	:= libata-core.o libata-scsi.o
+ifeq ($(CONFIG_SCSI_SATA_ACPI),y)
+  libata-objs	+= libata-acpi.o
+endif
 oktagon_esp_mod-objs	:= oktagon_esp.o oktagon_io.o
 
 # Files generated that shall be removed upon make clean
--- linux-2616-rc4-ata.orig/drivers/scsi/Kconfig
+++ linux-2616-rc4-ata/drivers/scsi/Kconfig
@@ -599,6 +599,11 @@ config SCSI_SATA_INTEL_COMBINED
 	depends on IDE=y && !BLK_DEV_IDE_SATA && (SCSI_SATA_AHCI || SCSI_ATA_PIIX)
 	default y
 
+config SCSI_SATA_ACPI
+	bool
+	depends on SCSI_SATA && ACPI
+	default y
+
 config SCSI_BUSLOGIC
 	tristate "BusLogic SCSI support"
 	depends on (PCI || ISA || MCA) && SCSI && ISA_DMA_API
--- linux-2616-rc4-ata.orig/include/linux/libata.h
+++ linux-2616-rc4-ata/include/linux/libata.h
@@ -33,6 +33,7 @@
 #include <asm/io.h>
 #include <linux/ata.h>
 #include <linux/workqueue.h>
+#include <acpi/acpi.h>
 
 /*
  * compile-time options
@@ -318,6 +319,11 @@ struct ata_device {
 	u16			cylinders;	/* Number of cylinders */
 	u16			heads;		/* Number of heads */
 	u16			sectors;	/* Number of sectors per track */
+
+#ifdef CONFIG_SCSI_SATA_ACPI
+	/* ACPI objects info */
+	acpi_handle		obj_handle;
+#endif
 };
 
 struct ata_port {
--- linux-2616-rc4-ata.orig/Documentation/DocBook/libata.tmpl
+++ linux-2616-rc4-ata/Documentation/DocBook/libata.tmpl
@@ -787,6 +787,12 @@ and other resources, etc.
 !Idrivers/scsi/libata-scsi.c
   </chapter>
 
+  <chapter id="libataAcpi">
+     <title>libata ACPI interfaces/methods</title>
+!Edrivers/scsi/ata_acpi.c
+!Idrivers/scsi/ata_acpi.c
+  </chapter>
+
   <chapter id="ataExceptions">
      <title>ATA errors &amp; exceptions</title>
 

[PATCH 2/13] ATA ACPI: debugging infrastructure

[Randy Dunlap]

(Jeff has already merged this in his tree.)

From: Borislav Petkov <petkov@uni-muenster.de>

libata new debugging macro definitions

Signed-off-by: Borislav Petkov <petkov@uni-muenster.de>
Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 include/linux/libata.h |   52 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

--- linux-2616-rc4-ata.orig/include/linux/libata.h
+++ linux-2616-rc4-ata/include/linux/libata.h
@@ -36,7 +36,8 @@
 #include <acpi/acpi.h>
 
 /*
- * compile-time options
+ * compile-time options: to be removed as soon as all the drivers are
+ * converted to the new debugging mechanism
  */
 #undef ATA_DEBUG		/* debugging output */
 #undef ATA_VERBOSE_DEBUG	/* yet more debugging output */
@@ -72,6 +73,38 @@
         }
 #endif
 
+/* NEW: debug levels */
+#define HAVE_LIBATA_MSG 1
+
+enum {
+	ATA_MSG_DRV	= 0x0001,
+	ATA_MSG_INFO	= 0x0002,
+	ATA_MSG_PROBE	= 0x0004,
+	ATA_MSG_WARN	= 0x0008,
+	ATA_MSG_MALLOC	= 0x0010,
+	ATA_MSG_CTL	= 0x0020,
+	ATA_MSG_INTR	= 0x0040,
+	ATA_MSG_ERR	= 0x0080,
+};
+
+#define ata_msg_drv(p)    ((p)->msg_enable & ATA_MSG_DRV)
+#define ata_msg_info(p)   ((p)->msg_enable & ATA_MSG_INFO)
+#define ata_msg_probe(p)  ((p)->msg_enable & ATA_MSG_PROBE)
+#define ata_msg_warn(p)   ((p)->msg_enable & ATA_MSG_WARN)
+#define ata_msg_malloc(p) ((p)->msg_enable & ATA_MSG_MALLOC)
+#define ata_msg_ctl(p)    ((p)->msg_enable & ATA_MSG_CTL)
+#define ata_msg_intr(p)   ((p)->msg_enable & ATA_MSG_INTR)
+#define ata_msg_err(p)    ((p)->msg_enable & ATA_MSG_ERR)
+
+static inline u32 ata_msg_init(int dval, int default_msg_enable_bits)
+{
+	if (dval < 0 || dval >= (sizeof(u32) * 8))
+		return default_msg_enable_bits; /* should be 0x1 - only driver info msgs */
+	if (!dval)
+		return 0;
+	return (1 << dval) - 1;
+}
+
 /* defines only for the constants which don't work well as enums */
 #define ATA_TAG_POISON		0xfafbfcfdU
 
@@ -365,6 +398,8 @@ struct ata_port {
 	unsigned int		hsm_task_state;
 	unsigned long		pio_task_timeout;
 
+	u32			msg_enable;
+
 	void			*private_data;
 };
 
@@ -651,9 +686,9 @@ static inline u8 ata_wait_idle(struct at
 
 	if (status & (ATA_BUSY | ATA_DRQ)) {
 		unsigned long l = ap->ioaddr.status_addr;
-		printk(KERN_WARNING
-		       "ATA: abnormal status 0x%X on port 0x%lX\n",
-		       status, l);
+		if (ata_msg_warn(ap))
+			printk(KERN_WARNING "ATA: abnormal status 0x%X on port 0x%lX\n",
+				status, l);
 	}
 
 	return status;
@@ -745,7 +780,8 @@ static inline u8 ata_irq_ack(struct ata_
 
 	status = ata_busy_wait(ap, bits, 1000);
 	if (status & bits)
-		DPRINTK("abnormal status 0x%X\n", status);
+		if (ata_msg_err(ap))
+			printk(KERN_ERR "abnormal status 0x%X\n", status);
 
 	/* get controller status; clear intr, err bits */
 	if (ap->flags & ATA_FLAG_MMIO) {
@@ -763,8 +799,10 @@ static inline u8 ata_irq_ack(struct ata_
 		post_stat = inb(ap->ioaddr.bmdma_addr + ATA_DMA_STATUS);
 	}
 
-	VPRINTK("irq ack: host_stat 0x%X, new host_stat 0x%X, drv_stat 0x%X\n",
-		host_stat, post_stat, status);
+	if (ata_msg_intr(ap))
+		printk(KERN_INFO "%s: irq ack: host_stat 0x%X, new host_stat 0x%X, drv_stat 0x%X\n",
+			__FUNCTION__,
+			host_stat, post_stat, status);
 
 	return status;
 }

[PATCH 3/13] ATA ACPI: SATA methods

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Add support for ACPI methods to SATA suspend/resume.
Add calls to ACPI methods for SATA drives.
Use ata_exec_internal().

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-acpi.c |  574 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libata-core.c |    7 
 drivers/scsi/libata.h      |   36 ++
 3 files changed, 616 insertions(+), 1 deletion(-)

--- /dev/null
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -0,0 +1,574 @@
+/*
+ * libata-acpi.c
+ * Provides ACPI support for PATA/SATA.
+ *
+ * Copyright (C) 2005 Intel Corp.
+ * Copyright (C) 2005 Randy Dunlap
+ */
+
+#include <linux/ata.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/kernel.h>
+#include <acpi/acpi.h>
+#include "scsi.h"
+#include <linux/libata.h>
+#include <linux/pci.h>
+#include "libata.h"
+
+#include <acpi/acpi_bus.h>
+#include <acpi/acnames.h>
+#include <acpi/acnamesp.h>
+#include <acpi/acparser.h>
+#include <acpi/acexcep.h>
+#include <acpi/acmacros.h>
+#include <acpi/actypes.h>
+
+#define SATA_ROOT_PORT(x)	(((x) >> 16) & 0xffff)
+#define SATA_PORT_NUMBER(x)	((x) & 0xffff)	/* or NO_PORT_MULT */
+#define NO_PORT_MULT		0xffff
+#define SATA_ADR_RSVD		0xffffffff
+
+#define REGS_PER_GTF		7
+struct taskfile_array {
+	u8	tfa[REGS_PER_GTF];	/* regs. 0x1f1 - 0x1f7 */
+};
+
+/**
+ * sata_get_dev_handle - finds acpi_handle and PCI device.function
+ * @dev: device to locate
+ * @handle: returned acpi_handle for @dev
+ * @pcidevfn: return PCI device.func for @dev
+ *
+ * This function is somewhat SATA-specific.  Or at least the
+ * IDE and SCSI versions of this function are different,
+ * so it's not entirely generic code.
+ *
+ * Returns 0 on success, <0 on error.
+ */
+static int sata_get_dev_handle(struct device *dev, acpi_handle *handle,
+					acpi_integer *pcidevfn)
+{
+	struct pci_dev	*pci_dev;
+	acpi_integer	addr;
+
+	pci_dev = to_pci_dev(dev);	/* NOTE: PCI-specific */
+	/* Please refer to the ACPI spec for the syntax of _ADR. */
+	addr = (PCI_SLOT(pci_dev->devfn) << 16) | PCI_FUNC(pci_dev->devfn);
+	*pcidevfn = addr;
+	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), addr);
+	if (!*handle)
+		return -ENODEV;
+	return 0;
+}
+
+struct walk_info {		/* can be trimmed some */
+	struct device	*dev;
+	struct acpi_device *adev;
+	acpi_handle	handle;
+	acpi_integer	pcidevfn;
+	unsigned int	drivenum;
+	acpi_handle	obj_handle;
+	struct ata_port *ataport;
+	struct ata_device *atadev;
+	u32		sata_adr;
+	int		status;
+	char		basepath[ACPI_PATHNAME_MAX];
+	int		basepath_len;
+};
+
+static acpi_status get_devices(acpi_handle handle,
+				u32 level, void *context, void **return_value)
+{
+	acpi_status		status;
+	struct walk_info	*winfo = context;
+	struct acpi_buffer	namebuf = {ACPI_ALLOCATE_BUFFER, NULL};
+	char			*pathname;
+	struct acpi_buffer	buffer;
+	struct acpi_device_info	*dinfo;
+
+	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &namebuf);
+	if (status)
+		goto ret;
+	pathname = namebuf.pointer;
+
+	buffer.length = ACPI_ALLOCATE_BUFFER;
+	buffer.pointer = NULL;
+	status = acpi_get_object_info(handle, &buffer);
+
+	if (ACPI_SUCCESS(status)) {
+		dinfo = buffer.pointer;
+
+		/* find full device path name for pcidevfn */
+		if (dinfo && (dinfo->valid & ACPI_VALID_ADR) &&
+		    dinfo->address == winfo->pcidevfn) {
+			if (ata_msg_probe(winfo->ataport))
+				printk(KERN_DEBUG
+					":%s: matches pcidevfn (0x%llx)\n",
+					pathname, winfo->pcidevfn);
+			strlcpy(winfo->basepath, pathname,
+				sizeof(winfo->basepath));
+			winfo->basepath_len = strlen(pathname);
+			goto out;
+		}
+
+		/* if basepath is not yet known, ignore this object */
+		if (!winfo->basepath_len)
+			goto out;
+
+		/* if this object is in scope of basepath, maybe use it */
+		if (strncmp(pathname, winfo->basepath,
+		    winfo->basepath_len) == 0) {
+			if (!(dinfo->valid & ACPI_VALID_ADR))
+				goto out;
+			if (ata_msg_probe(winfo->ataport))
+				printk(KERN_DEBUG "GOT ONE: (%s) "
+					"root_port = 0x%llx, port_num = 0x%llx\n",
+					pathname,
+					SATA_ROOT_PORT(dinfo->address),
+					SATA_PORT_NUMBER(dinfo->address));
+			/* heuristics: */
+			if (SATA_PORT_NUMBER(dinfo->address) != NO_PORT_MULT)
+				if (ata_msg_probe(winfo->ataport))
+					printk(KERN_DEBUG
+						"warning: don't know how to handle SATA port multiplier\n");
+			if (SATA_ROOT_PORT(dinfo->address) ==
+				winfo->ataport->port_no &&
+			    SATA_PORT_NUMBER(dinfo->address) == NO_PORT_MULT) {
+				if (ata_msg_probe(winfo->ataport))
+					printk(KERN_DEBUG
+						"THIS ^^^^^ is the requested SATA drive (handle = 0x%p)\n",
+						handle);
+				winfo->sata_adr = dinfo->address;
+				winfo->obj_handle = handle;
+			}
+		}
+out:
+		acpi_os_free(dinfo);
+	}
+	acpi_os_free(pathname);
+
+ret:
+	return status;
+}
+
+/* Get the SATA drive _ADR object. */
+static int get_sata_adr(struct device *dev, acpi_handle handle,
+			acpi_integer pcidevfn, unsigned int drive,
+			struct ata_port *ap,
+			struct ata_device *atadev, u32 *dev_adr)
+{
+	acpi_status	status;
+	struct walk_info *winfo;
+	int		err = -ENOMEM;
+
+	winfo = kzalloc(sizeof(struct walk_info), GFP_KERNEL);
+	if (!winfo)
+		goto out;
+
+	winfo->dev = dev;
+	winfo->atadev = atadev;
+	winfo->ataport = ap;
+	if (acpi_bus_get_device(handle, &winfo->adev) < 0)
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "acpi_bus_get_device failed\n");
+	winfo->handle = handle;
+	winfo->pcidevfn = pcidevfn;
+	winfo->drivenum = drive;
+
+	status = acpi_get_devices(NULL, get_devices, winfo, NULL);
+	if (ACPI_FAILURE(status)) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: acpi_get_devices failed\n",
+				__FUNCTION__);
+		err = -ENODEV;
+	} else {
+		*dev_adr = winfo->sata_adr;
+		atadev->obj_handle = winfo->obj_handle;
+		err = 0;
+	}
+	kfree(winfo);
+out:
+	return err;
+}
+
+/**
+ * ata_acpi_push_id - send Identify data to a drive
+ * @ap: the ata_port for the drive
+ * @ix: drive index
+ *
+ * Must be after Identify (Packet) Device -- uses its data.
+ */
+int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
+{
+	acpi_handle			handle;
+	acpi_integer			pcidevfn;
+	int				err = -ENODEV;
+	struct device			*dev = ap->host_set->dev;
+	struct ata_device		*atadev = &ap->device[ix];
+	u32				dev_adr;
+	acpi_status			status;
+	struct acpi_object_list		input;
+	union acpi_object 		in_params[1];
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG
+			"%s: ap->id: %d, ix = %d, port#: %d, hard_port#: %d\n",
+			__FUNCTION__, ap->id, ix,
+			ap->port_no, ap->hard_port_no);
+
+	/* Don't continue if not a SATA device. */
+	if (!ata_id_is_sata(atadev->id)) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: ata_id_is_sata is False\n",
+				__FUNCTION__);
+		goto out;
+	}
+
+	/* Don't continue if device has no _ADR method.
+	 * _SDD is intended for known motherboard devices. */
+	err = sata_get_dev_handle(dev, &handle, &pcidevfn);
+	if (err < 0) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: sata_get_dev_handle failed (%d\n",
+				__FUNCTION__, err);
+		goto out;
+	}
+
+	/* Get this drive's _ADR info. if not already known. */
+	if (!atadev->obj_handle) {
+		dev_adr = SATA_ADR_RSVD;
+		err = get_sata_adr(dev, handle, pcidevfn, ix, ap, atadev,
+				&dev_adr);
+		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
+		    !atadev->obj_handle) {
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG "%s: get_sata_adr failed: "
+					"err=%d, dev_adr=%u, obj_handle=0x%p\n",
+					__FUNCTION__, err, dev_adr,
+					atadev->obj_handle);
+			goto out;
+		}
+	}
+
+	/* Give the drive Identify data to the drive via the _SDD method */
+	/* _SDD: set up input parameters */
+	input.count = 1;
+	input.pointer = in_params;
+	in_params[0].type = ACPI_TYPE_BUFFER;
+	in_params[0].buffer.length = sizeof(atadev->id);
+	in_params[0].buffer.pointer = (u8 *)atadev->id;
+	/* Output buffer: _SDD has no output */
+
+	/* It's OK for _SDD to be missing too. */
+	swap_buf_le16(atadev->id, ATA_ID_WORDS);
+	status = acpi_evaluate_object(atadev->obj_handle, "_SDD", &input, NULL);
+	swap_buf_le16(atadev->id, ATA_ID_WORDS);
+
+	err = ACPI_FAILURE(status) ? -EIO : 0;
+	if (err < 0) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"ata%u(%u): %s _SDD error: status = 0x%x\n",
+				ap->id, ap->device->devno,
+				__FUNCTION__, status);
+	}
+out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_push_id);
+
+/**
+ * do_drive_get_GTF - get the drive bootup default taskfile settings
+ * @ap: the ata_port for the drive
+ * @atadev: target ata_device
+ * @gtf_length: number of bytes of _GTF data returned at @gtf_address
+ * @gtf_address: buffer containing _GTF taskfile arrays
+ *
+ * This applies to both PATA and SATA drives.
+ *
+ * The _GTF method has no input parameters.
+ * It returns a variable number of register set values (registers
+ * hex 1F1..1F7, taskfiles).
+ * The <variable number> is not known in advance, so have ACPI-CA
+ * allocate the buffer as needed and return it, then free it later.
+ *
+ * The returned @gtf_length and @gtf_address are only valid if the
+ * function return value is 0.
+ */
+int do_drive_get_GTF(struct ata_port *ap, struct ata_device *atadev,
+			unsigned int *gtf_length, unsigned long *gtf_address)
+{
+	acpi_status			status;
+	acpi_handle			handle;
+	acpi_integer			pcidevfn;
+	u32				dev_adr;
+	struct acpi_buffer		output;
+	union acpi_object 		*out_obj;
+	struct device			*dev = ap->host_set->dev;
+	int				err = -ENODEV;
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG
+			"%s: ENTER: ap->id: %d, port#: %d, hard_port#: %d\n",
+			__FUNCTION__, ap->id,
+		ap->port_no, ap->hard_port_no);
+
+	*gtf_length = 0;
+	*gtf_address = 0UL;
+
+	if (!ata_dev_present(atadev) ||
+	    (ap->flags & ATA_FLAG_PORT_DISABLED)) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: ERR: "
+				"ata_dev_present: %d, PORT_DISABLED: %lu\n",
+				__FUNCTION__, ata_dev_present(atadev),
+				ap->flags & ATA_FLAG_PORT_DISABLED);
+		goto out;
+	}
+
+	/* Don't continue if device has no _ADR method.
+	 * _GTF is intended for known motherboard devices. */
+	err = sata_get_dev_handle(dev, &handle, &pcidevfn);
+	if (err < 0) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: sata_get_dev_handle failed (%d\n",
+				__FUNCTION__, err);
+		goto out;
+	}
+
+	/* Get this drive's _ADR info. if not already known. */
+	if (!atadev->obj_handle) {
+		dev_adr = SATA_ADR_RSVD;
+		err = get_sata_adr(dev, handle, pcidevfn, 0, ap, atadev,
+				&dev_adr);
+		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
+		    !atadev->obj_handle) {
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG "%s: get_sata_adr failed: "
+					"err=%d, dev_adr=%u, obj_handle=0x%p\n",
+					__FUNCTION__, err, dev_adr,
+					atadev->obj_handle);
+			goto out;
+		}
+	}
+
+	/* Setting up output buffer */
+	output.length = ACPI_ALLOCATE_BUFFER;
+	output.pointer = NULL;	/* ACPI-CA sets this; save/free it later */
+
+	/* _GTF has no input parameters */
+	err = -EIO;
+	status = acpi_evaluate_object(atadev->obj_handle, "_GTF",
+					NULL, &output);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_DEBUG
+			"%s: Run _GTF error: status = 0x%x\n",
+			__FUNCTION__, status);
+		goto out;
+	}
+
+	if (!output.length || !output.pointer) {
+		printk(KERN_DEBUG
+			"%s: Run _GTF: length or ptr is NULL (0x%llx, 0x%p)\n",
+			__FUNCTION__,
+			(unsigned long long)output.length, output.pointer);
+		acpi_os_free(output.pointer);
+		goto out;
+	}
+
+	out_obj = output.pointer;
+	if (out_obj->type != ACPI_TYPE_BUFFER) {
+		acpi_os_free(output.pointer);
+		printk(KERN_DEBUG "%s: Run _GTF: error: "
+			"expected object type of ACPI_TYPE_BUFFER, got 0x%x\n",
+			__FUNCTION__, out_obj->type);
+		err = -ENOENT;
+		goto out;
+	}
+
+	if (out_obj->buffer.length % REGS_PER_GTF) {
+		if (ata_msg_drv(ap))
+			printk(KERN_ERR "%s: unexpected GTF length (%d)\n",
+				__FUNCTION__, out_obj->buffer.length);
+		err = -ENOENT;
+		goto out;
+	}
+
+	*gtf_length = out_obj->buffer.length;
+	*gtf_address = (unsigned long)out_obj->buffer.pointer;
+	err = 0;
+out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(do_drive_get_GTF);
+
+/**
+ * taskfile_load_raw - send taskfile registers to host controller
+ * @ap: Port to which output is sent
+ * @gtf: raw ATA taskfile register set (0x1f1 - 0x1f7)
+ *
+ * Outputs ATA taskfile to standard ATA host controller using MMIO
+ * or PIO as indicated by the ATA_FLAG_MMIO flag.
+ * Writes the control, feature, nsect, lbal, lbam, and lbah registers.
+ * Optionally (ATA_TFLAG_LBA48) writes hob_feature, hob_nsect,
+ * hob_lbal, hob_lbam, and hob_lbah.
+ *
+ * This function waits for idle (!BUSY and !DRQ) after writing
+ * registers.  If the control register has a new value, this
+ * function also waits for idle after writing control and before
+ * writing the remaining registers.
+ *
+ * LOCKING: TBD:
+ * Inherited from caller.
+ */
+static void taskfile_load_raw(struct ata_port *ap,
+				struct ata_device *atadev,
+				const struct taskfile_array *gtf)
+{
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: (0x1f1-1f7): hex: "
+			"%02x %02x %02x %02x %02x %02x %02x\n",
+			__FUNCTION__,
+			gtf->tfa[0], gtf->tfa[1], gtf->tfa[2],
+			gtf->tfa[3], gtf->tfa[4], gtf->tfa[5], gtf->tfa[6]);
+
+	if (ap->ops->qc_issue) {
+		struct ata_taskfile tf;
+		unsigned int err;
+
+		ata_tf_init(ap, &tf, atadev->devno);
+
+		/* convert gtf to tf */
+		tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE; /* TBD */
+		tf.protocol = atadev->class == ATA_DEV_ATAPI ?
+			ATA_PROT_ATAPI_NODATA : ATA_PROT_NODATA;
+		tf.feature = gtf->tfa[0];	/* 0x1f1 */
+		tf.nsect   = gtf->tfa[1];	/* 0x1f2 */
+		tf.lbal    = gtf->tfa[2];	/* 0x1f3 */
+		tf.lbam    = gtf->tfa[3];	/* 0x1f4 */
+		tf.lbah    = gtf->tfa[4];	/* 0x1f5 */
+		tf.device  = gtf->tfa[5];	/* 0x1f6 */
+		tf.command = gtf->tfa[6];	/* 0x1f7 */
+
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "call ata_exec_internal:\n");
+		err = ata_exec_internal(ap, atadev, &tf, DMA_NONE, NULL, 0);
+		if (err && ata_msg_probe(ap))
+			printk(KERN_ERR "%s: ata_exec_internal failed: %u\n",
+				__FUNCTION__, err);
+	} else
+		if (ata_msg_warn(ap))
+			printk(KERN_WARNING
+				"%s: SATA driver is missing qc_issue function entry points\n",
+				__FUNCTION__);
+}
+
+/**
+ * do_drive_set_taskfiles - write the drive taskfile settings from _GTF
+ * @ap: the ata_port for the drive
+ * @atadev: target ata_device
+ * @gtf_length: total number of bytes of _GTF taskfiles
+ * @gtf_address: location of _GTF taskfile arrays
+ *
+ * This applies to both PATA and SATA drives.
+ *
+ * Write {gtf_address, length gtf_length} in groups of
+ * REGS_PER_GTF bytes.
+ */
+int do_drive_set_taskfiles(struct ata_port *ap, struct ata_device *atadev,
+			unsigned int gtf_length, unsigned long gtf_address)
+{
+	int			err = -ENODEV;
+	int			gtf_count = gtf_length / REGS_PER_GTF;
+	int			ix;
+	struct taskfile_array	*gtf;
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG
+			"%s: ENTER: ap->id: %d, port#: %d, hard_port#: %d\n",
+			__FUNCTION__, ap->id,
+			ap->port_no, ap->hard_port_no);
+
+	if (!ata_dev_present(atadev) ||
+	    (ap->flags & ATA_FLAG_PORT_DISABLED))
+		goto out;
+	if (!gtf_count)		/* shouldn't be here */
+		goto out;
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG
+			"%s: total GTF bytes = %u (0x%x), gtf_count = %d\n",
+			__FUNCTION__, gtf_length, gtf_length, gtf_count);
+	if (gtf_length % REGS_PER_GTF) {
+		if (ata_msg_drv(ap))
+			printk(KERN_ERR "%s: unexpected GTF length (%d)\n",
+				__FUNCTION__, gtf_length);
+		goto out;
+	}
+
+	for (ix = 0; ix < gtf_count; ix++) {
+		gtf = (struct taskfile_array *)
+			(gtf_address + ix * REGS_PER_GTF);
+
+		/* send all TaskFile registers (0x1f1-0x1f7) *in*that*order* */
+		taskfile_load_raw(ap, atadev, gtf);
+	}
+
+	err = 0;
+out:
+	return err;
+}
+EXPORT_SYMBOL_GPL(do_drive_set_taskfiles);
+
+/**
+ * ata_acpi_exec_tfs - get then write drive taskfile settings
+ * @ap: the ata_port for the drive
+ *
+ * This applies to both PATA and SATA drives.
+ */
+int ata_acpi_exec_tfs(struct ata_port *ap)
+{
+	int ix;
+	int ret;
+	unsigned int gtf_length;
+	unsigned long gtf_address;
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
+
+	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
+		printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",
+			__FUNCTION__, ix);
+		ret = do_drive_get_GTF(ap, &ap->device[ix],
+				&gtf_length, &gtf_address);
+		if (ret < 0) {
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG "%s: get_GTF error (%d)\n",
+					__FUNCTION__, ret);
+			break;
+		}
+
+		printk(KERN_DEBUG "%s: call set_taskfiles, ix=%d\n",
+			__FUNCTION__, ix);
+		ret = do_drive_set_taskfiles(ap, &ap->device[ix],
+				gtf_length, gtf_address);
+		acpi_os_free((void *)gtf_address);
+		if (ret < 0) {
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG
+					"%s: set_taskfiles error (%d)\n",
+					__FUNCTION__, ret);
+			break;
+		}
+	}
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: ret=%d\n", __FUNCTION__, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_exec_tfs);
--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
@@ -1112,7 +1112,7 @@ int ata_qc_complete_internal(struct ata_
  *	None.  Should be called with kernel context, might sleep.
  */
 
-static unsigned
+unsigned int
 ata_exec_internal(struct ata_port *ap, struct ata_device *dev,
 		  struct ata_taskfile *tf,
 		  int dma_dir, void *buf, unsigned int buflen)
@@ -1461,6 +1461,8 @@ void ata_dev_config(struct ata_port *ap,
 
 	if (ap->ops->dev_config)
 		ap->ops->dev_config(ap, &ap->device[i]);
+
+	ata_acpi_push_id(ap, i);
 }
 
 /**
@@ -1501,6 +1503,8 @@ static int ata_bus_probe(struct ata_port
 	if (ap->flags & ATA_FLAG_PORT_DISABLED)
 		goto err_out_disable;
 
+	ata_acpi_exec_tfs(ap);
+
 	return 0;
 
 err_out_disable:
@@ -4291,6 +4295,7 @@ int ata_device_resume(struct ata_port *a
 	}
 	if (!ata_dev_present(dev))
 		return 0;
+	ata_acpi_exec_tfs(ap);
 	if (dev->class == ATA_DEV_ATA)
 		ata_start_drive(ap, dev);
 
--- linux-2616-rc4-ata.orig/drivers/scsi/libata.h
+++ linux-2616-rc4-ata/drivers/scsi/libata.h
@@ -52,6 +52,42 @@ extern void ata_dev_select(struct ata_po
 extern void swap_buf_le16(u16 *buf, unsigned int buf_words);
 extern int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
+extern unsigned int ata_exec_internal(struct ata_port *ap,
+				struct ata_device *dev,
+				struct ata_taskfile *tf,
+				int dma_dir, void *buf, unsigned int buflen);
+
+
+/* libata-acpi.c */
+#ifdef CONFIG_SCSI_SATA_ACPI
+extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
+extern int do_drive_get_GTF(struct ata_port *ap, struct ata_device *atadev,
+			unsigned int *gtf_length, unsigned long *gtf_address);
+extern int do_drive_set_taskfiles(struct ata_port *ap, struct ata_device *atadev,
+			unsigned int gtf_length, unsigned long gtf_address);
+extern int ata_acpi_exec_tfs(struct ata_port *ap);
+#else
+static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
+{
+	return 0;
+}
+static inline int do_drive_get_GTF(struct ata_port *ap,
+			struct ata_device *atadev,
+			unsigned int *gtf_length, unsigned long *gtf_address)
+{
+	return 0;
+}
+static inline int do_drive_set_taskfiles(struct ata_port *ap,
+			struct ata_device *atadev,
+			unsigned int gtf_length, unsigned long gtf_address)
+{
+	return 0;
+}
+static inline int ata_acpi_exec_tfs(struct ata_port *ap)
+{
+	return 0;
+}
+#endif
 
 
 /* libata-scsi.c */

[PATCH 4/13] ATA ACPI: add params/docs.

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Add and use 'noacpi' parameter for libata-acpi.
Add and use 'printk' parameter for libata (parts).
Update Documentation/kernel-parameters.txt, including atapi_enabled.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 Documentation/kernel-parameters.txt |   13 +++++++++++++
 drivers/scsi/libata-acpi.c          |   12 ++++++++++++
 drivers/scsi/libata-core.c          |   10 ++++++++++
 drivers/scsi/libata.h               |    2 ++
 4 files changed, 37 insertions(+)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
@@ -82,6 +82,14 @@ int atapi_enabled = 0;
 module_param(atapi_enabled, int, 0444);
 MODULE_PARM_DESC(atapi_enabled, "Enable discovery of ATAPI devices (0=off, 1=on)");
 
+int noacpi = 0;
+module_param(noacpi, int, 0444);
+MODULE_PARM_DESC(noacpi, "Disables use of ACPI in suspend/resume when set");
+
+int libata_printk = ATA_MSG_DRV;
+module_param_named(printk, libata_printk, int, 0644);
+MODULE_PARM_DESC(printk, "Set libata printk flags"); /* in linux/libata.h */
+
 MODULE_AUTHOR("Jeff Garzik");
 MODULE_DESCRIPTION("Library module for ATA devices");
 MODULE_LICENSE("GPL");
@@ -4545,6 +4553,8 @@ int ata_device_add(const struct ata_prob
 				(ap->mwdma_mask << ATA_SHIFT_MWDMA) |
 				(ap->pio_mask << ATA_SHIFT_PIO);
 
+		ap->msg_enable = libata_printk;
+
 		/* print per-port info to dmesg */
 		printk(KERN_INFO "ata%u: %cATA max %s cmd 0x%lX ctl 0x%lX "
 				 "bmdma 0x%lX irq %lu\n",
--- linux-2616-rc4-ata.orig/Documentation/kernel-parameters.txt
+++ linux-2616-rc4-ata/Documentation/kernel-parameters.txt
@@ -41,6 +41,7 @@ restrictions referred to are that the re
 	ISAPNP	ISA PnP code is enabled.
 	ISDN	Appropriate ISDN support is enabled.
 	JOY	Appropriate joystick support is enabled.
+	LIBATA	libata driver is enabled.
 	LP	Printer support is enabled.
 	LOOP	Loopback device support is enabled.
 	M68k	M68k architecture is enabled.
@@ -242,6 +243,9 @@ running once the system is up.
 
 	ataflop=	[HW,M68k]
 
+	atapi_enabled=	[LIBATA] Enable discovery & support of ATAPI devices
+			Format: <value> (0=off, 1=on)
+
 	atarimouse=	[HW,MOUSE] Atari Mouse
 
 	atascsi=	[HW,SCSI] Atari SCSI
@@ -981,6 +985,10 @@ running once the system is up.
 			emulation library even if a 387 maths coprocessor
 			is present.
 
+	noacpi=		[LIBATA] Disables use of ACPI in libata suspend/resume
+			when set.
+			Format: <int>
+
 	noalign		[KNL,ARM]
 
 	noapic		[SMP,APIC] Tells the kernel to not make use of any
@@ -1227,6 +1235,11 @@ running once the system is up.
 			autoconfiguration.
 			Ranges are in pairs (memory base and size).
 
+	printk=		[LIBATA] Set libata printk level (mask).
+			The values are defined in include/linux/libata.h.
+			The default value is 1 (ATA_MSG_DRV).
+			Format: <int>
+
 	profile=	[KNL] Enable kernel profiling via /proc/profile
 			Format: [schedule,]<number>
 			Param: "schedule" - profile schedule points.
--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -212,6 +212,9 @@ int ata_acpi_push_id(struct ata_port *ap
 	struct acpi_object_list		input;
 	union acpi_object 		in_params[1];
 
+	if (noacpi)
+		return 0;
+
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG
 			"%s: ap->id: %d, ix = %d, port#: %d, hard_port#: %d\n",
@@ -319,6 +322,9 @@ int do_drive_get_GTF(struct ata_port *ap
 	*gtf_length = 0;
 	*gtf_address = 0UL;
 
+	if (noacpi)
+		return 0;
+
 	if (!ata_dev_present(atadev) ||
 	    (ap->flags & ATA_FLAG_PORT_DISABLED)) {
 		if (ata_msg_probe(ap))
@@ -493,6 +499,9 @@ int do_drive_set_taskfiles(struct ata_po
 			__FUNCTION__, ap->id,
 			ap->port_no, ap->hard_port_no);
 
+	if (noacpi)
+		return 0;
+
 	if (!ata_dev_present(atadev) ||
 	    (ap->flags & ATA_FLAG_PORT_DISABLED))
 		goto out;
@@ -540,6 +549,9 @@ int ata_acpi_exec_tfs(struct ata_port *a
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
 
+	if (noacpi)
+		return 0;
+
 	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
 		printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",
 			__FUNCTION__, ix);
--- linux-2616-rc4-ata.orig/drivers/scsi/libata.h
+++ linux-2616-rc4-ata/drivers/scsi/libata.h
@@ -41,6 +41,8 @@ struct ata_scsi_args {
 
 /* libata-core.c */
 extern int atapi_enabled;
+extern int noacpi;
+extern int libata_printk;
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_port *ap,
 				      struct ata_device *dev);
 extern int ata_rwcmd_protocol(struct ata_queued_cmd *qc);

[PATCH 5/13] ATA ACPI: use debugging macros

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Add more libata-acpi debugging, plus controlled by libata.printk value.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-acpi.c |   53 ++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 19 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -371,17 +371,20 @@ int do_drive_get_GTF(struct ata_port *ap
 	status = acpi_evaluate_object(atadev->obj_handle, "_GTF",
 					NULL, &output);
 	if (ACPI_FAILURE(status)) {
-		printk(KERN_DEBUG
-			"%s: Run _GTF error: status = 0x%x\n",
-			__FUNCTION__, status);
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: Run _GTF error: status = 0x%x\n",
+				__FUNCTION__, status);
 		goto out;
 	}
 
 	if (!output.length || !output.pointer) {
-		printk(KERN_DEBUG
-			"%s: Run _GTF: length or ptr is NULL (0x%llx, 0x%p)\n",
-			__FUNCTION__,
-			(unsigned long long)output.length, output.pointer);
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: Run _GTF: "
+				"length or ptr is NULL (0x%llx, 0x%p)\n",
+				__FUNCTION__,
+				(unsigned long long)output.length,
+				output.pointer);
 		acpi_os_free(output.pointer);
 		goto out;
 	}
@@ -389,23 +392,32 @@ int do_drive_get_GTF(struct ata_port *ap
 	out_obj = output.pointer;
 	if (out_obj->type != ACPI_TYPE_BUFFER) {
 		acpi_os_free(output.pointer);
-		printk(KERN_DEBUG "%s: Run _GTF: error: "
-			"expected object type of ACPI_TYPE_BUFFER, got 0x%x\n",
-			__FUNCTION__, out_obj->type);
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: Run _GTF: error: "
+				"expected object type of ACPI_TYPE_BUFFER, "
+				"got 0x%x\n",
+				__FUNCTION__, out_obj->type);
 		err = -ENOENT;
 		goto out;
 	}
 
-	if (out_obj->buffer.length % REGS_PER_GTF) {
+	if (!out_obj->buffer.length || !out_obj->buffer.pointer ||
+	    out_obj->buffer.length % REGS_PER_GTF) {
 		if (ata_msg_drv(ap))
-			printk(KERN_ERR "%s: unexpected GTF length (%d)\n",
-				__FUNCTION__, out_obj->buffer.length);
+			printk(KERN_ERR
+				"%s: unexpected GTF length (%d) or addr (0x%p)\n",
+				__FUNCTION__, out_obj->buffer.length,
+				out_obj->buffer.pointer);
 		err = -ENOENT;
 		goto out;
 	}
 
 	*gtf_length = out_obj->buffer.length;
 	*gtf_address = (unsigned long)out_obj->buffer.pointer;
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: returning "
+			"gtf_length=%d, gtf_address=0x%lx\n",
+			__FUNCTION__, *gtf_length, *gtf_address);
 	err = 0;
 out:
 	return err;
@@ -510,8 +522,9 @@ int do_drive_set_taskfiles(struct ata_po
 
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG
-			"%s: total GTF bytes = %u (0x%x), gtf_count = %d\n",
-			__FUNCTION__, gtf_length, gtf_length, gtf_count);
+			"%s: total GTF bytes=%u (0x%x), gtf_count=%d, addr=0x%lx\n",
+			__FUNCTION__, gtf_length, gtf_length, gtf_count,
+			gtf_address);
 	if (gtf_length % REGS_PER_GTF) {
 		if (ata_msg_drv(ap))
 			printk(KERN_ERR "%s: unexpected GTF length (%d)\n",
@@ -553,8 +566,9 @@ int ata_acpi_exec_tfs(struct ata_port *a
 		return 0;
 
 	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
-		printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",
-			__FUNCTION__, ix);
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",
+				__FUNCTION__, ix);
 		ret = do_drive_get_GTF(ap, &ap->device[ix],
 				&gtf_length, &gtf_address);
 		if (ret < 0) {
@@ -564,8 +578,9 @@ int ata_acpi_exec_tfs(struct ata_port *a
 			break;
 		}
 
-		printk(KERN_DEBUG "%s: call set_taskfiles, ix=%d\n",
-			__FUNCTION__, ix);
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: call set_taskfiles, ix=%d\n",
+				__FUNCTION__, ix);
 		ret = do_drive_set_taskfiles(ap, &ap->device[ix],
 				gtf_length, gtf_address);
 		acpi_os_free((void *)gtf_address);

[PATCH 6/13] ATA ACPI: use correct acpi_object pointer

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Save and free the correct acpi_object pointer.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-acpi.c |   14 +++++++++-----
 drivers/scsi/libata.h      |    6 ++++--
 2 files changed, 13 insertions(+), 7 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -302,7 +302,8 @@ EXPORT_SYMBOL_GPL(ata_acpi_push_id);
  * function return value is 0.
  */
 int do_drive_get_GTF(struct ata_port *ap, struct ata_device *atadev,
-			unsigned int *gtf_length, unsigned long *gtf_address)
+			unsigned int *gtf_length, unsigned long *gtf_address,
+			unsigned long *obj_loc)
 {
 	acpi_status			status;
 	acpi_handle			handle;
@@ -321,6 +322,7 @@ int do_drive_get_GTF(struct ata_port *ap
 
 	*gtf_length = 0;
 	*gtf_address = 0UL;
+	*obj_loc = 0UL;
 
 	if (noacpi)
 		return 0;
@@ -414,10 +416,11 @@ int do_drive_get_GTF(struct ata_port *ap
 
 	*gtf_length = out_obj->buffer.length;
 	*gtf_address = (unsigned long)out_obj->buffer.pointer;
+	*obj_loc = (unsigned long)out_obj;
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG "%s: returning "
-			"gtf_length=%d, gtf_address=0x%lx\n",
-			__FUNCTION__, *gtf_length, *gtf_address);
+			"gtf_length=%d, gtf_address=0x%lx, obj_loc=0x%lx\n",
+			__FUNCTION__, *gtf_length, *gtf_address, *obj_loc);
 	err = 0;
 out:
 	return err;
@@ -558,6 +561,7 @@ int ata_acpi_exec_tfs(struct ata_port *a
 	int ret;
 	unsigned int gtf_length;
 	unsigned long gtf_address;
+	unsigned long obj_loc;
 
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
@@ -570,7 +574,7 @@ int ata_acpi_exec_tfs(struct ata_port *a
 			printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",
 				__FUNCTION__, ix);
 		ret = do_drive_get_GTF(ap, &ap->device[ix],
-				&gtf_length, &gtf_address);
+				&gtf_length, &gtf_address, &obj_loc);
 		if (ret < 0) {
 			if (ata_msg_probe(ap))
 				printk(KERN_DEBUG "%s: get_GTF error (%d)\n",
@@ -583,7 +587,7 @@ int ata_acpi_exec_tfs(struct ata_port *a
 				__FUNCTION__, ix);
 		ret = do_drive_set_taskfiles(ap, &ap->device[ix],
 				gtf_length, gtf_address);
-		acpi_os_free((void *)gtf_address);
+		acpi_os_free((void *)obj_loc);
 		if (ret < 0) {
 			if (ata_msg_probe(ap))
 				printk(KERN_DEBUG
--- linux-2616-rc4-ata.orig/drivers/scsi/libata.h
+++ linux-2616-rc4-ata/drivers/scsi/libata.h
@@ -64,7 +64,8 @@ extern unsigned int ata_exec_internal(st
 #ifdef CONFIG_SCSI_SATA_ACPI
 extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
 extern int do_drive_get_GTF(struct ata_port *ap, struct ata_device *atadev,
-			unsigned int *gtf_length, unsigned long *gtf_address);
+			unsigned int *gtf_length, unsigned long *gtf_address,
+			unsigned long *obj_loc);
 extern int do_drive_set_taskfiles(struct ata_port *ap, struct ata_device *atadev,
 			unsigned int gtf_length, unsigned long gtf_address);
 extern int ata_acpi_exec_tfs(struct ata_port *ap);
@@ -75,7 +76,8 @@ static inline int ata_acpi_push_id(struc
 }
 static inline int do_drive_get_GTF(struct ata_port *ap,
 			struct ata_device *atadev,
-			unsigned int *gtf_length, unsigned long *gtf_address)
+			unsigned int *gtf_length, unsigned long *gtf_address,
+			unsigned long *obj_loc)
 {
 	return 0;
 }

[PATCH 7/13] ATA ACPI: more Makefile/Kconfig

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Simplify Makefile.
Add Kconfig help.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/Kconfig  |   10 +++++++++-
 drivers/scsi/Makefile |    6 ++----
 2 files changed, 11 insertions(+), 5 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/Makefile
+++ linux-2616-rc4-ata/drivers/scsi/Makefile
@@ -163,10 +163,8 @@ ncr53c8xx-flags-$(CONFIG_SCSI_ZALON) \
 CFLAGS_ncr53c8xx.o	:= $(ncr53c8xx-flags-y) $(ncr53c8xx-flags-m)
 zalon7xx-objs	:= zalon.o ncr53c8xx.o
 NCR_Q720_mod-objs	:= NCR_Q720.o ncr53c8xx.o
-libata-objs	:= libata-core.o libata-scsi.o
-ifeq ($(CONFIG_SCSI_SATA_ACPI),y)
-  libata-objs	+= libata-acpi.o
-endif
+libata-y	:= libata-core.o libata-scsi.o
+libata-$(CONFIG_SCSI_SATA_ACPI) += libata-acpi.o
 oktagon_esp_mod-objs	:= oktagon_esp.o oktagon_io.o
 
 # Files generated that shall be removed upon make clean
--- linux-2616-rc4-ata.orig/drivers/scsi/Kconfig
+++ linux-2616-rc4-ata/drivers/scsi/Kconfig
@@ -601,8 +601,16 @@ config SCSI_SATA_INTEL_COMBINED
 
 config SCSI_SATA_ACPI
 	bool
-	depends on SCSI_SATA && ACPI
+	depends on SCSI_SATA && ACPI && PCI
 	default y
+	help
+	  This option adds support for SATA-related ACPI objects.
+	  These ACPI objects add the ability to retrieve taskfiles
+	  from the ACPI BIOS and write them to the disk controller.
+	  These objects may be related to performance, security,
+	  power management, or other areas.
+	  You can disable this at kernel boot time by using the
+	  option 'libata.noacpi'.
 
 config SCSI_BUSLOGIC
 	tristate "BusLogic SCSI support"

[PATCH 8/13] ATA ACPI: PATA methods

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Add PATA support to the previous SATA support.
Add _GTM and _STM methods and expose (export) them.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-acpi.c |  421 ++++++++++++++++++++++++++++++++++++++++++---
 drivers/scsi/libata-core.c |   23 ++
 drivers/scsi/libata.h      |   13 +
 include/linux/libata.h     |   10 -
 4 files changed, 433 insertions(+), 34 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -35,6 +35,23 @@ struct taskfile_array {
 	u8	tfa[REGS_PER_GTF];	/* regs. 0x1f1 - 0x1f7 */
 };
 
+struct GTM_buffer {
+	__u32	PIO_speed0;
+	__u32	DMA_speed0;
+	__u32	PIO_speed1;
+	__u32	DMA_speed1;
+	__u32	GTM_flags;
+};
+
+#define DEBUGGING	1
+/* note: adds function name and KERN_DEBUG */
+#ifdef DEBUGGING
+#define DEBPRINT(fmt, args...)	\
+		printk(KERN_DEBUG "%s: " fmt, __FUNCTION__, ## args)
+#else
+#define DEBPRINT(fmt, args...)	do {} while (0)
+#endif	/* DEBUGGING */
+
 /**
  * sata_get_dev_handle - finds acpi_handle and PCI device.function
  * @dev: device to locate
@@ -58,11 +75,84 @@ static int sata_get_dev_handle(struct de
 	addr = (PCI_SLOT(pci_dev->devfn) << 16) | PCI_FUNC(pci_dev->devfn);
 	*pcidevfn = addr;
 	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), addr);
+	printk(KERN_DEBUG "%s: SATA dev addr=0x%llx, handle=0x%p\n",
+		__FUNCTION__, (unsigned long long)addr, *handle);
 	if (!*handle)
 		return -ENODEV;
 	return 0;
 }
 
+/**
+ * pata_get_dev_handle - finds acpi_handle and PCI device.function
+ * @dev: device to locate
+ * @handle: returned acpi_handle for @dev
+ * @pcidevfn: return PCI device.func for @dev
+ *
+ * The PATA and SATA versions of this function are different.
+ *
+ * Returns 0 on success, <0 on error.
+ */
+static int pata_get_dev_handle(struct device *dev, acpi_handle *handle,
+					acpi_integer *pcidevfn)
+{
+	unsigned int domain, bus, devnum, func;
+	acpi_integer addr;
+	acpi_handle dev_handle, parent_handle;
+	int scanned;
+	struct acpi_buffer buffer = {.length = ACPI_ALLOCATE_BUFFER,
+					.pointer = NULL};
+	acpi_status status;
+	struct acpi_device_info	*dinfo = NULL;
+	int ret = -ENODEV;
+
+	printk(KERN_DEBUG "%s: ENTER: dev->bus_id='%s'\n",
+		__FUNCTION__, dev->bus_id);
+	if ((scanned = sscanf(dev->bus_id, "%x:%x:%x.%x",
+			&domain, &bus, &devnum, &func)) != 4) {
+		printk(KERN_DEBUG "%s: sscanf ret. %d\n",
+			__FUNCTION__, scanned);
+		goto err;
+	}
+
+	dev_handle = DEVICE_ACPI_HANDLE(dev);
+	parent_handle = DEVICE_ACPI_HANDLE(dev->parent);
+
+	status = acpi_get_object_info(parent_handle, &buffer);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_DEBUG "%s: get_object_info for parent failed\n",
+			__FUNCTION__);
+		goto err;
+	}
+	dinfo = buffer.pointer;
+	if (dinfo && (dinfo->valid & ACPI_VALID_ADR) &&
+	    dinfo->address == bus) {
+		/* ACPI spec for _ADR for PCI bus: */
+		addr = (acpi_integer)(devnum << 16 | func);
+		*pcidevfn = addr;
+		*handle = dev_handle;
+	} else {
+		printk(KERN_DEBUG "%s: get_object_info for parent has wrong "
+			" bus: %llu, should be %d\n",
+			__FUNCTION__,
+			dinfo ? (unsigned long long)dinfo->address : -1ULL,
+			bus);
+		goto err;
+	}
+
+	printk(KERN_DEBUG "%s: dev_handle: 0x%p, parent_handle: 0x%p\n",
+		__FUNCTION__, dev_handle, parent_handle);
+	printk(KERN_DEBUG
+		"%s: for dev=0x%x.%x, addr=0x%llx, parent=0x%p, *handle=0x%p\n",
+		__FUNCTION__, devnum, func, (unsigned long long)addr,
+		dev->parent, *handle);
+	if (!*handle)
+		goto err;
+	ret = 0;
+err:
+	acpi_os_free(dinfo);
+	return ret;
+}
+
 struct walk_info {		/* can be trimmed some */
 	struct device	*dev;
 	struct acpi_device *adev;
@@ -198,6 +288,7 @@ out:
  * @ap: the ata_port for the drive
  * @ix: drive index
  *
+ * _SDD ACPI object:  for SATA mode only.
  * Must be after Identify (Packet) Device -- uses its data.
  */
 int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
@@ -212,6 +303,11 @@ int ata_acpi_push_id(struct ata_port *ap
 	struct acpi_object_list		input;
 	union acpi_object 		in_params[1];
 
+	if (ap->legacy_mode) {
+		printk(KERN_DEBUG "%s: skipping for PATA mode\n",
+			__FUNCTION__);
+		return 0;
+	}
 	if (noacpi)
 		return 0;
 
@@ -286,7 +382,7 @@ EXPORT_SYMBOL_GPL(ata_acpi_push_id);
 /**
  * do_drive_get_GTF - get the drive bootup default taskfile settings
  * @ap: the ata_port for the drive
- * @atadev: target ata_device
+ * @ix: target ata_device (drive) index
  * @gtf_length: number of bytes of _GTF data returned at @gtf_address
  * @gtf_address: buffer containing _GTF taskfile arrays
  *
@@ -301,25 +397,21 @@ EXPORT_SYMBOL_GPL(ata_acpi_push_id);
  * The returned @gtf_length and @gtf_address are only valid if the
  * function return value is 0.
  */
-int do_drive_get_GTF(struct ata_port *ap, struct ata_device *atadev,
+int do_drive_get_GTF(struct ata_port *ap, int ix,
 			unsigned int *gtf_length, unsigned long *gtf_address,
 			unsigned long *obj_loc)
 {
 	acpi_status			status;
-	acpi_handle			handle;
+	acpi_handle			dev_handle;
+	acpi_handle			chan_handle, drive_handle;
 	acpi_integer			pcidevfn;
 	u32				dev_adr;
 	struct acpi_buffer		output;
 	union acpi_object 		*out_obj;
 	struct device			*dev = ap->host_set->dev;
+	struct ata_device		*atadev = &ap->device[ix];
 	int				err = -ENODEV;
 
-	if (ata_msg_probe(ap))
-		printk(KERN_DEBUG
-			"%s: ENTER: ap->id: %d, port#: %d, hard_port#: %d\n",
-			__FUNCTION__, ap->id,
-		ap->port_no, ap->hard_port_no);
-
 	*gtf_length = 0;
 	*gtf_address = 0UL;
 	*obj_loc = 0UL;
@@ -327,6 +419,12 @@ int do_drive_get_GTF(struct ata_port *ap
 	if (noacpi)
 		return 0;
 
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG
+			"%s: ENTER: ap->id: %d, port#: %d, hard_port#: %d\n",
+			__FUNCTION__, ap->id,
+		ap->port_no, ap->hard_port_no);
+
 	if (!ata_dev_present(atadev) ||
 	    (ap->flags & ATA_FLAG_PORT_DISABLED)) {
 		if (ata_msg_probe(ap))
@@ -339,24 +437,65 @@ int do_drive_get_GTF(struct ata_port *ap
 
 	/* Don't continue if device has no _ADR method.
 	 * _GTF is intended for known motherboard devices. */
-	err = sata_get_dev_handle(dev, &handle, &pcidevfn);
-	if (err < 0) {
-		if (ata_msg_probe(ap))
-			printk(KERN_DEBUG
-				"%s: sata_get_dev_handle failed (%d\n",
-				__FUNCTION__, err);
-		goto out;
+	if (ata_id_is_ata(atadev->id)) {
+		err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
+		if (err < 0) {
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG
+					"%s: pata_get_dev_handle failed (%d)\n",
+					__FUNCTION__, err);
+			goto out;
+		}
+	} else {
+		err = sata_get_dev_handle(dev, &dev_handle, &pcidevfn);
+		if (err < 0) {
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG
+					"%s: sata_get_dev_handle failed (%d\n",
+					__FUNCTION__, err);
+			goto out;
+		}
 	}
 
 	/* Get this drive's _ADR info. if not already known. */
 	if (!atadev->obj_handle) {
-		dev_adr = SATA_ADR_RSVD;
-		err = get_sata_adr(dev, handle, pcidevfn, 0, ap, atadev,
-				&dev_adr);
+		if (ata_id_is_ata(atadev->id)) {
+			/* get child objects of dev_handle == channel objects,
+	 		 * + _their_ children == drive objects */
+			/* channel is ap->hard_port_no */
+			chan_handle = acpi_get_child(dev_handle,
+						ap->hard_port_no);
+			if (ata_msg_probe(ap))
+				printk(KERN_DEBUG
+					"%s: chan adr=%d: chan_handle=0x%p\n",
+					__FUNCTION__, ap->hard_port_no,
+					chan_handle);
+			if (!chan_handle) {
+				err = -ENODEV;
+				goto out;
+			}
+			/* TBD: could also check ACPI object VALID bits */
+			drive_handle = acpi_get_child(chan_handle, ix);
+			printk(KERN_DEBUG "%s:   drive w/ adr=%d: %c: 0x%p\n",
+				__FUNCTION__, ix,
+				ap->device[0].class == ATA_DEV_NONE ? 'n' : 'v',
+				drive_handle);
+			if (!drive_handle) {
+				err = -ENODEV;
+				goto out;
+			}
+			dev_adr = ix;
+			atadev->obj_handle = drive_handle;
+		} else {	/* for SATA mode */
+			dev_adr = SATA_ADR_RSVD;
+			err = get_sata_adr(dev, dev_handle, pcidevfn, 0,
+					ap, atadev, &dev_adr);
+		}
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
 		    !atadev->obj_handle) {
 			if (ata_msg_probe(ap))
-				printk(KERN_DEBUG "%s: get_sata_adr failed: "
+				printk(KERN_DEBUG
+					"%s: get_sata/pata_adr failed: "
 					"err=%d, dev_adr=%u, obj_handle=0x%p\n",
 					__FUNCTION__, err, dev_adr,
 					atadev->obj_handle);
@@ -516,6 +655,11 @@ int do_drive_set_taskfiles(struct ata_po
 
 	if (noacpi)
 		return 0;
+	if (!ata_id_is_sata(atadev->id)) {
+		printk(KERN_DEBUG "%s: skipping non-SATA drive\n",
+			__FUNCTION__);
+		return 0;
+	}
 
 	if (!ata_dev_present(atadev) ||
 	    (ap->flags & ATA_FLAG_PORT_DISABLED))
@@ -557,11 +701,11 @@ EXPORT_SYMBOL_GPL(do_drive_set_taskfiles
  */
 int ata_acpi_exec_tfs(struct ata_port *ap)
 {
-	int ix;
-	int ret;
-	unsigned int gtf_length;
-	unsigned long gtf_address;
-	unsigned long obj_loc;
+	int		ix;
+	int		ret;
+	unsigned int	gtf_length;
+	unsigned long	gtf_address;
+	unsigned long	obj_loc;
 
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
@@ -573,7 +717,7 @@ int ata_acpi_exec_tfs(struct ata_port *a
 		if (ata_msg_probe(ap))
 			printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",
 				__FUNCTION__, ix);
-		ret = do_drive_get_GTF(ap, &ap->device[ix],
+		ret = do_drive_get_GTF(ap, ix,
 				&gtf_length, &gtf_address, &obj_loc);
 		if (ret < 0) {
 			if (ata_msg_probe(ap))
@@ -603,3 +747,228 @@ int ata_acpi_exec_tfs(struct ata_port *a
 	return ret;
 }
 EXPORT_SYMBOL_GPL(ata_acpi_exec_tfs);
+
+/**
+ * ata_acpi_get_timing - get the channel (controller) timings
+ * @ap: target ata_port (channel)
+ *
+ * For PATA ACPI, this function executes the _GTM ACPI method for the
+ * target channel.
+ *
+ * _GTM only applies to ATA controllers in PATA (legacy) mode, not to SATA.
+ * In legacy mode, ap->hard_port_no is channel (controller) number.
+ */
+void ata_acpi_get_timing(struct ata_port *ap)
+{
+	struct device		*dev = ap->dev;
+	int			err;
+	acpi_handle		dev_handle;
+	acpi_integer		pcidevfn;
+	acpi_handle		chan_handle;
+	acpi_status		status;
+	struct acpi_buffer	output;
+	union acpi_object 	*out_obj;
+	struct GTM_buffer	*gtm;
+
+	if (noacpi)
+		goto out;
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
+
+	if (!ap->legacy_mode) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: channel/controller not in legacy mode (%s)\n",
+				__FUNCTION__, dev->bus_id);
+		goto out;
+	}
+
+	err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
+	if (err < 0) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: pata_get_dev_handle failed (%d)\n",
+				__FUNCTION__, err);
+		goto out;
+	}
+
+	/* get child objects of dev_handle == channel objects,
+	 * + _their_ children == drive objects */
+	/* channel is ap->hard_port_no */
+	chan_handle = acpi_get_child(dev_handle, ap->hard_port_no);
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: chan adr=%d: handle=0x%p\n",
+			__FUNCTION__, ap->hard_port_no, chan_handle);
+	if (!chan_handle)
+		goto out;
+
+	/* Setting up output buffer for _GTM */
+	output.length = ACPI_ALLOCATE_BUFFER;
+	output.pointer = NULL;	/* ACPI-CA sets this; save/free it later */
+
+	/* _GTM has no input parameters */
+	status = acpi_evaluate_object(chan_handle, "_GTM",
+					NULL, &output);
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: _GTM status: %d, outptr: 0x%p, outlen: 0x%llx\n",
+			__FUNCTION__, status, output.pointer,
+			(unsigned long long)output.length);
+	if (ACPI_FAILURE(status)) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: Run _GTM error: status = 0x%x\n",
+				__FUNCTION__, status);
+		goto out;
+	}
+
+	if (!output.length || !output.pointer) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: Run _GTM: "
+				"length or ptr is NULL (0x%llx, 0x%p)\n",
+				__FUNCTION__,
+				(unsigned long long)output.length,
+				output.pointer);
+		acpi_os_free(output.pointer);
+		goto out;
+	}
+
+	out_obj = output.pointer;
+	if (out_obj->type != ACPI_TYPE_BUFFER) {
+		acpi_os_free(output.pointer);
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: Run _GTM: error: "
+				"expected object type of ACPI_TYPE_BUFFER, "
+				"got 0x%x\n",
+				__FUNCTION__, out_obj->type);
+		goto out;
+	}
+
+	if (!out_obj->buffer.length || !out_obj->buffer.pointer ||
+	    out_obj->buffer.length != sizeof(struct GTM_buffer)) {
+		acpi_os_free(output.pointer);
+		if (ata_msg_drv(ap))
+			printk(KERN_ERR
+				"%s: unexpected _GTM length (0x%x)[should be 0x%x] or addr (0x%p)\n",
+				__FUNCTION__, out_obj->buffer.length,
+				sizeof(struct GTM_buffer), out_obj->buffer.pointer);
+		goto out;
+	}
+
+	gtm = (struct GTM_buffer *)out_obj->buffer.pointer;
+	if (ata_msg_probe(ap)) {
+		printk(KERN_DEBUG "%s: _GTM info: ptr: 0x%p, len: 0x%x, exp.len: 0x%Zx\n",
+			__FUNCTION__, out_obj->buffer.pointer,
+			out_obj->buffer.length, sizeof(struct GTM_buffer));
+		printk(KERN_DEBUG "%s: _GTM fields: 0x%x, 0x%x, 0x%x, 0x%x, 0x%x\n",
+			__FUNCTION__, gtm->PIO_speed0, gtm->DMA_speed0,
+			gtm->PIO_speed1, gtm->DMA_speed1, gtm->GTM_flags);
+	}
+
+	/* TBD: when to free gtm */
+	ap->gtm = gtm;
+	kfree(ap->gtm_object_area); /* free previous then store new one */
+	ap->gtm_object_area = out_obj;
+out:;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_get_timing);
+
+/**
+ * ata_acpi_push_timing - set the channel (controller) timings
+ * @ap: target ata_port (channel)
+ *
+ * For PATA ACPI, this function executes the _STM ACPI method for the
+ * target channel.
+ *
+ * _STM only applies to ATA controllers in PATA (legacy) mode, not to SATA.
+ * In legacy mode, ap->hard_port_no is channel (controller) number.
+ *
+ * _STM requires Identify Drive data, which must already be present in
+ * ata_device->id[] (i.e., it's not fetched here).
+ */
+void ata_acpi_push_timing(struct ata_port *ap)
+{
+	struct device		*dev = ap->dev;
+	int			err;
+	acpi_handle		dev_handle;
+	acpi_integer		pcidevfn;
+	acpi_handle		chan_handle;
+	acpi_status		status;
+	struct acpi_object_list	input;
+	union acpi_object 	in_params[1];
+
+	if (noacpi)
+		goto out;
+
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
+
+	if (!ap->legacy_mode) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: channel/controller not in legacy mode (%s)\n",
+				__FUNCTION__, dev->bus_id);
+		goto out;
+	}
+
+	if (ap->device[0].id[49] || ap->device[1].id[49]) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG "%s: drive(s) on channel %d: missing Identify data\n",
+				__FUNCTION__, ap->hard_port_no);
+		goto out;
+	}
+
+	err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
+	if (err < 0) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: pata_get_dev_handle failed (%d)\n",
+				__FUNCTION__, err);
+		goto out;
+	}
+
+	/* get child objects of dev_handle == channel objects,
+	 * + _their_ children == drive objects */
+	/* channel is ap->hard_port_no */
+	chan_handle = acpi_get_child(dev_handle, ap->hard_port_no);
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: chan adr=%d: handle=0x%p\n",
+			__FUNCTION__, ap->hard_port_no, chan_handle);
+	if (!chan_handle)
+		goto out;
+
+	/* Give the GTM buffer + drive Identify data to the channel via the
+	 * _STM method: */
+	/* setup input parameters buffer for _STM */
+	input.count = 3;
+	input.pointer = in_params;
+	in_params[0].type = ACPI_TYPE_BUFFER;
+	in_params[0].buffer.length = sizeof(struct GTM_buffer);
+	in_params[0].buffer.pointer = (u8 *)ap->gtm;
+	in_params[1].type = ACPI_TYPE_BUFFER;
+	in_params[1].buffer.length = sizeof(ap->device[0].id);
+	in_params[1].buffer.pointer = (u8 *)ap->device[0].id;
+	in_params[2].type = ACPI_TYPE_BUFFER;
+	in_params[2].buffer.length = sizeof(ap->device[1].id);
+	in_params[2].buffer.pointer = (u8 *)ap->device[1].id;
+	/* Output buffer: _STM has no output */
+
+	swap_buf_le16(ap->device[0].id, ATA_ID_WORDS);
+	swap_buf_le16(ap->device[1].id, ATA_ID_WORDS);
+	status = acpi_evaluate_object(chan_handle, "_STM", &input, NULL);
+	swap_buf_le16(ap->device[0].id, ATA_ID_WORDS);
+	swap_buf_le16(ap->device[1].id, ATA_ID_WORDS);
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: _STM status: %d\n",
+			__FUNCTION__, status);
+	if (ACPI_FAILURE(status)) {
+		if (ata_msg_probe(ap))
+			printk(KERN_DEBUG
+				"%s: Run _STM error: status = 0x%x\n",
+				__FUNCTION__, status);
+		goto out;
+	}
+
+out:;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_push_timing);
--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
@@ -1309,11 +1309,11 @@ retry:
 
 	/* print device capabilities */
 	printk(KERN_DEBUG "ata%u: dev %u cfg "
-	       "49:%04x 82:%04x 83:%04x 84:%04x 85:%04x 86:%04x 87:%04x 88:%04x\n",
-	       ap->id, device, dev->id[49],
+	       "00:%04x 49:%04x 82:%04x 83:%04x 84:%04x 85:%04x 86:%04x 87:%04x 88:%04x 93:%04x\n",
+	       ap->id, device, dev->id[0], dev->id[49],
 	       dev->id[82], dev->id[83], dev->id[84],
 	       dev->id[85], dev->id[86], dev->id[87],
-	       dev->id[88]);
+	       dev->id[88], dev->id[93]);
 
 	/*
 	 * common ATA, ATAPI feature tests
@@ -4433,6 +4433,7 @@ static void ata_host_init(struct ata_por
 	ap->port_no = port_no;
 	ap->hard_port_no =
 		ent->legacy_mode ? ent->hard_port_no : port_no;
+	ap->legacy_mode = ent->legacy_mode;
 	ap->pio_mask = ent->pio_mask;
 	ap->mwdma_mask = ent->mwdma_mask;
 	ap->udma_mask = ent->udma_mask;
@@ -4441,6 +4442,7 @@ static void ata_host_init(struct ata_por
 	ap->cbl = ATA_CBL_NONE;
 	ap->active_tag = ATA_TAG_POISON;
 	ap->last_ctl = 0xFF;
+	ap->dev = ent->dev;
 
 	INIT_WORK(&ap->packet_task, atapi_packet_task, ap);
 	INIT_WORK(&ap->pio_task, ata_pio_task, ap);
@@ -4559,6 +4561,7 @@ int ata_device_add(const struct ata_prob
 		printk(KERN_INFO "ata%u: %cATA max %s cmd 0x%lX ctl 0x%lX "
 				 "bmdma 0x%lX irq %lu\n",
 			ap->id,
+			ap->flags & ATA_FLAG_PATA_MODE ? 'P' :
 			ap->flags & ATA_FLAG_SATA ? 'S' : 'P',
 			ata_mode_string(xfer_mode_mask),
 	       		ap->ioaddr.cmd_addr,
@@ -4620,6 +4623,12 @@ int ata_device_add(const struct ata_prob
 		ata_scsi_scan_host(ap);
 	}
 
+	for (i = 0; i < ent->n_ports; i++) {
+		struct ata_port *ap = host_set->ports[i];
+
+		ata_acpi_get_timing(ap);
+	}
+
 	dev_set_drvdata(dev, host_set);
 
 	VPRINTK("EXIT, returning %u\n", ent->n_ports);
@@ -4839,6 +4848,7 @@ static struct ata_probe_ent *ata_pci_ini
 	probe_ent->n_ports = 1;
 	probe_ent->hard_port_no = port_num;
 	probe_ent->private_data = port->private_data;
+	probe_ent->host_flags = port->host_flags;
 
 	switch(port_num)
 	{
@@ -4899,14 +4909,21 @@ int ata_pci_init_one (struct pci_dev *pd
 	else
 		port[1] = port[0];
 
+	printk(KERN_DEBUG "%s: pci_dev class+intf: 0x%x\n",
+		__FUNCTION__, pdev->class);
 	if ((port[0]->host_flags & ATA_FLAG_NO_LEGACY) == 0
 	    && (pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
+		printk(KERN_DEBUG "%s: NO_LEGACY == 0\n", __FUNCTION__);
+		port[0]->host_flags |= ATA_FLAG_PATA_MODE;
+		port[0]->host_flags &= ATA_FLAG_SATA;
 		/* TODO: What if one channel is in native mode ... */
 		pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
 		mask = (1 << 2) | (1 << 0);
 		if ((tmp8 & mask) != mask)
 			legacy_mode = (1 << 3);
 	}
+	else
+		printk(KERN_DEBUG "%s: NO_LEGACY == 1\n", __FUNCTION__);
 
 	/* FIXME... */
 	if ((!legacy_mode) && (n_ports > 2)) {
--- linux-2616-rc4-ata.orig/include/linux/libata.h
+++ linux-2616-rc4-ata/include/linux/libata.h
@@ -157,11 +157,10 @@ enum {
 					     * proper HSM is in place. */
 	ATA_FLAG_DEBUGMSG	= (1 << 10),
 	ATA_FLAG_NO_ATAPI	= (1 << 11), /* No ATAPI support */
-
 	ATA_FLAG_SUSPENDED	= (1 << 12), /* port is suspended */
-
 	ATA_FLAG_PIO_LBA48	= (1 << 13), /* Host DMA engine is LBA28 only */
 	ATA_FLAG_IRQ_MASK	= (1 << 14), /* Mask IRQ in PIO xfers */
+	ATA_FLAG_PATA_MODE	= (1 << 15), /* port in PATA mode */
 
 	ATA_QCFLAG_ACTIVE	= (1 << 1), /* cmd not yet ack'd to scsi lyer */
 	ATA_QCFLAG_SG		= (1 << 3), /* have s/g table? */
@@ -234,6 +233,7 @@ struct scsi_device;
 struct ata_port_operations;
 struct ata_port;
 struct ata_queued_cmd;
+struct GTM_buffer;
 
 /* typedefs */
 typedef int (*ata_qc_cb_t) (struct ata_queued_cmd *qc);
@@ -377,6 +377,7 @@ struct ata_port {
 
 	u8			ctl;	/* cache of ATA control register */
 	u8			last_ctl;	/* Cache last written value */
+	u8			legacy_mode;
 	unsigned int		pio_mask;
 	unsigned int		mwdma_mask;
 	unsigned int		udma_mask;
@@ -397,8 +398,13 @@ struct ata_port {
 	struct work_struct	pio_task;
 	unsigned int		hsm_task_state;
 	unsigned long		pio_task_timeout;
+	struct device		*dev;
 
 	u32			msg_enable;
+#ifdef CONFIG_SCSI_SATA_ACPI
+	struct GTM_buffer	*gtm;
+	void			*gtm_object_area;
+#endif
 
 	void			*private_data;
 };
--- linux-2616-rc4-ata.orig/drivers/scsi/libata.h
+++ linux-2616-rc4-ata/drivers/scsi/libata.h
@@ -63,19 +63,20 @@ extern unsigned int ata_exec_internal(st
 /* libata-acpi.c */
 #ifdef CONFIG_SCSI_SATA_ACPI
 extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
-extern int do_drive_get_GTF(struct ata_port *ap, struct ata_device *atadev,
+extern int do_drive_get_GTF(struct ata_port *ap, int ix,
 			unsigned int *gtf_length, unsigned long *gtf_address,
 			unsigned long *obj_loc);
 extern int do_drive_set_taskfiles(struct ata_port *ap, struct ata_device *atadev,
 			unsigned int gtf_length, unsigned long gtf_address);
 extern int ata_acpi_exec_tfs(struct ata_port *ap);
+extern void ata_acpi_get_timing(struct ata_port *ap);
+extern void ata_acpi_push_timing(struct ata_port *ap);
 #else
 static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 {
 	return 0;
 }
-static inline int do_drive_get_GTF(struct ata_port *ap,
-			struct ata_device *atadev,
+static inline int do_drive_get_GTF(struct ata_port *ap, int ix,
 			unsigned int *gtf_length, unsigned long *gtf_address,
 			unsigned long *obj_loc)
 {
@@ -91,6 +92,12 @@ static inline int ata_acpi_exec_tfs(stru
 {
 	return 0;
 }
+static void ata_acpi_get_timing(struct ata_port *ap)
+{
+}
+static void ata_acpi_push_timing(struct ata_port *ap)
+{
+}
 #endif
 
 

[PATCH 9/13] ATA ACPI: check SATA/PATA more carefully

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Use 'legacy_mode' to check for SATA vs. PATA mode.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-acpi.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -437,7 +437,7 @@ int do_drive_get_GTF(struct ata_port *ap
 
 	/* Don't continue if device has no _ADR method.
 	 * _GTF is intended for known motherboard devices. */
-	if (ata_id_is_ata(atadev->id)) {
+	if (ap->legacy_mode) {
 		err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
 		if (err < 0) {
 			if (ata_msg_probe(ap))
@@ -459,7 +459,7 @@ int do_drive_get_GTF(struct ata_port *ap
 
 	/* Get this drive's _ADR info. if not already known. */
 	if (!atadev->obj_handle) {
-		if (ata_id_is_ata(atadev->id)) {
+		if (ap->legacy_mode) {
 			/* get child objects of dev_handle == channel objects,
 	 		 * + _their_ children == drive objects */
 			/* channel is ap->hard_port_no */
@@ -655,7 +655,7 @@ int do_drive_set_taskfiles(struct ata_po
 
 	if (noacpi)
 		return 0;
-	if (!ata_id_is_sata(atadev->id)) {
+	if (ap->legacy_mode) {
 		printk(KERN_DEBUG "%s: skipping non-SATA drive\n",
 			__FUNCTION__);
 		return 0;

[PATCH 10/13] ATA ACPI: do taskfile before mode commands

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Do drive/taskfile-specific commands before setting the drive mode.
This allows the taskfile to unlock the drive before trying to
set the drive mode.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-core.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
@@ -4297,13 +4297,17 @@ static int ata_start_drive(struct ata_po
  */
 int ata_device_resume(struct ata_port *ap, struct ata_device *dev)
 {
+	printk(KERN_DEBUG "ata%d: resume device\n", ap->id);
+
+	WARN_ON (irqs_disabled());
+
+	if (!ata_dev_present(dev))
+		return 0;
+	ata_acpi_exec_tfs(ap);
 	if (ap->flags & ATA_FLAG_SUSPENDED) {
 		ap->flags &= ~ATA_FLAG_SUSPENDED;
 		ata_set_mode(ap);
 	}
-	if (!ata_dev_present(dev))
-		return 0;
-	ata_acpi_exec_tfs(ap);
 	if (dev->class == ATA_DEV_ATA)
 		ata_start_drive(ap, dev);
 
@@ -4319,6 +4323,7 @@ int ata_device_resume(struct ata_port *a
  */
 int ata_device_suspend(struct ata_port *ap, struct ata_device *dev)
 {
+	printk(KERN_DEBUG "ata%d: suspend device\n", ap->id);
 	if (!ata_dev_present(dev))
 		return 0;
 	if (dev->class == ATA_DEV_ATA)
@@ -5099,6 +5104,7 @@ int pci_test_config_bits(struct pci_dev 
 
 int ata_pci_device_suspend(struct pci_dev *pdev, pm_message_t state)
 {
+	dev_printk(KERN_DEBUG, &pdev->dev, "suspend PCI device\n");
 	pci_save_state(pdev);
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
@@ -5107,6 +5113,7 @@ int ata_pci_device_suspend(struct pci_de
 
 int ata_pci_device_resume(struct pci_dev *pdev)
 {
+	dev_printk(KERN_DEBUG, &pdev->dev, "resume PCI device\n");
 	pci_set_power_state(pdev, PCI_D0);
 	pci_restore_state(pdev);
 	pci_enable_device(pdev);

[PATCH 11/13] ATA ACPI: fix pata host typo

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Fix a typo.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
@@ -4920,7 +4920,7 @@ int ata_pci_init_one (struct pci_dev *pd
 	    && (pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
 		printk(KERN_DEBUG "%s: NO_LEGACY == 0\n", __FUNCTION__);
 		port[0]->host_flags |= ATA_FLAG_PATA_MODE;
-		port[0]->host_flags &= ATA_FLAG_SATA;
+		port[0]->host_flags &= ~ATA_FLAG_SATA;
 		/* TODO: What if one channel is in native mode ... */
 		pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
 		mask = (1 << 2) | (1 << 0);

[PATCH 12/13] ATA ACPI: use scsi_bus_shutdown for SATA/PATA

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Add ability for SCSI drivers to invoke a shutdown method.
This allows drivers to make drives safe for shutdown/poweroff,
for example.  Some drives need this to prevent possible problems.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/ahci.c        |    1 +
 drivers/scsi/ata_piix.c    |    1 +
 drivers/scsi/libata-core.c |   23 +++++++++++++++++++++++
 drivers/scsi/libata-scsi.c |    8 ++++++++
 drivers/scsi/scsi_sysfs.c  |   16 ++++++++++++++++
 include/linux/libata.h     |    2 ++
 include/scsi/scsi_host.h   |    1 +
 7 files changed, 52 insertions(+)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-scsi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-scsi.c
@@ -412,6 +412,14 @@ int ata_scsi_device_suspend(struct scsi_
 	return ata_device_suspend(ap, dev);
 }
 
+int ata_scsi_device_shutdown(struct scsi_device *sdev)
+{
+	struct ata_port *ap = (struct ata_port *) &sdev->host->hostdata[0];
+	struct ata_device *dev = &ap->device[sdev->id];
+
+	return ata_device_shutdown(ap, dev);
+}
+
 /**
  *	ata_to_sense_error - convert ATA error to SCSI error
  *	@id: ATA device number
--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
@@ -4334,6 +4334,27 @@ int ata_device_suspend(struct ata_port *
 	return 0;
 }
 
+/**
+ *	ata_device_shutdown - send Standby Immediate command to drive
+ *	@ap: target ata_port
+ *	@dev: target device on the ata_port
+ *
+ *	This command makes it safe to power-off a drive.
+ *	Otherwise the heads may be flying at the wrong place
+ *	when the power is removed.
+ */
+int ata_device_shutdown(struct ata_port *ap, struct ata_device *dev)
+{
+
+	if (!ata_dev_present(dev))
+		return 0;
+
+	ata_standby_drive(ap, dev);
+	ap->flags |= ATA_FLAG_SUSPENDED;
+
+	return 0;
+}
+
 int ata_port_start (struct ata_port *ap)
 {
 	struct device *dev = ap->host_set->dev;
@@ -5230,5 +5251,7 @@ EXPORT_SYMBOL_GPL(ata_pci_device_resume)
 
 EXPORT_SYMBOL_GPL(ata_device_suspend);
 EXPORT_SYMBOL_GPL(ata_device_resume);
+EXPORT_SYMBOL_GPL(ata_device_shutdown);
 EXPORT_SYMBOL_GPL(ata_scsi_device_suspend);
 EXPORT_SYMBOL_GPL(ata_scsi_device_resume);
+EXPORT_SYMBOL_GPL(ata_scsi_device_shutdown);
--- linux-2616-rc4-ata.orig/include/linux/libata.h
+++ linux-2616-rc4-ata/include/linux/libata.h
@@ -501,8 +501,10 @@ extern int ata_scsi_release(struct Scsi_
 extern unsigned int ata_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc);
 extern int ata_scsi_device_resume(struct scsi_device *);
 extern int ata_scsi_device_suspend(struct scsi_device *);
+extern int ata_scsi_device_shutdown(struct scsi_device *);
 extern int ata_device_resume(struct ata_port *, struct ata_device *);
 extern int ata_device_suspend(struct ata_port *, struct ata_device *);
+extern int ata_device_shutdown(struct ata_port *, struct ata_device *);
 extern int ata_ratelimit(void);
 
 /*
--- linux-2616-rc4-ata.orig/drivers/scsi/scsi_sysfs.c
+++ linux-2616-rc4-ata/drivers/scsi/scsi_sysfs.c
@@ -302,11 +302,27 @@ static int scsi_bus_resume(struct device
 	return err;
 }
 
+static void scsi_bus_shutdown(struct device * dev)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	struct scsi_host_template *sht = sdev->host->hostt;
+	int err;
+
+	err = scsi_device_quiesce(sdev);
+	if (err)
+		printk(KERN_DEBUG "%s: error (0x%x) during shutdown\n",
+			__FUNCTION__, err);
+
+	if (sht->shutdown)
+		sht->shutdown(sdev);
+}
+
 struct bus_type scsi_bus_type = {
         .name		= "scsi",
         .match		= scsi_bus_match,
 	.suspend	= scsi_bus_suspend,
 	.resume		= scsi_bus_resume,
+	.shutdown	= scsi_bus_shutdown,
 };
 
 int scsi_sysfs_register(void)
--- linux-2616-rc4-ata.orig/include/scsi/scsi_host.h
+++ linux-2616-rc4-ata/include/scsi/scsi_host.h
@@ -301,6 +301,7 @@ struct scsi_host_template {
 	 */
 	int (*resume)(struct scsi_device *);
 	int (*suspend)(struct scsi_device *);
+	int (*shutdown)(struct scsi_device *);
 
 	/*
 	 * Name of proc directory
--- linux-2616-rc4-ata.orig/drivers/scsi/ahci.c
+++ linux-2616-rc4-ata/drivers/scsi/ahci.c
@@ -214,6 +214,7 @@ static struct scsi_host_template ahci_sh
 	.dma_boundary		= AHCI_DMA_BOUNDARY,
 	.slave_configure	= ata_scsi_slave_config,
 	.bios_param		= ata_std_bios_param,
+	.shutdown		= ata_scsi_device_shutdown,
 };
 
 static const struct ata_port_operations ahci_ops = {
--- linux-2616-rc4-ata.orig/drivers/scsi/ata_piix.c
+++ linux-2616-rc4-ata/drivers/scsi/ata_piix.c
@@ -192,6 +192,7 @@ static struct scsi_host_template piix_sh
 	.bios_param		= ata_std_bios_param,
 	.resume			= ata_scsi_device_resume,
 	.suspend		= ata_scsi_device_suspend,
+	.shutdown		= ata_scsi_device_shutdown,
 };
 
 static const struct ata_port_operations piix_pata_ops = {

[PATCH 13/13] ATA ACPI: enable writing PATA taskfiles

[Randy Dunlap]

From: Randy Dunlap <randy_d_dunlap@linux.intel.com>

Move 'noacpi' option handling to top of functions.
Enable writing taskfiles for PATA drives.

Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
---
 drivers/scsi/libata-acpi.c |   16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
@@ -303,13 +303,14 @@ int ata_acpi_push_id(struct ata_port *ap
 	struct acpi_object_list		input;
 	union acpi_object 		in_params[1];
 
+	if (noacpi)
+		return 0;
+
 	if (ap->legacy_mode) {
 		printk(KERN_DEBUG "%s: skipping for PATA mode\n",
 			__FUNCTION__);
 		return 0;
 	}
-	if (noacpi)
-		return 0;
 
 	if (ata_msg_probe(ap))
 		printk(KERN_DEBUG
@@ -655,11 +656,6 @@ int do_drive_set_taskfiles(struct ata_po
 
 	if (noacpi)
 		return 0;
-	if (ap->legacy_mode) {
-		printk(KERN_DEBUG "%s: skipping non-SATA drive\n",
-			__FUNCTION__);
-		return 0;
-	}
 
 	if (!ata_dev_present(atadev) ||
 	    (ap->flags & ATA_FLAG_PORT_DISABLED))
@@ -707,12 +703,12 @@ int ata_acpi_exec_tfs(struct ata_port *a
 	unsigned long	gtf_address;
 	unsigned long	obj_loc;
 
-	if (ata_msg_probe(ap))
-		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
-
 	if (noacpi)
 		return 0;
 
+	if (ata_msg_probe(ap))
+		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
+
 	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
 		if (ata_msg_probe(ap))
 			printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",

Re: [PATCH 9/13] ATA ACPI: check SATA/PATA more carefully

[Alan Cox]

On Mer, 2006-02-22 at 14:00 -0800, Randy Dunlap wrote:
> +	if (ap->legacy_mode) {
>  		err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
>  		if (err < 0) {
>  			if (ata_msg_probe(ap))

ap->legacy mode tells you if one of a subset of PCI controllers is in
native or legacy compatibility mode. It tells you nothing about whether
the controller is SATA or PATA. In fact it may even be both at once as
there may be a SATA bridge on one channel in some configurations. The
field is also meaningless for PCI controllers that are not class IDE.

The cable type field will tell you in some situations if we are PATA or
SATA but we actually don't always know at the moment. We probably have
the info for 99.9% of cases if needed it just isnt ap->legacy_mode, its
cable type plus the PATA/SATA bridge knobbling check.

Alan

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Pavel Machek]

> --- linux-2616-rc4-ata.orig/include/linux/libata.h
> +++ linux-2616-rc4-ata/include/linux/libata.h
> @@ -36,7 +36,8 @@
>  #include <acpi/acpi.h>
>  
>  /*
> - * compile-time options
> + * compile-time options: to be removed as soon as all the drivers are
> + * converted to the new debugging mechanism
>   */
>  #undef ATA_DEBUG		/* debugging output */
>  #undef ATA_VERBOSE_DEBUG	/* yet more debugging output */
> @@ -72,6 +73,38 @@
>          }
>  #endif
>  
> +/* NEW: debug levels */
> +#define HAVE_LIBATA_MSG 1

What is new with them?

> +enum {
> +	ATA_MSG_DRV	= 0x0001,
> +	ATA_MSG_INFO	= 0x0002,
> +	ATA_MSG_PROBE	= 0x0004,
> +	ATA_MSG_WARN	= 0x0008,
> +	ATA_MSG_MALLOC	= 0x0010,
> +	ATA_MSG_CTL	= 0x0020,
> +	ATA_MSG_INTR	= 0x0040,
> +	ATA_MSG_ERR	= 0x0080,
> +};
> +
> +#define ata_msg_drv(p)    ((p)->msg_enable & ATA_MSG_DRV)
> +#define ata_msg_info(p)   ((p)->msg_enable & ATA_MSG_INFO)
> +#define ata_msg_probe(p)  ((p)->msg_enable & ATA_MSG_PROBE)
> +#define ata_msg_warn(p)   ((p)->msg_enable & ATA_MSG_WARN)
> +#define ata_msg_malloc(p) ((p)->msg_enable & ATA_MSG_MALLOC)
> +#define ata_msg_ctl(p)    ((p)->msg_enable & ATA_MSG_CTL)
> +#define ata_msg_intr(p)   ((p)->msg_enable & ATA_MSG_INTR)
> +#define ata_msg_err(p)    ((p)->msg_enable & ATA_MSG_ERR)

I hate to see debugging infrastructure like this. We already have it
in ACPI and it is nasty/useless. It hides serious errors during normal
run, while if you turn on the debugging, it floods logs so that
it is unusable, too. I end up having to replace dprintks with
printks... nasty.
								Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 4/13] ATA ACPI: add params/docs.

[Pavel Machek]

Hi!

> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Add and use 'noacpi' parameter for libata-acpi.

Why is this option needed? Either the code works, or it does not. If
it does not, it is not suitable for merging...
								Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 5/13] ATA ACPI: use debugging macros

[Pavel Machek]

On St 22-02-06 13:55:42, Randy Dunlap wrote:
> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Add more libata-acpi debugging, plus controlled by libata.printk
> value.

Please don't. Instead select messages so that it is not too noisy by
default...
								Pavel

-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 7/13] ATA ACPI: more Makefile/Kconfig

[Pavel Machek]

On St 22-02-06 13:58:02, Randy Dunlap wrote:
> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Simplify Makefile.
> Add Kconfig help.

Could you fold this with patch 1 of series? Introducing too complex
Makefile then fixing it makes review quite "interetsing". 

Is the config option really neccessary?
								Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 8/13] ATA ACPI: PATA methods

[Pavel Machek]

Hi!

> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Add PATA support to the previous SATA support.

Does it make your CONFIG_SATA_ACPI option missnamed?

> --- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
> +++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
> @@ -35,6 +35,23 @@ struct taskfile_array {
>  	u8	tfa[REGS_PER_GTF];	/* regs. 0x1f1 - 0x1f7 */
>  };
>  
> +struct GTM_buffer {
> +	__u32	PIO_speed0;
> +	__u32	DMA_speed0;
> +	__u32	PIO_speed1;
> +	__u32	DMA_speed1;
> +	__u32	GTM_flags;
> +};

No reason to use __u32 here, u32 should be okay.

> +static int pata_get_dev_handle(struct device *dev, acpi_handle *handle,
> +					acpi_integer *pcidevfn)
> +{
> +	unsigned int domain, bus, devnum, func;
> +	acpi_integer addr;
> +	acpi_handle dev_handle, parent_handle;
> +	int scanned;
> +	struct acpi_buffer buffer = {.length = ACPI_ALLOCATE_BUFFER,
> +					.pointer = NULL};
> +	acpi_status status;
> +	struct acpi_device_info	*dinfo = NULL;
> +	int ret = -ENODEV;
> +
> +	printk(KERN_DEBUG "%s: ENTER: dev->bus_id='%s'\n",
> +		__FUNCTION__, dev->bus_id);

Have you catched some nasty disease from acpi people or what? Having
"printk(enter)" at beggining of each function may be nice for your
development, but should be removed prior to merge.

> +	if ((scanned = sscanf(dev->bus_id, "%x:%x:%x.%x",
> +			&domain, &bus, &devnum, &func)) != 4) {
> +		printk(KERN_DEBUG "%s: sscanf ret. %d\n",
> +			__FUNCTION__, scanned);
> +		goto err;
> +	}

Is there no other way than to parse back name?

> +	if (ata_msg_probe(ap))
> +		printk(KERN_DEBUG
> +			"%s: ENTER: ap->id: %d, port#: %d, hard_port#: %d\n",
> +			__FUNCTION__, ap->id,
> +		ap->port_no, ap->hard_port_no);
> +

Again, please clean your printks.
> @@ -557,11 +701,11 @@ EXPORT_SYMBOL_GPL(do_drive_set_taskfiles
>   */
>  int ata_acpi_exec_tfs(struct ata_port *ap)
>  {
> -	int ix;
> -	int ret;
> -	unsigned int gtf_length;
> -	unsigned long gtf_address;
> -	unsigned long obj_loc;
> +	int		ix;
> +	int		ret;
> +	unsigned int	gtf_length;
> +	unsigned long	gtf_address;
> +	unsigned long	obj_loc;
>  
>  	if (ata_msg_probe(ap))
>  		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);

Again!

> +void ata_acpi_get_timing(struct ata_port *ap)
> +{
> +	struct device		*dev = ap->dev;
> +	int			err;
> +	acpi_handle		dev_handle;
> +	acpi_integer		pcidevfn;
> +	acpi_handle		chan_handle;
> +	acpi_status		status;
> +	struct acpi_buffer	output;
> +	union acpi_object 	*out_obj;
> +	struct GTM_buffer	*gtm;
> +
> +	if (noacpi)
> +		goto out;
> +
> +	if (ata_msg_probe(ap))
> +		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);

Again!

> +void ata_acpi_push_timing(struct ata_port *ap)
> +{
> +	struct device		*dev = ap->dev;
> +	int			err;
> +	acpi_handle		dev_handle;
> +	acpi_integer		pcidevfn;
> +	acpi_handle		chan_handle;
> +	acpi_status		status;
> +	struct acpi_object_list	input;
> +	union acpi_object 	in_params[1];
> +
> +	if (noacpi)
> +		goto out;
> +
> +	if (ata_msg_probe(ap))
> +		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);

And again!
							Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 10/13] ATA ACPI: do taskfile before mode commands

[Pavel Machek]

On St 22-02-06 14:01:40, Randy Dunlap wrote:
> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Do drive/taskfile-specific commands before setting the drive mode.
> This allows the taskfile to unlock the drive before trying to
> set the drive mode.
> 
> Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> ---
>  drivers/scsi/libata-core.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> --- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
> +++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
> @@ -4297,13 +4297,17 @@ static int ata_start_drive(struct ata_po
>   */
>  int ata_device_resume(struct ata_port *ap, struct ata_device *dev)
>  {
> +	printk(KERN_DEBUG "ata%d: resume device\n", ap->id);

Yep, one more helpful printk. Not. Actually this is four more of them
in this patch alone. Please remove your debugging code prior to merge.

								Pavel

-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 4/13] ATA ACPI: add params/docs.

[Jeff Garzik]

Pavel Machek wrote:
> Hi!
> 
> 
>>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
>>
>>Add and use 'noacpi' parameter for libata-acpi.
> 
> 
> Why is this option needed? Either the code works, or it does not. If
> it does not, it is not suitable for merging...

Invalid logic.  It is needed for the same reasons that acpi=off is 
supported in the generic ACPI code.

	Jeff

Re: [PATCH 5/13] ATA ACPI: use debugging macros

[Jeff Garzik]

Pavel Machek wrote:
> On St 22-02-06 13:55:42, Randy Dunlap wrote:
> 
>>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
>>
>>Add more libata-acpi debugging, plus controlled by libata.printk
>>value.
> 
> 
> Please don't. Instead select messages so that it is not too noisy by
> default...

Wrong.  We want fine-grained message selection, just like modern net 
drivers have.  See netif_msg_xxx and friends.

	Jeff

Re: [PATCH 12/13] ATA ACPI: use scsi_bus_shutdown for SATA/PATA

[Pavel Machek]

Hi!

> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Add ability for SCSI drivers to invoke a shutdown method.
> This allows drivers to make drives safe for shutdown/poweroff,
> for example.  Some drives need this to prevent possible problems.
> 
> Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>

> --- linux-2616-rc4-ata.orig/drivers/scsi/scsi_sysfs.c
> +++ linux-2616-rc4-ata/drivers/scsi/scsi_sysfs.c
> @@ -302,11 +302,27 @@ static int scsi_bus_resume(struct device
>  	return err;
>  }
>  
> +static void scsi_bus_shutdown(struct device * dev)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +	struct scsi_host_template *sht = sdev->host->hostt;
> +	int err;
> +
> +	err = scsi_device_quiesce(sdev);

int err = scsi_device_quiesce()?

> +	if (err)
> +		printk(KERN_DEBUG "%s: error (0x%x) during shutdown\n",
> +			__FUNCTION__, err);

If you get an error, and then ignore it... that should not be DEBUG
message, right?

> +	if (sht->shutdown)
> +		sht->shutdown(sdev);
> +}
> +
>  struct bus_type scsi_bus_type = {
>          .name		= "scsi",
>          .match		= scsi_bus_match,
>  	.suspend	= scsi_bus_suspend,
>  	.resume		= scsi_bus_resume,
> +	.shutdown	= scsi_bus_shutdown,
>  };

Whitespace?
								Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 13/13] ATA ACPI: enable writing PATA taskfiles

[Pavel Machek]

On St 22-02-06 14:07:14, Randy Dunlap wrote:
> From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> Move 'noacpi' option handling to top of functions.
> Enable writing taskfiles for PATA drives.

Changes order of return and debug message. It does not seem too useful
to me.
								Pavel

> Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> ---
>  drivers/scsi/libata-acpi.c |   16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> --- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
> +++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
> @@ -303,13 +303,14 @@ int ata_acpi_push_id(struct ata_port *ap
>  	struct acpi_object_list		input;
>  	union acpi_object 		in_params[1];
>  
> +	if (noacpi)
> +		return 0;
> +
>  	if (ap->legacy_mode) {
>  		printk(KERN_DEBUG "%s: skipping for PATA mode\n",
>  			__FUNCTION__);
>  		return 0;
>  	}
> -	if (noacpi)
> -		return 0;
>  
>  	if (ata_msg_probe(ap))
>  		printk(KERN_DEBUG
...
> @@ -707,12 +703,12 @@ int ata_acpi_exec_tfs(struct ata_port *a
>  	unsigned long	gtf_address;
>  	unsigned long	obj_loc;
>  
> -	if (ata_msg_probe(ap))
> -		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
> -
>  	if (noacpi)
>  		return 0;
>  
> +	if (ata_msg_probe(ap))
> +		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
> +
>  	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
>  		if (ata_msg_probe(ap))
>  			printk(KERN_DEBUG "%s: call get_GTF, ix=%d\n",


-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Jeff Garzik]

Pavel Machek wrote:
> I hate to see debugging infrastructure like this. We already have it
> in ACPI and it is nasty/useless. It hides serious errors during normal
> run, while if you turn on the debugging, it floods logs so that
> it is unusable, too. I end up having to replace dprintks with
> printks... nasty.

Then you clearly don't understand what the code is doing.  Fine-grained 
message selection allows one to turn on only the messages needed, and 
only for the controller desired.  Otherwise, it is nearly impossible to 
debug one SATA controller while booting off another.

	Jeff

Re: [PATCH 8/13] ATA ACPI: PATA methods

[Jeff Garzik]

Pavel Machek wrote:
> Hi!
> 
> 
>>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
>>
>>Add PATA support to the previous SATA support.
> 
> 
> Does it make your CONFIG_SATA_ACPI option missnamed?

Yes.


>>--- linux-2616-rc4-ata.orig/drivers/scsi/libata-acpi.c
>>+++ linux-2616-rc4-ata/drivers/scsi/libata-acpi.c
>>@@ -35,6 +35,23 @@ struct taskfile_array {
>> 	u8	tfa[REGS_PER_GTF];	/* regs. 0x1f1 - 0x1f7 */
>> };
>> 
>>+struct GTM_buffer {
>>+	__u32	PIO_speed0;
>>+	__u32	DMA_speed0;
>>+	__u32	PIO_speed1;
>>+	__u32	DMA_speed1;
>>+	__u32	GTM_flags;
>>+};
> 
> 
> No reason to use __u32 here, u32 should be okay.

Agreed.


>>+static int pata_get_dev_handle(struct device *dev, acpi_handle *handle,
>>+					acpi_integer *pcidevfn)
>>+{
>>+	unsigned int domain, bus, devnum, func;
>>+	acpi_integer addr;
>>+	acpi_handle dev_handle, parent_handle;
>>+	int scanned;
>>+	struct acpi_buffer buffer = {.length = ACPI_ALLOCATE_BUFFER,
>>+					.pointer = NULL};
>>+	acpi_status status;
>>+	struct acpi_device_info	*dinfo = NULL;
>>+	int ret = -ENODEV;
>>+
>>+	printk(KERN_DEBUG "%s: ENTER: dev->bus_id='%s'\n",
>>+		__FUNCTION__, dev->bus_id);
> 
> 
> Have you catched some nasty disease from acpi people or what? Having
> "printk(enter)" at beggining of each function may be nice for your
> development, but should be removed prior to merge.

Mostly agreed:  the above should be changed to

	if (ata_msg_xxx())
		...

prior to merging.


>>+	if ((scanned = sscanf(dev->bus_id, "%x:%x:%x.%x",
>>+			&domain, &bus, &devnum, &func)) != 4) {
>>+		printk(KERN_DEBUG "%s: sscanf ret. %d\n",
>>+			__FUNCTION__, scanned);
>>+		goto err;
>>+	}
> 
> 
> Is there no other way than to parse back name?

Agreed.  Should obtain the struct pci_dev pointer, and access the 
elements, rather than parsing a string.


>>+	if (ata_msg_probe(ap))
>>+		printk(KERN_DEBUG
>>+			"%s: ENTER: ap->id: %d, port#: %d, hard_port#: %d\n",
>>+			__FUNCTION__, ap->id,
>>+		ap->port_no, ap->hard_port_no);
>>+
> 
> 
> Again, please clean your printks.

NAK, the above is better form.


>>@@ -557,11 +701,11 @@ EXPORT_SYMBOL_GPL(do_drive_set_taskfiles
>>  */
>> int ata_acpi_exec_tfs(struct ata_port *ap)
>> {
>>-	int ix;
>>-	int ret;
>>-	unsigned int gtf_length;
>>-	unsigned long gtf_address;
>>-	unsigned long obj_loc;
>>+	int		ix;
>>+	int		ret;
>>+	unsigned int	gtf_length;
>>+	unsigned long	gtf_address;
>>+	unsigned long	obj_loc;
>> 
>> 	if (ata_msg_probe(ap))
>> 		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
> 
> 
> Again!

Ditto.


>>+void ata_acpi_get_timing(struct ata_port *ap)
>>+{
>>+	struct device		*dev = ap->dev;
>>+	int			err;
>>+	acpi_handle		dev_handle;
>>+	acpi_integer		pcidevfn;
>>+	acpi_handle		chan_handle;
>>+	acpi_status		status;
>>+	struct acpi_buffer	output;
>>+	union acpi_object 	*out_obj;
>>+	struct GTM_buffer	*gtm;
>>+
>>+	if (noacpi)
>>+		goto out;
>>+
>>+	if (ata_msg_probe(ap))
>>+		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
> 
> 
> Again!

Ditto.


>>+void ata_acpi_push_timing(struct ata_port *ap)
>>+{
>>+	struct device		*dev = ap->dev;
>>+	int			err;
>>+	acpi_handle		dev_handle;
>>+	acpi_integer		pcidevfn;
>>+	acpi_handle		chan_handle;
>>+	acpi_status		status;
>>+	struct acpi_object_list	input;
>>+	union acpi_object 	in_params[1];
>>+
>>+	if (noacpi)
>>+		goto out;
>>+
>>+	if (ata_msg_probe(ap))
>>+		printk(KERN_DEBUG "%s: ENTER:\n", __FUNCTION__);
> 
> 
> And again!

Ditto.  All ata_msg_xxx are OK.

	Jeff

Re: [PATCH 7/13] ATA ACPI: more Makefile/Kconfig

[Jeff Garzik]

Pavel Machek wrote:
> On St 22-02-06 13:58:02, Randy Dunlap wrote:
> 
>>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
>>
>>Simplify Makefile.
>>Add Kconfig help.
> 
> 
> Could you fold this with patch 1 of series? Introducing too complex
> Makefile then fixing it makes review quite "interetsing". 

Agreed.  Patches should be folded together...

	Jeff

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Pavel Machek]

On Út 28-02-06 07:00:14, Jeff Garzik wrote:
> Pavel Machek wrote:
> >I hate to see debugging infrastructure like this. We already have it
> >in ACPI and it is nasty/useless. It hides serious errors during normal
> >run, while if you turn on the debugging, it floods logs so that
> >it is unusable, too. I end up having to replace dprintks with
> >printks... nasty.
> 
> Then you clearly don't understand what the code is doing.
> Fine-grained 

No, I do not... code is so full of printk()s that it is unreadable.

> message selection allows one to turn on only the messages needed, and 
> only for the controller desired.  Otherwise, it is nearly impossible to 
> debug one SATA controller while booting off another.

Now, maybe message selection is neccessary, but having printk at
begining of each function is not way to go.
								Pavel

-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 10/13] ATA ACPI: do taskfile before mode commands

[Jeff Garzik]

Pavel Machek wrote:
> On St 22-02-06 14:01:40, Randy Dunlap wrote:
> 
>>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
>>
>>Do drive/taskfile-specific commands before setting the drive mode.
>>This allows the taskfile to unlock the drive before trying to
>>set the drive mode.
>>
>>Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
>>---
>> drivers/scsi/libata-core.c |   13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>>--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
>>+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
>>@@ -4297,13 +4297,17 @@ static int ata_start_drive(struct ata_po
>>  */
>> int ata_device_resume(struct ata_port *ap, struct ata_device *dev)
>> {
>>+	printk(KERN_DEBUG "ata%d: resume device\n", ap->id);
> 
> 
> Yep, one more helpful printk. Not. Actually this is four more of them
> in this patch alone. Please remove your debugging code prior to merge.

Agreed, with the modification s/remove/limit by ata_msg_xxx/

	Jeff

Re: [PATCH 10/13] ATA ACPI: do taskfile before mode commands

[Pavel Machek]

On Út 28-02-06 07:05:17, Jeff Garzik wrote:
> Pavel Machek wrote:
> >On St 22-02-06 14:01:40, Randy Dunlap wrote:
> >
> >>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> >>
> >>Do drive/taskfile-specific commands before setting the drive mode.
> >>This allows the taskfile to unlock the drive before trying to
> >>set the drive mode.
> >>
> >>Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> >>---
> >>drivers/scsi/libata-core.c |   13 ++++++++++---
> >>1 file changed, 10 insertions(+), 3 deletions(-)
> >>
> >>--- linux-2616-rc4-ata.orig/drivers/scsi/libata-core.c
> >>+++ linux-2616-rc4-ata/drivers/scsi/libata-core.c
> >>@@ -4297,13 +4297,17 @@ static int ata_start_drive(struct ata_po
> >> */
> >>int ata_device_resume(struct ata_port *ap, struct ata_device *dev)
> >>{
> >>+	printk(KERN_DEBUG "ata%d: resume device\n", ap->id);
> >
> >
> >Yep, one more helpful printk. Not. Actually this is four more of them
> >in this patch alone. Please remove your debugging code prior to merge.
> 
> Agreed, with the modification s/remove/limit by ata_msg_xxx/

Please, just remove them. Driver model core already has debugging that
can be enabled and prints what is called.

I do not think we want printk() at begining of every *_resume
function.
								Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Jeff Garzik]

Pavel Machek wrote:
> Now, maybe message selection is neccessary, but having printk at
> begining of each function is not way to go.

Clearly you have not read much libata code at all...

	Jeff

Re: [PATCH 10/13] ATA ACPI: do taskfile before mode commands

[Jeff Garzik]

Pavel Machek wrote:
> Please, just remove them. Driver model core already has debugging that
> can be enabled and prints what is called.
> 
> I do not think we want printk() at begining of every *_resume
> function.

They will not be removed, but instead limited by a bit test as I stated.

	Jeff

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Andrew Morton]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> Fine-grained 
>  message selection allows one to turn on only the messages needed, and 
>  only for the controller desired.

Except

- There's (presently) no way of making all the messages go away for a
  non-debug build.

- The code is structured as

	if (ata_msg_foo(p))
		printk("something");

  So if we later do

	#define ata_msg_foo(p)	0

  We'll still get copies of "something" in the kernel image (may be fixed
  in later gcc, dunno).

- The new debug stuff isn't documented.  One has funble around in the
  source to work out how to even turn it on.  Can it be altered at runtime?
  Dunno - the changelogs are risible.  What effect do the various flags
  have?

  Having spent (and re-spent) time grovelling through the ALSA source
  working out how to enable their debug stuff during a maintainer snooze
  I'd prefer we didn't have to do that with libata as well.

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Jeff Garzik]

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>Fine-grained 
>> message selection allows one to turn on only the messages needed, and 
>> only for the controller desired.
> 
> 
> Except
> 
> - There's (presently) no way of making all the messages go away for a
>   non-debug build.

They aren't supposed to go away.


> - The code is structured as
> 
> 	if (ata_msg_foo(p))
> 		printk("something");
> 
>   So if we later do
> 
> 	#define ata_msg_foo(p)	0
> 
>   We'll still get copies of "something" in the kernel image (may be fixed
>   in later gcc, dunno).

We don't do that in net driver land, and I don't wish to do it for 
libata either.  Its just a bit test, that jumps over code if the message 
class isn't enabled (see link below).

We want users to be able to enable specific messages for specific 
controllers, without recompiling their kernel.

grep for msg_enable in various net drivers.  ethtool(8) is used to 
select specific controllers and messages to print.


> - The new debug stuff isn't documented.  One has funble around in the
>   source to work out how to even turn it on.  Can it be altered at runtime?
>   Dunno - the changelogs are risible.  What effect do the various flags
>   have?

The model has always been documented:
http://www.scyld.com/pipermail/vortex/2001-November/001426.html
(scroll down a tad)

	Jeff

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Mark Lord]

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
>> Fine-grained 
>>  message selection allows one to turn on only the messages needed, and 
>>  only for the controller desired.
> 
> Except
> 
> - There's (presently) no way of making all the messages go away for a
>   non-debug build.

Agreed.  We need a way to make them all really go away
for embedded builds -- memory matters there.

Cheers

Re: [PATCH 7/13] ATA ACPI: more Makefile/Kconfig

[Randy.Dunlap]

On Tue, 28 Feb 2006, Jeff Garzik wrote:

> Pavel Machek wrote:
> > On St 22-02-06 13:58:02, Randy Dunlap wrote:
> >
> >>From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> >>
> >>Simplify Makefile.
> >>Add Kconfig help.
> >
> >
> > Could you fold this with patch 1 of series? Introducing too complex
> > Makefile then fixing it makes review quite "interetsing".
>
> Agreed.  Patches should be folded together...

Agreed.  and I'll rename the config option as well....

-- 
~Randy

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Phillip Susi]

Andrew Morton wrote:
> Except
> 
> - There's (presently) no way of making all the messages go away for a
>   non-debug build.
> 

I agree, there should be a config option to build the kernel with the 
debug support entirely shut off, though it's a good idea to leave it on 
if you aren't really cramped for space.

> - The code is structured as
> 
> 	if (ata_msg_foo(p))
> 		printk("something");
> 
>   So if we later do
> 
> 	#define ata_msg_foo(p)	0
> 
>   We'll still get copies of "something" in the kernel image (may be fixed
>   in later gcc, dunno).
> 
> - The new debug stuff isn't documented.  One has funble around in the
>   source to work out how to even turn it on.  Can it be altered at runtime?
>   Dunno - the changelogs are risible.  What effect do the various flags
>   have?
> 
>   Having spent (and re-spent) time grovelling through the ALSA source
>   working out how to enable their debug stuff during a maintainer snooze
>   I'd prefer we didn't have to do that with libata as well.
> 

Would you prefer there not be any debug messages at all, rather than 
ones you have to figure out how to turn on and interpret?  Documentation 
is always a good thing, but if you are at least somewhat familiar with 
the code, turning on the debug messages should be easy and rather helpful.

BTW, didn't I see something recently in the kernel about a debug fs? 
Sounded like that was intended for this sort of thing to provide a 
standard interface to configuring fine grained debug message filtering.

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Andrew Morton]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
> Andrew Morton wrote:
> > Jeff Garzik <jgarzik@pobox.com> wrote:
> > 
> >>Fine-grained 
> >> message selection allows one to turn on only the messages needed, and 
> >> only for the controller desired.
> > 
> > 
> > Except
> > 
> > - There's (presently) no way of making all the messages go away for a
> >   non-debug build.
> 
> They aren't supposed to go away.
> 

It is legitimate to elect to waste memory on every machine so as to make
the system more easily debugged by remote maintainers.  But that's an
unusual choice in the kernel context.

They can still get you by setting CONFIG_PRINTK=n ;)

> > - The code is structured as
> > 
> > 	if (ata_msg_foo(p))
> > 		printk("something");
> > 
> >   So if we later do
> > 
> > 	#define ata_msg_foo(p)	0
> > 
> >   We'll still get copies of "something" in the kernel image (may be fixed
> >   in later gcc, dunno).
> 
> We don't do that in net driver land, and I don't wish to do it for 
> libata either.  Its just a bit test, that jumps over code if the message 
> class isn't enabled (see link below).
> 
> We want users to be able to enable specific messages for specific 
> controllers, without recompiling their kernel.
> 
> grep for msg_enable in various net drivers.  ethtool(8) is used to 
> select specific controllers and messages to print.
> 

umm, that's unrelated to my point, but whatever.

> 
> > - The new debug stuff isn't documented.  One has funble around in the
> >   source to work out how to even turn it on.  Can it be altered at runtime?
> >   Dunno - the changelogs are risible.  What effect do the various flags
> >   have?
> 
> The model has always been documented:
> http://www.scyld.com/pipermail/vortex/2001-November/001426.html
> (scroll down a tad)

That's useless.

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Randy Dunlap]

On Tue, 28 Feb 2006 09:43:54 -0500
Mark Lord <liml@rtr.ca> wrote:

> Andrew Morton wrote:
> > Jeff Garzik <jgarzik@pobox.com> wrote:
> >> Fine-grained 
> >>  message selection allows one to turn on only the messages needed, and 
> >>  only for the controller desired.
> > 
> > Except
> > 
> > - There's (presently) no way of making all the messages go away for a
> >   non-debug build.
> 
> Agreed.  We need a way to make them all really go away
> for embedded builds -- memory matters there.

That's a libata infrastructure issue, not an ATA-ACPI issue.
I'll go with whatever $maintainer decides.

~Randy

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Jeff Garzik]

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>>Andrew Morton wrote:
>>
>>>Jeff Garzik <jgarzik@pobox.com> wrote:
>>>
>>>
>>>>Fine-grained 
>>>>message selection allows one to turn on only the messages needed, and 
>>>>only for the controller desired.
>>>
>>>
>>>Except
>>>
>>>- There's (presently) no way of making all the messages go away for a
>>>  non-debug build.
>>
>>They aren't supposed to go away.
>>
> 
> 
> It is legitimate to elect to waste memory on every machine so as to make
> the system more easily debugged by remote maintainers.  But that's an
> unusual choice in the kernel context.

Not unusual, as I said, its done in a ton of net drivers.

That said, I suppose its OK to do

	#define ata_msg_foo() 0

for wacky embedded situations.  But the default will be enabled for all 
users, not just debug kernels.


>>>- The new debug stuff isn't documented.  One has funble around in the
>>>  source to work out how to even turn it on.  Can it be altered at runtime?
>>>  Dunno - the changelogs are risible.  What effect do the various flags
>>>  have?
>>
>>The model has always been documented:
>>http://www.scyld.com/pipermail/vortex/2001-November/001426.html
>>(scroll down a tad)
> 
> 
> That's useless.

Not useless at all:  It documents the model that is being implemented 
quite well.  libata will use the same method of bitmasks, same method of 
increasing verbosity as set by debug level, same method of masking the 
more verbose messages by default, but always compiling the messages into 
the driver.  Its highly similar.

	Jeff

Re: [PATCH 12/13] ATA ACPI: use scsi_bus_shutdown for SATA/PATA

[Randy Dunlap]

On Tue, 28 Feb 2006 12:58:58 +0100
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > From: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> > 
> > Add ability for SCSI drivers to invoke a shutdown method.
> > This allows drivers to make drives safe for shutdown/poweroff,
> > for example.  Some drives need this to prevent possible problems.
> > 
> > Signed-off-by: Randy Dunlap <randy_d_dunlap@linux.intel.com>
> 
> > --- linux-2616-rc4-ata.orig/drivers/scsi/scsi_sysfs.c
> > +++ linux-2616-rc4-ata/drivers/scsi/scsi_sysfs.c
> > @@ -302,11 +302,27 @@ static int scsi_bus_resume(struct device
> >  	return err;
> >  }
> >  
> > +static void scsi_bus_shutdown(struct device * dev)
> > +{
> > +	struct scsi_device *sdev = to_scsi_device(dev);
> > +	struct scsi_host_template *sht = sdev->host->hostt;
> > +	int err;
> > +
> > +	err = scsi_device_quiesce(sdev);
> 
> int err = scsi_device_quiesce()?

Shouldn't matter for the generated code, right?

> > +	if (err)
> > +		printk(KERN_DEBUG "%s: error (0x%x) during shutdown\n",
> > +			__FUNCTION__, err);
> 
> If you get an error, and then ignore it... that should not be DEBUG
> message, right?
> 
> > +	if (sht->shutdown)
> > +		sht->shutdown(sdev);
> > +}
> > +
> >  struct bus_type scsi_bus_type = {
> >          .name		= "scsi",
> >          .match		= scsi_bus_match,
> >  	.suspend	= scsi_bus_suspend,
> >  	.resume		= scsi_bus_resume,
> > +	.shutdown	= scsi_bus_shutdown,
> >  };
> 
> Whitespace?

Not a problem in my addition.  Are you requesting me to fix
the other lines?

~Randy

Re: [PATCH 12/13] ATA ACPI: use scsi_bus_shutdown for SATA/PATA

[Pavel Machek]

Hi!

> > > +	if (sht->shutdown)
> > > +		sht->shutdown(sdev);
> > > +}
> > > +
> > >  struct bus_type scsi_bus_type = {
> > >          .name		= "scsi",
> > >          .match		= scsi_bus_match,
> > >  	.suspend	= scsi_bus_suspend,
> > >  	.resume		= scsi_bus_resume,
> > > +	.shutdown	= scsi_bus_shutdown,
> > >  };
> > 
> > Whitespace?
> 
> Not a problem in my addition.  Are you requesting me to fix
> the other lines?

Ahha, sorry, I seen wrong whitespace, and did not realize it was there
already. Yes, it would be nice to to fix whitespace around
modifications.

...and sorry about all the comments about added debugging. I did not
realize Jeff actually wants it that way. (I seriously dislike
excessive debugging in ACPI case.)
								Pavel
-- 
Web maintainer for suspend.sf.net (www.sf.net/projects/suspend) wanted...

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[James Courtier-Dutton]

Andrew Morton wrote:
> - The new debug stuff isn't documented.  One has funble around in the
>   source to work out how to even turn it on.  Can it be altered at runtime?
>   Dunno - the changelogs are risible.  What effect do the various flags
>   have?
>
>   Having spent (and re-spent) time grovelling through the ALSA source
>   working out how to enable their debug stuff during a maintainer snooze
>   I'd prefer we didn't have to do that with libata as well.
>
>   

Is there a particular debugging coding style that we should adopt for
all the kernel code.
For example,
kconfig option in order to compile a module/section of core code for
debug work.
A sysfs file to then control the debug level for each module.
A debug module option, in the cases where a particular level of debug is
required at module load time, and before the sysfs entry exists.
If particularly fine grained debug control is needed, the module could
have multiple entries in the sysfs to control different classes of debug
output.

One then implements all this debug support code at a global level,
making it easy for each kernel module to make use of it.

With regard to ALSA, we can make it fit in with the kernel preferred method.

James




This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.

Re: [PATCH 2/13] ATA ACPI: debugging infrastructure

[Andrew Morton]

James Courtier-Dutton <James@superbug.co.uk> wrote:
>
> Is there a particular debugging coding style that we should adopt for
>  all the kernel code.

Err, probably.  But we'd need to have a 1000-email argument first.

Right now many subsystems and often many individual drivers go and
implement their own set of debugging macros and knobs to twiddle.  This was
a great source of fun for me in trying to support gcc-2.95.x - each time a
new debug macro got implemented I had to go in there (again) and apply the
gcc-2.95.x-macro-expansion-bug-workaround to it.

Yes, one common toolset with a common way of controlling it would be much
more sensible than the present chaos.  I count 163 separate definitions of
dprintk(), and that's excluding all the non-x86 arch and include dirs.

>  For example,
>  kconfig option in order to compile a module/section of core code for
>  debug work.
>  A sysfs file to then control the debug level for each module.
>  A debug module option, in the cases where a particular level of debug is
>  required at module load time, and before the sysfs entry exists.
>  If particularly fine grained debug control is needed, the module could
>  have multiple entries in the sysfs to control different classes of debug
>  output.
> 

Something like that..   Just don't cc me while you work it out ;)