* [PATCH] PCI: Quirk for ASUS S3 issue
@ 2012-07-03 8:55 AceLan Kao
2012-07-03 16:16 ` Bjorn Helgaas
0 siblings, 1 reply; 13+ messages in thread
From: AceLan Kao @ 2012-07-03 8:55 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas
Some of ASUS machines have problem to enter S3.
The root cause of this issue is result from the the BIOS will try to
disable USBs which was already disabled by driver.
BIOS will to check the EHCI command register, if it's not zero, then
BIOS will think the USB is not disabled yet, so it will try to disable
USB again.
To resolve this, we should clear the EHCI command register before
entering S3. And this does no harm to the system, since it'll switch
off the power after enter S3, so the value in memory is not important
at all.
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
drivers/pci/quirks.c | 15 +++++++++++++++
include/linux/pci_ids.h | 2 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 194b243a..684cd1f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2955,6 +2955,21 @@ static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
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);
+/*
+ * ASUS BIOS will check EHCI command register to see if USB if disabled
+ * or not. BIOS will try to disable USB if the command register is not
+ * cleared. But, actually, USB is disabled by the driver while entering S3,
+ * so it'll hang in BIOS when it try to disable USB.
+ * Since it's going to enter S3, so it does no harm to clear the command
+ * register.
+ */
+static void asus_clear_pci_command(struct pci_dev *dev)
+{
+ pci_write_config_word(dev, PCI_COMMAND, 0);
+}
+DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON_EHCI, asus_clear_memory_bit);
+DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_EHCI, asus_clear_memory_bit);
+
static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
struct pci_fixup *end)
{
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index ab741b0..e79c469 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -216,6 +216,7 @@
#define PCI_DEVICE_ID_ATI_68800 0x4158
#define PCI_DEVICE_ID_ATI_215CT222 0x4354
#define PCI_DEVICE_ID_ATI_210888CX 0x4358
+#define PCI_DEVICE_ID_ATI_EHCI 0x4396
#define PCI_DEVICE_ID_ATI_215ET222 0x4554
/* Mach64 / Rage */
#define PCI_DEVICE_ID_ATI_215GB 0x4742
@@ -553,6 +554,7 @@
#define PCI_DEVICE_ID_AMD_8131_BRIDGE 0x7450
#define PCI_DEVICE_ID_AMD_8131_APIC 0x7451
#define PCI_DEVICE_ID_AMD_8132_BRIDGE 0x7458
+#define PCI_DEVICE_ID_AMD_HUDSON_EHCI 0x7808
#define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b
#define PCI_DEVICE_ID_AMD_CS5535_IDE 0x208F
#define PCI_DEVICE_ID_AMD_CS5536_ISA 0x2090
--
1.7.9.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-03 8:55 [PATCH] PCI: Quirk for ASUS S3 issue AceLan Kao
@ 2012-07-03 16:16 ` Bjorn Helgaas
2012-07-03 20:17 ` Alan Stern
0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2012-07-03 16:16 UTC (permalink / raw)
To: AceLan Kao; +Cc: linux-pci, Alan Stern, Rafael J. Wysocki
On Tue, Jul 3, 2012 at 2:55 AM, AceLan Kao <acelan.kao@canonical.com> wrote:
> Some of ASUS machines have problem to enter S3.
This should include a specific description of the problem the user
sees. From the code comment, it something like "Attempt to suspend
some ASUS systems causes hang. Power cycle required to recover."
might be appropriate.
The quirk you added is not specific to ASUS, though, so it must be
something that's safe on every system using the AMD and ATI devices
you specified. I don't know whether that's the case.
> The root cause of this issue is result from the the BIOS will try to
> disable USBs which was already disabled by driver.
> BIOS will to check the EHCI command register, if it's not zero, then
> BIOS will think the USB is not disabled yet, so it will try to disable
> USB again.
> To resolve this, we should clear the EHCI command register before
> entering S3. And this does no harm to the system, since it'll switch
> off the power after enter S3, so the value in memory is not important
> at all.
System RAM is preserved in the S3 state (ACPI spec sec 7.3.4.4), so I
don't know how to interpret this statement.
Please include a bugzilla or problem report URL if you have one.
I'm not a USB or suspend/resume expert, so please get folks like Alan
Stern and Rafael Wysocki to take a look at this (cc'd).
Please read the comment at the top of pci_ids.h; I don't think you
need to change that file.
> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
> ---
> drivers/pci/quirks.c | 15 +++++++++++++++
> include/linux/pci_ids.h | 2 ++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 194b243a..684cd1f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2955,6 +2955,21 @@ static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
> 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);
>
> +/*
> + * ASUS BIOS will check EHCI command register to see if USB if disabled
> + * or not. BIOS will try to disable USB if the command register is not
> + * cleared. But, actually, USB is disabled by the driver while entering S3,
> + * so it'll hang in BIOS when it try to disable USB.
> + * Since it's going to enter S3, so it does no harm to clear the command
> + * register.
> + */
> +static void asus_clear_pci_command(struct pci_dev *dev)
> +{
> + pci_write_config_word(dev, PCI_COMMAND, 0);
> +}
> +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON_EHCI, asus_clear_memory_bit);
> +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_EHCI, asus_clear_memory_bit);
> +
> static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
> struct pci_fixup *end)
> {
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index ab741b0..e79c469 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -216,6 +216,7 @@
> #define PCI_DEVICE_ID_ATI_68800 0x4158
> #define PCI_DEVICE_ID_ATI_215CT222 0x4354
> #define PCI_DEVICE_ID_ATI_210888CX 0x4358
> +#define PCI_DEVICE_ID_ATI_EHCI 0x4396
> #define PCI_DEVICE_ID_ATI_215ET222 0x4554
> /* Mach64 / Rage */
> #define PCI_DEVICE_ID_ATI_215GB 0x4742
> @@ -553,6 +554,7 @@
> #define PCI_DEVICE_ID_AMD_8131_BRIDGE 0x7450
> #define PCI_DEVICE_ID_AMD_8131_APIC 0x7451
> #define PCI_DEVICE_ID_AMD_8132_BRIDGE 0x7458
> +#define PCI_DEVICE_ID_AMD_HUDSON_EHCI 0x7808
> #define PCI_DEVICE_ID_AMD_HUDSON2_SMBUS 0x780b
> #define PCI_DEVICE_ID_AMD_CS5535_IDE 0x208F
> #define PCI_DEVICE_ID_AMD_CS5536_ISA 0x2090
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-03 16:16 ` Bjorn Helgaas
@ 2012-07-03 20:17 ` Alan Stern
2012-07-04 2:03 ` AceLan Kao
0 siblings, 1 reply; 13+ messages in thread
From: Alan Stern @ 2012-07-03 20:17 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: AceLan Kao, linux-pci, Rafael J. Wysocki
On Tue, 3 Jul 2012, Bjorn Helgaas wrote:
> On Tue, Jul 3, 2012 at 2:55 AM, AceLan Kao <acelan.kao@canonical.com> wrote:
> > Some of ASUS machines have problem to enter S3.
>
> This should include a specific description of the problem the user
> sees. From the code comment, it something like "Attempt to suspend
> some ASUS systems causes hang. Power cycle required to recover."
> might be appropriate.
>
> The quirk you added is not specific to ASUS, though, so it must be
> something that's safe on every system using the AMD and ATI devices
> you specified. I don't know whether that's the case.
Is this the same bug as the one addressed by commit c2fb8a3fa255 (USB:
add NO_D3_DURING_SLEEP flag and revert 151b61284776be2)?
That bug seemed to occur only when the affected systems were put into
S3 sleep while an EHCI controller was in power level D3.
> > The root cause of this issue is result from the the BIOS will try to
> > disable USBs which was already disabled by driver.
> > BIOS will to check the EHCI command register, if it's not zero, then
You don't mean "EHCI command register"; you mean "the EHCI controller's
PCI COMMAND register". The EHCI command register is a completely
different register; it's part of the memory-mapped interface rather
than the PCI config space.
> > BIOS will think the USB is not disabled yet, so it will try to disable
> > USB again.
How do you know this?
Why should disabling an already-disabled USB controller cause the
system to hang?
> > To resolve this, we should clear the EHCI command register before
> > entering S3. And this does no harm to the system, since it'll switch
Is there any reason not to clear the PCI COMMAND register of every PCI
USB host controller when entering S3?
> > off the power after enter S3, so the value in memory is not important
> > at all.
>
> System RAM is preserved in the S3 state (ACPI spec sec 7.3.4.4), so I
> don't know how to interpret this statement.
>
> Please include a bugzilla or problem report URL if you have one.
>
> I'm not a USB or suspend/resume expert, so please get folks like Alan
> Stern and Rafael Wysocki to take a look at this (cc'd).
This really has nothing to do with USB, except the fact that the
affected parts are USB host controllers. It's entirely a PCI issue.
> Please read the comment at the top of pci_ids.h; I don't think you
> need to change that file.
>
> > Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
> > ---
> > drivers/pci/quirks.c | 15 +++++++++++++++
> > include/linux/pci_ids.h | 2 ++
> > 2 files changed, 17 insertions(+)
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 194b243a..684cd1f 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -2955,6 +2955,21 @@ static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
> > 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);
> >
> > +/*
> > + * ASUS BIOS will check EHCI command register to see if USB if disabled
> > + * or not. BIOS will try to disable USB if the command register is not
> > + * cleared. But, actually, USB is disabled by the driver while entering S3,
> > + * so it'll hang in BIOS when it try to disable USB.
Why does the system hang? There should be no problem clearing the PCI
COMMAND register even while the controller is in D3.
Does the BIOS try to access the memory-mapped registers while disabling
the controller?
> > + * Since it's going to enter S3, so it does no harm to clear the command
> > + * register.
> > + */
> > +static void asus_clear_pci_command(struct pci_dev *dev)
> > +{
> > + pci_write_config_word(dev, PCI_COMMAND, 0);
> > +}
> > +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON_EHCI, asus_clear_memory_bit);
> > +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_EHCI, asus_clear_memory_bit);
Why not do this for all EHCI controllers? Or even all PCI devices?
Alan Stern
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-03 20:17 ` Alan Stern
@ 2012-07-04 2:03 ` AceLan Kao
2012-07-04 18:47 ` Alan Stern
0 siblings, 1 reply; 13+ messages in thread
From: AceLan Kao @ 2012-07-04 2:03 UTC (permalink / raw)
To: Alan Stern
Cc: Bjorn Helgaas, linux-pci, Rafael J. Wysocki,
洪(Alex)茂耕
Dear Bjorn and Alan,
2012/7/4 Alan Stern <stern@rowland.harvard.edu>:
> On Tue, 3 Jul 2012, Bjorn Helgaas wrote:
>
>> On Tue, Jul 3, 2012 at 2:55 AM, AceLan Kao <acelan.kao@canonical.com> wrote:
>> > Some of ASUS machines have problem to enter S3.
>>
>> This should include a specific description of the problem the user
>> sees. From the code comment, it something like "Attempt to suspend
>> some ASUS systems causes hang. Power cycle required to recover."
>> might be appropriate.
>>
>> The quirk you added is not specific to ASUS, though, so it must be
>> something that's safe on every system using the AMD and ATI devices
>> you specified. I don't know whether that's the case.
>
> Is this the same bug as the one addressed by commit c2fb8a3fa255 (USB:
> add NO_D3_DURING_SLEEP flag and revert 151b61284776be2)?
>
> That bug seemed to occur only when the affected systems were put into
> S3 sleep while an EHCI controller was in power level D3.
We contacted the ASUS' BIOS engineer and try to verify this issue, for
it happened
on many ASUS' machines. Then they found that the system hangs in their
BIOS code.
The code will try to disable the USB if they found the PCI COMMAND
register is not zero.
That's not a correct behavior that BIOS should do, the PCI COMMAND
register is not
represent if the USB is disabled or not, It's a workaround they tried to fix
another issue in windows long time ago, but ASUS' BIOS engineer refuse to remove
that part of code. But windows will clear the PCI COMMAND register if
windows is already
disabled the USB.
So, I try to write this quirk.
>
>> > The root cause of this issue is result from the the BIOS will try to
>> > disable USBs which was already disabled by driver.
>> > BIOS will to check the EHCI command register, if it's not zero, then
>
> You don't mean "EHCI command register"; you mean "the EHCI controller's
> PCI COMMAND register". The EHCI command register is a completely
> different register; it's part of the memory-mapped interface rather
> than the PCI config space.
Thank you very much, I'm not familiar with this.
It's EHCI controller's PCI COMMAND register.
>
>> > BIOS will think the USB is not disabled yet, so it will try to disable
>> > USB again.
>
> How do you know this?
ASUS' BIOS engineer told me :p
>
> Why should disabling an already-disabled USB controller cause the
> system to hang?
I don't know, it hangs in BIOS, not in kernel.
>
>> > To resolve this, we should clear the EHCI command register before
>> > entering S3. And this does no harm to the system, since it'll switch
>
> Is there any reason not to clear the PCI COMMAND register of every PCI
> USB host controller when entering S3?
Quote from ASUS, they only mentioned EHCI
---------
BIOS will check EHCI command register PCI regiter offset 04h to see if
USB is disabled or not.
Because regiter offset 04h is not cleared, BIOS think USB is not disabled.
Then BIOS will try to disabled USB, but the USB is disabled by Ubuntu
already. This conflict will cause system hang.
ASUS think since Ubuntu will disable USB, it also need to clear the
register too.
---------
>
>> > off the power after enter S3, so the value in memory is not important
>> > at all.
>>
>> System RAM is preserved in the S3 state (ACPI spec sec 7.3.4.4), so I
>> don't know how to interpret this statement.
Sorry for my bad wording, it's not System RAM, it's PCI COMMAND register.
H/W will reset it's value after wake up.
I'm not pretty sure of this part, but I can verify it later.
>>
>> Please include a bugzilla or problem report URL if you have one.
I didn't grep the bugzilla before sending out this patch, it's easy to find
that there are plenty of them. I'll add them and rewording the patch description
http://thread.gmane.org/gmane.linux.usb.general/61396/focus=61396 #
ehci_hcd related S3 lockup on ASUS laptops, again
https://bugzilla.kernel.org/show_bug.cgi?id=43064 # [00:1a.0 Intel
USB] Asus U36SD kernel freeze at suspend
https://bugzilla.kernel.org/show_bug.cgi?id=42883 # Unable to use
suspension and hibernation on ASUS N53JQ-SZ197X
https://bugzilla.kernel.org/show_bug.cgi?id=37632 # Many ASUS laptops
can't suspend with ehci_hcd loaded
http://www.mail-archive.com/acpi-bugzilla@lists.sourceforge.net/msg35833.html
# New: suspend Asus P5N-E SLI
>>
>> I'm not a USB or suspend/resume expert, so please get folks like Alan
>> Stern and Rafael Wysocki to take a look at this (cc'd).
>
> This really has nothing to do with USB, except the fact that the
> affected parts are USB host controllers. It's entirely a PCI issue.
>
>> Please read the comment at the top of pci_ids.h; I don't think you
>> need to change that file.
Sorry, I'll remove the definition I added in pci_ids.h
>>
>> > Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
>> > ---
>> > drivers/pci/quirks.c | 15 +++++++++++++++
>> > include/linux/pci_ids.h | 2 ++
>> > 2 files changed, 17 insertions(+)
>> >
>> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> > index 194b243a..684cd1f 100644
>> > --- a/drivers/pci/quirks.c
>> > +++ b/drivers/pci/quirks.c
>> > @@ -2955,6 +2955,21 @@ static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
>> > 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);
>> >
>> > +/*
>> > + * ASUS BIOS will check EHCI command register to see if USB if disabled
>> > + * or not. BIOS will try to disable USB if the command register is not
>> > + * cleared. But, actually, USB is disabled by the driver while entering S3,
>> > + * so it'll hang in BIOS when it try to disable USB.
>
> Why does the system hang? There should be no problem clearing the PCI
> COMMAND register even while the controller is in D3.
>
> Does the BIOS try to access the memory-mapped registers while disabling
> the controller?
>
>> > + * Since it's going to enter S3, so it does no harm to clear the command
>> > + * register.
>> > + */
>> > +static void asus_clear_pci_command(struct pci_dev *dev)
>> > +{
>> > + pci_write_config_word(dev, PCI_COMMAND, 0);
>> > +}
>> > +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_HUDSON_EHCI, asus_clear_memory_bit);
>> > +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_EHCI, asus_clear_memory_bit);
>
> Why not do this for all EHCI controllers? Or even all PCI devices?
>
> Alan Stern
>
--
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-04 2:03 ` AceLan Kao
@ 2012-07-04 18:47 ` Alan Stern
2012-07-04 21:14 ` Rafael J. Wysocki
0 siblings, 1 reply; 13+ messages in thread
From: Alan Stern @ 2012-07-04 18:47 UTC (permalink / raw)
To: AceLan Kao
Cc: Bjorn Helgaas, linux-pci, Rafael J. Wysocki,
洪(Alex)茂耕
On Wed, 4 Jul 2012, AceLan Kao wrote:
> We contacted the ASUS' BIOS engineer and try to verify this issue, for
> it happened
> on many ASUS' machines. Then they found that the system hangs in their
> BIOS code.
> The code will try to disable the USB if they found the PCI COMMAND
> register is not zero.
>
> That's not a correct behavior that BIOS should do, the PCI COMMAND
> register is not
> represent if the USB is disabled or not, It's a workaround they tried to fix
> another issue in windows long time ago, but ASUS' BIOS engineer refuse to remove
> that part of code. But windows will clear the PCI COMMAND register if
> windows is already
> disabled the USB.
> So, I try to write this quirk.
> > Is there any reason not to clear the PCI COMMAND register of every PCI
> > USB host controller when entering S3?
> Quote from ASUS, they only mentioned EHCI
> ---------
> BIOS will check EHCI command register PCI regiter offset 04h to see if
> USB is disabled or not.
> Because regiter offset 04h is not cleared, BIOS think USB is not disabled.
> Then BIOS will try to disabled USB, but the USB is disabled by Ubuntu
> already. This conflict will cause system hang.
> ASUS think since Ubuntu will disable USB, it also need to clear the
> register too.
> ---------
How about this patch instead? (I haven't tested it yet...)
Rafael, does this seem okay with no special quirk flag? Or should the
command register be cleared as part of the USB code?
Alan Stern
Index: usb-3.5/drivers/pci/pci-driver.c
===================================================================
--- usb-3.5.orig/drivers/pci/pci-driver.c
+++ usb-3.5/drivers/pci/pci-driver.c
@@ -748,6 +748,18 @@ static int pci_pm_suspend_noirq(struct d
pci_pm_set_unknown_state(pci_dev);
+ /*
+ * Some BIOSes from ASUS have a bug: If a USB EHCI host controller's
+ * PCI COMMAND register isn't 0, the BIOS assumes that the controller
+ * hasn't been quiesced and tries to turn it off. If the controller
+ * is already in D3, this can hang or cause memory corruption.
+ *
+ * Since the value of the COMMAND register doesn't matter once the
+ * device has been suspended, we can safely set it to 0 here.
+ */
+ if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
+ pci_write_config_word(pci_dev, PCI_COMMAND, 0);
+
return 0;
}
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-04 18:47 ` Alan Stern
@ 2012-07-04 21:14 ` Rafael J. Wysocki
2012-07-05 18:17 ` Bjorn Helgaas
0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-07-04 21:14 UTC (permalink / raw)
To: Alan Stern, Bjorn Helgaas
Cc: AceLan Kao, linux-pci, 洪(Alex)茂耕
On Wednesday, July 04, 2012, Alan Stern wrote:
> On Wed, 4 Jul 2012, AceLan Kao wrote:
>
> > We contacted the ASUS' BIOS engineer and try to verify this issue, for
> > it happened
> > on many ASUS' machines. Then they found that the system hangs in their
> > BIOS code.
> > The code will try to disable the USB if they found the PCI COMMAND
> > register is not zero.
> >
> > That's not a correct behavior that BIOS should do, the PCI COMMAND
> > register is not
> > represent if the USB is disabled or not, It's a workaround they tried to fix
> > another issue in windows long time ago, but ASUS' BIOS engineer refuse to remove
> > that part of code. But windows will clear the PCI COMMAND register if
> > windows is already
> > disabled the USB.
> > So, I try to write this quirk.
>
> > > Is there any reason not to clear the PCI COMMAND register of every PCI
> > > USB host controller when entering S3?
> > Quote from ASUS, they only mentioned EHCI
> > ---------
> > BIOS will check EHCI command register PCI regiter offset 04h to see if
> > USB is disabled or not.
> > Because regiter offset 04h is not cleared, BIOS think USB is not disabled.
> > Then BIOS will try to disabled USB, but the USB is disabled by Ubuntu
> > already. This conflict will cause system hang.
> > ASUS think since Ubuntu will disable USB, it also need to clear the
> > register too.
> > ---------
>
> How about this patch instead? (I haven't tested it yet...)
>
> Rafael, does this seem okay with no special quirk flag? Or should the
> command register be cleared as part of the USB code?
Well, the clearing of it in the USB code would probably be too early,
so we don't really have much choice.
I suppose we can add pci_fixup_suspend_noirq for that in analogy with
pci_fixup_suspend, but I'm not sure it's worth the effort.
Bjorn, what do you think?
Rafael
> Index: usb-3.5/drivers/pci/pci-driver.c
> ===================================================================
> --- usb-3.5.orig/drivers/pci/pci-driver.c
> +++ usb-3.5/drivers/pci/pci-driver.c
> @@ -748,6 +748,18 @@ static int pci_pm_suspend_noirq(struct d
>
> pci_pm_set_unknown_state(pci_dev);
>
> + /*
> + * Some BIOSes from ASUS have a bug: If a USB EHCI host controller's
> + * PCI COMMAND register isn't 0, the BIOS assumes that the controller
> + * hasn't been quiesced and tries to turn it off. If the controller
> + * is already in D3, this can hang or cause memory corruption.
> + *
> + * Since the value of the COMMAND register doesn't matter once the
> + * device has been suspended, we can safely set it to 0 here.
> + */
> + if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
> + pci_write_config_word(pci_dev, PCI_COMMAND, 0);
> +
> return 0;
> }
>
>
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-04 21:14 ` Rafael J. Wysocki
@ 2012-07-05 18:17 ` Bjorn Helgaas
2012-07-05 21:33 ` Alan Stern
0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2012-07-05 18:17 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alan Stern, AceLan Kao, linux-pci, 洪(Alex)茂耕
On Wed, Jul 4, 2012 at 3:14 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday, July 04, 2012, Alan Stern wrote:
>> On Wed, 4 Jul 2012, AceLan Kao wrote:
>>
>> > We contacted the ASUS' BIOS engineer and try to verify this issue, for
>> > it happened
>> > on many ASUS' machines. Then they found that the system hangs in their
>> > BIOS code.
>> > The code will try to disable the USB if they found the PCI COMMAND
>> > register is not zero.
>> >
>> > That's not a correct behavior that BIOS should do, the PCI COMMAND
>> > register is not
>> > represent if the USB is disabled or not, It's a workaround they tried to fix
>> > another issue in windows long time ago, but ASUS' BIOS engineer refuse to remove
>> > that part of code. But windows will clear the PCI COMMAND register if
>> > windows is already
>> > disabled the USB.
>> > So, I try to write this quirk.
>>
>> > > Is there any reason not to clear the PCI COMMAND register of every PCI
>> > > USB host controller when entering S3?
>> > Quote from ASUS, they only mentioned EHCI
>> > ---------
>> > BIOS will check EHCI command register PCI regiter offset 04h to see if
>> > USB is disabled or not.
>> > Because regiter offset 04h is not cleared, BIOS think USB is not disabled.
>> > Then BIOS will try to disabled USB, but the USB is disabled by Ubuntu
>> > already. This conflict will cause system hang.
>> > ASUS think since Ubuntu will disable USB, it also need to clear the
>> > register too.
>> > ---------
>>
>> How about this patch instead? (I haven't tested it yet...)
>>
>> Rafael, does this seem okay with no special quirk flag? Or should the
>> command register be cleared as part of the USB code?
>
> Well, the clearing of it in the USB code would probably be too early,
> so we don't really have much choice.
>
> I suppose we can add pci_fixup_suspend_noirq for that in analogy with
> pci_fixup_suspend, but I'm not sure it's worth the effort.
>
> Bjorn, what do you think?
I agree; it doesn't seem like a new fixup type is worth the trouble.
I wish we knew more about what's really going on here. I looked
through the bug reports AceLan dug up, and I don't get the feeling
that anybody really has a grand unified theory about why Windows works
but Linux doesn't. But I'm willing to apply one of these patches if
you and/or Alan sign off on it.
>> Index: usb-3.5/drivers/pci/pci-driver.c
>> ===================================================================
>> --- usb-3.5.orig/drivers/pci/pci-driver.c
>> +++ usb-3.5/drivers/pci/pci-driver.c
>> @@ -748,6 +748,18 @@ static int pci_pm_suspend_noirq(struct d
>>
>> pci_pm_set_unknown_state(pci_dev);
>>
>> + /*
>> + * Some BIOSes from ASUS have a bug: If a USB EHCI host controller's
>> + * PCI COMMAND register isn't 0, the BIOS assumes that the controller
>> + * hasn't been quiesced and tries to turn it off. If the controller
>> + * is already in D3, this can hang or cause memory corruption.
>> + *
>> + * Since the value of the COMMAND register doesn't matter once the
>> + * device has been suspended, we can safely set it to 0 here.
>> + */
>> + if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
>> + pci_write_config_word(pci_dev, PCI_COMMAND, 0);
>> +
>> return 0;
>> }
>>
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-05 18:17 ` Bjorn Helgaas
@ 2012-07-05 21:33 ` Alan Stern
2012-07-05 22:00 ` Rafael J. Wysocki
0 siblings, 1 reply; 13+ messages in thread
From: Alan Stern @ 2012-07-05 21:33 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rafael J. Wysocki, AceLan Kao, linux-pci, 洪(Alex)茂耕
On Thu, 5 Jul 2012, Bjorn Helgaas wrote:
> I wish we knew more about what's really going on here. I looked
> through the bug reports AceLan dug up, and I don't get the feeling
> that anybody really has a grand unified theory about why Windows works
> but Linux doesn't. But I'm willing to apply one of these patches if
> you and/or Alan sign off on it.
His description seems to make sense, even if it may not be complete.
Windows sets the COMMAND register to 0 before suspending, so it doesn't
trigger the BIOS bus. Linux doesn't, so the BIOS tries to quiesce the
EHCI controller. This involves doing various MMIO accesses, which of
course don't work if the controller is already in D3.
For obvious reasons, I'm in favor of my two-line change over AceLan's
new PCI quirk. So far there have been positive responses from a few
volunteers testing the patch.
Alan Stern
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-05 21:33 ` Alan Stern
@ 2012-07-05 22:00 ` Rafael J. Wysocki
2012-07-05 22:39 ` Bjorn Helgaas
0 siblings, 1 reply; 13+ messages in thread
From: Rafael J. Wysocki @ 2012-07-05 22:00 UTC (permalink / raw)
To: Alan Stern
Cc: Bjorn Helgaas, AceLan Kao, linux-pci, 洪(Alex)茂耕
On Thursday, July 05, 2012, Alan Stern wrote:
> On Thu, 5 Jul 2012, Bjorn Helgaas wrote:
>
> > I wish we knew more about what's really going on here. I looked
> > through the bug reports AceLan dug up, and I don't get the feeling
> > that anybody really has a grand unified theory about why Windows works
> > but Linux doesn't. But I'm willing to apply one of these patches if
> > you and/or Alan sign off on it.
>
> His description seems to make sense, even if it may not be complete.
>
> Windows sets the COMMAND register to 0 before suspending, so it doesn't
> trigger the BIOS bus. Linux doesn't, so the BIOS tries to quiesce the
> EHCI controller. This involves doing various MMIO accesses, which of
> course don't work if the controller is already in D3.
>
> For obvious reasons, I'm in favor of my two-line change over AceLan's
> new PCI quirk. So far there have been positive responses from a few
> volunteers testing the patch.
The two-liner is good enough, I think, and tested. We can always add a quirk
based on it in the future.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-05 22:00 ` Rafael J. Wysocki
@ 2012-07-05 22:39 ` Bjorn Helgaas
2012-07-06 2:01 ` Alan Stern
0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2012-07-05 22:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Alan Stern, AceLan Kao, linux-pci, 洪(Alex)茂耕
On Thu, Jul 5, 2012 at 4:00 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Thursday, July 05, 2012, Alan Stern wrote:
>> On Thu, 5 Jul 2012, Bjorn Helgaas wrote:
>>
>> > I wish we knew more about what's really going on here. I looked
>> > through the bug reports AceLan dug up, and I don't get the feeling
>> > that anybody really has a grand unified theory about why Windows works
>> > but Linux doesn't. But I'm willing to apply one of these patches if
>> > you and/or Alan sign off on it.
>>
>> His description seems to make sense, even if it may not be complete.
>>
>> Windows sets the COMMAND register to 0 before suspending, so it doesn't
>> trigger the BIOS bus. Linux doesn't, so the BIOS tries to quiesce the
>> EHCI controller. This involves doing various MMIO accesses, which of
>> course don't work if the controller is already in D3.
Should we clear the COMMAND register for more than just EHCI devices?
It seems like known differences from Windows are just waiting to bite
us.
I like your patch and explanation for how this can cause a hang...
that helps make sense of things.
>> For obvious reasons, I'm in favor of my two-line change over AceLan's
>> new PCI quirk. So far there have been positive responses from a few
>> volunteers testing the patch.
>
> The two-liner is good enough, I think, and tested. We can always add a quirk
> based on it in the future.
Cool, do you want to send me a patch with Signed-off-by, etc?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-05 22:39 ` Bjorn Helgaas
@ 2012-07-06 2:01 ` Alan Stern
0 siblings, 0 replies; 13+ messages in thread
From: Alan Stern @ 2012-07-06 2:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rafael J. Wysocki, AceLan Kao, linux-pci, 洪(Alex)茂耕
On Thu, 5 Jul 2012, Bjorn Helgaas wrote:
> >> Windows sets the COMMAND register to 0 before suspending, so it doesn't
> >> trigger the BIOS bus. Linux doesn't, so the BIOS tries to quiesce the
> >> EHCI controller. This involves doing various MMIO accesses, which of
> >> course don't work if the controller is already in D3.
>
> Should we clear the COMMAND register for more than just EHCI devices?
> It seems like known differences from Windows are just waiting to bite
> us.
You're undoubtedly right, but I don't know what Windows does! I'm just
relying on what AceLan reported.
For the patch, I wanted to change the system behavior as little as
possible -- hence the restriction to EHCI controllers. For all I know
it might be perfectly okay to do this with all PCI devices, but it
seems more likely that it would cause trouble in some obscure cases.
And at least with EHCI controllers, I know what the underlying driver
is doing.
> I like your patch and explanation for how this can cause a hang...
> that helps make sense of things.
>
> >> For obvious reasons, I'm in favor of my two-line change over AceLan's
> >> new PCI quirk. So far there have been positive responses from a few
> >> volunteers testing the patch.
> >
> > The two-liner is good enough, I think, and tested. We can always add a quirk
> > based on it in the future.
>
> Cool, do you want to send me a patch with Signed-off-by, etc?
I'll send it early next week.
Alan Stern
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] PCI: Quirk for ASUS S3 issue
@ 2012-07-04 8:34 AceLan Kao
2012-07-04 8:39 ` AceLan Kao
0 siblings, 1 reply; 13+ messages in thread
From: AceLan Kao @ 2012-07-04 8:34 UTC (permalink / raw)
To: linux-pci, Bjorn Helgaas
Attempt to suspend some ASUS systems causes hang. Power cycle required
to recover.
The root cause of this issue is result from the the BIOS will try to
disable USB which is already disabled by the driver.
BIOS will check the EHCI controller's PCI COMMAND register,
if it's not zero, then BIOS will think the USB is not disabled yet,
so it will try to disable USB again, and then hang in the BIOS code.
To resolve this, we should clear the EHCI controller's PCI COMMAND
register before entering S3. And this does no harm to the system,
since it'll switch off the power after enter S3, and the value in the
register will be reset by BIOS after wakeup.
Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=43064
BugLink: http://bugs.launchpad.net/bugs/951143
Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
---
drivers/pci/quirks.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 194b243a..d1bb4db 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2955,6 +2955,25 @@ static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
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);
+/*
+ * ASUS BIOS will check the EHCI controller's PCI COMMAND register to see
+ * if the value is all zero. If not, BIOS will think the USB is not disabled
+ * and will try to disable USB. But, actually, USB is disabled by the driver
+ * while entering S3, so it'll hang in BIOS when it try to disable USB.
+ * Since it's going to enter S3, so it does no harm to clear the EHCI
+ * controller's PCI COMMAND register. And the value of the PCI COMMAND
+ * register value will be restored correctly after S3.
+ */
+static void asus_clear_pci_command(struct pci_dev *dev)
+{
+ if (dev->subsystem_vendor != PCI_VENDOR_ID_ASUSTEK)
+ return;
+
+ pci_write_config_word(dev, PCI_COMMAND, 0);
+}
+DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_AMD, 0x7808, asus_clear_memory_bit);
+DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_ATI, 0x4396, asus_clear_memory_bit);
+
static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
struct pci_fixup *end)
{
--
1.7.9.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] PCI: Quirk for ASUS S3 issue
2012-07-04 8:34 AceLan Kao
@ 2012-07-04 8:39 ` AceLan Kao
0 siblings, 0 replies; 13+ messages in thread
From: AceLan Kao @ 2012-07-04 8:39 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, Rafael J. Wysocki, Alan Stern
Hi all,
I take the following weekday as personal leave days,
so I send out the patch earlier for discussion.
I'll read email next week.
Best regards,
AceLan Kao.
2012/7/4 AceLan Kao <acelan.kao@canonical.com>:
> Attempt to suspend some ASUS systems causes hang. Power cycle required
> to recover.
> The root cause of this issue is result from the the BIOS will try to
> disable USB which is already disabled by the driver.
> BIOS will check the EHCI controller's PCI COMMAND register,
> if it's not zero, then BIOS will think the USB is not disabled yet,
> so it will try to disable USB again, and then hang in the BIOS code.
> To resolve this, we should clear the EHCI controller's PCI COMMAND
> register before entering S3. And this does no harm to the system,
> since it'll switch off the power after enter S3, and the value in the
> register will be reset by BIOS after wakeup.
>
> Resolves-bug: https://bugzilla.kernel.org/show_bug.cgi?id=43064
> BugLink: http://bugs.launchpad.net/bugs/951143
>
> Signed-off-by: AceLan Kao <acelan.kao@canonical.com>
> ---
> drivers/pci/quirks.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 194b243a..d1bb4db 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2955,6 +2955,25 @@ static void __devinit asus_ehci_no_d3(struct pci_dev *dev)
> 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);
>
> +/*
> + * ASUS BIOS will check the EHCI controller's PCI COMMAND register to see
> + * if the value is all zero. If not, BIOS will think the USB is not disabled
> + * and will try to disable USB. But, actually, USB is disabled by the driver
> + * while entering S3, so it'll hang in BIOS when it try to disable USB.
> + * Since it's going to enter S3, so it does no harm to clear the EHCI
> + * controller's PCI COMMAND register. And the value of the PCI COMMAND
> + * register value will be restored correctly after S3.
> + */
> +static void asus_clear_pci_command(struct pci_dev *dev)
> +{
> + if (dev->subsystem_vendor != PCI_VENDOR_ID_ASUSTEK)
> + return;
> +
> + pci_write_config_word(dev, PCI_COMMAND, 0);
> +}
> +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_AMD, 0x7808, asus_clear_memory_bit);
> +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_ATI, 0x4396, asus_clear_memory_bit);
> +
> static void pci_do_fixups(struct pci_dev *dev, struct pci_fixup *f,
> struct pci_fixup *end)
> {
> --
> 1.7.9.5
>
--
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-07-06 2:01 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-03 8:55 [PATCH] PCI: Quirk for ASUS S3 issue AceLan Kao
2012-07-03 16:16 ` Bjorn Helgaas
2012-07-03 20:17 ` Alan Stern
2012-07-04 2:03 ` AceLan Kao
2012-07-04 18:47 ` Alan Stern
2012-07-04 21:14 ` Rafael J. Wysocki
2012-07-05 18:17 ` Bjorn Helgaas
2012-07-05 21:33 ` Alan Stern
2012-07-05 22:00 ` Rafael J. Wysocki
2012-07-05 22:39 ` Bjorn Helgaas
2012-07-06 2:01 ` Alan Stern
2012-07-04 8:34 AceLan Kao
2012-07-04 8:39 ` AceLan Kao
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).