[RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Carl-Daniel Hailfinger]

Hi,

machines with the asus_hides_smbus "feature" have the problem
that the smbus is disabled again after suspend-to-RAM. This
causes all sorts of problems, the worst being a total fan
failure on my Samsung P35 notebook after STR and STD.

References: Novell bugzilla #173420.

This (totally ugly) patch fixes it.
Comments/criticism welcome.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
-- 
http://www.hailfinger.org/

--- linux-2.6.16.14/drivers/acpi/sleep/main.c	2006-05-09 19:02:39.000000000 +0200
+++ linux-2.6.16.14/drivers/acpi/sleep/main.c	2006-05-10 01:47:07.000000000 +0200
@@ -10,6 +10,7 @@
  *
  */

+#include <linux/pci.h>
 #include <linux/delay.h>
 #include <linux/irq.h>
 #include <linux/dmi.h>
@@ -24,6 +25,7 @@
 static struct pm_ops acpi_pm_ops;

 extern void do_suspend_lowlevel(void);
+extern struct pci_dev *asus_lpc_pcidev;

 static u32 acpi_suspend_states[] = {
 	[PM_SUSPEND_ON] = ACPI_STATE_S0,
@@ -116,6 +118,9 @@
 	if (pm_state > PM_SUSPEND_STANDBY)
 		acpi_restore_state_mem();

+	if (asus_lpc_pcidev)
+		pci_fixup_device(pci_fixup_header, asus_lpc_pcidev);
+
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }

--- linux-2.6.16.14/drivers/pci/quirks.c	2006-05-05 02:03:45.000000000 +0200
+++ linux-2.6.16.14/drivers/pci/quirks.c	2006-05-10 01:51:06.000000000 +0200
@@ -873,9 +873,10 @@
  * becomes necessary to do this tweak in two steps -- I've chosen the Host
  * bridge as trigger.
  */
-static int __initdata asus_hides_smbus = 0;
+static int asus_hides_smbus = 0;
+struct pci_dev *asus_lpc_pcidev = NULL;

-static void __init asus_hides_smbus_hostbridge(struct pci_dev *dev)
+static void asus_hides_smbus_hostbridge(struct pci_dev *dev)
 {
 	if (unlikely(dev->subsystem_vendor == PCI_VENDOR_ID_ASUSTEK)) {
 		if (dev->device == PCI_DEVICE_ID_INTEL_82845_HB)
@@ -968,7 +969,7 @@
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82855GM_HB,	asus_hides_smbus_hostbridge );
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82915GM_HB, asus_hides_smbus_hostbridge );

-static void __init asus_hides_smbus_lpc(struct pci_dev *dev)
+static void asus_hides_smbus_lpc(struct pci_dev *dev)
 {
 	u16 val;
 	
@@ -981,8 +982,11 @@
 		pci_read_config_word(dev, 0xF2, &val);
 		if (val & 0x8)
 			printk(KERN_INFO "PCI: i801 SMBus device continues to play 'hide and seek'! 0x%x\n", val);
-		else
+		else {
 			printk(KERN_INFO "PCI: Enabled i801 SMBus device\n");
+			if (!asus_lpc_pcidev)
+				asus_lpc_pcidev = dev;
+		}
 	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82801DB_0,	asus_hides_smbus_lpc );
@@ -991,7 +995,7 @@
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82801DB_12,	asus_hides_smbus_lpc );
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82801EB_0,	asus_hides_smbus_lpc );

-static void __init asus_hides_smbus_lpc_ich6(struct pci_dev *dev)
+static void asus_hides_smbus_lpc_ich6(struct pci_dev *dev)
 {
 	u32 val, rcba;
 	void __iomem *base;
@@ -1374,4 +1378,5 @@
 EXPORT_SYMBOL(pcie_mch_quirk);
 #ifdef CONFIG_HOTPLUG
 EXPORT_SYMBOL(pci_fixup_device);
+EXPORT_SYMBOL(asus_lpc_pcidev);
 #endif

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Pavel Machek]

Hi!

> machines with the asus_hides_smbus "feature" have the problem
> that the smbus is disabled again after suspend-to-RAM. This
> causes all sorts of problems, the worst being a total fan
> failure on my Samsung P35 notebook after STR and STD.

What happens if we disable hiding altogether? ASUS decided software
should not see smbus, perhaps they had a reason?

If we decide that we want to keep unhiding, redoing quirks after
resume is probably neccessary...
							Pavel


> References: Novell bugzilla #173420.
> 
> This (totally ugly) patch fixes it.
> Comments/criticism welcome.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Carl-Daniel Hailfinger]

Pavel Machek wrote:
> Hi!
> 
>> machines with the asus_hides_smbus "feature" have the problem
>> that the smbus is disabled again after suspend-to-RAM. This
>> causes all sorts of problems, the worst being a total fan
>> failure on my Samsung P35 notebook after STR and STD.
> 
> What happens if we disable hiding altogether? ASUS decided software
> should not see smbus, perhaps they had a reason?

IIRC this was introduced only to keep older ms-windows versions
from complaining about hardware for which no driver existed.
Newer ms-windows versions seem to unhide the smbus like we do.

> If we decide that we want to keep unhiding, redoing quirks after
> resume is probably neccessary...

Yes. Now the question is where exactly we want to execute these
quirks. Before or after restoring pci config space? If we do it
before restoring config space, it might cause some yet unknown
side effects. If we do it after restoring config space, it might
be worse because we would restore config space of a device not
existing anymore.

Thinking about it again, if we restored the full pci config space
on resume, this quirk handling would be completely unnecessary.
Any reasons why we don't do that?


>> References: Novell bugzilla #173420.
>>
>> This (totally ugly) patch fixes it.
>> Comments/criticism welcome.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
> 

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Greg KH]

On Wed, May 10, 2006 at 12:30:34PM +0200, Carl-Daniel Hailfinger wrote:
> 
> Thinking about it again, if we restored the full pci config space
> on resume, this quirk handling would be completely unnecessary.
> Any reasons why we don't do that?

We do do that.  Look at pci_restore_state().

Actually, look at it in the latest -mm tree, that version works better
than mainline does right now :)

thanks,

greg k-h

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Carl-Daniel Hailfinger]

Greg KH wrote:
> On Wed, May 10, 2006 at 12:30:34PM +0200, Carl-Daniel Hailfinger wrote:
>> Thinking about it again, if we restored the full pci config space
>> on resume, this quirk handling would be completely unnecessary.
>> Any reasons why we don't do that?
> 
> We do do that.  Look at pci_restore_state().
> 
> Actually, look at it in the latest -mm tree, that version works better
> than mainline does right now :)

Sorry. Even the version in -mm does not restore all 256 bytes, so it
will not change anything. The quirk sits at offset 0xf2 in config space.
So either we really restore the full config space (probably a good idea
by itself) or we add the quirk handling to the suspend-to-ram codepath.

On a related note, what happens if we try to restore the config space of
a device which has vanished? Right now this happens on all mainboards
with asus_hides_smbus==1 during resume-from-ram.

Greg, do you have any comments on the patch starting this thread?

Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Dave Jones]

On Wed, May 10, 2006 at 11:36:41PM +0200, Carl-Daniel Hailfinger wrote:
 > Greg KH wrote:
 > > On Wed, May 10, 2006 at 12:30:34PM +0200, Carl-Daniel Hailfinger wrote:
 > >> Thinking about it again, if we restored the full pci config space
 > >> on resume, this quirk handling would be completely unnecessary.
 > >> Any reasons why we don't do that?
 > > 
 > > We do do that.  Look at pci_restore_state().
 > > 
 > > Actually, look at it in the latest -mm tree, that version works better
 > > than mainline does right now :)
 > 
 > Sorry. Even the version in -mm does not restore all 256 bytes, so it
 > will not change anything.

You can't generically look at a PCI device past the first 32 bytes.
*anything* could be there, including registers which cause the machine
to lock up when they get read.

This is exactly the reason that lspci by default only shows 32 bytes,
and you need to be root to see past that limit.

 > So either we really restore the full config space (probably a good idea
 > by itself)

No, *really* *really* bad idea :)

		Dave

-- 
http://www.codemonkey.org.uk

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Carl-Daniel Hailfinger]

Dave Jones wrote:
> On Wed, May 10, 2006 at 11:36:41PM +0200, Carl-Daniel Hailfinger wrote:
>  > Greg KH wrote:
>  > > On Wed, May 10, 2006 at 12:30:34PM +0200, Carl-Daniel Hailfinger wrote:
>  > >> Thinking about it again, if we restored the full pci config space
>  > >> on resume, this quirk handling would be completely unnecessary.
>  > >> Any reasons why we don't do that?
>  > > 
>  > > We do do that.  Look at pci_restore_state().
>  > > 
>  > > Actually, look at it in the latest -mm tree, that version works better
>  > > than mainline does right now :)
>  > 
>  > Sorry. Even the version in -mm does not restore all 256 bytes, so it
>  > will not change anything.
> 
> You can't generically look at a PCI device past the first 32 bytes.
> *anything* could be there, including registers which cause the machine
> to lock up when they get read.
> 
> This is exactly the reason that lspci by default only shows 32 bytes,
> and you need to be root to see past that limit.

You mean 64 bytes?

>  > So either we really restore the full config space (probably a good idea
>  > by itself)
> 
> No, *really* *really* bad idea :)

I had hoped the warnings in the lspci man page would be obsolete now.
Wishful thinking, it appears. Thanks for the hint.


Unfortunately, that means we either have to introduce a new PCI_FIXUP_
type or we execute PCI_FIXUP_HEADER also on resume. Which is better?


Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Carl-Daniel Hailfinger]

Suggestion for a minimal fix for -stable:

Do not enable the SMBus device on Asus boards if software suspend
is used. We do not reenable the device on resume, leading to all sorts
of undesirable effects, the worst being a total fan failure after
resume on my Samsung P35 laptop.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

--- linux-2.6.16.14/drivers/pci/quirks.c.vanilla	2006-05-05 02:03:45.000000000 +0200
+++ linux-2.6.16.14/drivers/pci/quirks.c	2006-05-11 17:09:15.000000000 +0200
@@ -861,6 +861,8 @@
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82375,	quirk_eisa_bridge );

+#ifndef CONFIG_SOFTWARE_SUSPEND
+
 /*
  * On ASUS P4B boards, the SMBus PCI Device within the ICH2/4 southbridge
  * is not activated. The myth is that Asus said that they do not want the
@@ -1008,6 +1010,8 @@
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_ICH6_1,	asus_hides_smbus_lpc_ich6 );

+#endif
+
 /*
  * SiS 96x south bridge: BIOS typically hides SMBus device...
  */


-- 
http://www.hailfinger.org/

Re: [RFC] [PATCH] Execute PCI quirks on resume from suspend-to-RAM

[Matthew Garrett]

Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:

> +#ifndef CONFIG_SOFTWARE_SUSPEND

ACPI_SLEEP doesn't depend on SOFTWARE_SUSPEND, so this looks like the 
wrong fix.

-- 
Matthew Garrett | mjg59-chiark.mail.linux-rutgers.kernel@srcf.ucam.org