On Wed, 2008-05-21 at 16:28 +0800, Zhang, Rui wrote: > Hi, all, > > With Yakui's help, we've root caused this problem. > > For the GPEs with _Lxx/_Exx methods, Linux updates the enable mask but > doesn't enable them in HW in acpi_ev_gpe_initialize. > And in acpi_ev_install_fadt_gpes, the GPEs that are shown in _PRW > methods but don't have _Lxx/_Exx methods are disabled in > acpi_ev_set_type, before 6217 is applied, the acpi_ev_disable_gpe won't > write the GPE enable register and everything goes well. > But with 6217 applied, acpi_ev_disable_gpe will write the GPE enable > register and enable all the _Lxx/_Exx GPEs as a side effect. > > And the error message is printed out if a _Lxx/_Exx GPE is fired and try > to access EC address space. > > Of cource this can be solved by Alexey's patch which installs the EC > address space handler before any GPE is enabled in HW. > > But there is still one problem with both 6217 and 9916 applied which > I'll talk about in the next email. :) Please refer to bug 9781 and 10224, where a GPE is used as a wake GPE and a RUN_WAKE GPE at the same time, Before 6217 and 9916 are applied, Linux disable the wake GPEs in acpi_ev_install_fadt_gpes, call _PSW to disable the device's wakeup ability in acpi_scan_init, and re-enable the GPE as a RUN_WAKE GPE in acpi_button_add. But with both 6217 and 9916 applied, Linux enable all the _Lxx/_Exx GPEs in acpi_ec_ecdt_probe, and won't disable it until acpi_ev_install_fadt_gpes. And this may bring an interrupt storm because the _PSW is not called and the wakeup GPE keeps on firing. IMO, we should revert the 9916 patch because Yakui reproduced the problem on T61 and confirm it's fixed by a BIOS update, maybe we should ask the bug reporter to test the new BIOS as well. :) For bug 6217, we should do the following for stray GPEs: 1. get the current value of GPE_ENABLE register 2. clear the stray GPE bit of GPE_ENABLE register 3. write the value back Any comments on this? thanks, rui > > Thanks, > Rui > > >-----Original Message----- > >From: Moore, Robert > >Sent: Friday, April 25, 2008 9:57 AM > >To: Zhang, Rui; Len Brown > >Cc: Alexey Starikovskiy; linux-acpi@xxxxxxxxxxxxxxx > >Subject: RE: [PATCH 70/73] ACPICA: Fix to disable unknown spurious GPEs > > > >Yes, this is my question also. > > > >I hope Alexey can help with this. I'm very nervous about any code that > works > >or does not work for reasons unknown. When this happens in the GPE > code, I get > >more than nervous. > > > >Bob > > > > > >-----Original Message----- > >From: Zhang, Rui > >Sent: Thursday, April 24, 2008 6:53 PM > >To: Len Brown > >Cc: Moore, Robert; Alexey Starikovskiy; linux-acpi@xxxxxxxxxxxxxxx > >Subject: Re: [PATCH 70/73] ACPICA: Fix to disable unknown spurious GPEs > > > > > >On Fri, 2008-04-25 at 09:40 +0800, Len Brown wrote: > >> oh goody, i thought this one rang a bell... > >> > >> I'll apply 6217 on top of 9916 (which was already in the test tree, > >> but not on the acpica branch) > >> > >> the error goes away with both applied. > > > >I still don't see why the error messages pop up if 6217 is applied. > >IMO, 6217 won't enable the EC GPE. :( > > > >thanks, > >rui > > > >> On Wednesday 23 April 2008, Moore, Robert wrote: > >> > Alexey, > >> > > >> > I remember there was some discussion on this, but I'm still not > sure > >> why > >> > the patch for 6217 (Ignore GPE if valid and not enabled) makes the > >> other > >> > problem (too early enabling of GPEs) apparent. Please explain. > >> > > >> > Thanks, > >> > Bob > >> > > >> > > >> > >-----Original Message----- > >> > >From: Alexey Starikovskiy [mailto:astarikovskiy@xxxxxxx] > >> > >Sent: Tuesday, April 22, 2008 10:29 PM > >> > >To: Len Brown > >> > >Cc: linux-acpi@xxxxxxxxxxxxxxx; Moore, Robert; Zhang, Rui > >> > >Subject: Re: [PATCH 70/73] ACPICA: Fix to disable unknown spurious > >> GPEs > >> > > > >> > >Len Brown wrote: > >> > >> On Saturday 12 April 2008, Len Brown wrote: > >> > >>> From: Bob Moore <robert.moore@xxxxxxxxx> > >> > >>> > >> > >>> Implemented another change to eliminate/suppress spurious or > >> > >>> stray GPEs. The AcpiEvDisableGpe function will now disable GPEs > >> > >>> that are neither enabled nor disabled -- meaning that the GPE > is > >> > >>> unknown to the system. This will prevent future interrupts from > >> > >>> that GPE. (Zhang Rui) BZ 6217 > >> > >>> > >> > >>> Signed-off-by: Bob Moore <robert.moore@xxxxxxxxx> > >> > >>> Signed-off-by: Alexey Starikovskiy <astarikovskiy@xxxxxxx> > >> > >>> Signed-off-by: Len Brown <len.brown@xxxxxxxxx> > >> > >>> --- > >> > >>> drivers/acpi/events/evgpe.c | 9 ++++++++- > >> > >>> 1 files changed, 8 insertions(+), 1 deletions(-) > >> > >>> > >> > >>> diff --git a/drivers/acpi/events/evgpe.c > >> > b/drivers/acpi/events/evgpe.c > >> > >>> index 897baed..6940f5d 100644 > >> > >>> --- a/drivers/acpi/events/evgpe.c > >> > >>> +++ b/drivers/acpi/events/evgpe.c > >> > >>> @@ -250,7 +250,14 @@ acpi_status acpi_ev_disable_gpe(struct > >> > >acpi_gpe_event_info *gpe_event_info) > >> > >>> > >> > >>> ACPI_FUNCTION_TRACE(ev_disable_gpe); > >> > >>> > >> > >>> - if (!(gpe_event_info->flags & ACPI_GPE_ENABLE_MASK)) { > >> > >>> + /* > >> > >>> + * Ignore this if the GPE is valid and not enabled. > >> > >>> + * > >> > >>> + * Flags is only zero if GPE is neither enabled or disabled > -- > >> > it may > >> > >>> + * be a spurious or stray GPE -- disable it in the default > >> case > >> > >below. > >> > >>> + */ > >> > >>> + if (gpe_event_info->flags && > >> > >>> + (!(gpe_event_info->flags & ACPI_GPE_ENABLE_MASK))) { > >> > >>> return_ACPI_STATUS(AE_OK); > >> > >>> } > >> > >>> > >> > >> > >> > >> > >> > >> When applied to 2.6.25, the patch above causes ACPI Error > >> messages > >> > below > >> > >on my T61. > >> > >> Certainly this is a case of the EC getting an interrupt (GPE > x02) > >> > before > >> > >it has initialized. > >> > >> > >> > >> Perhaps it breaks the call of acpi_install_gpe_handler() to > >> > >acpi_ev_disable_gpe()? > >> > >Congratulations, now you too hit this one, patch is already > >> there... > >> > >It is wroong to enable GPE before handler for it is installed... > >> > >http://bugzilla.kernel.org/show_bug.cgi?id=9916 > >> > > > >> > >> > >> > >> -Len > >> > >> > >> > >> > >> > >> PCI: Using MMCONFIG at f0000000 - f3ffffff > >> > >> PCI: Using configuration type 1 > >> > >> evgpeblk-0956 [00] ev_create_gpe_block : GPE 00 to 1F [_GPE] > 4 > >> > regs on > >> > >int 0x9 > >> > >> +ACPI Error (evregion-0316): No handler for Region [ECOR] > >> > >(ffff81007d35dda0) [EmbeddedControl] [20070126] > >> > >> +ACPI Error (exfldio-0289): Region EmbeddedControl(3) has no > >> handler > >> > >[20070126] > >> > >> +ACPI Error (psparse-0537): Method parse/execution failed > >> > >[\_SB_.PCI0.LPC_.EC__.AC__._PSR] (Node ffff81007d364ab0), > >> AE_NOT_EXIST > >> > >> +ACPI Error (psparse-0537): Method parse/execution failed > >> > >[\_SB_.PCI0.LPC_.EC__.HKEY.CKC4] (Node ffff81007d364f48), > >> AE_NOT_EXIST > >> > >> +ACPI Error (psparse-0537): Method parse/execution failed > >> > [\_GPE._L02] > >> > >(Node ffff81007d376420), AE_NOT_EXIST > >> > >> +ACPI Exception (evgpe-0584): AE_NOT_EXIST, while evaluating GPE > >> > method > >> > >[_L02] [20070126] > >> > >> evgpeblk-1052 [00] ev_initialize_gpe_bloc: Found 9 Wake, > Enabled > >> 3 > >> > >Runtime GPEs in this block > >> > >> ACPI: EC: EC description table is found, configuring boot EC > >> > >> -ACPI: EC: non-query interrupt received, switching to interrupt > >> mode > >> > >> -Completing Region/Field/Buffer/Package > >> > > >> > >initialization:........................................................ > >> > .... > >> > >............................................................ > >> > >> > >> > > >> > >....................................................................... > >> > .... > >> > > >> > >....................................................................... > >> > .... > >> > >................ > >> > >> -Initialized 29/33 Regions 138/138 Fields 64/64 Buffers 55/72 > >> > Packages > >> > >(1888 nodes) > >> > >> +Completing Region/Field/Buffer/Package > >> > > >> > >initialization:........................................................ > >> > .... > >> > >............................................................ > >> > >> > >> > > >> > >....................................................................... > >> > .... > >> > > >> > >....................................................................... > >> > .... > >> > >................. > >> > >> +Initialized 30/33 Regions 138/138 Fields 64/64 Buffers 55/72 > >> > Packages > >> > >(1888 nodes) > >> > >> Initializing Device/Processor/Thermal objects by executing _INI > >> > >methods:.<5>ACPI: BIOS _OSI(Linux) query honored via DMI > >> > >> +ACPI: EC: non-query interrupt received, switching to interrupt > >> mode > >> > >> ....... > >> > >> Executed 8 _INI methods requiring 2 _STA executions (examined > 88 > >> > >objects) > >> > >> ACPI: Interpreter enabled > >> > > >> > > >> > >> > >> > >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
