Add _GTM and _STM support to libata

PATCH NAK Matthew Garrett 2 participants 9 messages Mar 26, 2007

Subsystems (2)

ACPI libata

lore

Share & tools

lore

Matthew Garrett

author

2007-03-26 03:22 UTC #0 lore raw

PATCH 1/1 Add _GTM and _STM support to libata

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 <[email protected]>

---

drivers/ata/libata-acpi.c +134 -0

index c428a56..0e542ea 100644
@@ -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);

drivers/ata/libata-core.c +6 -0

index bf327d4..df38fc3 100644
@@ -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;

drivers/ata/libata-eh.c +0 -0

drivers/ata/libata-scsi.c +0 -0

drivers/ata/libata.h +10 -0

index c426714..277982c 100644
@@ -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 */

include/linux/libata.h +8 -0

index e3f32f3..b141863 100644
@@ -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 {

Signature · 1 line

Matthew Garrett | [email protected]

Matthew Garrett

author

2007-03-26 03:39 UTC #1 lore raw

drivers/ata/libata-acpi.c out: · line 782

Quoted 6 lines

drivers/ata/libata-acpi.c +6 -0

+                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.

Quoted 3 lines

+ * _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.

Signature · 1 line

Matthew Garrett | [email protected]

Alan Cox

2007-03-26 09:55 UTC #2 lore raw

In reply to Matthew Garrett

Quoted 2 lines · On Mon, 26 Mar 2007 04:22:23 +0100 Matthew Garrett <[email protected]> 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

Matthew Garrett

author

2007-03-26 20:15 UTC #3 lore raw

Quoted 8 lines · Alan Cox

Quoted 2 lines · On Mon, 26 Mar 2007 04:22:23 +0100 Matthew Garrett <[email protected]> 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 <[email protected]>

---

drivers/ata/libata-acpi.c +71 ~~-72~~

index 0bd4789..99e534c 100644
@@ -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);

drivers/ata/libata-acpi.h +39 -1

index d630ec3..339768a 100644
@@ -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

drivers/ata/libata-core.c +8 -0

index bf327d4..079606b 100644
@@ -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);

drivers/ata/libata.h +0 ~~-15~~

index c426714..eb41263 100644
@@ -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;

include/linux/libata.h +3 -0

index e3f32f3..9fb8586 100644
@@ -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 {

Signature · 1 line

Matthew Garrett | [email protected]

Alan Cox

2007-03-26 20:37 UTC #4 lore raw

Quoted 3 lines

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.

drivers/ata/libata-acpi.c out: · line 659

Quoted 5 lines

drivers/ata/libata-acpi.c +2 -0

 	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.

drivers/ata/libata-acpi.c int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm) · line 679

Quoted 2 lines

drivers/ata/libata-acpi.c +1 -1

-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.

Matthew Garrett

author

2007-03-26 21:48 UTC #5 lore raw

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

drivers/ata/libata-acpi.c +81 ~~-74~~

index 0bd4789..b434c39 100644
@@ -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);

drivers/ata/libata-acpi.h +42 -3

51 lines not on this page · raw message

drivers/ata/libata-core.c +10 -0

index bf327d4..a7258b6 100644
@@ -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);

drivers/ata/libata.h +0 ~~-15~~

index c426714..eb41263 100644
@@ -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;

include/linux/libata.h +4 -0

index e3f32f3..4774c08 100644
@@ -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 {

Signature · 1 line

Matthew Garrett | [email protected]

Alan Cox

2007-03-26 22:36 UTC #6 lore raw

Quoted 2 lines · On Mon, 26 Mar 2007 22:48:00 +0100 Matthew Garrett <[email protected]> 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

drivers/ata/libata-acpi.c int ata_pata_acpi_present(struct pci_dev *pdev) · line 783

Quoted 2 lines

drivers/ata/libata-acpi.c +2 -0

+	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

Matthew Garrett

author

2007-03-26 22:44 UTC #7 lore raw

Quoted 7 lines · Alan Cox

Review
Quoted 2 lines
(quoted diff) +2 -0
+	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.

drivers/ata/libata-acpi.c +89 ~~-74~~

index 0bd4789..8fab5fd 100644
@@ -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);

drivers/ata/libata-acpi.h +42 -3

51 lines not on this page · raw message

drivers/ata/libata-core.c +10 -0

index bf327d4..a7258b6 100644
@@ -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);

drivers/ata/libata.h +0 ~~-15~~

index c426714..eb41263 100644
@@ -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;

include/linux/libata.h +4 -0

index e3f32f3..4774c08 100644
@@ -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 {

Signature · 1 line

Matthew Garrett | [email protected]

Alan Cox

NAK

2007-03-26 09:58 UTC #8 lore raw

In reply to Matthew Garrett

Quoted 5 lines

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 <[email protected]>

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

drivers/ata/libata-acpi.c out: · line 743

Quoted 3 lines

drivers/ata/libata-acpi.c +3 -0
+
+	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.