| Message ID | 201210072100.09813.linux@rainbow-software.org (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Sunday 07 of October 2012 21:00:09 Ondrej Zary wrote: > On Sunday 07 October 2012 15:13:27 Ondrej Zary wrote: > > Hello, > > Intel D845BG board comes out of S3 with PS/2 keyboard and mouse completely > > dead. The machine works otherwise (with USB keyboard or over network). When > > rebooted in this state, the BIOS hangs with blank screen. I have the latest > > BIOS installed (P08). > > > > Old kernels worked. Bisection pointed to commit: > > b6dacf63e9fb2e7a1369843d6cef332f76fca6a3 > > (ACPI: Unconditionally set SCI_EN on resume) > > > > Commenting out this line in drivers/acpi/sleep.c: > > acpi_write_bit_register(ACPI_BITREG_SCI_ENABLE, 1); > > fixes the problem. > > > > Any ideas why this breaks on this system? > > Added acpi_read_bit_register there and it seems that SCI_EN is already set! > > This patch fixes the problem here. I wonder how this affects systems that > require SCI_EN to be set. I'm not sure if reading SCI_EN is actually safe on all systems. Matthew, what do you think? Rafael > --- a/drivers/acpi/sleep.c > +++ b/drivers/acpi/sleep.c > @@ -271,6 +271,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) > acpi_status status = AE_OK; > u32 acpi_state = acpi_target_sleep_state; > int error; > + u32 sci_enabled; > > ACPI_FLUSH_CPU_CACHE(); > > @@ -289,7 +290,9 @@ static int acpi_suspend_enter(suspend_state_t pm_state) > } > > /* This violates the spec but is required for bug compatibility. */ > - acpi_write_bit_register(ACPI_BITREG_SCI_ENABLE, 1); > + acpi_read_bit_register(ACPI_BITREG_SCI_ENABLE, &sci_enabled); > + if (!sci_enabled) > + acpi_write_bit_register(ACPI_BITREG_SCI_ENABLE, 1); > > /* Reprogram control registers */ > acpi_leave_sleep_state_prep(acpi_state); > >
On Sun, Oct 07, 2012 at 09:00:09PM +0200, Ondrej Zary wrote: > Added acpi_read_bit_register there and it seems that SCI_EN is already set! > > This patch fixes the problem here. I wonder how this affects systems that > require SCI_EN to be set. I /think/ that this will be safe, but it doesn't match my recollection of how Windows behaves so it may break something. Any chance you can find someone with one of the machines mentioned in https://bugzilla.kernel.org/show_bug.cgi?id=13745 and make sure that they still work with your patch?
On 10/08/2012 10:42 AM, Matthew Garrett wrote: > On Sun, Oct 07, 2012 at 09:00:09PM +0200, Ondrej Zary wrote: > >> Added acpi_read_bit_register there and it seems that SCI_EN is already set! >> >> This patch fixes the problem here. I wonder how this affects systems that >> require SCI_EN to be set. > > I /think/ that this will be safe, but it doesn't match my recollection > of how Windows behaves so it may break something. Any chance you can > find someone with one of the machines mentioned in > https://bugzilla.kernel.org/show_bug.cgi?id=13745 and make sure that > they still work with your patch? yikes. we started with a white-list, then we made setting SCI_EN the default, and here is at least one box that wants to be on a black list so that we don't set SCI_EN:-( -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 09 October 2012, Len Brown wrote: > On 10/08/2012 10:42 AM, Matthew Garrett wrote: > > On Sun, Oct 07, 2012 at 09:00:09PM +0200, Ondrej Zary wrote: > >> Added acpi_read_bit_register there and it seems that SCI_EN is already > >> set! > >> > >> This patch fixes the problem here. I wonder how this affects systems > >> that require SCI_EN to be set. > > > > I /think/ that this will be safe, but it doesn't match my recollection > > of how Windows behaves so it may break something. Any chance you can > > find someone with one of the machines mentioned in > > https://bugzilla.kernel.org/show_bug.cgi?id=13745 and make sure that > > they still work with your patch? > > yikes. > we started with a white-list, > then we made setting SCI_EN the default, > and here is at least one box that wants to be on a black list > so that we don't set SCI_EN:-( I wonder what Windows does here. Haven't tested it on this machine but I assume that it works.
--- a/drivers/acpi/sleep.c +++ b/drivers/acpi/sleep.c @@ -271,6 +271,7 @@ static int acpi_suspend_enter(suspend_state_t pm_state) acpi_status status = AE_OK; u32 acpi_state = acpi_target_sleep_state; int error; + u32 sci_enabled; ACPI_FLUSH_CPU_CACHE(); @@ -289,7 +290,9 @@ static int acpi_suspend_enter(suspend_state_t pm_state) } /* This violates the spec but is required for bug compatibility. */ - acpi_write_bit_register(ACPI_BITREG_SCI_ENABLE, 1); + acpi_read_bit_register(ACPI_BITREG_SCI_ENABLE, &sci_enabled); + if (!sci_enabled) + acpi_write_bit_register(ACPI_BITREG_SCI_ENABLE, 1); /* Reprogram control registers */ acpi_leave_sleep_state_prep(acpi_state);