| Message ID | 6344941.kJ2o2XerZ0@aspire.rjw.lan |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi, Rafael > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J. > Wysocki > Subject: [PATCH 3/3] ACPI / scan: Enable GPEs before scanning the namespace > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > On some systems the platform firmware expects GPEs to be enabled > before the enumeration of devices and if that expectation is not > met, the systems in question may not boot in some situations. > > For this reason, change the initialization ordering of the ACPI > subsystem to make it enable GPEs before scanning the namespace > for the first time in order to enumerate devices. This indeed is worthy of a try. Acked-by: Lv Zheng <lv.zheng@intel.com> Thanks and best regards Lv > > Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/scan.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > acpi_get_spcr_uart_addr(); > } > > + acpi_gpe_apply_masked_gpes(); > + acpi_update_all_gpes(); > + acpi_ec_ecdt_start(); > + > mutex_lock(&acpi_scan_lock); > /* > * Enumerate devices in the ACPI namespace. > @@ -2163,10 +2167,6 @@ int __init acpi_scan_init(void) > } > } > > - acpi_gpe_apply_masked_gpes(); > - acpi_update_all_gpes(); > - acpi_ec_ecdt_start(); > - > acpi_scan_initialized = true; > > out: > > > -- > 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 Thu, Aug 10, 2017 at 12:34:23AM +0200, Rafael J. Wysocki wrote: > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > acpi_get_spcr_uart_addr(); > } > > + acpi_gpe_apply_masked_gpes(); > + acpi_update_all_gpes(); > + acpi_ec_ecdt_start(); > + > mutex_lock(&acpi_scan_lock); > /* > * Enumerate devices in the ACPI namespace. I notice this is called from a subsys_initcall(). We scan the PCI bus much earlier in arch/x86/kernel/early-quirks.c and it would be possible to identify presence of Thunderbolt host controllers in an early quirk (using the method of pci_is_thunderbolt_attached()) and, if found, enable their GPEs or all GPEs. Just as an aside in case your method doesn't work, I'm not affected by this issue being a Mac user... ;-) Thanks, Lukas
Hi, > From: Lukas Wunner [mailto:lukas@wunner.de] > Subject: Re: [PATCH 3/3] ACPI / scan: Enable GPEs before scanning the namespace > > On Thu, Aug 10, 2017 at 12:34:23AM +0200, Rafael J. Wysocki wrote: > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > > acpi_get_spcr_uart_addr(); > > } > > > > + acpi_gpe_apply_masked_gpes(); > > + acpi_update_all_gpes(); > > + acpi_ec_ecdt_start(); > > + > > mutex_lock(&acpi_scan_lock); > > /* > > * Enumerate devices in the ACPI namespace. > > I notice this is called from a subsys_initcall(). We scan the PCI bus > much earlier in arch/x86/kernel/early-quirks.c and it would be possible > to identify presence of Thunderbolt host controllers in an early quirk > (using the method of pci_is_thunderbolt_attached()) and, if found, > enable their GPEs or all GPEs. We have 2 choices here: 1. GPE is a part of device enumeration. GPE must be enabled one by one after making sure that all related bus/devices are powered on. But it seems there is no such relationship between GPE and bus/device in ACPI spec. However if Windows works in this way, we'll regress by applying this patch. 2. GPE is not a part of device enumeration. Then probably we can even move GPE enabling into acip_initialize_objects(), before calling acpi_ns_initialize_devices(). As after preparing ACPI namespace, _Lxx/_Exx is ready, and ACPI subsystem should be able to handle early GPEs. And ACPI subsystem's device enumeration starts from acpi_ns_initialize_devices(). So this patch should be correct in theory. ;) Thanks and best regards Lv > > Just as an aside in case your method doesn't work, I'm not affected by > this issue being a Mac user... ;-) > > Thanks, > > Lukas
On Thu, Aug 10, 2017 at 07:10:16AM +0200, Lukas Wunner wrote: > On Thu, Aug 10, 2017 at 12:34:23AM +0200, Rafael J. Wysocki wrote: > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > > acpi_get_spcr_uart_addr(); > > } > > > > + acpi_gpe_apply_masked_gpes(); > > + acpi_update_all_gpes(); > > + acpi_ec_ecdt_start(); > > + > > mutex_lock(&acpi_scan_lock); > > /* > > * Enumerate devices in the ACPI namespace. > > I notice this is called from a subsys_initcall(). We scan the PCI bus > much earlier in arch/x86/kernel/early-quirks.c and it would be possible > to identify presence of Thunderbolt host controllers in an early quirk > (using the method of pci_is_thunderbolt_attached()) and, if found, > enable their GPEs or all GPEs. I don't think we want to differentiate between Thunderbolt controller and anything else. Point here is that we need to enable GPEs in the same order than Windows does (before PCI scan) to be on the path that is at least somehow tested. > Just as an aside in case your method doesn't work, I'm not affected by > this issue being a Mac user... ;-) ;-)
Hi, Rafael > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J. > Wysocki > Subject: [PATCH 3/3] ACPI / scan: Enable GPEs before scanning the namespace > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > On some systems the platform firmware expects GPEs to be enabled > before the enumeration of devices and if that expectation is not > met, the systems in question may not boot in some situations. > > For this reason, change the initialization ordering of the ACPI > subsystem to make it enable GPEs before scanning the namespace > for the first time in order to enumerate devices. > > Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/scan.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > Index: linux-pm/drivers/acpi/scan.c > =================================================================== > --- linux-pm.orig/drivers/acpi/scan.c > +++ linux-pm/drivers/acpi/scan.c > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > acpi_get_spcr_uart_addr(); > } > > + acpi_gpe_apply_masked_gpes(); > + acpi_update_all_gpes(); > + acpi_ec_ecdt_start(); > + Just for your information. A recent internal bug reveals that acpi_ec_ecdt_start() should only be invoked after the enumeration (acpi_ec_add()) for now. The function contains logics that need to be altered by acpi_ec_add(). So it seems we can only do less aggressive change by moving the GPE related 2 lines up. Thanks and best regards Lv > mutex_lock(&acpi_scan_lock); > /* > * Enumerate devices in the ACPI namespace. > @@ -2163,10 +2167,6 @@ int __init acpi_scan_init(void) > } > } > > - acpi_gpe_apply_masked_gpes(); > - acpi_update_all_gpes(); > - acpi_ec_ecdt_start(); > - > acpi_scan_initialized = true; > > out: > > > -- > 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, August 15, 2017 4:12:24 AM CEST Zheng, Lv wrote: > Hi, Rafael > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Rafael J. > > Wysocki > > Subject: [PATCH 3/3] ACPI / scan: Enable GPEs before scanning the namespace > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > On some systems the platform firmware expects GPEs to be enabled > > before the enumeration of devices and if that expectation is not > > met, the systems in question may not boot in some situations. > > > > For this reason, change the initialization ordering of the ACPI > > subsystem to make it enable GPEs before scanning the namespace > > for the first time in order to enumerate devices. > > > > Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/acpi/scan.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > Index: linux-pm/drivers/acpi/scan.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/scan.c > > +++ linux-pm/drivers/acpi/scan.c > > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > > acpi_get_spcr_uart_addr(); > > } > > > > + acpi_gpe_apply_masked_gpes(); > > + acpi_update_all_gpes(); > > + acpi_ec_ecdt_start(); > > + > > Just for your information. > A recent internal bug reveals that acpi_ec_ecdt_start() should only be > invoked after the enumeration (acpi_ec_add()) for now. > The function contains logics that need to be altered by acpi_ec_add(). > > So it seems we can only do less aggressive change by moving the GPE > related 2 lines up. OK, done. Please check my linux-next branch and see if that's what it should be. Thanks, Rafael
Hi, Rafael > From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net] > Subject: Re: [PATCH 3/3] ACPI / scan: Enable GPEs before scanning the namespace > > On Tuesday, August 15, 2017 4:12:24 AM CEST Zheng, Lv wrote: > > Hi, Rafael > > > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of > Rafael J. > > > Wysocki > > > Subject: [PATCH 3/3] ACPI / scan: Enable GPEs before scanning the namespace > > > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > > > On some systems the platform firmware expects GPEs to be enabled > > > before the enumeration of devices and if that expectation is not > > > met, the systems in question may not boot in some situations. > > > > > > For this reason, change the initialization ordering of the ACPI > > > subsystem to make it enable GPEs before scanning the namespace > > > for the first time in order to enumerate devices. > > > > > > Reported-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > --- > > > drivers/acpi/scan.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > Index: linux-pm/drivers/acpi/scan.c > > > =================================================================== > > > --- linux-pm.orig/drivers/acpi/scan.c > > > +++ linux-pm/drivers/acpi/scan.c > > > @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) > > > acpi_get_spcr_uart_addr(); > > > } > > > > > > + acpi_gpe_apply_masked_gpes(); > > > + acpi_update_all_gpes(); > > > + acpi_ec_ecdt_start(); > > > + > > > > Just for your information. > > A recent internal bug reveals that acpi_ec_ecdt_start() should only be > > invoked after the enumeration (acpi_ec_add()) for now. > > The function contains logics that need to be altered by acpi_ec_add(). > > > > So it seems we can only do less aggressive change by moving the GPE > > related 2 lines up. > > OK, done. > > Please check my linux-next branch and see if that's what it should be. I confirmed. And refreshed my EC regression fix on top of that with v2 tagged in the subjects. Thanks and best regards Lv
Index: linux-pm/drivers/acpi/scan.c =================================================================== --- linux-pm.orig/drivers/acpi/scan.c +++ linux-pm/drivers/acpi/scan.c @@ -2139,6 +2139,10 @@ int __init acpi_scan_init(void) acpi_get_spcr_uart_addr(); } + acpi_gpe_apply_masked_gpes(); + acpi_update_all_gpes(); + acpi_ec_ecdt_start(); + mutex_lock(&acpi_scan_lock); /* * Enumerate devices in the ACPI namespace. @@ -2163,10 +2167,6 @@ int __init acpi_scan_init(void) } } - acpi_gpe_apply_masked_gpes(); - acpi_update_all_gpes(); - acpi_ec_ecdt_start(); - acpi_scan_initialized = true; out: