Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Mon, Jul 9, 2012 at 10:47 AM, Greg KH gre...@linuxfoundation.org wrote: On Mon, Jul 09, 2012 at 06:50:24PM +0200, Rafael J. Wysocki wrote: On Monday, July 09, 2012, Alan Stern wrote: Quite a few ASUS computers experience a nasty problem, related to the EHCI controllers, when going into system suspend. It was observed that the problem didn't occur if the controllers were not put into the D3 power state before starting the suspend, and commit 151b61284776be2d6f02d48c23c3625678960b97 (USB: EHCI: fix crash during suspend on ASUS computers) was created to do this. It turned out this approach messed up other computers that didn't have the problem -- it prevented USB wakeup from working. Consequently commit c2fb8a3fa25513de8fedb38509b1f15a5bbee47b (USB: add NO_D3_DURING_SLEEP flag and revert 151b61284776be2) was merged; it reverted the earlier commit and added a whitelist of known good board names. Now we know the actual cause of the problem. Thanks to AceLan Kao for tracking it down. According to him, an engineer at ASUS explained that some of their BIOSes contain a bug that was added in an attempt to work around a problem in early versions of Windows. When the computer goes into S3 suspend, the BIOS tries to verify that the EHCI controllers were first quiesced by the OS. Nothing's wrong with this, but the BIOS does it by checking that the PCI COMMAND registers contain 0 without checking the controllers' power state. If the register isn't 0, the BIOS assumes the controller needs to be quiesced and tries to do so. This involves making various MMIO accesses to the controller, which don't work very well if the controller is already in D3. The end result is a system hang or memory corruption. Since the value in the PCI COMMAND register doesn't matter once the controller has been suspended, and since the value will be restored anyway when the controller is resumed, we can work around the BIOS bug simply by setting the register to 0 during system suspend. This patch (as1590) does so and also reverts the second commit mentioned above, which is now unnecessary. In theory we could do this for every PCI device. However to avoid introducing new problems, the patch restricts itself to EHCI host controllers. Finally the affected systems can suspend with USB wakeup working properly. Signed-off-by: Alan Stern st...@rowland.harvard.edu Tested-by: Dâniel Fraga frag...@gmail.com Tested-by: Javier Marcet jmar...@gmail.com Tested-by: Andrey Rahmatullin w...@wrar.name Tested-by: Oleksij Rempel bug-tr...@fisher-privat.net Tested-by: Pavel Pisa p...@cmp.felk.cvut.cz CC: sta...@vger.kernel.org Acked-by: Rafael J. Wysocki r...@sisk.pl Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org I'm fine with this patch. I was going to add these: Based-on-patch-by: AceLan Kao acelan@canonical.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 I don't have the previous iteration (c2fb8a3fa25513d) in my next branch. I think it went through you, Greg. Do you want to handle this one as well? I *could* do it, but it looks like a messy merge -- I think I'd have to rebase almost everything in my next branch -- so I'd rather not. Of course, I do have some D3-related updates in pci_prepare_to_sleep() which will conflict with this, too, so I guess it will be a bit of work for somebody either way. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, 10 Jul 2012, Greg KH wrote: I'm fine with this patch. I was going to add these: Based-on-patch-by: AceLan Kao acelan@canonical.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 I don't have the previous iteration (c2fb8a3fa25513d) in my next The commit is already in 3.5-rc6. And I intended this fix to get into 3.5-final, not into Linux-next (I should have said so explicitly in the patch). branch. I think it went through you, Greg. Do you want to handle this one as well? I can easily take it if you don't want to. I *could* do it, but it looks like a messy merge -- I think I'd have to rebase almost everything in my next branch -- so I'd rather not. Of course, I do have some D3-related updates in pci_prepare_to_sleep() which will conflict with this, too, so I guess it will be a bit of work for somebody either way. Ick, no, don't rebase anything. If I take this then we will have merge issues in linux-next, which we can work out, and then we will have the same issues for 3.6-rc1 as well. Or, Alan can redo the patch based on your next branch, which might make it easier for everyone involved? If Bjorn's next branch diverged from Linus's tree before c2fb8a3 was added, that would make things more difficult. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Tue, Jul 10, 2012 at 10:17 AM, Greg KH gre...@linuxfoundation.org wrote: On Tue, Jul 10, 2012 at 12:11:41PM -0400, Alan Stern wrote: On Tue, 10 Jul 2012, Greg KH wrote: I'm fine with this patch. I was going to add these: Based-on-patch-by: AceLan Kao acelan@canonical.com Reference: https://bugzilla.kernel.org/show_bug.cgi?id=37632 Reference: https://bugzilla.kernel.org/show_bug.cgi?id=42728 I don't have the previous iteration (c2fb8a3fa25513d) in my next The commit is already in 3.5-rc6. And I intended this fix to get into 3.5-final, not into Linux-next (I should have said so explicitly in the patch). Ah, ok, if so, and Bjorn doesn't mind, I can add it to the other USB patches that I have queued up to go in before 3.5-final is released. Bjorn, want me to take it? If so, can I get your ack? Yes, please. Acked-by: Bjorn Helgaas bhelg...@google.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
Quite a few ASUS computers experience a nasty problem, related to the
EHCI controllers, when going into system suspend. It was observed
that the problem didn't occur if the controllers were not put into the
D3 power state before starting the suspend, and commit
151b61284776be2d6f02d48c23c3625678960b97 (USB: EHCI: fix crash during
suspend on ASUS computers) was created to do this.
It turned out this approach messed up other computers that didn't have
the problem -- it prevented USB wakeup from working. Consequently
commit c2fb8a3fa25513de8fedb38509b1f15a5bbee47b (USB: add
NO_D3_DURING_SLEEP flag and revert 151b61284776be2) was merged; it
reverted the earlier commit and added a whitelist of known good board
names.
Now we know the actual cause of the problem. Thanks to AceLan Kao for
tracking it down.
According to him, an engineer at ASUS explained that some of their
BIOSes contain a bug that was added in an attempt to work around a
problem in early versions of Windows. When the computer goes into S3
suspend, the BIOS tries to verify that the EHCI controllers were first
quiesced by the OS. Nothing's wrong with this, but the BIOS does it
by checking that the PCI COMMAND registers contain 0 without checking
the controllers' power state. If the register isn't 0, the BIOS
assumes the controller needs to be quiesced and tries to do so. This
involves making various MMIO accesses to the controller, which don't
work very well if the controller is already in D3. The end result is
a system hang or memory corruption.
Since the value in the PCI COMMAND register doesn't matter once the
controller has been suspended, and since the value will be restored
anyway when the controller is resumed, we can work around the BIOS bug
simply by setting the register to 0 during system suspend. This patch
(as1590) does so and also reverts the second commit mentioned above,
which is now unnecessary.
In theory we could do this for every PCI device. However to avoid
introducing new problems, the patch restricts itself to EHCI host
controllers.
Finally the affected systems can suspend with USB wakeup working
properly.
Signed-off-by: Alan Stern st...@rowland.harvard.edu
Tested-by: Dâniel Fraga frag...@gmail.com
Tested-by: Javier Marcet jmar...@gmail.com
Tested-by: Andrey Rahmatullin w...@wrar.name
Tested-by: Oleksij Rempel bug-tr...@fisher-privat.net
Tested-by: Pavel Pisa p...@cmp.felk.cvut.cz
CC: sta...@vger.kernel.org
---
drivers/pci/pci-driver.c | 12
drivers/pci/pci.c|5 -
drivers/pci/quirks.c | 26 --
include/linux/pci.h |2 --
4 files changed, 12 insertions(+), 33 deletions(-)
Index: usb-3.4/drivers/pci/pci.c
==
--- usb-3.4.orig/drivers/pci/pci.c
+++ usb-3.4/drivers/pci/pci.c
@@ -1744,11 +1744,6 @@ int pci_prepare_to_sleep(struct pci_dev
if (target_state == PCI_POWER_ERROR)
return -EIO;
- /* Some devices mustn't be in D3 during system sleep */
- if (target_state == PCI_D3hot
- (dev-dev_flags PCI_DEV_FLAGS_NO_D3_DURING_SLEEP))
- return 0;
-
pci_enable_wake(dev, target_state, device_may_wakeup(dev-dev));
error = pci_set_power_state(dev, target_state);
Index: usb-3.4/drivers/pci/quirks.c
===
--- usb-3.4.orig/drivers/pci/quirks.c
+++ usb-3.4/drivers/pci/quirks.c
@@ -2929,32 +2929,6 @@ static void __devinit disable_igfx_irq(s
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0102, disable_igfx_irq);
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x010a, disable_igfx_irq);
-/*
- * The Intel 6 Series/C200 Series chipset's EHCI controllers on many
- * ASUS motherboards will cause memory corruption or a system crash
- * if they are in D3 while the system is put into S3 sleep.
- */
-static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
-{
- const char *sys_info;
- static const char good_Asus_board[] = P8Z68-V;
-
- if (dev-dev_flags PCI_DEV_FLAGS_NO_D3_DURING_SLEEP)
- return;
- if (dev-subsystem_vendor != PCI_VENDOR_ID_ASUSTEK)
- return;
- sys_info = dmi_get_system_info(DMI_BOARD_NAME);
- if (sys_info memcmp(sys_info, good_Asus_board,
- sizeof(good_Asus_board) - 1) == 0)
- return;
-
- dev_info(dev-dev, broken D3 during system sleep on ASUS\n);
- dev-dev_flags |= PCI_DEV_FLAGS_NO_D3_DURING_SLEEP;
- device_set_wakeup_capable(dev-dev, false);
-}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1c26, asus_ehci_no_d3);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1c2d, asus_ehci_no_d3);
-
static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
struct pci_fixup *end)
{
Index: usb-3.4/include/linux/pci.h
===
---
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Monday, July 09, 2012, Alan Stern wrote:
Quite a few ASUS computers experience a nasty problem, related to the
EHCI controllers, when going into system suspend. It was observed
that the problem didn't occur if the controllers were not put into the
D3 power state before starting the suspend, and commit
151b61284776be2d6f02d48c23c3625678960b97 (USB: EHCI: fix crash during
suspend on ASUS computers) was created to do this.
It turned out this approach messed up other computers that didn't have
the problem -- it prevented USB wakeup from working. Consequently
commit c2fb8a3fa25513de8fedb38509b1f15a5bbee47b (USB: add
NO_D3_DURING_SLEEP flag and revert 151b61284776be2) was merged; it
reverted the earlier commit and added a whitelist of known good board
names.
Now we know the actual cause of the problem. Thanks to AceLan Kao for
tracking it down.
According to him, an engineer at ASUS explained that some of their
BIOSes contain a bug that was added in an attempt to work around a
problem in early versions of Windows. When the computer goes into S3
suspend, the BIOS tries to verify that the EHCI controllers were first
quiesced by the OS. Nothing's wrong with this, but the BIOS does it
by checking that the PCI COMMAND registers contain 0 without checking
the controllers' power state. If the register isn't 0, the BIOS
assumes the controller needs to be quiesced and tries to do so. This
involves making various MMIO accesses to the controller, which don't
work very well if the controller is already in D3. The end result is
a system hang or memory corruption.
Since the value in the PCI COMMAND register doesn't matter once the
controller has been suspended, and since the value will be restored
anyway when the controller is resumed, we can work around the BIOS bug
simply by setting the register to 0 during system suspend. This patch
(as1590) does so and also reverts the second commit mentioned above,
which is now unnecessary.
In theory we could do this for every PCI device. However to avoid
introducing new problems, the patch restricts itself to EHCI host
controllers.
Finally the affected systems can suspend with USB wakeup working
properly.
Signed-off-by: Alan Stern st...@rowland.harvard.edu
Tested-by: Dâniel Fraga frag...@gmail.com
Tested-by: Javier Marcet jmar...@gmail.com
Tested-by: Andrey Rahmatullin w...@wrar.name
Tested-by: Oleksij Rempel bug-tr...@fisher-privat.net
Tested-by: Pavel Pisa p...@cmp.felk.cvut.cz
CC: sta...@vger.kernel.org
Acked-by: Rafael J. Wysocki r...@sisk.pl
---
drivers/pci/pci-driver.c | 12
drivers/pci/pci.c|5 -
drivers/pci/quirks.c | 26 --
include/linux/pci.h |2 --
4 files changed, 12 insertions(+), 33 deletions(-)
Index: usb-3.4/drivers/pci/pci.c
==
--- usb-3.4.orig/drivers/pci/pci.c
+++ usb-3.4/drivers/pci/pci.c
@@ -1744,11 +1744,6 @@ int pci_prepare_to_sleep(struct pci_dev
if (target_state == PCI_POWER_ERROR)
return -EIO;
- /* Some devices mustn't be in D3 during system sleep */
- if (target_state == PCI_D3hot
- (dev-dev_flags PCI_DEV_FLAGS_NO_D3_DURING_SLEEP))
- return 0;
-
pci_enable_wake(dev, target_state, device_may_wakeup(dev-dev));
error = pci_set_power_state(dev, target_state);
Index: usb-3.4/drivers/pci/quirks.c
===
--- usb-3.4.orig/drivers/pci/quirks.c
+++ usb-3.4/drivers/pci/quirks.c
@@ -2929,32 +2929,6 @@ static void __devinit disable_igfx_irq(s
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0102, disable_igfx_irq);
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x010a, disable_igfx_irq);
-/*
- * The Intel 6 Series/C200 Series chipset's EHCI controllers on many
- * ASUS motherboards will cause memory corruption or a system crash
- * if they are in D3 while the system is put into S3 sleep.
- */
-static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
-{
- const char *sys_info;
- static const char good_Asus_board[] = P8Z68-V;
-
- if (dev-dev_flags PCI_DEV_FLAGS_NO_D3_DURING_SLEEP)
- return;
- if (dev-subsystem_vendor != PCI_VENDOR_ID_ASUSTEK)
- return;
- sys_info = dmi_get_system_info(DMI_BOARD_NAME);
- if (sys_info memcmp(sys_info, good_Asus_board,
- sizeof(good_Asus_board) - 1) == 0)
- return;
-
- dev_info(dev-dev, broken D3 during system sleep on ASUS\n);
- dev-dev_flags |= PCI_DEV_FLAGS_NO_D3_DURING_SLEEP;
- device_set_wakeup_capable(dev-dev, false);
-}
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1c26, asus_ehci_no_d3);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1c2d, asus_ehci_no_d3);
-
static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
Re: [PATCH] PCI: EHCI: fix crash during suspend on ASUS computers
On Mon, Jul 09, 2012 at 06:50:24PM +0200, Rafael J. Wysocki wrote: On Monday, July 09, 2012, Alan Stern wrote: Quite a few ASUS computers experience a nasty problem, related to the EHCI controllers, when going into system suspend. It was observed that the problem didn't occur if the controllers were not put into the D3 power state before starting the suspend, and commit 151b61284776be2d6f02d48c23c3625678960b97 (USB: EHCI: fix crash during suspend on ASUS computers) was created to do this. It turned out this approach messed up other computers that didn't have the problem -- it prevented USB wakeup from working. Consequently commit c2fb8a3fa25513de8fedb38509b1f15a5bbee47b (USB: add NO_D3_DURING_SLEEP flag and revert 151b61284776be2) was merged; it reverted the earlier commit and added a whitelist of known good board names. Now we know the actual cause of the problem. Thanks to AceLan Kao for tracking it down. According to him, an engineer at ASUS explained that some of their BIOSes contain a bug that was added in an attempt to work around a problem in early versions of Windows. When the computer goes into S3 suspend, the BIOS tries to verify that the EHCI controllers were first quiesced by the OS. Nothing's wrong with this, but the BIOS does it by checking that the PCI COMMAND registers contain 0 without checking the controllers' power state. If the register isn't 0, the BIOS assumes the controller needs to be quiesced and tries to do so. This involves making various MMIO accesses to the controller, which don't work very well if the controller is already in D3. The end result is a system hang or memory corruption. Since the value in the PCI COMMAND register doesn't matter once the controller has been suspended, and since the value will be restored anyway when the controller is resumed, we can work around the BIOS bug simply by setting the register to 0 during system suspend. This patch (as1590) does so and also reverts the second commit mentioned above, which is now unnecessary. In theory we could do this for every PCI device. However to avoid introducing new problems, the patch restricts itself to EHCI host controllers. Finally the affected systems can suspend with USB wakeup working properly. Signed-off-by: Alan Stern st...@rowland.harvard.edu Tested-by: Dâniel Fraga frag...@gmail.com Tested-by: Javier Marcet jmar...@gmail.com Tested-by: Andrey Rahmatullin w...@wrar.name Tested-by: Oleksij Rempel bug-tr...@fisher-privat.net Tested-by: Pavel Pisa p...@cmp.felk.cvut.cz CC: sta...@vger.kernel.org Acked-by: Rafael J. Wysocki r...@sisk.pl Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
