[PATCH] Add _GTM and _STM support to libata

[PATCH] Add _GTM and _STM support to libata

[Matthew Garrett]

I've ported the drivers/ide code for handling _GTM and _STM to libata. 
I'm not utterly convinced that I'm doing the calls in the right place - 
should they be attached to the scsi code rather than the ata host code? 
Anyway, this fixes suspend/resume on nc6220, which is what I was aiming 
for...

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

--- 

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index c428a56..0e542ea 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -34,6 +34,19 @@ 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;
+};
+
+struct ata_acpi_port_link {
+        acpi_handle                      obj_handle;
+        struct GTM_buffer                gtm;
+};
+
 /*
  *	Helper - belongs in the PCI layer somewhere eventually
  */
@@ -706,4 +719,125 @@ out:
 	return 0;
 }
 
+/**
+ * ata_acpi_get_timings - get the channel (controller) timings
+ * @hwif: target IDE interface (channel)
+ *
+ * This function executes the _GTM ACPI method for the target channel.
+ *
+ */
+void ata_acpi_get_timings(struct ata_port *ap)
+{
+        acpi_status                     status;
+	int                             err;
+        acpi_handle                     dev_handle = NULL;
+        acpi_integer                    pcidevfn = 0;
+	struct device			*dev = ap->host->dev;
+        struct acpi_buffer              output;
+        union acpi_object               *out_obj;
+
+        if (noacpi)
+		return;
+
+	if (ap->cbl == ATA_CBL_SATA)
+		return;
 
+	err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
+
+	if (err < 0)
+		return;
+
+	ap->port_handle = acpi_get_child(dev_handle, ap->port_no);
+
+        /* 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(ap->port_handle, "_GTM",
+                                      NULL, &output);
+
+        if (ACPI_FAILURE(status))
+	  return;
+
+        if (!output.length || !output.pointer) {
+                kfree(output.pointer);
+                return;
+        }
+
+        out_obj = output.pointer;
+        if (out_obj->type != ACPI_TYPE_BUFFER) {
+                kfree(output.pointer);
+                return;
+        }
+
+        if (!out_obj->buffer.length || !out_obj->buffer.pointer ||
+            out_obj->buffer.length != sizeof(struct GTM_buffer)) {
+                kfree(output.pointer);
+                printk(KERN_ERR
+                        "%s: unexpected _GTM length (0x%x)[should be 0x%zx] or \
+"
+		       "addr (0x%p)\n",
+		       __FUNCTION__, out_obj->buffer.length,
+		       sizeof(struct GTM_buffer), out_obj->buffer.pointer);
+                return;
+        }
+
+	if (!ap->acpidata) {
+		ap->acpidata = kzalloc(sizeof(struct ata_acpi_port_link), GFP_KERNEL);
+		if (!ap->acpidata)
+			return;
+	}
+
+        memcpy(&ap->acpidata->gtm, out_obj->buffer.pointer,
+               sizeof(struct GTM_buffer));
+
+        kfree(output.pointer);
+}
+EXPORT_SYMBOL_GPL(ata_acpi_get_timings);
+
+/**
+ * ata_acpi_push_timings - set the channel (controller) timings
+ * @hwif: target IDE interface (channel)
+ *
+ * This function executes the _STM ACPI method for the target channel.
+ *
+ * _STM requires Identify Drive data, which has to passed as an argument.
+ * Unfortunately hd_driveid is a mangled version which we can't readily
+ * use; hence we'll get the information afresh.
+ */
+void ata_acpi_push_timings(struct ata_port *ap)
+{
+        acpi_status             status;
+        struct acpi_object_list input;
+        union acpi_object       in_params[3];
+
+        if (noacpi)
+                return;
+
+        if (!ap->acpidata)
+                return;
+
+        /* 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->acpidata->gtm;
+        in_params[1].type = ACPI_TYPE_BUFFER;
+        in_params[1].buffer.length = sizeof(ap->device[0].id[0]) * ATA_ID_WORDS;
+        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[0]) * ATA_ID_WORDS;
+        in_params[2].buffer.pointer = (u8 *)&ap->device[1].id;
+        /* Output buffer: _STM has no output */
+
+        status = acpi_evaluate_object(ap->port_handle, "_STM",
+                                      &input, NULL);
+
+        if (ACPI_FAILURE(status))
+	  return;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_push_timings);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bf327d4..df38fc3 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5444,6 +5444,7 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
 				goto fail;
 			}
 		}
+		ata_acpi_get_timings(ap);
 	}
 
 	host->dev->power.power_state = mesg;
@@ -5467,6 +5468,11 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
  */
 void ata_host_resume(struct ata_host *host)
 {
+        int i;
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		ata_acpi_push_timings(ap);
+	}
 	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
 			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
 	host->dev->power.power_state = PMSG_ON;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index c426714..277982c 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -101,6 +101,8 @@ extern struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev,
 #ifdef CONFIG_SATA_ACPI
 extern int ata_acpi_exec_tfs(struct ata_port *ap);
 extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
+extern void ata_acpi_get_timings(struct ata_port *ap);
+extern void ata_acpi_push_timings(struct ata_port *ap);
 #else
 static inline int ata_acpi_exec_tfs(struct ata_port *ap)
 {
@@ -110,6 +112,14 @@ static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 {
 	return 0;
 }
+static inline void ata_acpi_get_timings(struct ata_port *ap)
+{
+        return;
+}
+extern void ata_acpi_push_timings(struct ata_port *ap)
+{
+        return;
+}
 #endif
 
 /* libata-scsi.c */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e3f32f3..b141863 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -340,6 +340,9 @@ struct scsi_device;
 struct ata_port_operations;
 struct ata_port;
 struct ata_queued_cmd;
+#ifdef CONFIG_SATA_ACPI
+struct ata_acpi_port_link;
+#endif
 
 /* typedefs */
 typedef void (*ata_qc_cb_t) (struct ata_queued_cmd *qc);
@@ -590,6 +593,11 @@ struct ata_port {
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
+#ifdef CONFIG_SATA_ACPI
+	/* ACPI objects info */
+	acpi_handle port_handle;
+	struct ata_acpi_port_link *acpidata;
+#endif
 };
 
 struct ata_port_operations {

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add _GTM and _STM support to libata

[Matthew Garrett]

> +                printk(KERN_ERR
> +                        "%s: unexpected _GTM length (0x%x)[should be 0x%zx] or \
> +"
> +		       "addr (0x%p)\n",
> +		       __FUNCTION__, out_obj->buffer.length,
> +		       sizeof(struct GTM_buffer), out_obj->buffer.pointer);

Yeah, erm.

> + * _STM requires Identify Drive data, which has to passed as an argument.
> + * Unfortunately hd_driveid is a mangled version which we can't readily
> + * use; hence we'll get the information afresh.

And clearly this doesn't actually match reality either. I'll send a 
tidied up patch when I've actually slept.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add _GTM and _STM support to libata

[Alan Cox]

On Mon, 26 Mar 2007 04:22:23 +0100
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> I've ported the drivers/ide code for handling _GTM and _STM to libata. 
> I'm not utterly convinced that I'm doing the calls in the right place - 

Please see the various patches I've posted or the -mm tree. I did that
some time ago and the pata_acpi driver makes full use of them already.

All there, all done.

Alan

Re: [PATCH] Add _GTM and _STM support to libata

[Alan Cox]

> should they be attached to the scsi code rather than the ata host code? 
> Anyway, this fixes suspend/resume on nc6220, which is what I was aiming 
> for...
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

NAK - there is code to do the _gtm/_stm parts in the -mm tree already
used by pata_acpi, waiting to be merged and it's a lot cleaner. It does
want hooking to the suspend/resume paths as well as to pata_acpi but that
is all.

This code also has bugs

> +
> +	if (ap->cbl == ATA_CBL_SATA)
> +		return;

Bangs head against wall. As I keep saying you *CANNOT* do any PATA v SATA
decisions based upon cable type. It just doesn't work. The kernel
sometimes doesn't know. The hardware sometimes turns up in weird formats
with ATA bridges.

You *MUST* see if the device has GTF/GTM or SATA methods in its ACPI
space and decide based upon that alone what the BIOS thinks should be
used.

Re: [PATCH] Add _GTM and _STM support to libata

[Matthew Garrett]

On Mon, Mar 26, 2007 at 11:58:07AM +0100, Alan Cox wrote:
> On Mon, 26 Mar 2007 04:22:23 +0100
> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> 
> > I've ported the drivers/ide code for handling _GTM and _STM to libata. 
> > I'm not utterly convinced that I'm doing the calls in the right place - 
> 
> Please see the various patches I've posted or the -mm tree. I did that
> some time ago and the pata_acpi driver makes full use of them already.

Ok, missed that. How about something like the following? I've changed 
some of the interfaces slightly, so the pata_acpi driver would need to 
be altered to match. This gets rid of the CABLE_SATA checking in 
libata-acpi.c, stores the handle in the ata_port structure so we can 
delete a load of code that keeps regenerating it, and ensures that the 
acpi code is setup up at device creation time. Finally, it hooks the 
calls into the core libata code.

Any comments?

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

--- 
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 0bd4789..99e534c 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -291,8 +291,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 			unsigned long *obj_loc)
 {
 	acpi_status			status;
-	acpi_handle			dev_handle = NULL;
-	acpi_handle			chan_handle, drive_handle;
+	acpi_handle			drive_handle;
 	acpi_integer			pcidevfn = 0;
 	u32				dev_adr;
 	struct acpi_buffer		output;
@@ -321,47 +320,12 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 		goto out;
 	}
 
-	/* Don't continue if device has no _ADR method.
-	 * _GTF is intended for known motherboard devices. */
-	if (!(ap->cbl == ATA_CBL_SATA)) {
-		err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
-		if (err < 0) {
-			if (ata_msg_probe(ap))
-				ata_dev_printk(atadev, 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))
-				ata_dev_printk(atadev, 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) {
-		if (!(ap->cbl == ATA_CBL_SATA)) {
-			/* get child objects of dev_handle == channel objects,
-	 		 * + _their_ children == drive objects */
-			/* channel is ap->port_no */
-			chan_handle = acpi_get_child(dev_handle,
-						ap->port_no);
-			if (ata_msg_probe(ap))
-				ata_dev_printk(atadev, KERN_DEBUG,
-					"%s: chan adr=%d: chan_handle=0x%p\n",
-					__FUNCTION__, ap->port_no,
-					chan_handle);
-			if (!chan_handle) {
-				err = -ENODEV;
-				goto out;
-			}
+		if (ap->acpi_port_link->is_pata) {
 			/* TBD: could also check ACPI object VALID bits */
-			drive_handle = acpi_get_child(chan_handle, ix);
+			drive_handle = 
+				acpi_get_child(ap->acpi_port_link->handle, ix);
 			if (!drive_handle) {
 				err = -ENODEV;
 				goto out;
@@ -370,7 +334,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 			atadev->obj_handle = drive_handle;
 		} else {	/* for SATA mode */
 			dev_adr = SATA_ADR_RSVD;
-			err = get_sata_adr(dev, dev_handle, pcidevfn, 0,
+			err = get_sata_adr(dev, ap->acpi_port_link->handle, pcidevfn, 0,
 					ap, atadev, &dev_adr);
 		}
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
@@ -531,7 +495,7 @@ static int do_drive_set_taskfiles(struct ata_port *ap,
 		ata_dev_printk(atadev, KERN_DEBUG, "%s: ENTER: port#: %d\n",
 			       __FUNCTION__, ap->port_no);
 
-	if (noacpi || !(ap->cbl == ATA_CBL_SATA))
+	if (noacpi || !ap->acpi_port_link || ap->acpi_port_link->is_pata)
 		return 0;
 
 	if (!ata_dev_enabled(atadev) || (ap->flags & ATA_FLAG_DISABLED))
@@ -581,7 +545,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
 	 * we should not run GTF on PATA devices since some
 	 * PATA require execution of GTM/STM before GTF.
 	 */
-	if (!(ap->cbl == ATA_CBL_SATA))
+	if (!ap->acpi_port_link || ap->acpi_port_link->is_pata)
 		return 0;
 
 	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
@@ -626,7 +590,6 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
  */
 int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 {
-	acpi_handle                     handle;
 	acpi_integer                    pcidevfn;
 	int                             err;
 	struct device                   *dev = ap->host->dev;
@@ -636,36 +599,17 @@ int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 	struct acpi_object_list         input;
 	union acpi_object               in_params[1];
 
-	if (noacpi)
+	if (noacpi || !ap->acpi_port_link || ap->acpi_port_link->is_pata)
 		return 0;
 
 	if (ata_msg_probe(ap))
 		ata_dev_printk(atadev, KERN_DEBUG, "%s: ix = %d, port#: %d\n",
 			       __FUNCTION__, ix, ap->port_no);
 
-	/* Don't continue if not a SATA device. */
-	if (!(ap->cbl == ATA_CBL_SATA)) {
-		if (ata_msg_probe(ap))
-			ata_dev_printk(atadev, KERN_DEBUG,
-				"%s: Not a SATA device\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))
-			ata_dev_printk(atadev, 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,
+		err = get_sata_adr(dev, ap->acpi_port_link->handle, pcidevfn, ix, ap, atadev,
 					&dev_adr);
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
 			!atadev->obj_handle) {
@@ -706,16 +650,18 @@ out:
 	return 0;
 }
 
-
-int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
+int ata_acpi_gtm(const struct ata_port *ap)
 {
 	acpi_status status;
 	struct acpi_buffer output;
 
+	if (!ap->acpi_port_link || !ap->acpi_port_link->is_pata)
+		return -ENODEV;
+
 	output.length = ACPI_ALLOCATE_BUFFER;
 	output.pointer = NULL;
 
-	status = acpi_evaluate_object(handle, "_GTM", NULL, &output);
+	status = acpi_evaluate_object(ap->acpi_port_link->handle, "_GTM", NULL, &output);
 
 	if (ACPI_FAILURE(status)) {
 		ata_port_printk(ap, KERN_ERR, "ACPI get timing mode failed.\n");
@@ -725,20 +671,23 @@ int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
 		ata_port_printk(ap, KERN_ERR, "ACPI get timing provided invalid data.\n");
 		return -EINVAL;
 	}
-	memcpy(gtm, output.pointer, sizeof(struct acpi_gtm));
+	memcpy(ap->acpi_port_link->gtm, output.pointer, sizeof(struct acpi_gtm));
 	kfree(output.pointer);
 	return 0;
 }
 
-int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
+int ata_acpi_stm(const struct ata_port *ap)
 {
 	acpi_status status;
 	struct acpi_object_list         input;
 	union acpi_object               in_params[3];
 
+	if (!ap->acpi_port_link || !ap->acpi_port_link->is_pata || !ap->acpi_port_link->gtm)
+		return -ENODEV;
+
 	in_params[0].type = ACPI_TYPE_BUFFER;
 	in_params[0].buffer.length = sizeof(struct acpi_gtm);
-	in_params[0].buffer.pointer = (u8 *)stm;
+	in_params[0].buffer.pointer = (u8 *)ap->acpi_port_link->gtm;
 	/* Buffers for id may need byteswapping ? */
 	in_params[1].type = ACPI_TYPE_BUFFER;
 	in_params[1].buffer.length = 512;
@@ -750,7 +699,7 @@ int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
 	input.count = 3;
 	input.pointer = in_params;
 
-	status = acpi_evaluate_object(handle, "_STM", &input, NULL);
+	status = acpi_evaluate_object(ap->acpi_port_link->handle, "_STM", &input, NULL);
 
 	if (ACPI_FAILURE(status)) {
 		ata_port_printk(ap, KERN_ERR, "ACPI set timing mode failed.\n");
@@ -787,8 +736,58 @@ int ata_pata_acpi_present(struct pci_dev *pdev)
 	return present;
 }
 
+static void *ata_sata_find_handle(struct device *dev, int port)
+{
+	acpi_handle handle;
+	acpi_integer devfn;
+
+	if (noacpi)
+		return NULL;
+	if (sata_get_dev_handle(dev, &handle, &devfn) < 0)
+		return NULL;
+	return acpi_get_child(handle, port);
+}
+
+int ata_sata_acpi_present(struct pci_dev *pdev)
+{
+	if (ata_sata_find_handle(&pdev->dev, 0))
+		return 1;
+	return 0;
+}
+
+void ata_acpi_setup(struct ata_port *ap)
+{
+	struct device *dev = ap->host->dev;	
+	struct pci_dev pci_dev;
+	int pata;
+
+	if (!is_pci_dev (dev))
+		return;
+
+	pci_dev = to_pci_dev(dev);
+
+	if (ata_sata_acpi_present(pci_dev)) 
+		pata = 0;
+	else if (ata_pata_acpi_present(pci_dev))
+		pata = 1;
+	else
+		return;
+
+	ap->acpi_port_link = kzalloc(sizeof(struct ata_acpi_port_link), 
+				     GFP_KERNEL);
+	ap->acpi_port_link->is_pata = pata;
+	if (pata) {
+		ap->acpi_port_link->handle = 
+			ata_pata_find_handle(ap->host->dev, ap->port_no);
+		ap->acpi_port_link->gtm = kzalloc(sizeof(struct acpi_gtm), 
+						  GFP_KERNEL);
+	} else
+		ap->acpi_port_link->handle = 
+			ata_sata_find_handle(ap->host->dev, ap->port_no);
+}
 
 EXPORT_SYMBOL_GPL(ata_acpi_gtm);
 EXPORT_SYMBOL_GPL(ata_acpi_stm);
 EXPORT_SYMBOL_GPL(ata_pata_acpi_handle);
 EXPORT_SYMBOL_GPL(ata_pata_acpi_present);
+EXPORT_SYMBOL_GPL(ata_acpi_setup);
diff --git a/drivers/ata/libata-acpi.h b/drivers/ata/libata-acpi.h
index d630ec3..339768a 100644
--- a/drivers/ata/libata-acpi.h
+++ b/drivers/ata/libata-acpi.h
@@ -39,9 +39,47 @@ struct acpi_gtm {
 	u32 flags;
 };
 
+struct acpi_port_link {
+	int is_pata;
+	acpi_handle *handle;
+	struct acpi_gtm *gtm;
+}
+
+#ifdef CONFIG_SATA_ACPI
+extern int ata_acpi_exec_tfs(struct ata_port *ap);
+extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
 extern int ata_acpi_gtm(const struct ata_port *p, void *handle, struct acpi_gtm *gtm);
 extern int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm);
 extern void *ata_pata_acpi_handle(const struct ata_port *ap);
 extern int ata_pata_acpi_present(struct pci_dev *pdev);
-
+extern void ata_acpi_setup(const struct ata_port *ap);
+#else
+static inline int ata_acpi_exec_tfs(struct ata_port *ap)
+{
+        return 0;
+}
+static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
+{
+        return 0;
+}
+extern inline int ata_acpi_gtm(const struct ata_port *p, void *handle, struct acpi_gtm *gtm)
+{
+	return 0;
+}
+extern inline int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
+{
+	return 0;
+}
+extern inline void *ata_pata_acpi_handle(const struct ata_port *ap)
+{
+	return 0;
+}
+extern inline int ata_pata_acpi_present(struct pci_dev *pdev)
+{
+	return 0;
+}
+extern inline void ata_acpi_setup(const struct ata_port *ap)
+{
+	return;
+}
 #endif
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bf327d4..079606b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -58,6 +58,7 @@
 #include <asm/byteorder.h>
 
 #include "libata.h"
+#include "libata-acpi.h"
 
 #define DRV_VERSION	"2.20"	/* must be exactly four chars */
 
@@ -5444,6 +5445,7 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
 				goto fail;
 			}
 		}
+		ata_acpi_gtm(ap);
 	}
 
 	host->dev->power.power_state = mesg;
@@ -5467,6 +5469,11 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
  */
 void ata_host_resume(struct ata_host *host)
 {
+	int i;
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		ata_acpi_stm(ap);
+	}
 	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
 			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
 	host->dev->power.power_state = PMSG_ON;
@@ -5917,6 +5924,7 @@ int ata_device_add(const struct ata_probe_ent *ent)
 		struct ata_port *ap = host->ports[i];
 
 		ata_scsi_scan_host(ap);
+		ata_acpi_setup(ap);
 	}
 
 	VPRINTK("EXIT, returning %u\n", ent->n_ports);
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index c426714..eb41263 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -97,21 +97,6 @@ extern void ata_port_init(struct ata_port *ap, struct ata_host *host,
 extern struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev,
 						 const struct ata_port_info *port);
 
-/* libata-acpi.c */
-#ifdef CONFIG_SATA_ACPI
-extern int ata_acpi_exec_tfs(struct ata_port *ap);
-extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
-#else
-static inline int ata_acpi_exec_tfs(struct ata_port *ap)
-{
-	return 0;
-}
-static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
-{
-	return 0;
-}
-#endif
-
 /* libata-scsi.c */
 extern struct scsi_transport_template ata_scsi_transport_template;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e3f32f3..9fb8586 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -590,6 +590,9 @@ struct ata_port {
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
+#ifdef CONFIG_SATA_ACPI
+	struct ata_acpi_port_link *acpi_port_link;
+#endif
 };
 
 struct ata_port_operations {

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add _GTM and _STM support to libata

[Alan Cox]

> Ok, missed that. How about something like the following? I've changed 
> some of the interfaces slightly, so the pata_acpi driver would need to 
> be altered to match. This gets rid of the CABLE_SATA checking in 

Close but no cigar. The pata_acpi driver needs to be able to pass its own
pointer for gtf as it wants to keep multiple states to hand.


>  	acpi_status status;
>  	struct acpi_buffer output;
>  
> +	if (!ap->acpi_port_link || !ap->acpi_port_link->is_pata)
> +		return -ENODEV;

Make ap->acpi_port_link a local variable and it is easier to read and
smaller and faster, need to pass the struct to gtm/stm from the caller -
easily pushed up a layer.


> -int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
> +int ata_acpi_stm(const struct ata_port *ap)

Ditto 

Otherwise looks good.

Re: [PATCH] Add _GTM and _STM support to libata

[Matthew Garrett]

Ok. How about this? Couple of cleanups, and leaves the possibility for 
drivers to pass in their own gtf structure.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 0bd4789..b434c39 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -291,13 +291,12 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 			unsigned long *obj_loc)
 {
 	acpi_status			status;
-	acpi_handle			dev_handle = NULL;
-	acpi_handle			chan_handle, drive_handle;
-	acpi_integer			pcidevfn = 0;
+	acpi_handle			drive_handle;
 	u32				dev_adr;
 	struct acpi_buffer		output;
 	union acpi_object 		*out_obj;
 	struct device			*dev = ap->host->dev;
+	struct ata_acpi_port_link           *acpidata = ap->acpi_port_link;
 	struct ata_device		*atadev = &ap->device[ix];
 	int				err = -ENODEV;
 
@@ -321,47 +320,12 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 		goto out;
 	}
 
-	/* Don't continue if device has no _ADR method.
-	 * _GTF is intended for known motherboard devices. */
-	if (!(ap->cbl == ATA_CBL_SATA)) {
-		err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
-		if (err < 0) {
-			if (ata_msg_probe(ap))
-				ata_dev_printk(atadev, 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))
-				ata_dev_printk(atadev, 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) {
-		if (!(ap->cbl == ATA_CBL_SATA)) {
-			/* get child objects of dev_handle == channel objects,
-	 		 * + _their_ children == drive objects */
-			/* channel is ap->port_no */
-			chan_handle = acpi_get_child(dev_handle,
-						ap->port_no);
-			if (ata_msg_probe(ap))
-				ata_dev_printk(atadev, KERN_DEBUG,
-					"%s: chan adr=%d: chan_handle=0x%p\n",
-					__FUNCTION__, ap->port_no,
-					chan_handle);
-			if (!chan_handle) {
-				err = -ENODEV;
-				goto out;
-			}
+		if (acpidata->is_pata) {
 			/* TBD: could also check ACPI object VALID bits */
-			drive_handle = acpi_get_child(chan_handle, ix);
+			drive_handle = 
+				acpi_get_child(acpidata->handle, ix);
 			if (!drive_handle) {
 				err = -ENODEV;
 				goto out;
@@ -370,8 +334,9 @@ static int do_drive_get_GTF(struct ata_port *ap, int 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);
+			err = get_sata_adr(dev, acpidata->handle, 
+					   acpidata->devfn, 0, ap, atadev, 
+					   &dev_adr);
 		}
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
 		    !atadev->obj_handle) {
@@ -526,12 +491,13 @@ static int do_drive_set_taskfiles(struct ata_port *ap,
 	int			gtf_count = gtf_length / REGS_PER_GTF;
 	int			ix;
 	struct taskfile_array	*gtf;
+	struct ata_acpi_port_link   *acpidata = ap->acpi_port_link;
 
 	if (ata_msg_probe(ap))
 		ata_dev_printk(atadev, KERN_DEBUG, "%s: ENTER: port#: %d\n",
 			       __FUNCTION__, ap->port_no);
 
-	if (noacpi || !(ap->cbl == ATA_CBL_SATA))
+	if (noacpi || !acpidata || acpidata->is_pata)
 		return 0;
 
 	if (!ata_dev_enabled(atadev) || (ap->flags & ATA_FLAG_DISABLED))
@@ -573,6 +539,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
 	unsigned int	gtf_length;
 	unsigned long	gtf_address;
 	unsigned long	obj_loc;
+	struct ata_acpi_port_link *acpidata = ap->acpi_port_link;
 
 	if (noacpi)
 		return 0;
@@ -581,7 +548,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
 	 * we should not run GTF on PATA devices since some
 	 * PATA require execution of GTM/STM before GTF.
 	 */
-	if (!(ap->cbl == ATA_CBL_SATA))
+	if (!acpidata || acpidata->is_pata)
 		return 0;
 
 	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
@@ -626,8 +593,6 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
  */
 int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 {
-	acpi_handle                     handle;
-	acpi_integer                    pcidevfn;
 	int                             err;
 	struct device                   *dev = ap->host->dev;
 	struct ata_device               *atadev = &ap->device[ix];
@@ -635,38 +600,20 @@ int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 	acpi_status                     status;
 	struct acpi_object_list         input;
 	union acpi_object               in_params[1];
+	struct ata_acpi_port_link           *acpidata = ap->acpi_port_link;
 
-	if (noacpi)
+	if (noacpi || !acpidata || acpidata->is_pata)
 		return 0;
 
 	if (ata_msg_probe(ap))
 		ata_dev_printk(atadev, KERN_DEBUG, "%s: ix = %d, port#: %d\n",
 			       __FUNCTION__, ix, ap->port_no);
 
-	/* Don't continue if not a SATA device. */
-	if (!(ap->cbl == ATA_CBL_SATA)) {
-		if (ata_msg_probe(ap))
-			ata_dev_printk(atadev, KERN_DEBUG,
-				"%s: Not a SATA device\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))
-			ata_dev_printk(atadev, 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);
+		err = get_sata_adr(dev, acpidata->handle, acpidata->devfn, ix,
+				   ap, atadev, &dev_adr);
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
 			!atadev->obj_handle) {
 			if (ata_msg_probe(ap))
@@ -706,16 +653,19 @@ out:
 	return 0;
 }
 
-
-int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
+int ata_acpi_gtm(const struct ata_port *ap, struct acpi_gtm *gtm)
 {
 	acpi_status status;
 	struct acpi_buffer output;
+	struct ata_acpi_port_link *acpidata = ap->acpi_port_link;
+
+	if (!acpidata || !acpidata->is_pata)
+		return -ENODEV;
 
 	output.length = ACPI_ALLOCATE_BUFFER;
 	output.pointer = NULL;
 
-	status = acpi_evaluate_object(handle, "_GTM", NULL, &output);
+	status = acpi_evaluate_object(acpidata->handle, "_GTM", NULL, &output);
 
 	if (ACPI_FAILURE(status)) {
 		ata_port_printk(ap, KERN_ERR, "ACPI get timing mode failed.\n");
@@ -730,11 +680,15 @@ int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
 	return 0;
 }
 
-int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
+int ata_acpi_stm(const struct ata_port *ap, struct acpi_gtm *stm)
 {
 	acpi_status status;
 	struct acpi_object_list         input;
 	union acpi_object               in_params[3];
+	struct ata_acpi_port_link       *acpidata = ap->acpi_port_link;
+
+	if (!acpidata || !acpidata->is_pata)
+		return -ENODEV;
 
 	in_params[0].type = ACPI_TYPE_BUFFER;
 	in_params[0].buffer.length = sizeof(struct acpi_gtm);
@@ -750,7 +704,7 @@ int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
 	input.count = 3;
 	input.pointer = in_params;
 
-	status = acpi_evaluate_object(handle, "_STM", &input, NULL);
+	status = acpi_evaluate_object(acpidata->handle, "_STM", &input, NULL);
 
 	if (ACPI_FAILURE(status)) {
 		ata_port_printk(ap, KERN_ERR, "ACPI set timing mode failed.\n");
@@ -787,8 +741,61 @@ int ata_pata_acpi_present(struct pci_dev *pdev)
 	return present;
 }
 
+static void *ata_sata_find_handle(struct device *dev, int port)
+{
+	acpi_handle handle;
+	acpi_integer devfn;
+
+	if (noacpi)
+		return NULL;
+	if (sata_get_dev_handle(dev, &handle, &devfn) < 0)
+		return NULL;
+	return acpi_get_child(handle, port);
+}
+
+int ata_sata_acpi_present(struct pci_dev *pdev)
+{
+	if (ata_sata_find_handle(&pdev->dev, 0))
+		return 1;
+	return 0;
+}
+
+void ata_acpi_setup(struct ata_port *ap)
+{
+	struct device *dev = ap->host->dev;	
+	struct pci_dev *pci_dev;
+	struct ata_acpi_port_link *acpidata;
+	int pata;
+
+	if (!is_pci_dev (dev))
+		return;
+
+	pci_dev = to_pci_dev(dev);
+
+	if (ata_pata_acpi_present(pci_dev))
+		pata = 1;
+	else if (ata_sata_acpi_present(pci_dev)) 
+		pata = 0;
+	else
+		return;
+
+	acpidata = kzalloc(sizeof(struct ata_acpi_port_link), GFP_KERNEL);
+	acpidata->is_pata = pata;
+	acpidata->devfn = (PCI_SLOT(pci_dev->devfn) << 16) | 
+		PCI_FUNC(pci_dev->devfn);
+	if (pata) {
+		acpidata->handle = 
+			ata_pata_find_handle(ap->host->dev, ap->port_no);
+		ap->gtm = kzalloc(sizeof(struct acpi_gtm), GFP_KERNEL);
+	} else
+		acpidata->handle = 
+			ata_sata_find_handle(ap->host->dev, ap->port_no);
+
+	ap->acpi_port_link = acpidata;
+}
 
 EXPORT_SYMBOL_GPL(ata_acpi_gtm);
 EXPORT_SYMBOL_GPL(ata_acpi_stm);
 EXPORT_SYMBOL_GPL(ata_pata_acpi_handle);
 EXPORT_SYMBOL_GPL(ata_pata_acpi_present);
+EXPORT_SYMBOL_GPL(ata_acpi_setup);
diff --git a/drivers/ata/libata-acpi.h b/drivers/ata/libata-acpi.h
index d630ec3..5948310 100644
--- a/drivers/ata/libata-acpi.h
+++ b/drivers/ata/libata-acpi.h
@@ -39,9 +39,48 @@ struct acpi_gtm {
 	u32 flags;
 };
 
-extern int ata_acpi_gtm(const struct ata_port *p, void *handle, struct acpi_gtm *gtm);
-extern int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm);
+struct ata_acpi_port_link {
+	int is_pata;
+	acpi_handle *handle;
+	acpi_integer devfn;
+};
+
+#ifdef CONFIG_SATA_ACPI
+extern int ata_acpi_exec_tfs(struct ata_port *ap);
+extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
+extern int ata_acpi_gtm(const struct ata_port *p, struct acpi_gtm *gtm);
+extern int ata_acpi_stm(const struct ata_port *ap, struct acpi_gtm *stm);
 extern void *ata_pata_acpi_handle(const struct ata_port *ap);
 extern int ata_pata_acpi_present(struct pci_dev *pdev);
-
+extern void ata_acpi_setup(struct ata_port *ap);
+#else
+static inline int ata_acpi_exec_tfs(struct ata_port *ap)
+{
+        return 0;
+}
+static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
+{
+        return 0;
+}
+extern inline int ata_acpi_gtm(const struct ata_port *p, struct acpi_gtm *gtm)
+{
+	return 0;
+}
+extern inline int ata_acpi_stm(const struct ata_port *ap, struct acpi_gtm *stm)
+{
+	return 0;
+}
+extern inline void *ata_pata_acpi_handle(const struct ata_port *ap)
+{
+	return 0;
+}
+extern inline int ata_pata_acpi_present(struct pci_dev *pdev)
+{
+	return 0;
+}
+extern inline void ata_acpi_setup(struct ata_port *ap)
+{
+	return;
+}
+#endif /* CONFIG_SATA_ACPI */
 #endif
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bf327d4..a7258b6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -58,6 +58,7 @@
 #include <asm/byteorder.h>
 
 #include "libata.h"
+#include "libata-acpi.h"
 
 #define DRV_VERSION	"2.20"	/* must be exactly four chars */
 
@@ -5444,6 +5445,8 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
 				goto fail;
 			}
 		}
+
+		ata_acpi_gtm(ap, ap->gtm);
 	}
 
 	host->dev->power.power_state = mesg;
@@ -5467,6 +5470,12 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
  */
 void ata_host_resume(struct ata_host *host)
 {
+	int i;
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		ata_acpi_stm(ap, ap->gtm);
+	}
+
 	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
 			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
 	host->dev->power.power_state = PMSG_ON;
@@ -5917,6 +5926,7 @@ int ata_device_add(const struct ata_probe_ent *ent)
 		struct ata_port *ap = host->ports[i];
 
 		ata_scsi_scan_host(ap);
+		ata_acpi_setup(ap);
 	}
 
 	VPRINTK("EXIT, returning %u\n", ent->n_ports);
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index c426714..eb41263 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -97,21 +97,6 @@ extern void ata_port_init(struct ata_port *ap, struct ata_host *host,
 extern struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev,
 						 const struct ata_port_info *port);
 
-/* libata-acpi.c */
-#ifdef CONFIG_SATA_ACPI
-extern int ata_acpi_exec_tfs(struct ata_port *ap);
-extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
-#else
-static inline int ata_acpi_exec_tfs(struct ata_port *ap)
-{
-	return 0;
-}
-static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
-{
-	return 0;
-}
-#endif
-
 /* libata-scsi.c */
 extern struct scsi_transport_template ata_scsi_transport_template;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e3f32f3..4774c08 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -590,6 +590,10 @@ struct ata_port {
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
+	struct acpi_gtm         *gtm;
+#ifdef CONFIG_SATA_ACPI
+	struct ata_acpi_port_link *acpi_port_link;
+#endif
 };
 
 struct ata_port_operations {

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add _GTM and _STM support to libata

[Matthew Garrett]

On Tue, Mar 27, 2007 at 12:39:06AM +0100, Alan Cox wrote:
> > +	acpidata = kzalloc(sizeof(struct ata_acpi_port_link), GFP_KERNEL);
> > +	acpidata->is_pata = pata;
> 
> Out of memory -> NULL - > OOps *bang*
> 
> 
> Looks ok but for the memory handling now

Ok. Only changes in this one are checking the return from kzalloc and 
bailing appropriately.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 0bd4789..8fab5fd 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -291,13 +291,12 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 			unsigned long *obj_loc)
 {
 	acpi_status			status;
-	acpi_handle			dev_handle = NULL;
-	acpi_handle			chan_handle, drive_handle;
-	acpi_integer			pcidevfn = 0;
+	acpi_handle			drive_handle;
 	u32				dev_adr;
 	struct acpi_buffer		output;
 	union acpi_object 		*out_obj;
 	struct device			*dev = ap->host->dev;
+	struct ata_acpi_port_link           *acpidata = ap->acpi_port_link;
 	struct ata_device		*atadev = &ap->device[ix];
 	int				err = -ENODEV;
 
@@ -321,47 +320,12 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
 		goto out;
 	}
 
-	/* Don't continue if device has no _ADR method.
-	 * _GTF is intended for known motherboard devices. */
-	if (!(ap->cbl == ATA_CBL_SATA)) {
-		err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
-		if (err < 0) {
-			if (ata_msg_probe(ap))
-				ata_dev_printk(atadev, 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))
-				ata_dev_printk(atadev, 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) {
-		if (!(ap->cbl == ATA_CBL_SATA)) {
-			/* get child objects of dev_handle == channel objects,
-	 		 * + _their_ children == drive objects */
-			/* channel is ap->port_no */
-			chan_handle = acpi_get_child(dev_handle,
-						ap->port_no);
-			if (ata_msg_probe(ap))
-				ata_dev_printk(atadev, KERN_DEBUG,
-					"%s: chan adr=%d: chan_handle=0x%p\n",
-					__FUNCTION__, ap->port_no,
-					chan_handle);
-			if (!chan_handle) {
-				err = -ENODEV;
-				goto out;
-			}
+		if (acpidata->is_pata) {
 			/* TBD: could also check ACPI object VALID bits */
-			drive_handle = acpi_get_child(chan_handle, ix);
+			drive_handle = 
+				acpi_get_child(acpidata->handle, ix);
 			if (!drive_handle) {
 				err = -ENODEV;
 				goto out;
@@ -370,8 +334,9 @@ static int do_drive_get_GTF(struct ata_port *ap, int 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);
+			err = get_sata_adr(dev, acpidata->handle, 
+					   acpidata->devfn, 0, ap, atadev, 
+					   &dev_adr);
 		}
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
 		    !atadev->obj_handle) {
@@ -526,12 +491,13 @@ static int do_drive_set_taskfiles(struct ata_port *ap,
 	int			gtf_count = gtf_length / REGS_PER_GTF;
 	int			ix;
 	struct taskfile_array	*gtf;
+	struct ata_acpi_port_link   *acpidata = ap->acpi_port_link;
 
 	if (ata_msg_probe(ap))
 		ata_dev_printk(atadev, KERN_DEBUG, "%s: ENTER: port#: %d\n",
 			       __FUNCTION__, ap->port_no);
 
-	if (noacpi || !(ap->cbl == ATA_CBL_SATA))
+	if (noacpi || !acpidata || acpidata->is_pata)
 		return 0;
 
 	if (!ata_dev_enabled(atadev) || (ap->flags & ATA_FLAG_DISABLED))
@@ -573,6 +539,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
 	unsigned int	gtf_length;
 	unsigned long	gtf_address;
 	unsigned long	obj_loc;
+	struct ata_acpi_port_link *acpidata = ap->acpi_port_link;
 
 	if (noacpi)
 		return 0;
@@ -581,7 +548,7 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
 	 * we should not run GTF on PATA devices since some
 	 * PATA require execution of GTM/STM before GTF.
 	 */
-	if (!(ap->cbl == ATA_CBL_SATA))
+	if (!acpidata || acpidata->is_pata)
 		return 0;
 
 	for (ix = 0; ix < ATA_MAX_DEVICES; ix++) {
@@ -626,8 +593,6 @@ int ata_acpi_exec_tfs(struct ata_port *ap)
  */
 int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 {
-	acpi_handle                     handle;
-	acpi_integer                    pcidevfn;
 	int                             err;
 	struct device                   *dev = ap->host->dev;
 	struct ata_device               *atadev = &ap->device[ix];
@@ -635,38 +600,20 @@ int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
 	acpi_status                     status;
 	struct acpi_object_list         input;
 	union acpi_object               in_params[1];
+	struct ata_acpi_port_link           *acpidata = ap->acpi_port_link;
 
-	if (noacpi)
+	if (noacpi || !acpidata || acpidata->is_pata)
 		return 0;
 
 	if (ata_msg_probe(ap))
 		ata_dev_printk(atadev, KERN_DEBUG, "%s: ix = %d, port#: %d\n",
 			       __FUNCTION__, ix, ap->port_no);
 
-	/* Don't continue if not a SATA device. */
-	if (!(ap->cbl == ATA_CBL_SATA)) {
-		if (ata_msg_probe(ap))
-			ata_dev_printk(atadev, KERN_DEBUG,
-				"%s: Not a SATA device\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))
-			ata_dev_printk(atadev, 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);
+		err = get_sata_adr(dev, acpidata->handle, acpidata->devfn, ix,
+				   ap, atadev, &dev_adr);
 		if (err < 0 || dev_adr == SATA_ADR_RSVD ||
 			!atadev->obj_handle) {
 			if (ata_msg_probe(ap))
@@ -706,16 +653,19 @@ out:
 	return 0;
 }
 
-
-int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
+int ata_acpi_gtm(const struct ata_port *ap, struct acpi_gtm *gtm)
 {
 	acpi_status status;
 	struct acpi_buffer output;
+	struct ata_acpi_port_link *acpidata = ap->acpi_port_link;
+
+	if (!acpidata || !acpidata->is_pata)
+		return -ENODEV;
 
 	output.length = ACPI_ALLOCATE_BUFFER;
 	output.pointer = NULL;
 
-	status = acpi_evaluate_object(handle, "_GTM", NULL, &output);
+	status = acpi_evaluate_object(acpidata->handle, "_GTM", NULL, &output);
 
 	if (ACPI_FAILURE(status)) {
 		ata_port_printk(ap, KERN_ERR, "ACPI get timing mode failed.\n");
@@ -730,11 +680,15 @@ int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
 	return 0;
 }
 
-int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
+int ata_acpi_stm(const struct ata_port *ap, struct acpi_gtm *stm)
 {
 	acpi_status status;
 	struct acpi_object_list         input;
 	union acpi_object               in_params[3];
+	struct ata_acpi_port_link       *acpidata = ap->acpi_port_link;
+
+	if (!acpidata || !acpidata->is_pata)
+		return -ENODEV;
 
 	in_params[0].type = ACPI_TYPE_BUFFER;
 	in_params[0].buffer.length = sizeof(struct acpi_gtm);
@@ -750,7 +704,7 @@ int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
 	input.count = 3;
 	input.pointer = in_params;
 
-	status = acpi_evaluate_object(handle, "_STM", &input, NULL);
+	status = acpi_evaluate_object(acpidata->handle, "_STM", &input, NULL);
 
 	if (ACPI_FAILURE(status)) {
 		ata_port_printk(ap, KERN_ERR, "ACPI set timing mode failed.\n");
@@ -787,8 +741,69 @@ int ata_pata_acpi_present(struct pci_dev *pdev)
 	return present;
 }
 
+static void *ata_sata_find_handle(struct device *dev, int port)
+{
+	acpi_handle handle;
+	acpi_integer devfn;
+
+	if (noacpi)
+		return NULL;
+	if (sata_get_dev_handle(dev, &handle, &devfn) < 0)
+		return NULL;
+	return acpi_get_child(handle, port);
+}
+
+int ata_sata_acpi_present(struct pci_dev *pdev)
+{
+	if (ata_sata_find_handle(&pdev->dev, 0))
+		return 1;
+	return 0;
+}
+
+void ata_acpi_setup(struct ata_port *ap)
+{
+	struct device *dev = ap->host->dev;	
+	struct pci_dev *pci_dev;
+	struct ata_acpi_port_link *acpidata;
+	int pata;
+
+	if (!is_pci_dev (dev))
+		return;
+
+	pci_dev = to_pci_dev(dev);
+
+	if (ata_pata_acpi_present(pci_dev))
+		pata = 1;
+	else if (ata_sata_acpi_present(pci_dev)) 
+		pata = 0;
+	else
+		return;
+
+	acpidata = kzalloc(sizeof(struct ata_acpi_port_link), GFP_KERNEL);
+
+	if (!acpidata)
+		return;
+
+	acpidata->is_pata = pata;
+	acpidata->devfn = (PCI_SLOT(pci_dev->devfn) << 16) | 
+		PCI_FUNC(pci_dev->devfn);
+	if (pata) {
+		acpidata->handle = 
+			ata_pata_find_handle(ap->host->dev, ap->port_no);
+		ap->gtm = kzalloc(sizeof(struct acpi_gtm), GFP_KERNEL);
+		if (!ap->gtm) {
+			kfree(ap);
+			return;
+		}
+	} else
+		acpidata->handle = 
+			ata_sata_find_handle(ap->host->dev, ap->port_no);
+
+	ap->acpi_port_link = acpidata;
+}
 
 EXPORT_SYMBOL_GPL(ata_acpi_gtm);
 EXPORT_SYMBOL_GPL(ata_acpi_stm);
 EXPORT_SYMBOL_GPL(ata_pata_acpi_handle);
 EXPORT_SYMBOL_GPL(ata_pata_acpi_present);
+EXPORT_SYMBOL_GPL(ata_acpi_setup);
diff --git a/drivers/ata/libata-acpi.h b/drivers/ata/libata-acpi.h
index d630ec3..5948310 100644
--- a/drivers/ata/libata-acpi.h
+++ b/drivers/ata/libata-acpi.h
@@ -39,9 +39,48 @@ struct acpi_gtm {
 	u32 flags;
 };
 
-extern int ata_acpi_gtm(const struct ata_port *p, void *handle, struct acpi_gtm *gtm);
-extern int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm);
+struct ata_acpi_port_link {
+	int is_pata;
+	acpi_handle *handle;
+	acpi_integer devfn;
+};
+
+#ifdef CONFIG_SATA_ACPI
+extern int ata_acpi_exec_tfs(struct ata_port *ap);
+extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
+extern int ata_acpi_gtm(const struct ata_port *p, struct acpi_gtm *gtm);
+extern int ata_acpi_stm(const struct ata_port *ap, struct acpi_gtm *stm);
 extern void *ata_pata_acpi_handle(const struct ata_port *ap);
 extern int ata_pata_acpi_present(struct pci_dev *pdev);
-
+extern void ata_acpi_setup(struct ata_port *ap);
+#else
+static inline int ata_acpi_exec_tfs(struct ata_port *ap)
+{
+        return 0;
+}
+static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
+{
+        return 0;
+}
+extern inline int ata_acpi_gtm(const struct ata_port *p, struct acpi_gtm *gtm)
+{
+	return 0;
+}
+extern inline int ata_acpi_stm(const struct ata_port *ap, struct acpi_gtm *stm)
+{
+	return 0;
+}
+extern inline void *ata_pata_acpi_handle(const struct ata_port *ap)
+{
+	return 0;
+}
+extern inline int ata_pata_acpi_present(struct pci_dev *pdev)
+{
+	return 0;
+}
+extern inline void ata_acpi_setup(struct ata_port *ap)
+{
+	return;
+}
+#endif /* CONFIG_SATA_ACPI */
 #endif
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index bf327d4..a7258b6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -58,6 +58,7 @@
 #include <asm/byteorder.h>
 
 #include "libata.h"
+#include "libata-acpi.h"
 
 #define DRV_VERSION	"2.20"	/* must be exactly four chars */
 
@@ -5444,6 +5445,8 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
 				goto fail;
 			}
 		}
+
+		ata_acpi_gtm(ap, ap->gtm);
 	}
 
 	host->dev->power.power_state = mesg;
@@ -5467,6 +5470,12 @@ int ata_host_suspend(struct ata_host *host, pm_message_t mesg)
  */
 void ata_host_resume(struct ata_host *host)
 {
+	int i;
+	for (i = 0; i < host->n_ports; i++) {
+		struct ata_port *ap = host->ports[i];
+		ata_acpi_stm(ap, ap->gtm);
+	}
+
 	ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
 			    ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
 	host->dev->power.power_state = PMSG_ON;
@@ -5917,6 +5926,7 @@ int ata_device_add(const struct ata_probe_ent *ent)
 		struct ata_port *ap = host->ports[i];
 
 		ata_scsi_scan_host(ap);
+		ata_acpi_setup(ap);
 	}
 
 	VPRINTK("EXIT, returning %u\n", ent->n_ports);
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index c426714..eb41263 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -97,21 +97,6 @@ extern void ata_port_init(struct ata_port *ap, struct ata_host *host,
 extern struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev,
 						 const struct ata_port_info *port);
 
-/* libata-acpi.c */
-#ifdef CONFIG_SATA_ACPI
-extern int ata_acpi_exec_tfs(struct ata_port *ap);
-extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
-#else
-static inline int ata_acpi_exec_tfs(struct ata_port *ap)
-{
-	return 0;
-}
-static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
-{
-	return 0;
-}
-#endif
-
 /* libata-scsi.c */
 extern struct scsi_transport_template ata_scsi_transport_template;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e3f32f3..4774c08 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -590,6 +590,10 @@ struct ata_port {
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
+	struct acpi_gtm         *gtm;
+#ifdef CONFIG_SATA_ACPI
+	struct ata_acpi_port_link *acpi_port_link;
+#endif
 };
 
 struct ata_port_operations {

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] Add _GTM and _STM support to libata

[Alan Cox]

On Mon, 26 Mar 2007 22:48:00 +0100
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> Ok. How about this? Couple of cleanups, and leaves the possibility for 
> drivers to pass in their own gtf structure.

Looks better, I can work with that for pata_acpi nicely


> +	acpidata = kzalloc(sizeof(struct ata_acpi_port_link), GFP_KERNEL);
> +	acpidata->is_pata = pata;

Out of memory -> NULL - > OOps *bang*


Looks ok but for the memory handling now