linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] PCI: export __pci_reenable_device()
@ 2007-07-10  6:36 Tejun Heo
  2007-07-10  6:55 ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
  2007-07-10 16:17 ` [PATCH 1/2] PCI: export __pci_reenable_device() Greg KH
  0 siblings, 2 replies; 14+ messages in thread
From: Tejun Heo @ 2007-07-10  6:36 UTC (permalink / raw)
  To: Jeff Garzik, greg, linux-ide, owner-linux-pci

Some odd ACPI implementations choke if certain controller is disabled
when ACPI suspend is invoked but we still need to make sure the PCI
device is enabled during resume.  Simply using pci_enable_device()
unbalances device enable count.  Export __pci_reenable_device().

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/pci/pci.c   |    1 +
 drivers/pci/pci.h   |    1 -
 include/linux/pci.h |    1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -1427,6 +1427,7 @@ early_param("pci", pci_setup);
 device_initcall(pci_init);
 
 EXPORT_SYMBOL_GPL(pci_restore_bars);
+EXPORT_SYMBOL(__pci_reenable_device);
 EXPORT_SYMBOL(pci_enable_device_bars);
 EXPORT_SYMBOL(pci_enable_device);
 EXPORT_SYMBOL(pcim_enable_device);
Index: work/drivers/pci/pci.h
===================================================================
--- work.orig/drivers/pci/pci.h
+++ work/drivers/pci/pci.h
@@ -1,6 +1,5 @@
 /* Functions internal to the PCI core code */
 
-extern int __must_check __pci_reenable_device(struct pci_dev *);
 extern int pci_uevent(struct device *dev, char **envp, int num_envp,
 		      char *buffer, int buffer_size);
 extern int pci_create_sysfs_dev_files(struct pci_dev *pdev);
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -531,6 +531,7 @@ static inline int pci_write_config_dword
 
 int __must_check pci_enable_device(struct pci_dev *dev);
 int __must_check pci_enable_device_bars(struct pci_dev *dev, int mask);
+int __must_check __pci_reenable_device(struct pci_dev *);
 int __must_check pcim_enable_device(struct pci_dev *pdev);
 void pcim_pin_device(struct pci_dev *pdev);
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops
  2007-07-10  6:36 [PATCH 1/2] PCI: export __pci_reenable_device() Tejun Heo
@ 2007-07-10  6:55 ` Tejun Heo
  2007-07-24 20:59   ` Jeff Garzik
  2007-07-10 16:17 ` [PATCH 1/2] PCI: export __pci_reenable_device() Greg KH
  1 sibling, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2007-07-10  6:55 UTC (permalink / raw)
  To: Jeff Garzik, greg, linux-ide, owner-linux-pci

ACPI implementations in several TOSHIBA laptops are weird and burn cpu
cycles for tens of seconds while trying to suspend if the PCI device
for the ATA controller is disabled when the ACPI suspend is called.

This patch uses DMI to match those machines and bypass device disable
on those machines during suspend.  As the device needs to be put into
enabled state on resume without affecting PCI enable count, matching
resume callback uses __pci_reenable_device().

This bug is reported in bugzilla bug 7780.

  http://bugzilla.kernel.org/show_bug.cgi?id=7780

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

Index: work/drivers/ata/ata_piix.c
===================================================================
--- work.orig/drivers/ata/ata_piix.c
+++ work/drivers/ata/ata_piix.c
@@ -91,6 +91,7 @@
 #include <linux/device.h>
 #include <scsi/scsi_host.h>
 #include <linux/libata.h>
+#include <linux/dmi.h>
 
 #define DRV_NAME	"ata_piix"
 #define DRV_VERSION	"2.11"
@@ -140,6 +141,9 @@ enum {
 	RV			= -3, /* reserved */
 
 	PIIX_AHCI_DEVICE	= 6,
+
+	/* host->flags bits */
+	PIIX_HOST_BROKEN_SUSPEND = (1 << 24),
 };
 
 struct piix_map_db {
@@ -159,6 +163,10 @@ static void piix_set_piomode (struct ata
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
 static void ich_set_dmamode (struct ata_port *ap, struct ata_device *adev);
 static int ich_pata_cable_detect(struct ata_port *ap);
+#ifdef CONFIG_PM
+static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
+static int piix_pci_device_resume(struct pci_dev *pdev);
+#endif
 
 static unsigned int in_module_init = 1;
 
@@ -255,8 +263,8 @@ static struct pci_driver piix_pci_driver
 	.probe			= piix_init_one,
 	.remove			= ata_pci_remove_one,
 #ifdef CONFIG_PM
-	.suspend		= ata_pci_device_suspend,
-	.resume			= ata_pci_device_resume,
+	.suspend		= piix_pci_device_suspend,
+	.resume			= piix_pci_device_resume,
 #endif
 };
 
@@ -878,6 +886,107 @@ static void ich_set_dmamode (struct ata_
 	do_pata_set_dmamode(ap, adev, 1);
 }
 
+#ifdef CONFIG_PM
+static struct dmi_system_id piix_broken_suspend_dmi_table[] = {
+	{
+		.ident = "TECRA M5",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TECRA M5"),
+		},
+	},
+	{
+		.ident = "Satellite U200",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U200"),
+		},
+	},
+	{
+		.ident = "Satellite U205",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U205"),
+		},
+	},
+	{
+		.ident = "Portege M500",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE M500"),
+		},
+	},
+	{ }
+};
+
+static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	unsigned long flags;
+	int rc = 0;
+
+	rc = ata_host_suspend(host, mesg);
+	if (rc)
+		return rc;
+
+	/* Some braindamaged ACPI suspend implementations expect the
+	 * controller to be awake on entry; otherwise, it burns cpu
+	 * cycles and power trying to do something to the sleeping
+	 * beauty.
+	 */
+	if (dmi_check_system(piix_broken_suspend_dmi_table) &&
+	    mesg.event == PM_EVENT_SUSPEND) {
+		pci_save_state(pdev);
+
+		/* mark its power state as "unknown", since we don't
+		 * know if e.g. the BIOS will change its device state
+		 * when we suspend.
+		 */
+		if (pdev->current_state == PCI_D0)
+			pdev->current_state = PCI_UNKNOWN;
+
+		/* tell resume that it's waking up from broken suspend */
+		spin_lock_irqsave(&host->lock, flags);
+		host->flags |= PIIX_HOST_BROKEN_SUSPEND;
+		spin_unlock_irqrestore(&host->lock, flags);
+	} else
+		ata_pci_device_do_suspend(pdev, mesg);
+
+	return 0;
+}
+
+static int piix_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	unsigned long flags;
+	int rc;
+
+	if (host->flags & PIIX_HOST_BROKEN_SUSPEND) {
+		spin_lock_irqsave(&host->lock, flags);
+		host->flags &= ~PIIX_HOST_BROKEN_SUSPEND;
+		spin_unlock_irqrestore(&host->lock, flags);
+
+		pci_set_power_state(pdev, PCI_D0);
+		pci_restore_state(pdev);
+
+		/* PCI device wasn't disabled during suspend.  Use
+		 * __pci_reenable_device() to avoid affecting the
+		 * enable count.
+		 */
+		rc = __pci_reenable_device(pdev);
+		if (rc)
+			dev_printk(KERN_ERR, &pdev->dev, "failed to enable "
+				   "device after resume (%d)\n", rc);
+	} else
+		rc = ata_pci_device_do_resume(pdev);
+
+	if (rc == 0)
+		ata_host_resume(host);
+
+	return rc;
+}
+#endif
+
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h
+++ work/include/linux/libata.h
@@ -217,6 +217,8 @@ enum {
 	ATA_HOST_SIMPLEX	= (1 << 0),	/* Host is simplex, one DMA channel per host only */
 	ATA_HOST_STARTED	= (1 << 1),	/* Host started */
 
+	/* bits 24:31 of host->flags are reserved for LLD specific flags */
+
 	/* various lengths of time */
 	ATA_TMOUT_BOOT		= 30 * HZ,	/* heuristic */
 	ATA_TMOUT_BOOT_QUICK	= 7 * HZ,	/* heuristic */

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] PCI: export __pci_reenable_device()
  2007-07-10  6:36 [PATCH 1/2] PCI: export __pci_reenable_device() Tejun Heo
  2007-07-10  6:55 ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
@ 2007-07-10 16:17 ` Greg KH
  2007-07-11 10:32   ` [PATCH 1/2] PCI: rename and " Tejun Heo
  1 sibling, 1 reply; 14+ messages in thread
From: Greg KH @ 2007-07-10 16:17 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Jeff Garzik, linux-ide, owner-linux-pci

On Tue, Jul 10, 2007 at 03:36:05PM +0900, Tejun Heo wrote:
> Some odd ACPI implementations choke if certain controller is disabled
> when ACPI suspend is invoked but we still need to make sure the PCI
> device is enabled during resume.  Simply using pci_enable_device()
> unbalances device enable count.  Export __pci_reenable_device().

We should rename it to pci_reenable_device() instead of the leading "__"
as that should denote an internal-only function.

Care to do that and respin this series?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] PCI: rename and export __pci_reenable_device()
  2007-07-10 16:17 ` [PATCH 1/2] PCI: export __pci_reenable_device() Greg KH
@ 2007-07-11 10:32   ` Tejun Heo
  2007-07-11 10:32     ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
  2007-07-17 17:05     ` [PATCH 1/2] PCI: rename and export __pci_reenable_device() Greg KH
  0 siblings, 2 replies; 14+ messages in thread
From: Tejun Heo @ 2007-07-11 10:32 UTC (permalink / raw)
  To: Greg KH; +Cc: Jeff Garzik, linux-ide, owner-linux-pci

Some odd ACPI implementations choke if certain controller is disabled
when ACPI suspend is invoked but we still need to make sure the PCI
device is enabled during resume.  Simply using pci_enable_device()
unbalances device enable count.

Drop leading underscores from __pci_reenable_device() and export it.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Sure, here it is.

 drivers/pci/pci-driver.c |    2 +-
 drivers/pci/pci.c        |    6 +++---
 drivers/pci/pci.h        |    1 -
 include/linux/pci.h      |    1 +
 4 files changed, 5 insertions(+), 5 deletions(-)

Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -695,14 +695,13 @@ static int do_pci_enable_device(struct p
 }
 
 /**
- * __pci_reenable_device - Resume abandoned device
+ * pci_reenable_device - Resume abandoned device
  * @dev: PCI device to be resumed
  *
  *  Note this function is a backend of pci_default_resume and is not supposed
  *  to be called by normal code, write proper resume handler and use it instead.
  */
-int
-__pci_reenable_device(struct pci_dev *dev)
+int pci_reenable_device(struct pci_dev *dev)
 {
 	if (atomic_read(&dev->enable_cnt))
 		return do_pci_enable_device(dev, (1 << PCI_NUM_RESOURCES) - 1);
@@ -1427,6 +1426,7 @@ early_param("pci", pci_setup);
 device_initcall(pci_init);
 
 EXPORT_SYMBOL_GPL(pci_restore_bars);
+EXPORT_SYMBOL(pci_reenable_device);
 EXPORT_SYMBOL(pci_enable_device_bars);
 EXPORT_SYMBOL(pci_enable_device);
 EXPORT_SYMBOL(pcim_enable_device);
Index: work/drivers/pci/pci.h
===================================================================
--- work.orig/drivers/pci/pci.h
+++ work/drivers/pci/pci.h
@@ -1,6 +1,5 @@
 /* Functions internal to the PCI core code */
 
-extern int __must_check __pci_reenable_device(struct pci_dev *);
 extern int pci_uevent(struct device *dev, char **envp, int num_envp,
 		      char *buffer, int buffer_size);
 extern int pci_create_sysfs_dev_files(struct pci_dev *pdev);
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -531,6 +531,7 @@ static inline int pci_write_config_dword
 
 int __must_check pci_enable_device(struct pci_dev *dev);
 int __must_check pci_enable_device_bars(struct pci_dev *dev, int mask);
+int __must_check pci_reenable_device(struct pci_dev *);
 int __must_check pcim_enable_device(struct pci_dev *pdev);
 void pcim_pin_device(struct pci_dev *pdev);
 
Index: work/drivers/pci/pci-driver.c
===================================================================
--- work.orig/drivers/pci/pci-driver.c
+++ work/drivers/pci/pci-driver.c
@@ -310,7 +310,7 @@ static int pci_default_resume(struct pci
 	/* restore the PCI config space */
 	pci_restore_state(pci_dev);
 	/* if the device was enabled before suspend, reenable */
-	retval = __pci_reenable_device(pci_dev);
+	retval = pci_reenable_device(pci_dev);
 	/* if the device was busmaster before the suspend, make it busmaster again */
 	if (pci_dev->is_busmaster)
 		pci_set_master(pci_dev);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops
  2007-07-11 10:32   ` [PATCH 1/2] PCI: rename and " Tejun Heo
@ 2007-07-11 10:32     ` Tejun Heo
  2007-07-17 17:05     ` [PATCH 1/2] PCI: rename and export __pci_reenable_device() Greg KH
  1 sibling, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2007-07-11 10:32 UTC (permalink / raw)
  To: Greg KH; +Cc: Jeff Garzik, linux-ide, owner-linux-pci

ACPI implementations in several TOSHIBA laptops are weird and burn cpu
cycles for tens of seconds while trying to suspend if the PCI device
for the ATA controller is disabled when the ACPI suspend is called.

This patch uses DMI to match those machines and bypass device disable
on those machines during suspend.  As the device needs to be put into
enabled state on resume without affecting PCI enable count, matching
resume callback uses pci_reenable_device().

This bug is reported in bugzilla bug 7780.

  http://bugzilla.kernel.org/show_bug.cgi?id=7780

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Updated to fit renamed pci_reenable_device().

 drivers/ata/ata_piix.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/libata.h |    2 
 2 files changed, 113 insertions(+), 2 deletions(-)

Index: work/drivers/ata/ata_piix.c
===================================================================
--- work.orig/drivers/ata/ata_piix.c
+++ work/drivers/ata/ata_piix.c
@@ -91,6 +91,7 @@
 #include <linux/device.h>
 #include <scsi/scsi_host.h>
 #include <linux/libata.h>
+#include <linux/dmi.h>
 
 #define DRV_NAME	"ata_piix"
 #define DRV_VERSION	"2.11"
@@ -140,6 +141,9 @@ enum {
 	RV			= -3, /* reserved */
 
 	PIIX_AHCI_DEVICE	= 6,
+
+	/* host->flags bits */
+	PIIX_HOST_BROKEN_SUSPEND = (1 << 24),
 };
 
 struct piix_map_db {
@@ -159,6 +163,10 @@ static void piix_set_piomode (struct ata
 static void piix_set_dmamode (struct ata_port *ap, struct ata_device *adev);
 static void ich_set_dmamode (struct ata_port *ap, struct ata_device *adev);
 static int ich_pata_cable_detect(struct ata_port *ap);
+#ifdef CONFIG_PM
+static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg);
+static int piix_pci_device_resume(struct pci_dev *pdev);
+#endif
 
 static unsigned int in_module_init = 1;
 
@@ -253,8 +261,8 @@ static struct pci_driver piix_pci_driver
 	.probe			= piix_init_one,
 	.remove			= ata_pci_remove_one,
 #ifdef CONFIG_PM
-	.suspend		= ata_pci_device_suspend,
-	.resume			= ata_pci_device_resume,
+	.suspend		= piix_pci_device_suspend,
+	.resume			= piix_pci_device_resume,
 #endif
 };
 
@@ -868,6 +876,107 @@ static void ich_set_dmamode (struct ata_
 	do_pata_set_dmamode(ap, adev, 1);
 }
 
+#ifdef CONFIG_PM
+static struct dmi_system_id piix_broken_suspend_dmi_table[] = {
+	{
+		.ident = "TECRA M5",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TECRA M5"),
+		},
+	},
+	{
+		.ident = "Satellite U200",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U200"),
+		},
+	},
+	{
+		.ident = "Satellite U205",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U205"),
+		},
+	},
+	{
+		.ident = "Portege M500",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE M500"),
+		},
+	},
+	{ }
+};
+
+static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	unsigned long flags;
+	int rc = 0;
+
+	rc = ata_host_suspend(host, mesg);
+	if (rc)
+		return rc;
+
+	/* Some braindamaged ACPI suspend implementations expect the
+	 * controller to be awake on entry; otherwise, it burns cpu
+	 * cycles and power trying to do something to the sleeping
+	 * beauty.
+	 */
+	if (dmi_check_system(piix_broken_suspend_dmi_table) &&
+	    mesg.event == PM_EVENT_SUSPEND) {
+		pci_save_state(pdev);
+
+		/* mark its power state as "unknown", since we don't
+		 * know if e.g. the BIOS will change its device state
+		 * when we suspend.
+		 */
+		if (pdev->current_state == PCI_D0)
+			pdev->current_state = PCI_UNKNOWN;
+
+		/* tell resume that it's waking up from broken suspend */
+		spin_lock_irqsave(&host->lock, flags);
+		host->flags |= PIIX_HOST_BROKEN_SUSPEND;
+		spin_unlock_irqrestore(&host->lock, flags);
+	} else
+		ata_pci_device_do_suspend(pdev, mesg);
+
+	return 0;
+}
+
+static int piix_pci_device_resume(struct pci_dev *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	unsigned long flags;
+	int rc;
+
+	if (host->flags & PIIX_HOST_BROKEN_SUSPEND) {
+		spin_lock_irqsave(&host->lock, flags);
+		host->flags &= ~PIIX_HOST_BROKEN_SUSPEND;
+		spin_unlock_irqrestore(&host->lock, flags);
+
+		pci_set_power_state(pdev, PCI_D0);
+		pci_restore_state(pdev);
+
+		/* PCI device wasn't disabled during suspend.  Use
+		 * pci_reenable_device() to avoid unbalancing pdev
+		 * enable count.
+		 */
+		rc = pci_reenable_device(pdev);
+		if (rc)
+			dev_printk(KERN_ERR, &pdev->dev, "failed to enable "
+				   "device after resume (%d)\n", rc);
+	} else
+		rc = ata_pci_device_do_resume(pdev);
+
+	if (rc == 0)
+		ata_host_resume(host);
+
+	return rc;
+}
+#endif
+
 #define AHCI_PCI_BAR 5
 #define AHCI_GLOBAL_CTL 0x04
 #define AHCI_ENABLE (1 << 31)
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h
+++ work/include/linux/libata.h
@@ -213,6 +213,8 @@ enum {
 	ATA_HOST_SIMPLEX	= (1 << 0),	/* Host is simplex, one DMA channel per host only */
 	ATA_HOST_STARTED	= (1 << 1),	/* Host started */
 
+	/* bits 24:31 of host->flags are reserved for LLD specific flags */
+
 	/* various lengths of time */
 	ATA_TMOUT_BOOT		= 30 * HZ,	/* heuristic */
 	ATA_TMOUT_BOOT_QUICK	= 7 * HZ,	/* heuristic */

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] PCI: rename and export __pci_reenable_device()
  2007-07-11 10:32   ` [PATCH 1/2] PCI: rename and " Tejun Heo
  2007-07-11 10:32     ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
@ 2007-07-17 17:05     ` Greg KH
  1 sibling, 0 replies; 14+ messages in thread
From: Greg KH @ 2007-07-17 17:05 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Jeff Garzik, linux-ide, owner-linux-pci

On Wed, Jul 11, 2007 at 07:32:04PM +0900, Tejun Heo wrote:
> Some odd ACPI implementations choke if certain controller is disabled
> when ACPI suspend is invoked but we still need to make sure the PCI
> device is enabled during resume.  Simply using pci_enable_device()
> unbalances device enable count.
> 
> Drop leading underscores from __pci_reenable_device() and export it.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

Jeff, I'm guessing this will go through your tree as the ata patch after
this needs this change.

If so, feel free to add my:

	Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

to the patch, I have no objection to it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops
  2007-07-10  6:55 ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
@ 2007-07-24 20:59   ` Jeff Garzik
  2007-07-24 21:57     ` Alan Cox
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Garzik @ 2007-07-24 20:59 UTC (permalink / raw)
  To: Tejun Heo; +Cc: greg, linux-ide, owner-linux-pci

Tejun Heo wrote:
> +	/* Some braindamaged ACPI suspend implementations expect the
> +	 * controller to be awake on entry; otherwise, it burns cpu
> +	 * cycles and power trying to do something to the sleeping
> +	 * beauty.
> +	 */


applied 1-2... HOWEVER...

I would not classify this as braindead.  I have applied these patches, 
but it must be emphasized that this is a temporary fix only.

Overall, it is unfortunately but libata must be aware of the state the 
controller should be in, for suspend and resume.  I bet some more 
suspend/resume problems can be isolated to a hardware state mismatch, 
where BIOS expects one thing but Linux has configured the hardware 
differently.

A more generalized rule to consider for the future would be to ensure 
that ALL suspend routines put the hardware back into its pre-Linux init 
mode before suspending.  i.e. that means if we turned on some enhanced 
mode, we must switch back to legacy mode before calling pci_xxx to 
suspend our device.

	Jeff





^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops
  2007-07-24 20:59   ` Jeff Garzik
@ 2007-07-24 21:57     ` Alan Cox
  2007-07-24 22:05       ` Jeff Garzik
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Cox @ 2007-07-24 21:57 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Tejun Heo, greg, linux-ide, owner-linux-pci

> A more generalized rule to consider for the future would be to ensure 
> that ALL suspend routines put the hardware back into its pre-Linux init 
> mode before suspending.  i.e. that means if we turned on some enhanced 
> mode, we must switch back to legacy mode before calling pci_xxx to 
> suspend our device.

This sounds good but for some hardware you can't get it back into that
state and there is no guarantee some laptops don't have ACPI suspend
paths that "know" Windows will have reconfigured the hardware.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops
  2007-07-24 21:57     ` Alan Cox
@ 2007-07-24 22:05       ` Jeff Garzik
  2007-07-27  5:43         ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Tejun Heo
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Garzik @ 2007-07-24 22:05 UTC (permalink / raw)
  To: Alan Cox; +Cc: Tejun Heo, greg, linux-ide, owner-linux-pci

Alan Cox wrote:
>> A more generalized rule to consider for the future would be to ensure 
>> that ALL suspend routines put the hardware back into its pre-Linux init 
>> mode before suspending.  i.e. that means if we turned on some enhanced 
>> mode, we must switch back to legacy mode before calling pci_xxx to 
>> suspend our device.
> 
> This sounds good but for some hardware you can't get it back into that
> state and there is no guarantee some laptops don't have ACPI suspend
> paths that "know" Windows will have reconfigured the hardware.

Good point.  To generalize a bit more:  it is quite possible that many 
situations will require us to put the hardware device into a certain 
mode before suspending, a mode that Linux may not currently be operating 
in.  ata_piix is definitely not going to be the only one needing this 
change, long term.

	Jeff




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device()
  2007-07-24 22:05       ` Jeff Garzik
@ 2007-07-27  5:43         ` Tejun Heo
  2007-07-27  5:53           ` [PATCH] ata_piix: implement piix_borken_suspend() Tejun Heo
                             ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Tejun Heo @ 2007-07-27  5:43 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Alan Cox, greg, linux-ide, owner-linux-pci

Rename __pci_reenable_device() to pci_reenable_device().

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Jeff, Greg wanted to drop the preceding underscores before exporting
the function and the updated patch was posted but the earlier version
was applied.  This patch renames the function.

 drivers/ata/ata_piix.c   |    6 +++---
 drivers/pci/pci-driver.c |    2 +-
 drivers/pci/pci.c        |    7 +++----
 include/linux/pci.h      |    2 +-
 4 files changed, 8 insertions(+), 9 deletions(-)

Index: work/drivers/ata/ata_piix.c
===================================================================
--- work.orig/drivers/ata/ata_piix.c
+++ work/drivers/ata/ata_piix.c
@@ -973,10 +973,10 @@ static int piix_pci_device_resume(struct
 		pci_restore_state(pdev);
 
 		/* PCI device wasn't disabled during suspend.  Use
-		 * __pci_reenable_device() to avoid affecting the
-		 * enable count.
+		 * pci_reenable_device() to avoid affecting the enable
+		 * count.
 		 */
-		rc = __pci_reenable_device(pdev);
+		rc = pci_reenable_device(pdev);
 		if (rc)
 			dev_printk(KERN_ERR, &pdev->dev, "failed to enable "
 				   "device after resume (%d)\n", rc);
Index: work/drivers/pci/pci-driver.c
===================================================================
--- work.orig/drivers/pci/pci-driver.c
+++ work/drivers/pci/pci-driver.c
@@ -310,7 +310,7 @@ static int pci_default_resume(struct pci
 	/* restore the PCI config space */
 	pci_restore_state(pci_dev);
 	/* if the device was enabled before suspend, reenable */
-	retval = __pci_reenable_device(pci_dev);
+	retval = pci_reenable_device(pci_dev);
 	/* if the device was busmaster before the suspend, make it busmaster again */
 	if (pci_dev->is_busmaster)
 		pci_set_master(pci_dev);
Index: work/drivers/pci/pci.c
===================================================================
--- work.orig/drivers/pci/pci.c
+++ work/drivers/pci/pci.c
@@ -695,14 +695,13 @@ static int do_pci_enable_device(struct p
 }
 
 /**
- * __pci_reenable_device - Resume abandoned device
+ * pci_reenable_device - Resume abandoned device
  * @dev: PCI device to be resumed
  *
  *  Note this function is a backend of pci_default_resume and is not supposed
  *  to be called by normal code, write proper resume handler and use it instead.
  */
-int
-__pci_reenable_device(struct pci_dev *dev)
+int pci_reenable_device(struct pci_dev *dev)
 {
 	if (atomic_read(&dev->enable_cnt))
 		return do_pci_enable_device(dev, (1 << PCI_NUM_RESOURCES) - 1);
@@ -1604,7 +1603,7 @@ early_param("pci", pci_setup);
 device_initcall(pci_init);
 
 EXPORT_SYMBOL_GPL(pci_restore_bars);
-EXPORT_SYMBOL(__pci_reenable_device);
+EXPORT_SYMBOL(pci_reenable_device);
 EXPORT_SYMBOL(pci_enable_device_bars);
 EXPORT_SYMBOL(pci_enable_device);
 EXPORT_SYMBOL(pcim_enable_device);
Index: work/include/linux/pci.h
===================================================================
--- work.orig/include/linux/pci.h
+++ work/include/linux/pci.h
@@ -534,7 +534,7 @@ static inline int pci_write_config_dword
 
 int __must_check pci_enable_device(struct pci_dev *dev);
 int __must_check pci_enable_device_bars(struct pci_dev *dev, int mask);
-int __must_check __pci_reenable_device(struct pci_dev *);
+int __must_check pci_reenable_device(struct pci_dev *);
 int __must_check pcim_enable_device(struct pci_dev *pdev);
 void pcim_pin_device(struct pci_dev *pdev);
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] ata_piix: implement piix_borken_suspend()
  2007-07-27  5:43         ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Tejun Heo
@ 2007-07-27  5:53           ` Tejun Heo
  2007-07-27  5:55             ` [PATCH] ata_piix: add Tecra M3 to broken suspend blacklist Tejun Heo
  2007-07-27  6:22           ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Greg KH
  2007-08-01 14:02           ` Jeff Garzik
  2 siblings, 1 reply; 14+ messages in thread
From: Tejun Heo @ 2007-07-27  5:53 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Jeff Garzik, Alan Cox, greg, linux-ide, owner-linux-pci

Separate out broken suspend blacklist matching into
piix_broken_suspend().

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ata_piix.c |   60 ++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

Index: work/drivers/ata/ata_piix.c
===================================================================
--- work.orig/drivers/ata/ata_piix.c
+++ work/drivers/ata/ata_piix.c
@@ -890,37 +890,38 @@ static void ich_set_dmamode (struct ata_
 }
 
 #ifdef CONFIG_PM
-static struct dmi_system_id piix_broken_suspend_dmi_table[] = {
-	{
-		.ident = "TECRA M5",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "TECRA M5"),
-		},
-	},
-	{
-		.ident = "Satellite U200",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U200"),
+static int piix_broken_suspend(void)
+{
+	static struct dmi_system_id sysids[] = {
+		{
+			.ident = "TECRA M5",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "TECRA M5"),
+			},
 		},
-	},
-	{
-		.ident = "Satellite U205",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U205"),
+		{
+			.ident = "Satellite U205",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "Satellite U205"),
+			},
 		},
-	},
-	{
-		.ident = "Portege M500",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE M500"),
+		{
+			.ident = "Portege M500",
+			.matches = {
+				DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"),
+				DMI_MATCH(DMI_PRODUCT_NAME, "PORTEGE M500"),
+			},
 		},
-	},
-	{ }
-};
+		{ }
+	};
+
+	if (dmi_check_system(sysids))
+		return 1;
+
+	return 0;
+}
 
 static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
 {
@@ -937,8 +938,7 @@ static int piix_pci_device_suspend(struc
 	 * cycles and power trying to do something to the sleeping
 	 * beauty.
 	 */
-	if (dmi_check_system(piix_broken_suspend_dmi_table) &&
-	    mesg.event == PM_EVENT_SUSPEND) {
+	if (piix_broken_suspend() && mesg.event == PM_EVENT_SUSPEND) {
 		pci_save_state(pdev);
 
 		/* mark its power state as "unknown", since we don't

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] ata_piix: add Tecra M3 to broken suspend blacklist
  2007-07-27  5:53           ` [PATCH] ata_piix: implement piix_borken_suspend() Tejun Heo
@ 2007-07-27  5:55             ` Tejun Heo
  0 siblings, 0 replies; 14+ messages in thread
From: Tejun Heo @ 2007-07-27  5:55 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Jeff Garzik, Alan Cox, greg, linux-ide, owner-linux-pci

Add Tecra M3 to the broken suspend blacklist.  Tecra M3 doesn't have
proper DMI_PRODUCT_NAME but has an OEM_STRING instead.  Match it.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ata_piix.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: work/drivers/ata/ata_piix.c
===================================================================
--- work.orig/drivers/ata/ata_piix.c
+++ work/drivers/ata/ata_piix.c
@@ -916,10 +916,18 @@ static int piix_broken_suspend(void)
 		},
 		{ }
 	};
+	static const char *oemstrs[] = {
+		"Tecra M3,",
+	};
+	int i;
 
 	if (dmi_check_system(sysids))
 		return 1;
 
+	for (i = 0; i < ARRAY_SIZE(oemstrs); i++)
+		if (dmi_find_device(DMI_DEV_TYPE_OEM_STRING, oemstrs[i], NULL))
+			return 1;
+
 	return 0;
 }
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device()
  2007-07-27  5:43         ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Tejun Heo
  2007-07-27  5:53           ` [PATCH] ata_piix: implement piix_borken_suspend() Tejun Heo
@ 2007-07-27  6:22           ` Greg KH
  2007-08-01 14:02           ` Jeff Garzik
  2 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2007-07-27  6:22 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Jeff Garzik, Alan Cox, linux-ide, owner-linux-pci

On Fri, Jul 27, 2007 at 02:43:35PM +0900, Tejun Heo wrote:
> Rename __pci_reenable_device() to pci_reenable_device().
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> Jeff, Greg wanted to drop the preceding underscores before exporting
> the function and the updated patch was posted but the earlier version
> was applied.  This patch renames the function.

Yeah, that is nicer :)

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>


thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device()
  2007-07-27  5:43         ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Tejun Heo
  2007-07-27  5:53           ` [PATCH] ata_piix: implement piix_borken_suspend() Tejun Heo
  2007-07-27  6:22           ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Greg KH
@ 2007-08-01 14:02           ` Jeff Garzik
  2 siblings, 0 replies; 14+ messages in thread
From: Jeff Garzik @ 2007-08-01 14:02 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Alan Cox, greg, linux-ide, owner-linux-pci

Tejun Heo wrote:
> Rename __pci_reenable_device() to pci_reenable_device().
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> Jeff, Greg wanted to drop the preceding underscores before exporting
> the function and the updated patch was posted but the earlier version
> was applied.  This patch renames the function.

applied these three to #upstream-fixes



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-08-01 14:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-10  6:36 [PATCH 1/2] PCI: export __pci_reenable_device() Tejun Heo
2007-07-10  6:55 ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
2007-07-24 20:59   ` Jeff Garzik
2007-07-24 21:57     ` Alan Cox
2007-07-24 22:05       ` Jeff Garzik
2007-07-27  5:43         ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Tejun Heo
2007-07-27  5:53           ` [PATCH] ata_piix: implement piix_borken_suspend() Tejun Heo
2007-07-27  5:55             ` [PATCH] ata_piix: add Tecra M3 to broken suspend blacklist Tejun Heo
2007-07-27  6:22           ` [PATCH] pci: rename __pci_reenable_device() to pci_reenable_device() Greg KH
2007-08-01 14:02           ` Jeff Garzik
2007-07-10 16:17 ` [PATCH 1/2] PCI: export __pci_reenable_device() Greg KH
2007-07-11 10:32   ` [PATCH 1/2] PCI: rename and " Tejun Heo
2007-07-11 10:32     ` [PATCH 2/2] ata_piix: fix suspend/resume for some TOSHIBA laptops Tejun Heo
2007-07-17 17:05     ` [PATCH 1/2] PCI: rename and export __pci_reenable_device() Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).