Skip to contentSkip to conversation

too late: updated version of the ACPI patches

PATCH In review Hannes Reinecke 2 participants 3 messages Sep 28, 2006 – Sep 29, 2006

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 too late: updated version of the ACPI patches

Hi Jeff,

argl. Appearently I'm in Yeltsin mode.

Anyway: I'm also have finished an updated version of the ACPI patches. They are also based upon the patches by Randy Dunlap but with some improvements:

- Omit the namespace walk for SATA devices. We can really trust the ACPI layer to find out the correct device. If not the ACPI is buggered anyway and we shouldn't even try to continue.
- Make the control over the ACPI execution more finegrained as some methods (most notably _GTF) are downright disastrous on PATA devices, whereas you really want to call _GTM / _STM on these to have them properly resumed after suspend to RAM.
- Only export the symbols we really have to :-)
- Proper integration with the new EH code. This is actually an error in the patch by Kristen; for new-style EH the ACPI functions will never be called :-(

If you consider this a duplicate I'm happy to rebase my patch on top of Kristens.

Comments etc are welcome.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
Documentation/kernel-parameters.txt +5 -0
index b50595a..73e73ef 100644
@@ -48,6 +48,7 @@ parameter is applicable:
 	ISAPNP	ISA PnP code is enabled.
 	ISDN	Appropriate ISDN support is enabled.
 	JOY	Appropriate joystick support is enabled.
+	LIBATA  Libata driver is enabled.
 	LP	Printer support is enabled.
 	LOOP	Loopback device support is enabled.
 	M68k	M68k architecture is enabled.
@@ -257,6 +258,10 @@ running once the system is up.
 	arcrimi=	[HW,NET] ARCnet - "RIM I" (entirely mem-mapped) cards
 			Format: <io>,<irq>,<nodeID>
 
+	ata_acpi=	[LIBATA] Disables use of ACPI in libata suspend/resume
+			when set.
+			Format: <int>
+
 	ataflop=	[HW,M68k]
 
 	atarimouse=	[HW,MOUSE] Atari Mouse
drivers/ata/Kconfig +13 -0
index 13027d5..4b90b2d 100644
@@ -145,6 +145,19 @@ config SATA_INTEL_COMBINED
 	depends on IDE=y && !BLK_DEV_IDE_SATA && (SATA_AHCI || ATA_PIIX)
 	default y
 
+config ATA_ACPI
+	bool "Handle ATA-related ACPI objects"
+	depends on ACPI && PCI
+	default y
+	help
+	  This option adds support for ATA-related ACPI objects.
+	  These ACPI objects add the ability to retrieve taskfiles
+	  from the ACPI BIOS and write them to the disk controller.
+	  These objects may be related to performance, security,
+	  power management, or other areas.
+	  You can disable this at kernel boot time by using the
+	  option 'libata.ata_acpi=0'.
+
 endif
 endmenu
 
drivers/ata/Makefile +1 -0
index e260e3f..875b5d9 100644
@@ -18,4 +18,5 @@ obj-$(CONFIG_SATA_MV)		+= sata_mv.o
 obj-$(CONFIG_PDC_ADMA)		+= pdc_adma.o
 
 libata-objs	:= libata-core.o libata-scsi.o libata-sff.o libata-eh.o
+libata-$(CONFIG_ATA_ACPI) += libata-acpi.o
 
drivers/ata/libata-core.c +46 -3

86 lines not on this page · raw message

drivers/ata/libata-eh.c +21 -0
index b1b5104..93723c1 100644
@@ -1991,6 +1991,21 @@ static int ata_eh_recover(struct ata_por
 			down_xfermask = 1;
 			goto dev_fail;
 		}
+
+#ifdef CONFIG_ATA_ACPI
+		for (i = 0; i < ATA_MAX_DEVICES; i++) {
+			dev = &ap->device[i];
+
+			if (!ata_dev_enabled(dev))
+				continue;
+
+			/* Send down drive data via _SDD */
+			ata_acpi_push_id(dev);
+
+			/* retrieve and execute the ATA task file of _GTF */
+			ata_acpi_exec_tfs(dev);			
+		}
+#endif
 	}
 
 	/* suspend devices */
@@ -2215,6 +2230,12 @@ static void ata_eh_handle_port_resume(st
 	if (!(ap->pflags & ATA_PFLAG_SUSPENDED))
 		goto done;
 
+#ifdef CONFIG_ATA_ACPI
+	if (!(ap->flags & ATA_FLAG_SATA)) {
+		/* Call _STM for PATA ports */
+		ata_acpi_push_timing(ap);
+	}
+#endif
 	if (ap->ops->port_resume)
 		rc = ap->ops->port_resume(ap);
 
drivers/ata/libata-sff.c +4 -0
index 7605028..c8aee20 100644
@@ -959,6 +959,10 @@ int ata_pci_init_one (struct pci_dev *pd
 
 	if ((port[0]->flags & ATA_FLAG_NO_LEGACY) == 0
 	    && (pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
+		printk(KERN_DEBUG "%s: NO_LEGACY == 0\n", __FUNCTION__);
+		port[0]->flags |= ATA_FLAG_PATA_MODE;
+		port[0]->flags &= ~ATA_FLAG_SATA;
+	    
 		/* TODO: What if one channel is in native mode ... */
 		pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
 		mask = (1 << 2) | (1 << 0);
drivers/ata/libata.h +11 -0
index a5ecb71..0b826eb 100644
@@ -43,6 +43,9 @@ extern struct workqueue_struct *ata_aux_
 extern int atapi_enabled;
 extern int atapi_dmadir;
 extern int libata_fua;
+#ifdef CONFIG_ATA_ACPI
+extern int libata_acpi;
+#endif
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
 extern int ata_rwcmd_protocol(struct ata_queued_cmd *qc);
 extern void ata_dev_disable(struct ata_device *dev);
@@ -119,4 +122,12 @@ extern void ata_scsi_error(struct Scsi_H
 extern void ata_port_wait_eh(struct ata_port *ap);
 extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
 
+/* libata-acpi.c */
+#ifdef CONFIG_ATA_ACPI
+extern int ata_acpi_push_id(struct ata_device *atadev);
+extern int ata_acpi_exec_tfs(struct ata_device *atadev);
+extern void ata_acpi_get_timing(struct ata_port *ap);
+extern void ata_acpi_push_timing(struct ata_port *ap);
+#endif
+
 #endif /* __LIBATA_H__ */
include/linux/libata.h +35 -1
index 0ddf16c..d9104c1 100644
@@ -35,6 +35,9 @@ #include <asm/io.h>
 #include <linux/ata.h>
 #include <linux/workqueue.h>
 #include <scsi/scsi_host.h>
+#ifdef CONFIG_ATA_ACPI
+#include <acpi/acpi.h>
+#endif
 
 #include <asm/libata-portmap.h>
 
@@ -46,7 +49,7 @@ #undef ATA_DEBUG		/* debugging output */
 #undef ATA_VERBOSE_DEBUG	/* yet more debugging output */
 #undef ATA_IRQ_TRAP		/* define to ack screaming irqs */
 #undef ATA_NDEBUG		/* define to disable quick runtime checks */
-#undef ATA_ENABLE_PATA		/* define to enable PATA support in some
+#define ATA_ENABLE_PATA		/* define to enable PATA support in some
 				 * low-level drivers */
 
 
@@ -97,6 +100,25 @@ static inline u32 ata_msg_init(int dval,
 	return (1 << dval) - 1;
 }
 
+#ifdef CONFIG_ATA_ACPI
+enum {
+    ATA_ACPI_SATA_MASK = 0xf0,
+    ATA_ACPI_SATA_SDD  = 0x10, /* Execute _SDD method */
+    ATA_ACPI_SATA_GTF  = 0x20, /* Execute _GTF method */
+    ATA_ACPI_SATA_TFX  = 0x40, /* Execute tf registers received via _GTF */
+    ATA_ACPI_PATA_MASK = 0x0f,
+    ATA_ACPI_PATA_GTM  = 0x01, /* Execute _GTM & _STM method */
+    ATA_ACPI_PATA_GTF  = 0x02, /* Execute _GTF method */
+    ATA_ACPI_PATA_TFX  = 0x04, /* Execute tf registers received via _GTF */
+
+    ATA_ACPI_GTF  = 0x02, /* Execute _GTF method */
+    ATA_ACPI_TFX  = 0x04, /* Execute tf registers received via _GTF */
+};
+
+#define ata_acpi_flags(a,f)  ata_id_is_sata((a)->id)?((f)>>4):(f)
+
+#endif
+
 /* defines only for the constants which don't work well as enums */
 #define ATA_TAG_POISON		0xfafbfcfdU
 
@@ -162,6 +184,7 @@ enum {
 	ATA_FLAG_SKIP_D2H_BSY	= (1 << 12), /* can't wait for the first D2H
 					      * Register FIS clearing BSY */
 	ATA_FLAG_DEBUGMSG	= (1 << 13),
+	ATA_FLAG_PATA_MODE	= (1 << 14), /* port in PATA mode */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -318,6 +341,7 @@ struct scsi_device;
 struct ata_port_operations;
 struct ata_port;
 struct ata_queued_cmd;
+struct GTM_buffer;
 
 /* typedefs */
 typedef void (*ata_qc_cb_t) (struct ata_queued_cmd *qc);
@@ -468,6 +492,11 @@ struct ata_device {
 
 	/* error history */
 	struct ata_ering	ering;
+
+#ifdef CONFIG_ATA_ACPI
+	/* ACPI objects info */
+	acpi_handle		obj_handle;
+#endif
 };
 
 /* Offset into struct ata_device.  Fields above it are maintained
@@ -554,6 +583,11 @@ struct ata_port {
 	pm_message_t		pm_mesg;
 	int			*pm_result;
 
+#ifdef CONFIG_ATA_ACPI
+	struct GTM_buffer	*gtm;
+	void			*gtm_object_area;
+#endif
+
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
#1 lore raw
Quoted 7 lines · On Thu, 28 Sep 2006 10:38:29 +0200 Hannes Reinecke <[email protected]> wrote:

Hi Jeff,

argl. Appearently I'm in Yeltsin mode.

Anyway: I'm also have finished an updated version of the ACPI patches. They are also based upon the patches by Randy Dunlap but with some improvements:

Hey,
I don't care who gets this stuff in, I just want it done with :). I'm going to go ahead and make the changes to my patches that Jeff suggested yesterday, but I'm also glad to help review your patches. But having looked at your attachment I feel like I'm missing something. Was there more than this? I don't see the patch that has libata-acpi.c in it.

Quoted 4 lines

- Omit the namespace walk for SATA devices. We can really trust the ACPI layer to find out the correct device. If not the ACPI is buggered anyway and we shouldn't even try to continue.

Not sure about this one - how does this work for devices that may not be present at boot time? Normally this is something we have to walk namespace for in order to discover -- at least with removable bay devices.

Quoted 23 lines

- Make the control over the ACPI execution more finegrained as some methods (most notably _GTF) are downright disastrous on PATA devices, whereas you really want to call _GTM / _STM on these to have them properly resumed after suspend to RAM.
- Only export the symbols we really have to :-)
- Proper integration with the new EH code. This is actually an error in the patch by Kristen; for new-style EH the ACPI functions will never be called :-(

If you consider this a duplicate I'm happy to rebase my patch on top of Kristens.

Comments etc are welcome.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
#2 lore raw
Quoted 5 lines · Kristen Carlson Accardi
Quoted 2 lines · On Thu, 28 Sep 2006 10:38:29 +0200 Hannes Reinecke <[email protected]> wrote:

Hi Jeff,

Quoted 9 lines
Quoted 4 lines

argl. Appearently I'm in Yeltsin mode.

Anyway: I'm also have finished an updated version of the ACPI patches. They are also based upon the patches by Randy Dunlap but with some improvements:

Hey,
I don't care who gets this stuff in, I just want it done with :).
I'm going to go ahead and make the changes to my patches that Jeff suggested yesterday, but I'm also glad to help review your patches. But having looked at your attachment I feel like I'm missing something. Was there more than this? I don't see the patch that has libata-acpi.c in it.

Oh, not again. Feeling like a git newbie :-(

Right. Fixed now.

Quoted 8 lines
Quoted 2 lines

- Omit the namespace walk for SATA devices. We can really trust the ACPI
layer to find out the correct device. If not the ACPI is buggered anyway and we shouldn't even try to continue.

Not sure about this one - how does this work for devices that may not be present at boot time? Normally this is something we have to walk namespace for in order to discover -- at least with removable bay devices.

Tricky that. Don't think so. Reasoning as follows:

ACPI is _really_ not designed for hotplugging. The entire ACPI code has to be present at boot time; it's not possible to modify it during run-time. Hence it already _knows_ about all possible controllers and the objects are already present in memory. And all removable bay controllers I've seen so far are either USB devices (which aren't controlled by ACPI anyway) or already present on the mainboard. There might not be a drive attached to them (ie if the bay is not present), but the controller is. And the controller is the only one which actually has a representation in sysfs, hence the correct ACPI object is already attached to it.
The SATA-device or PATA channel / PATA-drive don't have a direct representation in sysfs, hence we have to take care of identifying the correct ACPI object ourselves.
Which is what we're already do.
But the controller itself will already be present.
And even if not we can be reasonably sure that the driver core already has assigned the correct handle for us. But to be on the safe side I've put in a sanity check anyway.

So, attached is an updated version of my patch, based on the 'acpi' branch from libata-dev. We now even do _GTM & _STM and have the possiblity to call _GTF for PATA devices, too. Not that I recommend that.

Comments etc are welcome.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke			[email protected]
SuSE Linux Products GmbH		S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de
Documentation/kernel-parameters.txt +4 -4
index 50adcc5..bebf339 100644
@@ -258,6 +258,10 @@ running once the system is up.
 	arcrimi=	[HW,NET] ARCnet - "RIM I" (entirely mem-mapped) cards
 			Format: <io>,<irq>,<nodeID>
 
+	ata_acpi=	[LIBATA] Disables use of ACPI in libata suspend/resume
+			when set.
+			Format: <int>
+
 	ataflop=	[HW,M68k]
 
 	atarimouse=	[HW,MOUSE] Atari Mouse
@@ -1021,10 +1025,6 @@ running once the system is up.
 			emulation library even if a 387 maths coprocessor
 			is present.
 
-	noacpi		[LIBATA] Disables use of ACPI in libata suspend/resume
-			when set.
-			Format: <int>
-
 	noalign		[KNL,ARM]
 
 	noapic		[SMP,APIC] Tells the kernel to not make use of any
drivers/ata/Kconfig +5 -5
index f5a5694..96de687 100644
@@ -144,21 +144,21 @@ config SATA_VITESSE
 
 config SATA_INTEL_COMBINED
 	bool
-	depends on IDE=y && !BLK_DEV_IDE_SATA && (SATA_AHCI || ATA_PIIX)
+	depends on IDE!=n && !BLK_DEV_IDE_SATA && (SATA_AHCI || ATA_PIIX)
 	default y
 
-config SATA_ACPI
-	bool
+config ATA_ACPI
+	bool "Handle ATA-related ACPI objects"
 	depends on ACPI && PCI
 	default y
 	help
-	  This option adds support for SATA-related ACPI objects.
+	  This option adds support for ATA-related ACPI objects.
 	  These ACPI objects add the ability to retrieve taskfiles
 	  from the ACPI BIOS and write them to the disk controller.
 	  These objects may be related to performance, security,
 	  power management, or other areas.
 	  You can disable this at kernel boot time by using the
-	  option libata.noacpi=1
+	  option 'libata.ata_acpi=0'.
 
 config PATA_ALI
 	tristate "ALi PATA support (Experimental)"
drivers/ata/Makefile +2 -1
index 1d77823..26d72ee 100644
@@ -59,4 +59,5 @@ # Should be last libata driver
 obj-$(CONFIG_PATA_LEGACY)	+= pata_legacy.o
 
 libata-objs	:= libata-core.o libata-scsi.o libata-sff.o libata-eh.o
-libata-$(CONFIG_SATA_ACPI) += libata-acpi.o
+libata-$(CONFIG_ATA_ACPI) += libata-acpi.o
+
drivers/ata/libata-acpi.c +673 -384

1,293 lines not on this page · raw message

drivers/ata/libata-core.c +45 -17

111 lines not on this page · raw message

drivers/ata/libata-eh.c +21 -0
index 02b2b27..8735138 100644
@@ -1994,6 +1994,21 @@ static int ata_eh_recover(struct ata_por
 			down_xfermask = 1;
 			goto dev_fail;
 		}
+
+#ifdef CONFIG_ATA_ACPI
+		for (i = 0; i < ATA_MAX_DEVICES; i++) {
+			dev = &ap->device[i];
+
+			if (!ata_dev_enabled(dev))
+				continue;
+
+			/* Send down drive data via _SDD */
+			ata_acpi_push_id(dev);
+
+			/* retrieve and execute the ATA task file of _GTF */
+			ata_acpi_exec_tfs(dev);			
+		}
+#endif
 	}
 
 	/* suspend devices */
@@ -2218,6 +2233,12 @@ static void ata_eh_handle_port_resume(st
 	if (!(ap->pflags & ATA_PFLAG_SUSPENDED))
 		goto done;
 
+#ifdef CONFIG_ATA_ACPI
+	if (!(ap->flags & ATA_FLAG_SATA)) {
+		/* Call _STM for PATA ports */
+		ata_acpi_push_timing(ap);
+	}
+#endif
 	if (ap->ops->port_resume)
 		rc = ap->ops->port_resume(ap);
 
drivers/ata/libata-sff.c +5 -0
index 06daaa3..430ac1c 100644
@@ -976,6 +976,11 @@ int ata_pci_init_one (struct pci_dev *pd
 	if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
 		u8 tmp8;
 
+		if ((port[0]->host_flags & ATA_FLAG_NO_LEGACY) == 0) {
+			port[0]->host_flags |= ATA_FLAG_PATA_MODE;
+			port[0]->host_flags &= ~ATA_FLAG_SATA;
+		}
+
 		/* TODO: What if one channel is in native mode ... */
 		pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
 		mask = (1 << 2) | (1 << 0);
drivers/ata/libata.h +11 -1
index d565025..2b16dcc 100644
@@ -43,7 +43,9 @@ extern struct workqueue_struct *ata_aux_
 extern int atapi_enabled;
 extern int atapi_dmadir;
 extern int libata_fua;
-extern int noacpi;
+#ifdef CONFIG_ATA_ACPI
+extern int libata_acpi;
+#endif
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev);
 extern int ata_rwcmd_protocol(struct ata_queued_cmd *qc);
 extern void ata_dev_disable(struct ata_device *dev);
@@ -134,4 +136,12 @@ extern void ata_scsi_error(struct Scsi_H
 extern void ata_port_wait_eh(struct ata_port *ap);
 extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
 
+/* libata-acpi.c */
+#ifdef CONFIG_ATA_ACPI
+extern int ata_acpi_push_id(struct ata_device *atadev);
+extern int ata_acpi_exec_tfs(struct ata_device *atadev);
+extern void ata_acpi_get_timing(struct ata_port *ap);
+extern void ata_acpi_push_timing(struct ata_port *ap);
+#endif
+
 #endif /* __LIBATA_H__ */
include/linux/libata.h +29 -2
index a7bb6e4..f90b3e3 100644
@@ -106,6 +106,25 @@ static inline u32 ata_msg_init(int dval,
 	return (1 << dval) - 1;
 }
 
+#ifdef CONFIG_ATA_ACPI
+enum {
+    ATA_ACPI_SATA_MASK = 0xf0,
+    ATA_ACPI_SATA_SDD  = 0x10, /* Execute _SDD method */
+    ATA_ACPI_SATA_GTF  = 0x20, /* Execute _GTF method */
+    ATA_ACPI_SATA_TFX  = 0x40, /* Execute tf registers received via _GTF */
+    ATA_ACPI_PATA_MASK = 0x0f,
+    ATA_ACPI_PATA_GTM  = 0x01, /* Execute _GTM & _STM method */
+    ATA_ACPI_PATA_GTF  = 0x02, /* Execute _GTF method */
+    ATA_ACPI_PATA_TFX  = 0x04, /* Execute tf registers received via _GTF */
+
+    ATA_ACPI_GTF  = 0x02, /* Execute _GTF method */
+    ATA_ACPI_TFX  = 0x04, /* Execute tf registers received via _GTF */
+};
+
+#define ata_acpi_flags(a,f)  ata_id_is_sata((a)->id)?((f)>>4):(f)
+
+#endif
+
 /* defines only for the constants which don't work well as enums */
 #define ATA_TAG_POISON		0xfafbfcfdU
 
@@ -175,6 +194,7 @@ enum {
 	ATA_FLAG_SKIP_D2H_BSY	= (1 << 12), /* can't wait for the first D2H
 					      * Register FIS clearing BSY */
 	ATA_FLAG_DEBUGMSG	= (1 << 13),
+	ATA_FLAG_PATA_MODE	= (1 << 14), /* port in PATA mode */
 
 	/* The following flag belongs to ap->pflags but is kept in
 	 * ap->flags because it's referenced in many LLDs and will be
@@ -336,6 +356,7 @@ struct scsi_device;
 struct ata_port_operations;
 struct ata_port;
 struct ata_queued_cmd;
+struct GTM_buffer;
 
 /* typedefs */
 typedef void (*ata_qc_cb_t) (struct ata_queued_cmd *qc);
@@ -495,9 +516,10 @@ struct ata_device {
 	/* error history */
 	struct ata_ering	ering;
 	unsigned int		horkage;	/* List of broken features */
-#ifdef CONFIG_SATA_ACPI
+
+#ifdef CONFIG_ATA_ACPI
 	/* ACPI objects info */
-	acpi_handle obj_handle;
+	acpi_handle		obj_handle;
 #endif
 };
 
@@ -585,6 +607,11 @@ struct ata_port {
 	pm_message_t		pm_mesg;
 	int			*pm_result;
 
+#ifdef CONFIG_ATA_ACPI
+	struct GTM_buffer	*gtm;
+	void			*gtm_object_area;
+#endif
+
 	void			*private_data;
 
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */