* Regression between rc2 and rc6: ACPI EC problems
@ 2016-07-06 14:14 Wolfram Sang
2016-07-06 21:28 ` Rafael J. Wysocki
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Wolfram Sang @ 2016-07-06 14:14 UTC (permalink / raw)
To: linux-acpi; +Cc: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]
Hi,
when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
temperature status info got bogus. Here is the major change between the
boot logs:
ACPI : EC: EC stopped
ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for [EmbeddedControl] (20160422/evregion-300)
ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG] (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
ACPI Exception: AE_BAD_PARAMETER, from region _REG, [EmbeddedControl] (20160422/evregion-397)
ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for [EmbeddedControl] (20160422/evregion-300)
ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG] (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
ACPI Exception: AE_BAD_PARAMETER, from region _REG, [EmbeddedControl] (20160422/evregion-397)
ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
ACPI : EC: EC started
The only config changes between the kernels were enabling MMC block
devices and DM_CRYPT. Not likely to cause the above, I'd think.
Does that ring a bell for someone? This little machine has only an SSD
without TRIM, so bisecting is not really an option. I can provide more
info and test patches, though.
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Regression between rc2 and rc6: ACPI EC problems
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
@ 2016-07-06 21:28 ` Rafael J. Wysocki
2016-07-07 0:26 ` Zheng, Lv
2016-07-07 0:30 ` Zheng, Lv
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2016-07-06 21:28 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-acpi, linux-kernel, Lv Zheng
[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]
On Wednesday, July 06, 2016 11:14:11 PM Wolfram Sang wrote:
> Hi,
>
> when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> temperature status info got bogus. Here is the major change between the
> boot logs:
>
> ACPI : EC: EC stopped
> ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for [EmbeddedControl] (20160422/evregion-300)
> ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG] (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> ACPI Exception: AE_BAD_PARAMETER, from region _REG, [EmbeddedControl] (20160422/evregion-397)
> ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for [EmbeddedControl] (20160422/evregion-300)
> ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG] (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> ACPI Exception: AE_BAD_PARAMETER, from region _REG, [EmbeddedControl] (20160422/evregion-397)
> ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
> ACPI : EC: EC started
>
> The only config changes between the kernels were enabling MMC block
> devices and DM_CRYPT. Not likely to cause the above, I'd think.
>
> Does that ring a bell for someone? This little machine has only an SSD
> without TRIM, so bisecting is not really an option. I can provide more
> info and test patches, though.
There is one commit related directly to EC between -rc2 and -rc6:
dcf15cbded65 ACPI / EC: Fix a boot EC regresion by restoring boot EC support for the DSDT EC
There also are two ACPICA commits in that frame that may be related to it
2f38b1b16d92 ACPICA: Namespace: Fix deadlock triggered by MLC support in dynamic table loading
da4e792550a8 Revert "ACPICA: ACPI 2.0, Hardware: Add access_width/bit_offset support for acpi_hw_write()"
but one of them is a revert, so can you please check dcf15cbded65 and 2f38b1b16d92?
Thanks,
Rafael
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Regression between rc2 and rc6: ACPI EC problems
2016-07-06 21:28 ` Rafael J. Wysocki
@ 2016-07-07 0:26 ` Zheng, Lv
2016-07-07 0:36 ` Rafael J. Wysocki
0 siblings, 1 reply; 15+ messages in thread
From: Zheng, Lv @ 2016-07-07 0:26 UTC (permalink / raw)
To: Rafael J. Wysocki, Wolfram Sang; +Cc: linux-acpi, linux-kernel
Hi, Rafael
> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Thursday, July 7, 2016 5:29 AM
> Subject: Re: Regression between rc2 and rc6: ACPI EC problems
>
> On Wednesday, July 06, 2016 11:14:11 PM Wolfram Sang wrote:
> > Hi,
> >
> > when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> > temperature status info got bogus. Here is the major change between
> the
> > boot logs:
> >
> > ACPI : EC: EC stopped
> > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> [EmbeddedControl] (20160422/evregion-300)
> > ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> > ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> [EmbeddedControl] (20160422/evregion-397)
> > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> [EmbeddedControl] (20160422/evregion-300)
> > ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> > ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> [EmbeddedControl] (20160422/evregion-397)
> > ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
> > ACPI : EC: EC started
[Lv Zheng]
This is a known order issue that ACPICA executes _REG(DISCONENCT) after removing the opregion handler.
But this issue is not a function failure, it only generates error garbage in the kernel log.
So we didn't fix it in a hurry.
There is another order issue detected in the same Bugzilla entry that:
_REG(CONNECT/DISCONNECT) shouldn't be invoked for SystemMemory/SystemIo and PCI_Config that under a PCI root bus containing _BBN method.
Which is also proven by the Windows behavior and spec definitions.
I was planning to fix the both issues in the ACPICA upstream.
The bug is here for your reference:
https://bugzilla.kernel.org/show_bug.cgi?id=102421
For now, since this is just a non-function-failure error report, we can ignore it.
Because sometimes, when there are several problems related, we have to make a choice to fix one of them first.
Thanks and best regards
-Lv
> >
> > The only config changes between the kernels were enabling MMC block
> > devices and DM_CRYPT. Not likely to cause the above, I'd think.
> >
> > Does that ring a bell for someone? This little machine has only an SSD
> > without TRIM, so bisecting is not really an option. I can provide more
> > info and test patches, though.
>
> There is one commit related directly to EC between -rc2 and -rc6:
>
> dcf15cbded65 ACPI / EC: Fix a boot EC regresion by restoring boot EC
> support for the DSDT EC
>
> There also are two ACPICA commits in that frame that may be related to it
>
> 2f38b1b16d92 ACPICA: Namespace: Fix deadlock triggered by MLC
> support in dynamic table loading
> da4e792550a8 Revert "ACPICA: ACPI 2.0, Hardware: Add
> access_width/bit_offset support for acpi_hw_write()"
>
> but one of them is a revert, so can you please check dcf15cbded65 and
> 2f38b1b16d92?
>
> Thanks,
> Rafael
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Regression between rc2 and rc6: ACPI EC problems
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
2016-07-06 21:28 ` Rafael J. Wysocki
@ 2016-07-07 0:30 ` Zheng, Lv
2016-07-07 0:31 ` Zheng, Lv
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Zheng, Lv @ 2016-07-07 0:30 UTC (permalink / raw)
To: Wolfram Sang, linux-acpi; +Cc: linux-kernel
Hi, Wolfram
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Wolfram Sang
> Subject: Regression between rc2 and rc6: ACPI EC problems
>
> Hi,
>
> when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> temperature status info got bogus. Here is the major change between the
> boot logs:
>
> ACPI : EC: EC stopped
> ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> [EmbeddedControl] (20160422/evregion-300)
> ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> [EmbeddedControl] (20160422/evregion-397)
> ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> [EmbeddedControl] (20160422/evregion-300)
> ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> [EmbeddedControl] (20160422/evregion-397)
> ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
> ACPI : EC: EC started
>
> The only config changes between the kernels were enabling MMC block
> devices and DM_CRYPT. Not likely to cause the above, I'd think.
>
> Does that ring a bell for someone? This little machine has only an SSD
> without TRIM, so bisecting is not really an option. I can provide more
> info and test patches, though.
[Lv Zheng]
Please just ignore this for several release cycles.
We need strategy to fix the ACPI initialization order issues.
This is already a noted non-function-failure issue.
We just need to fix the functional ones prior than this.
Thanks and best regards
-Lv
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Regression between rc2 and rc6: ACPI EC problems
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
2016-07-06 21:28 ` Rafael J. Wysocki
2016-07-07 0:30 ` Zheng, Lv
@ 2016-07-07 0:31 ` Zheng, Lv
2016-07-07 1:49 ` Lv Zheng
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Zheng, Lv @ 2016-07-07 0:31 UTC (permalink / raw)
To: Wolfram Sang, linux-acpi; +Cc: linux-kernel
Hi, Walfram
You can register onto the kernel Bugzilla and monitor this bug for further updates:
https://bugzilla.kernel.org/show_bug.cgi?id=102421
Thanks and best regards
-Lv
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Wolfram Sang
> Subject: Regression between rc2 and rc6: ACPI EC problems
>
> Hi,
>
> when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> temperature status info got bogus. Here is the major change between the
> boot logs:
>
> ACPI : EC: EC stopped
> ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> [EmbeddedControl] (20160422/evregion-300)
> ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> [EmbeddedControl] (20160422/evregion-397)
> ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> [EmbeddedControl] (20160422/evregion-300)
> ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> [EmbeddedControl] (20160422/evregion-397)
> ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
> ACPI : EC: EC started
>
> The only config changes between the kernels were enabling MMC block
> devices and DM_CRYPT. Not likely to cause the above, I'd think.
>
> Does that ring a bell for someone? This little machine has only an SSD
> without TRIM, so bisecting is not really an option. I can provide more
> info and test patches, though.
>
> Thanks,
>
> Wolfram
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Regression between rc2 and rc6: ACPI EC problems
2016-07-07 0:36 ` Rafael J. Wysocki
@ 2016-07-07 0:34 ` Zheng, Lv
0 siblings, 0 replies; 15+ messages in thread
From: Zheng, Lv @ 2016-07-07 0:34 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Wolfram Sang, linux-acpi, linux-kernel
Hi, Rafael
> From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> owner@vger.kernel.org] On Behalf Of Rafael J. Wysocki
> Subject: Re: Regression between rc2 and rc6: ACPI EC problems
>
> On Thursday, July 07, 2016 12:26:49 AM Zheng, Lv wrote:
> > Hi, Rafael
> >
> > > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > > Sent: Thursday, July 7, 2016 5:29 AM
> > > Subject: Re: Regression between rc2 and rc6: ACPI EC problems
> > >
> > > On Wednesday, July 06, 2016 11:14:11 PM Wolfram Sang wrote:
> > > > Hi,
> > > >
> > > > when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> > > > temperature status info got bogus. Here is the major change
> between
> > > the
> > > > boot logs:
> > > >
> > > > ACPI : EC: EC stopped
> > > > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> > > [EmbeddedControl] (20160422/evregion-300)
> > > > ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> > > (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> > > > ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> > > [EmbeddedControl] (20160422/evregion-397)
> > > > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> > > [EmbeddedControl] (20160422/evregion-300)
> > > > ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> > > (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> > > > ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> > > [EmbeddedControl] (20160422/evregion-397)
> > > > ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
> > > > ACPI : EC: EC started
> > [Lv Zheng]
> > This is a known order issue that ACPICA executes _REG(DISCONENCT)
> after removing the opregion handler.
> > But this issue is not a function failure, it only generates error garbage in
> the kernel log.
> > So we didn't fix it in a hurry.
> >
> > There is another order issue detected in the same Bugzilla entry that:
> > _REG(CONNECT/DISCONNECT) shouldn't be invoked for
> SystemMemory/SystemIo and PCI_Config that under a PCI root bus
> containing _BBN method.
> > Which is also proven by the Windows behavior and spec definitions.
> >
> > I was planning to fix the both issues in the ACPICA upstream.
> > The bug is here for your reference:
> > https://bugzilla.kernel.org/show_bug.cgi?id=102421
> >
> > For now, since this is just a non-function-failure error report, we can
> ignore it.
> > Because sometimes, when there are several problems related, we have
> to make a choice to fix one of them first.
>
> Well, please look at the report again. It says:
>
> "when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> temperature status info got bogus".
>
> So, it very much *is* a functional issue and I'm not going to ignore it.
[Lv Zheng]
What does the bogus mean here?
Anyway, I'll post a fix for this first.
I'll do this right now.
Thanks and best regards
-Lv
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Regression between rc2 and rc6: ACPI EC problems
2016-07-07 0:26 ` Zheng, Lv
@ 2016-07-07 0:36 ` Rafael J. Wysocki
2016-07-07 0:34 ` Zheng, Lv
0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2016-07-07 0:36 UTC (permalink / raw)
To: Zheng, Lv; +Cc: Wolfram Sang, linux-acpi, linux-kernel
On Thursday, July 07, 2016 12:26:49 AM Zheng, Lv wrote:
> Hi, Rafael
>
> > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> > Sent: Thursday, July 7, 2016 5:29 AM
> > Subject: Re: Regression between rc2 and rc6: ACPI EC problems
> >
> > On Wednesday, July 06, 2016 11:14:11 PM Wolfram Sang wrote:
> > > Hi,
> > >
> > > when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
> > > temperature status info got bogus. Here is the major change between
> > the
> > > boot logs:
> > >
> > > ACPI : EC: EC stopped
> > > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> > [EmbeddedControl] (20160422/evregion-300)
> > > ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> > (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> > > ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> > [EmbeddedControl] (20160422/evregion-397)
> > > ACPI Exception: AE_BAD_PARAMETER, Returned by Handler for
> > [EmbeddedControl] (20160422/evregion-300)
> > > ACPI Error: Method parse/execution failed [\_SB.PCI0.LPCB.EC0._REG]
> > (Node f5d09870), AE_BAD_PARAMETER (20160422/psparse-542)
> > > ACPI Exception: AE_BAD_PARAMETER, from region _REG,
> > [EmbeddedControl] (20160422/evregion-397)
> > > ACPI : EC: GPE = 0x19, I/O: command/status = 0x66, data = 0x62
> > > ACPI : EC: EC started
> [Lv Zheng]
> This is a known order issue that ACPICA executes _REG(DISCONENCT) after removing the opregion handler.
> But this issue is not a function failure, it only generates error garbage in the kernel log.
> So we didn't fix it in a hurry.
>
> There is another order issue detected in the same Bugzilla entry that:
> _REG(CONNECT/DISCONNECT) shouldn't be invoked for SystemMemory/SystemIo and PCI_Config that under a PCI root bus containing _BBN method.
> Which is also proven by the Windows behavior and spec definitions.
>
> I was planning to fix the both issues in the ACPICA upstream.
> The bug is here for your reference:
> https://bugzilla.kernel.org/show_bug.cgi?id=102421
>
> For now, since this is just a non-function-failure error report, we can ignore it.
> Because sometimes, when there are several problems related, we have to make a choice to fix one of them first.
Well, please look at the report again. It says:
"when upgrading from v4.7-rc2 to rc6 on my Dell Mini 9, battery and
temperature status info got bogus".
So, it very much *is* a functional issue and I'm not going to ignore it.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH] ACPI / EC: Fix an order issue in ec_remove_handlers()
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
@ 2016-07-07 1:49 ` Lv Zheng
2016-07-07 0:30 ` Zheng, Lv
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Lv Zheng @ 2016-07-07 1:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Wolfram Sang, Nicholas
There is an order issue in ec_remove_handlers() that the functions invoked
in it are not invoked in the reversed order of their appearance in
ec_install_handlers(). This existing issue has been triggered by the
following commit:
Commit: dcf15cbded656a12335bc4151f3f75f10080a375
Subject: ACPI / EC: Fix a boot EC regresion by restoring boot EC
The commit invokes ec_remove_handlers() during runtime, thus uncovers this
issue. This patch fixes this regression.
Fixes: dcf15cbded65 ("ACPI / EC: Fix a boot EC regresion by restoring boot EC")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
Reported-by: Wolfram Sang <wsa@the-dreams.de>
Reported-by: Nicholas <nkudriavtsev@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Nicholas <nkudriavtsev@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/ec.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b1050a0..9ff3d4b 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1331,7 +1331,12 @@ static int ec_install_handlers(struct acpi_ec *ec)
static void ec_remove_handlers(struct acpi_ec *ec)
{
- acpi_ec_stop(ec, false);
+ if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
+ if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
+ &acpi_ec_gpe_handler)))
+ pr_err("failed to remove gpe handler\n");
+ clear_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags);
+ }
if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
@@ -1340,12 +1345,7 @@ static void ec_remove_handlers(struct acpi_ec *ec)
clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
}
- if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
- if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
- &acpi_ec_gpe_handler)))
- pr_err("failed to remove gpe handler\n");
- clear_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags);
- }
+ acpi_ec_stop(ec, false);
}
static struct acpi_ec *acpi_ec_alloc(void)
--
1.7.10
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [RFC PATCH] ACPI / EC: Fix an order issue in ec_remove_handlers()
@ 2016-07-07 1:49 ` Lv Zheng
0 siblings, 0 replies; 15+ messages in thread
From: Lv Zheng @ 2016-07-07 1:49 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi, Wolfram Sang, Nicholas
There is an order issue in ec_remove_handlers() that the functions invoked
in it are not invoked in the reversed order of their appearance in
ec_install_handlers(). This existing issue has been triggered by the
following commit:
Commit: dcf15cbded656a12335bc4151f3f75f10080a375
Subject: ACPI / EC: Fix a boot EC regresion by restoring boot EC
The commit invokes ec_remove_handlers() during runtime, thus uncovers this
issue. This patch fixes this regression.
Fixes: dcf15cbded65 ("ACPI / EC: Fix a boot EC regresion by restoring boot EC")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
Reported-by: Wolfram Sang <wsa@the-dreams.de>
Reported-by: Nicholas <nkudriavtsev@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Nicholas <nkudriavtsev@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/ec.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b1050a0..9ff3d4b 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1331,7 +1331,12 @@ static int ec_install_handlers(struct acpi_ec *ec)
static void ec_remove_handlers(struct acpi_ec *ec)
{
- acpi_ec_stop(ec, false);
+ if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
+ if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
+ &acpi_ec_gpe_handler)))
+ pr_err("failed to remove gpe handler\n");
+ clear_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags);
+ }
if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
@@ -1340,12 +1345,7 @@ static void ec_remove_handlers(struct acpi_ec *ec)
clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
}
- if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
- if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
- &acpi_ec_gpe_handler)))
- pr_err("failed to remove gpe handler\n");
- clear_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags);
- }
+ acpi_ec_stop(ec, false);
}
static struct acpi_ec *acpi_ec_alloc(void)
--
1.7.10
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [UPDATE RFC PATCH v2] ACPI / EC: Fix an order issue in ec_remove_handlers()
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
` (3 preceding siblings ...)
2016-07-07 1:49 ` Lv Zheng
@ 2016-07-07 4:37 ` Lv Zheng
2016-07-07 11:46 ` Wolfram Sang
2016-07-08 1:25 ` Lv Zheng
5 siblings, 1 reply; 15+ messages in thread
From: Lv Zheng @ 2016-07-07 4:37 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-acpi, Wolfram Sang, Nicholas
(Updated to add v2 indicator)
There is an order issue in ec_remove_handlers() that the functions invoked
in it are not invoked in the reversed order of their appearance in
ec_install_handlers(). This existing issue has been triggered by the
following commit:
Commit: dcf15cbded656a12335bc4151f3f75f10080a375
Subject: ACPI / EC: Fix a boot EC regresion by restoring boot EC
The commit invokes ec_remove_handlers() during runtime, thus uncovers this
issue. This patch fixes this regression.
Fixes: dcf15cbded65 ("ACPI / EC: Fix a boot EC regresion by restoring boot EC")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
Reported-by: Wolfram Sang <wsa@the-dreams.de>
Reported-by: Nicholas <nkudriavtsev@gmail.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: Nicholas <nkudriavtsev@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/ec.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b1050a0..d513208 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1331,8 +1331,6 @@ static int ec_install_handlers(struct acpi_ec *ec)
static void ec_remove_handlers(struct acpi_ec *ec)
{
- acpi_ec_stop(ec, false);
-
if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
@@ -1340,6 +1338,17 @@ static void ec_remove_handlers(struct acpi_ec *ec)
clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
}
+ /*
+ * Disabling EC (transactions) after removing the operation region
+ * handler. This order is required because _REG(DISCONNECT) may
+ * access the EmbeddedControl operation regions.
+ *
+ * Flushing transactions before removing the GPE handler. This is
+ * required by the current ACPICA GPE design. ACPICA GPE will block
+ * a GPE if there is no way to handle it.
+ */
+ acpi_ec_stop(ec, false);
+
if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
&acpi_ec_gpe_handler)))
--
1.7.10
^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [RFC PATCH] ACPI / EC: Fix an order issue in ec_remove_handlers()
2016-07-07 1:49 ` Lv Zheng
(?)
@ 2016-07-07 4:38 ` Zheng, Lv
-1 siblings, 0 replies; 15+ messages in thread
From: Zheng, Lv @ 2016-07-07 4:38 UTC (permalink / raw)
To: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len
Cc: Lv Zheng, linux-kernel, linux-acpi, Wolfram Sang, Nicholas
This one and the previous one contain problems.
A patch marked as [UPDATE RFC v2] is the correct fix.
Sorry for the noise.
Thanks and best regards
-Lv
> From: Zheng, Lv
> Subject: [RFC PATCH] ACPI / EC: Fix an order issue in ec_remove_handlers()
>
> There is an order issue in ec_remove_handlers() that the functions invoked
> in it are not invoked in the reversed order of their appearance in
> ec_install_handlers(). This existing issue has been triggered by the
> following commit:
> Commit: dcf15cbded656a12335bc4151f3f75f10080a375
> Subject: ACPI / EC: Fix a boot EC regresion by restoring boot EC
> The commit invokes ec_remove_handlers() during runtime, thus uncovers
> this
> issue. This patch fixes this regression.
>
> Fixes: dcf15cbded65 ("ACPI / EC: Fix a boot EC regresion by restoring boot
> EC")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
> Reported-by: Wolfram Sang <wsa@the-dreams.de>
> Reported-by: Nicholas <nkudriavtsev@gmail.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Nicholas <nkudriavtsev@gmail.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> ---
> drivers/acpi/ec.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> Index: linux-acpica/drivers/acpi/ec.c
> ==========================================================
> =========
> --- linux-acpica.orig/drivers/acpi/ec.c
> +++ linux-acpica/drivers/acpi/ec.c
> @@ -1331,8 +1331,6 @@ static int ec_install_handlers(struct ac
>
> static void ec_remove_handlers(struct acpi_ec *ec)
> {
> - acpi_ec_stop(ec, false);
> -
> if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
> if (ACPI_FAILURE(acpi_remove_address_space_handler(ec-
> >handle,
> ACPI_ADR_SPACE_EC,
> &acpi_ec_space_handler)))
> @@ -1340,6 +1338,17 @@ static void ec_remove_handlers(struct ac
> clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
> }
>
> + /*
> + * Disabling EC (transactions) after removing the operation region
> + * handler. This order is required because _REG(DISCONNECT) may
> + * access the EmbeddedControl operation regions.
> + *
> + * Flushing transactions before removing the GPE handler. This is
> + * required by the current ACPICA GPE design. ACPICA GPE will
> block
> + * a GPE if there is no way to handle it.
> + */
> + acpi_ec_stop(ec, false);
> +
> if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
> if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
> &acpi_ec_gpe_handler)))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [UPDATE RFC PATCH v2] ACPI / EC: Fix an order issue in ec_remove_handlers()
2016-07-07 4:37 ` [UPDATE RFC PATCH v2] " Lv Zheng
@ 2016-07-07 11:46 ` Wolfram Sang
2016-07-08 0:38 ` Zheng, Lv
0 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2016-07-07 11:46 UTC (permalink / raw)
To: Lv Zheng
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown, Lv Zheng,
linux-acpi, Nicholas
[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]
On Thu, Jul 07, 2016 at 12:37:10PM +0800, Lv Zheng wrote:
> (Updated to add v2 indicator)
>
> There is an order issue in ec_remove_handlers() that the functions invoked
> in it are not invoked in the reversed order of their appearance in
> ec_install_handlers(). This existing issue has been triggered by the
> following commit:
> Commit: dcf15cbded656a12335bc4151f3f75f10080a375
> Subject: ACPI / EC: Fix a boot EC regresion by restoring boot EC
> The commit invokes ec_remove_handlers() during runtime, thus uncovers this
> issue. This patch fixes this regression.
>
> Fixes: dcf15cbded65 ("ACPI / EC: Fix a boot EC regresion by restoring boot EC")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
> Reported-by: Wolfram Sang <wsa@the-dreams.de>
> Reported-by: Nicholas <nkudriavtsev@gmail.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Nicholas <nkudriavtsev@gmail.com>
> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Tested-by: Wolfram Sang <wsa@the-dreams.de>
The status infos are back to normal and the EC error reports are gone
from the log. Looks good! Thanks for the quick fix.
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [UPDATE RFC PATCH v2] ACPI / EC: Fix an order issue in ec_remove_handlers()
2016-07-07 11:46 ` Wolfram Sang
@ 2016-07-08 0:38 ` Zheng, Lv
0 siblings, 0 replies; 15+ messages in thread
From: Zheng, Lv @ 2016-07-08 0:38 UTC (permalink / raw)
To: Wolfram Sang
Cc: Wysocki, Rafael J, Rafael J. Wysocki, Brown, Len, Lv Zheng,
linux-acpi, Nicholas
Hi,
> From: Wolfram Sang [mailto:wsa@the-dreams.de]
> Subject: Re: [UPDATE RFC PATCH v2] ACPI / EC: Fix an order issue in
> ec_remove_handlers()
>
> On Thu, Jul 07, 2016 at 12:37:10PM +0800, Lv Zheng wrote:
> > (Updated to add v2 indicator)
> >
> > There is an order issue in ec_remove_handlers() that the functions
> invoked
> > in it are not invoked in the reversed order of their appearance in
> > ec_install_handlers(). This existing issue has been triggered by the
> > following commit:
> > Commit: dcf15cbded656a12335bc4151f3f75f10080a375
> > Subject: ACPI / EC: Fix a boot EC regresion by restoring boot EC
> > The commit invokes ec_remove_handlers() during runtime, thus
> uncovers this
> > issue. This patch fixes this regression.
> >
> > Fixes: dcf15cbded65 ("ACPI / EC: Fix a boot EC regresion by restoring
> boot EC")
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
> > Reported-by: Wolfram Sang <wsa@the-dreams.de>
> > Reported-by: Nicholas <nkudriavtsev@gmail.com>
> > Cc: Wolfram Sang <wsa@the-dreams.de>
> > Cc: Nicholas <nkudriavtsev@gmail.com>
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>
> Tested-by: Wolfram Sang <wsa@the-dreams.de>
>
> The status infos are back to normal and the EC error reports are gone
> from the log. Looks good! Thanks for the quick fix.
[Lv Zheng]
Thanks for the test, I'll refresh the patch with your tested-by added and RFC removed.
Best regards
-Lv
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3] ACPI / EC: Fix an order issue in ec_remove_handlers()
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
@ 2016-07-08 1:25 ` Lv Zheng
2016-07-07 0:30 ` Zheng, Lv
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Lv Zheng @ 2016-07-08 1:25 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi
There is an order issue in ec_remove_handlers() that the functions invoked
in it are not able to handle the current restrictions (see comments below).
If ec_remove_handlers() is invoked during runtime after _REG(CONNECT) is
invoked, this issue will be uncovered.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
Reported-and-tested-by: Wolfram Sang <wsa@the-dreams.de>
Reported-by: Nicholas <nkudriavtsev@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/ec.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b1050a0..84073a6 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1331,8 +1331,6 @@ static int ec_install_handlers(struct acpi_ec *ec)
static void ec_remove_handlers(struct acpi_ec *ec)
{
- acpi_ec_stop(ec, false);
-
if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
@@ -1340,6 +1338,19 @@ static void ec_remove_handlers(struct acpi_ec *ec)
clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
}
+ /*
+ * Stops handling the EC transactions after removing the operation
+ * region handler. This is required because _REG(DISCONNECT)
+ * invoked during the removal can result in new EC transactions.
+ *
+ * Flushes the EC requests and thus disables the GPE before
+ * removing the GPE handler. This is required by the current ACPICA
+ * GPE core. ACPICA GPE core will automatically disable a GPE when
+ * it is indicated but there is no way to handle it. So the drivers
+ * must disable the GPEs prior than removing the GPE handlers.
+ */
+ acpi_ec_stop(ec, false);
+
if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
&acpi_ec_gpe_handler)))
--
1.7.10
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3] ACPI / EC: Fix an order issue in ec_remove_handlers()
@ 2016-07-08 1:25 ` Lv Zheng
0 siblings, 0 replies; 15+ messages in thread
From: Lv Zheng @ 2016-07-08 1:25 UTC (permalink / raw)
To: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown
Cc: Lv Zheng, Lv Zheng, linux-kernel, linux-acpi
There is an order issue in ec_remove_handlers() that the functions invoked
in it are not able to handle the current restrictions (see comments below).
If ec_remove_handlers() is invoked during runtime after _REG(CONNECT) is
invoked, this issue will be uncovered.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=102421
Reported-and-tested-by: Wolfram Sang <wsa@the-dreams.de>
Reported-by: Nicholas <nkudriavtsev@gmail.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
drivers/acpi/ec.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index b1050a0..84073a6 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1331,8 +1331,6 @@ static int ec_install_handlers(struct acpi_ec *ec)
static void ec_remove_handlers(struct acpi_ec *ec)
{
- acpi_ec_stop(ec, false);
-
if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_address_space_handler(ec->handle,
ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
@@ -1340,6 +1338,19 @@ static void ec_remove_handlers(struct acpi_ec *ec)
clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
}
+ /*
+ * Stops handling the EC transactions after removing the operation
+ * region handler. This is required because _REG(DISCONNECT)
+ * invoked during the removal can result in new EC transactions.
+ *
+ * Flushes the EC requests and thus disables the GPE before
+ * removing the GPE handler. This is required by the current ACPICA
+ * GPE core. ACPICA GPE core will automatically disable a GPE when
+ * it is indicated but there is no way to handle it. So the drivers
+ * must disable the GPEs prior than removing the GPE handlers.
+ */
+ acpi_ec_stop(ec, false);
+
if (test_bit(EC_FLAGS_GPE_HANDLER_INSTALLED, &ec->flags)) {
if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe,
&acpi_ec_gpe_handler)))
--
1.7.10
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-07-08 1:25 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-06 14:14 Regression between rc2 and rc6: ACPI EC problems Wolfram Sang
2016-07-06 21:28 ` Rafael J. Wysocki
2016-07-07 0:26 ` Zheng, Lv
2016-07-07 0:36 ` Rafael J. Wysocki
2016-07-07 0:34 ` Zheng, Lv
2016-07-07 0:30 ` Zheng, Lv
2016-07-07 0:31 ` Zheng, Lv
2016-07-07 1:49 ` [RFC PATCH] ACPI / EC: Fix an order issue in ec_remove_handlers() Lv Zheng
2016-07-07 1:49 ` Lv Zheng
2016-07-07 4:38 ` Zheng, Lv
2016-07-07 4:37 ` [UPDATE RFC PATCH v2] " Lv Zheng
2016-07-07 11:46 ` Wolfram Sang
2016-07-08 0:38 ` Zheng, Lv
2016-07-08 1:25 ` [PATCH v3] " Lv Zheng
2016-07-08 1:25 ` Lv Zheng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.