* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc [not found] ` <201009101210.31942.bjorn.helgaas@hp.com> @ 2010-09-10 19:20 ` Yinghai Lu 2010-09-10 23:45 ` Andrew Morton 0 siblings, 1 reply; 9+ messages in thread From: Yinghai Lu @ 2010-09-10 19:20 UTC (permalink / raw) To: Bjorn Helgaas Cc: Suresh Siddha, Andrew Morton, Brown, Len, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner On 09/10/2010 11:10 AM, Bjorn Helgaas wrote: > On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote: >> >> When bios switch to use Device object instead of Processor statement. >> >> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically. >> >> So try to scan ACPI0007 in addition to Processor. >> >> this fix regression: 2.6.32 is ok. > > Can you include the URL of the regression bug report? And maybe > the commit that introduced the regression? the commit should be commit d8191fa4a33fdc817277da4f2b7f771ff605a41c Author: Alex Chiang <achiang@hp.com> Date: Mon Feb 22 12:11:39 2010 -0700 ACPI: processor: driver doesn't need to evaluate _PDC Now that the early _PDC evaluation path knows how to correctly evaluate _PDC on only physically present processors, there's no need for the processor driver to evaluate it later when it loads. To cover the hotplug case, push _PDC evaluation down into the hotplug paths. Cc: x86@kernel.org Cc: Tony Luck <tony.luck@intel.com> Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> Signed-off-by: Alex Chiang <achiang@hp.com> Signed-off-by: Len Brown <len.brown@intel.com> that is between 2.6.34-rc1 and 2.6.34-rc2. So we need put this patch in stable tree for 2.6.34, .35, .36 Yinghai > >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> >> >> --- >> drivers/acpi/processor_core.c | 1 + >> 1 file changed, 1 insertion(+) >> >> Index: linux-2.6/drivers/acpi/processor_core.c >> =================================================================== >> --- linux-2.6.orig/drivers/acpi/processor_core.c >> +++ linux-2.6/drivers/acpi/processor_core.c >> @@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc >> acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, >> ACPI_UINT32_MAX, >> early_init_pdc, NULL, NULL, NULL); >> + acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL); > > I hate having to walk the namespace. Usually that's a clue that > there's something wrong with our ACPI device model, because it'd > be better to handle everything in a driver .add() method. But > maybe this early _PDC thing is so special that it can't be helped > in this case. > > But I do think you could probably fix this to walk the namespace > once rather than twice. Maybe you could use something like > acpi_walk_namespace(ACPI_TYPE_ANY, ...) with a callback that > recognizes both ACPI_TYPE_PROCESSOR and "ACPI_TYPE_DEVICE with > HID ACPI0007". > > Bjorn ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc 2010-09-10 19:20 ` [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc Yinghai Lu @ 2010-09-10 23:45 ` Andrew Morton 2010-09-11 2:06 ` Yinghai Lu 0 siblings, 1 reply; 9+ messages in thread From: Andrew Morton @ 2010-09-10 23:45 UTC (permalink / raw) To: Yinghai Lu Cc: Bjorn Helgaas, Suresh Siddha, Brown, Len, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner On Fri, 10 Sep 2010 12:20:36 -0700 Yinghai Lu <yinghai@kernel.org> wrote: > On 09/10/2010 11:10 AM, Bjorn Helgaas wrote: > > On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote: > >> > >> When bios switch to use Device object instead of Processor statement. > >> > >> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically. > >> > >> So try to scan ACPI0007 in addition to Processor. > >> > >> this fix regression: 2.6.32 is ok. > > > > Can you include the URL of the regression bug report? And maybe > > the commit that introduced the regression? > > the commit should be > > commit d8191fa4a33fdc817277da4f2b7f771ff605a41c > Author: Alex Chiang <achiang@hp.com> > Date: Mon Feb 22 12:11:39 2010 -0700 > > ACPI: processor: driver doesn't need to evaluate _PDC > > Now that the early _PDC evaluation path knows how to correctly > evaluate _PDC on only physically present processors, there's no > need for the processor driver to evaluate it later when it loads. > > To cover the hotplug case, push _PDC evaluation down into the > hotplug paths. > > Cc: x86@kernel.org > Cc: Tony Luck <tony.luck@intel.com> > Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> > Signed-off-by: Alex Chiang <achiang@hp.com> > Signed-off-by: Len Brown <len.brown@intel.com> > > that is between 2.6.34-rc1 and 2.6.34-rc2. > > So we need put this patch in stable tree for 2.6.34, .35, .36 > Maybe. But first can you please address Bjorn's suggestions below? > > > > >> Signed-off-by: Yinghai Lu <yinghai@kernel.org> > >> > >> --- > >> drivers/acpi/processor_core.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> Index: linux-2.6/drivers/acpi/processor_core.c > >> =================================================================== > >> --- linux-2.6.orig/drivers/acpi/processor_core.c > >> +++ linux-2.6/drivers/acpi/processor_core.c > >> @@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc > >> acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, > >> ACPI_UINT32_MAX, > >> early_init_pdc, NULL, NULL, NULL); > >> + acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL); > > > > I hate having to walk the namespace. Usually that's a clue that > > there's something wrong with our ACPI device model, because it'd > > be better to handle everything in a driver .add() method. But > > maybe this early _PDC thing is so special that it can't be helped > > in this case. > > > > But I do think you could probably fix this to walk the namespace > > once rather than twice. Maybe you could use something like > > acpi_walk_namespace(ACPI_TYPE_ANY, ...) with a callback that > > recognizes both ACPI_TYPE_PROCESSOR and "ACPI_TYPE_DEVICE with > > HID ACPI0007". > > > > Bjorn ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc 2010-09-10 23:45 ` Andrew Morton @ 2010-09-11 2:06 ` Yinghai Lu 2010-09-13 17:28 ` Bjorn Helgaas 0 siblings, 1 reply; 9+ messages in thread From: Yinghai Lu @ 2010-09-11 2:06 UTC (permalink / raw) To: Andrew Morton, Brown, Len Cc: Bjorn Helgaas, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore On 09/10/2010 04:45 PM, Andrew Morton wrote: > On Fri, 10 Sep 2010 12:20:36 -0700 > Yinghai Lu <yinghai@kernel.org> wrote: > >> On 09/10/2010 11:10 AM, Bjorn Helgaas wrote: >>> On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote: >>>> >>>> When bios switch to use Device object instead of Processor statement. >>>> >>>> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically. >>>> >>>> So try to scan ACPI0007 in addition to Processor. >>>> >>>> this fix regression: 2.6.32 is ok. >>> >>> Can you include the URL of the regression bug report? And maybe >>> the commit that introduced the regression? >> >> the commit should be >> >> commit d8191fa4a33fdc817277da4f2b7f771ff605a41c >> Author: Alex Chiang <achiang@hp.com> >> Date: Mon Feb 22 12:11:39 2010 -0700 >> >> ACPI: processor: driver doesn't need to evaluate _PDC >> >> Now that the early _PDC evaluation path knows how to correctly >> evaluate _PDC on only physically present processors, there's no >> need for the processor driver to evaluate it later when it loads. >> >> To cover the hotplug case, push _PDC evaluation down into the >> hotplug paths. >> >> Cc: x86@kernel.org >> Cc: Tony Luck <tony.luck@intel.com> >> Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> >> Signed-off-by: Alex Chiang <achiang@hp.com> >> Signed-off-by: Len Brown <len.brown@intel.com> >> >> that is between 2.6.34-rc1 and 2.6.34-rc2. >> >> So we need put this patch in stable tree for 2.6.34, .35, .36 >> > > Maybe. But first can you please address Bjorn's suggestions below? fine, if you prefer 47 lines instead one line changes. [PATCH -v2] acpi: handle ACPI0007 Device in acpi_early_set_pdc When bios switch to use Device object instead of Processor statement. the SSDT for Pstate/Cstate/Tstate can not loaded dynamically. So try to scan ACPI0007 in addition to Processor. -v2: add 47 lines instead of 1 line to walk namespace one time only. Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/acpi/acpica/nsxfeval.c | 43 +++++++++++++++++++++++++++++++++++++++++ drivers/acpi/processor_core.c | 8 ++++--- include/acpi/acpixf.h | 2 + 3 files changed, 50 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/acpi/processor_core.c =================================================================== --- linux-2.6.orig/drivers/acpi/processor_core.c +++ linux-2.6/drivers/acpi/processor_core.c @@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i EXPORT_SYMBOL_GPL(acpi_get_cpuid); #endif -static bool processor_physically_present(acpi_handle handle) +static bool is_processor_and_physically_present(acpi_handle handle) { int cpuid, type; u32 acpi_id; @@ -211,6 +211,8 @@ static bool processor_physically_present acpi_id = object.processor.proc_id; break; case ACPI_TYPE_DEVICE: + if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK) + return false; status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp); if (ACPI_FAILURE(status)) return false; @@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc static acpi_status early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv) { - if (processor_physically_present(handle) == false) + if (is_processor_and_physically_present(handle) == false) return AE_OK; acpi_processor_set_pdc(handle); @@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc */ dmi_check_system(processor_idle_dmi_table); - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, + acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, early_init_pdc, NULL, NULL, NULL); } Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c +++ linux-2.6/drivers/acpi/acpica/nsxfeval.c @@ -524,6 +524,49 @@ ACPI_EXPORT_SYMBOL(acpi_walk_namespace) /******************************************************************************* * + * FUNCTION: acpi_match_device_hid + * + * PARAMETERS: handle and HID to be compared + * + * RETURN: bool + * + * DESCRIPTION: check if device coresponding to handle has HID + * + ******************************************************************************/ +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID) +{ + acpi_status status; + struct acpi_namespace_node *node; + struct acpica_device_id *hid; + int no_match; + + status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); + if (ACPI_FAILURE(status)) + return status; + + node = acpi_ns_validate_handle(obj_handle); + status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); + if (ACPI_FAILURE(status)) + return status; + + if (!node) + return AE_BAD_PARAMETER; + + status = acpi_ut_execute_HID(node, &hid); + if (ACPI_FAILURE(status)) + return status; + + no_match = ACPI_STRCMP(hid->string, HID); + ACPI_FREE(hid); + + if (no_match) + return AE_NOT_FOUND; + + return AE_OK; +} + +/******************************************************************************* + * * FUNCTION: acpi_ns_get_device_callback * * PARAMETERS: Callback from acpi_get_device Index: linux-2.6/include/acpi/acpixf.h =================================================================== --- linux-2.6.orig/include/acpi/acpixf.h +++ linux-2.6/include/acpi/acpixf.h @@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ acpi_walk_callback post_order_visit, void *context, void **return_value); +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID); + acpi_status acpi_get_devices(const char *HID, acpi_walk_callback user_function, ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc 2010-09-11 2:06 ` Yinghai Lu @ 2010-09-13 17:28 ` Bjorn Helgaas 2010-09-17 7:21 ` Yinghai Lu 0 siblings, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2010-09-13 17:28 UTC (permalink / raw) To: Yinghai Lu Cc: Andrew Morton, Brown, Len, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote: > On 09/10/2010 04:45 PM, Andrew Morton wrote: > > On Fri, 10 Sep 2010 12:20:36 -0700 > > Yinghai Lu <yinghai@kernel.org> wrote: > > > >> On 09/10/2010 11:10 AM, Bjorn Helgaas wrote: > >>> On Thursday, September 09, 2010 07:56:59 pm Yinghai Lu wrote: > >>>> > >>>> When bios switch to use Device object instead of Processor statement. > >>>> > >>>> the SSDT for Pstate/Cstate/Tstate can not be loaded dynamically. > >>>> > >>>> So try to scan ACPI0007 in addition to Processor. > >>>> > >>>> this fix regression: 2.6.32 is ok. > >>> > >>> Can you include the URL of the regression bug report? And maybe > >>> the commit that introduced the regression? > >> > >> the commit should be > >> > >> commit d8191fa4a33fdc817277da4f2b7f771ff605a41c > >> Author: Alex Chiang <achiang@hp.com> > >> Date: Mon Feb 22 12:11:39 2010 -0700 > >> > >> ACPI: processor: driver doesn't need to evaluate _PDC > >> > >> Now that the early _PDC evaluation path knows how to correctly > >> evaluate _PDC on only physically present processors, there's no > >> need for the processor driver to evaluate it later when it loads. > >> > >> To cover the hotplug case, push _PDC evaluation down into the > >> hotplug paths. > >> > >> Cc: x86@kernel.org > >> Cc: Tony Luck <tony.luck@intel.com> > >> Acked-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com> > >> Signed-off-by: Alex Chiang <achiang@hp.com> > >> Signed-off-by: Len Brown <len.brown@intel.com> > >> > >> that is between 2.6.34-rc1 and 2.6.34-rc2. There's no need to repeat the entire changelog of Alex's commit here; all we need is the commit ID so we can find it later. > > Maybe. But first can you please address Bjorn's suggestions below? > > fine, if you prefer 47 lines instead one line changes. It's more important for a change to be clear and correct than for it to be tiny. But I do see your point, and having seen the patch below, I prefer the original one-line version. I timed the original on a 64-CPU box where the namespace walk cost should be relatively high, and the walk you added takes about 0.01 seconds. The one already there took about 0.03 seconds, so neither costs very much. And I don't like the fact that the patch below changes the ACPI CA and it only partly implements acpi_get_devices(), since it ignores _CID and _STA. Bjorn > [PATCH -v2] acpi: handle ACPI0007 Device in acpi_early_set_pdc > > When bios switch to use Device object instead of Processor statement. > > the SSDT for Pstate/Cstate/Tstate can not loaded dynamically. > > So try to scan ACPI0007 in addition to Processor. > > -v2: add 47 lines instead of 1 line to walk namespace one time only. > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > drivers/acpi/acpica/nsxfeval.c | 43 +++++++++++++++++++++++++++++++++++++++++ > drivers/acpi/processor_core.c | 8 ++++--- > include/acpi/acpixf.h | 2 + > 3 files changed, 50 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/acpi/processor_core.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/processor_core.c > +++ linux-2.6/drivers/acpi/processor_core.c > @@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i > EXPORT_SYMBOL_GPL(acpi_get_cpuid); > #endif > > -static bool processor_physically_present(acpi_handle handle) > +static bool is_processor_and_physically_present(acpi_handle handle) > { > int cpuid, type; > u32 acpi_id; > @@ -211,6 +211,8 @@ static bool processor_physically_present > acpi_id = object.processor.proc_id; > break; > case ACPI_TYPE_DEVICE: > + if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK) > + return false; > status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp); > if (ACPI_FAILURE(status)) > return false; > @@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc > static acpi_status > early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv) > { > - if (processor_physically_present(handle) == false) > + if (is_processor_and_physically_present(handle) == false) > return AE_OK; > > acpi_processor_set_pdc(handle); > @@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc > */ > dmi_check_system(processor_idle_dmi_table); > > - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, > + acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT, > ACPI_UINT32_MAX, > early_init_pdc, NULL, NULL, NULL); > } > Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c > +++ linux-2.6/drivers/acpi/acpica/nsxfeval.c > @@ -524,6 +524,49 @@ ACPI_EXPORT_SYMBOL(acpi_walk_namespace) > > /******************************************************************************* > * > + * FUNCTION: acpi_match_device_hid > + * > + * PARAMETERS: handle and HID to be compared > + * > + * RETURN: bool > + * > + * DESCRIPTION: check if device coresponding to handle has HID > + * > + ******************************************************************************/ > +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID) > +{ > + acpi_status status; > + struct acpi_namespace_node *node; > + struct acpica_device_id *hid; > + int no_match; > + > + status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE); > + if (ACPI_FAILURE(status)) > + return status; > + > + node = acpi_ns_validate_handle(obj_handle); > + status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE); > + if (ACPI_FAILURE(status)) > + return status; > + > + if (!node) > + return AE_BAD_PARAMETER; > + > + status = acpi_ut_execute_HID(node, &hid); > + if (ACPI_FAILURE(status)) > + return status; > + > + no_match = ACPI_STRCMP(hid->string, HID); > + ACPI_FREE(hid); > + > + if (no_match) > + return AE_NOT_FOUND; > + > + return AE_OK; > +} > + > +/******************************************************************************* > + * > * FUNCTION: acpi_ns_get_device_callback > * > * PARAMETERS: Callback from acpi_get_device > Index: linux-2.6/include/acpi/acpixf.h > =================================================================== > --- linux-2.6.orig/include/acpi/acpixf.h > +++ linux-2.6/include/acpi/acpixf.h > @@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ > acpi_walk_callback post_order_visit, > void *context, void **return_value); > > +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID); > + > acpi_status > acpi_get_devices(const char *HID, > acpi_walk_callback user_function, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc 2010-09-13 17:28 ` Bjorn Helgaas @ 2010-09-17 7:21 ` Yinghai Lu 2010-09-17 13:56 ` Bjorn Helgaas 0 siblings, 1 reply; 9+ messages in thread From: Yinghai Lu @ 2010-09-17 7:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andrew Morton, Brown, Len, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore On 09/13/2010 10:28 AM, Bjorn Helgaas wrote: > On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote: > > I timed the original on a 64-CPU box where the namespace walk cost > should be relatively high, and the walk you added takes about 0.01 > seconds. The one already there took about 0.03 seconds, so neither > costs very much. > > And I don't like the fact that the patch below changes the ACPI CA > and it only partly implements acpi_get_devices(), since it ignores > _CID and _STA. please check this one. it still keep _STA etc. [PATCH -v3] acpi: Handle ACPI0007 Device in acpi_early_set_pdc After | commit d8191fa4a33fdc817277da4f2b7f771ff605a41c | Author: Alex Chiang <achiang@hp.com> | Date: Mon Feb 22 12:11:39 2010 -0700 | | ACPI: processor: driver doesn't need to evaluate _PDC | | Now that the early _PDC evaluation path knows how to correctly | evaluate _PDC on only physically present processors, there's no | need for the processor driver to evaluate it later when it loads. | | To cover the hotplug case, push _PDC evaluation down into the | hotplug paths. only cpu with Processor Statement get processed with _PDC If bios is using Device object instead of Processor statement. SSDTs for Pstate/Cstate/Tstate can not be loaded dynamically. Need to try to scan ACPI0007 in addition to Processor. That commit is between 2.6.34-rc1 and 2.6.34-rc2, so stable tree for 2.6.34+ need this patch. -v2: add 47 lines instead of 1 line to walk namespace one time only. -v3: Let acpi_match_device_hid() call acpi_ns_get_device_callback() Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/acpi/acpica/nsxfeval.c | 41 +++++++++++++++++++++++++++++++++++++++++ drivers/acpi/processor_core.c | 8 +++++--- include/acpi/acpixf.h | 2 ++ 3 files changed, 48 insertions(+), 3 deletions(-) Index: linux-2.6/drivers/acpi/processor_core.c =================================================================== --- linux-2.6.orig/drivers/acpi/processor_core.c +++ linux-2.6/drivers/acpi/processor_core.c @@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i EXPORT_SYMBOL_GPL(acpi_get_cpuid); #endif -static bool processor_physically_present(acpi_handle handle) +static bool is_processor_and_physically_present(acpi_handle handle) { int cpuid, type; u32 acpi_id; @@ -211,6 +211,8 @@ static bool processor_physically_present acpi_id = object.processor.proc_id; break; case ACPI_TYPE_DEVICE: + if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK) + return false; status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp); if (ACPI_FAILURE(status)) return false; @@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc static acpi_status early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv) { - if (processor_physically_present(handle) == false) + if (is_processor_and_physically_present(handle) == false) return AE_OK; acpi_processor_set_pdc(handle); @@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc */ dmi_check_system(processor_idle_dmi_table); - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, + acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, early_init_pdc, NULL, NULL, NULL); } Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c +++ linux-2.6/drivers/acpi/acpica/nsxfeval.c @@ -642,6 +642,47 @@ acpi_ns_get_device_callback(acpi_handle return (status); } +static acpi_status +acpi_match_device_hid_callback(acpi_handle obj_handle, + u32 nesting_level, + void *context, void **return_value) +{ + *return_value = (void *)0xff; + + return AE_OK; +} + +/******************************************************************************* + * + * FUNCTION: acpi_match_device_hid + * + * PARAMETERS: handle and HID to be compared + * + * RETURN: AE_OK mean found match one + * + * DESCRIPTION: check if device coresponding to handle has HID + * + ******************************************************************************/ +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID) +{ + struct acpi_get_devices_info info; + void *return_value = NULL; + + ACPI_FUNCTION_TRACE(acpi_match_device_hid); + + info.hid = HID; + info.context = NULL; + info.user_function = acpi_match_device_hid_callback; + + acpi_ns_get_device_callback(obj_handle, ACPI_UINT32_MAX, + &info, &return_value); + + if (!return_value) + return AE_NOT_FOUND; + + return AE_OK; +} + /******************************************************************************* * * FUNCTION: acpi_get_devices Index: linux-2.6/include/acpi/acpixf.h =================================================================== --- linux-2.6.orig/include/acpi/acpixf.h +++ linux-2.6/include/acpi/acpixf.h @@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ acpi_walk_callback post_order_visit, void *context, void **return_value); +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID); + acpi_status acpi_get_devices(const char *HID, acpi_walk_callback user_function, ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc 2010-09-17 7:21 ` Yinghai Lu @ 2010-09-17 13:56 ` Bjorn Helgaas 2010-09-17 18:37 ` Yinghai Lu 2010-09-18 6:26 ` [PATCH -v4] acpi: Handle " Yinghai Lu 0 siblings, 2 replies; 9+ messages in thread From: Bjorn Helgaas @ 2010-09-17 13:56 UTC (permalink / raw) To: Yinghai Lu Cc: Andrew Morton, Brown, Len, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore On Friday, September 17, 2010 01:21:56 am Yinghai Lu wrote: > On 09/13/2010 10:28 AM, Bjorn Helgaas wrote: > > On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote: > > > > I timed the original on a 64-CPU box where the namespace walk cost > > should be relatively high, and the walk you added takes about 0.01 > > seconds. The one already there took about 0.03 seconds, so neither > > costs very much. > > > > And I don't like the fact that the patch below changes the ACPI CA > > and it only partly implements acpi_get_devices(), since it ignores > > _CID and _STA. > > please check this one. it still keep _STA etc. I think you missed this line in my previous response: > > But I do see your point, and having seen the patch > > below, I prefer the original one-line version. The patch below is better, because it doesn't reimplement quite so much stuff, but it still changes the CA, and I think the end result is still quite confusing to read. So I still prefer the original one-line version. Bjorn > [PATCH -v3] acpi: Handle ACPI0007 Device in acpi_early_set_pdc > > After > | commit d8191fa4a33fdc817277da4f2b7f771ff605a41c > | Author: Alex Chiang <achiang@hp.com> > | Date: Mon Feb 22 12:11:39 2010 -0700 > | > | ACPI: processor: driver doesn't need to evaluate _PDC > | > | Now that the early _PDC evaluation path knows how to correctly > | evaluate _PDC on only physically present processors, there's no > | need for the processor driver to evaluate it later when it loads. > | > | To cover the hotplug case, push _PDC evaluation down into the > | hotplug paths. > > only cpu with Processor Statement get processed with _PDC > > If bios is using Device object instead of Processor statement. > SSDTs for Pstate/Cstate/Tstate can not be loaded dynamically. > > Need to try to scan ACPI0007 in addition to Processor. > > That commit is between 2.6.34-rc1 and 2.6.34-rc2, so stable tree for 2.6.34+ > need this patch. > > -v2: add 47 lines instead of 1 line to walk namespace one time only. > -v3: Let acpi_match_device_hid() call acpi_ns_get_device_callback() > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > drivers/acpi/acpica/nsxfeval.c | 41 +++++++++++++++++++++++++++++++++++++++++ > drivers/acpi/processor_core.c | 8 +++++--- > include/acpi/acpixf.h | 2 ++ > 3 files changed, 48 insertions(+), 3 deletions(-) > > Index: linux-2.6/drivers/acpi/processor_core.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/processor_core.c > +++ linux-2.6/drivers/acpi/processor_core.c > @@ -189,7 +189,7 @@ int acpi_get_cpuid(acpi_handle handle, i > EXPORT_SYMBOL_GPL(acpi_get_cpuid); > #endif > > -static bool processor_physically_present(acpi_handle handle) > +static bool is_processor_and_physically_present(acpi_handle handle) > { > int cpuid, type; > u32 acpi_id; > @@ -211,6 +211,8 @@ static bool processor_physically_present > acpi_id = object.processor.proc_id; > break; > case ACPI_TYPE_DEVICE: > + if (acpi_match_device_hid(handle, "ACPI0007") != AE_OK) > + return false; > status = acpi_evaluate_integer(handle, "_UID", NULL, &tmp); > if (ACPI_FAILURE(status)) > return false; > @@ -334,7 +336,7 @@ EXPORT_SYMBOL_GPL(acpi_processor_set_pdc > static acpi_status > early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv) > { > - if (processor_physically_present(handle) == false) > + if (is_processor_and_physically_present(handle) == false) > return AE_OK; > > acpi_processor_set_pdc(handle); > @@ -349,7 +351,7 @@ void __init acpi_early_processor_set_pdc > */ > dmi_check_system(processor_idle_dmi_table); > > - acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, > + acpi_walk_namespace(ACPI_TYPE_ANY, ACPI_ROOT_OBJECT, > ACPI_UINT32_MAX, > early_init_pdc, NULL, NULL, NULL); > } > Index: linux-2.6/drivers/acpi/acpica/nsxfeval.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/acpica/nsxfeval.c > +++ linux-2.6/drivers/acpi/acpica/nsxfeval.c > @@ -642,6 +642,47 @@ acpi_ns_get_device_callback(acpi_handle > return (status); > } > > +static acpi_status > +acpi_match_device_hid_callback(acpi_handle obj_handle, > + u32 nesting_level, > + void *context, void **return_value) > +{ > + *return_value = (void *)0xff; > + > + return AE_OK; > +} > + > +/******************************************************************************* > + * > + * FUNCTION: acpi_match_device_hid > + * > + * PARAMETERS: handle and HID to be compared > + * > + * RETURN: AE_OK mean found match one > + * > + * DESCRIPTION: check if device coresponding to handle has HID > + * > + ******************************************************************************/ > +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID) > +{ > + struct acpi_get_devices_info info; > + void *return_value = NULL; > + > + ACPI_FUNCTION_TRACE(acpi_match_device_hid); > + > + info.hid = HID; > + info.context = NULL; > + info.user_function = acpi_match_device_hid_callback; > + > + acpi_ns_get_device_callback(obj_handle, ACPI_UINT32_MAX, > + &info, &return_value); > + > + if (!return_value) > + return AE_NOT_FOUND; > + > + return AE_OK; > +} > + > /******************************************************************************* > * > * FUNCTION: acpi_get_devices > Index: linux-2.6/include/acpi/acpixf.h > =================================================================== > --- linux-2.6.orig/include/acpi/acpixf.h > +++ linux-2.6/include/acpi/acpixf.h > @@ -160,6 +160,8 @@ acpi_walk_namespace(acpi_object_type typ > acpi_walk_callback post_order_visit, > void *context, void **return_value); > > +acpi_status acpi_match_device_hid(acpi_handle obj_handle, const char *HID); > + > acpi_status > acpi_get_devices(const char *HID, > acpi_walk_callback user_function, > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc 2010-09-17 13:56 ` Bjorn Helgaas @ 2010-09-17 18:37 ` Yinghai Lu 2010-09-18 6:26 ` [PATCH -v4] acpi: Handle " Yinghai Lu 1 sibling, 0 replies; 9+ messages in thread From: Yinghai Lu @ 2010-09-17 18:37 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andrew Morton, Brown, Len, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore On 09/17/2010 06:56 AM, Bjorn Helgaas wrote: > On Friday, September 17, 2010 01:21:56 am Yinghai Lu wrote: >> On 09/13/2010 10:28 AM, Bjorn Helgaas wrote: >>> On Friday, September 10, 2010 08:06:26 pm Yinghai Lu wrote: >>> >>> I timed the original on a 64-CPU box where the namespace walk cost >>> should be relatively high, and the walk you added takes about 0.01 >>> seconds. The one already there took about 0.03 seconds, so neither >>> costs very much. >>> >>> And I don't like the fact that the patch below changes the ACPI CA >>> and it only partly implements acpi_get_devices(), since it ignores >>> _CID and _STA. >> >> please check this one. it still keep _STA etc. > > I think you missed this line in my previous response: > >>> But I do see your point, and having seen the patch >>> below, I prefer the original one-line version. > > The patch below is better, because it doesn't reimplement quite > so much stuff, but it still changes the CA, not sure. > and I think the end > result is still quite confusing to read. yeah, it is confusing to read.... callback with callback. > > So I still prefer the original one-line version. ok, Will resend out original one with updated changelog. Yinghai ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH -v4] acpi: Handle ACPI0007 Device in acpi_early_set_pdc 2010-09-17 13:56 ` Bjorn Helgaas 2010-09-17 18:37 ` Yinghai Lu @ 2010-09-18 6:26 ` Yinghai Lu 2010-10-01 6:07 ` Len Brown 1 sibling, 1 reply; 9+ messages in thread From: Yinghai Lu @ 2010-09-18 6:26 UTC (permalink / raw) To: Brown, Len Cc: Bjorn Helgaas, Andrew Morton, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore After | commit d8191fa4a33fdc817277da4f2b7f771ff605a41c | Author: Alex Chiang <achiang@hp.com> | Date: Mon Feb 22 12:11:39 2010 -0700 | | ACPI: processor: driver doesn't need to evaluate _PDC | | Now that the early _PDC evaluation path knows how to correctly | evaluate _PDC on only physically present processors, there's no | need for the processor driver to evaluate it later when it loads. | | To cover the hotplug case, push _PDC evaluation down into the | hotplug paths. only cpu with Processor Statement get processed with _PDC If bios is using Device object instead of Processor statement. SSDTs for Pstate/Cstate/Tstate can not be loaded dynamically. Need to try to scan ACPI0007 in addition to Processor. That commit is between 2.6.34-rc1 and 2.6.34-rc2, so stable tree for 2.6.34+ need this patch. Signed-off-by: Yinghai Lu <yinghai@kernel.org> --- drivers/acpi/processor_core.c | 1 + 1 file changed, 1 insertion(+) Index: linux-2.6/drivers/acpi/processor_core.c =================================================================== --- linux-2.6.orig/drivers/acpi/processor_core.c +++ linux-2.6/drivers/acpi/processor_core.c @@ -352,4 +352,5 @@ void __init acpi_early_processor_set_pdc acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, early_init_pdc, NULL, NULL, NULL); + acpi_get_devices("ACPI0007", early_init_pdc, NULL, NULL); } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH -v4] acpi: Handle ACPI0007 Device in acpi_early_set_pdc 2010-09-18 6:26 ` [PATCH -v4] acpi: Handle " Yinghai Lu @ 2010-10-01 6:07 ` Len Brown 0 siblings, 0 replies; 9+ messages in thread From: Len Brown @ 2010-10-01 6:07 UTC (permalink / raw) To: Yinghai Lu Cc: Brown, Len, Bjorn Helgaas, Andrew Morton, Suresh Siddha, ACPI Devel Maling List, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner, Lin Ming, Bob Moore applied to acpi-test thanks, Len Brown, Intel Open Source Technology Center ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-10-01 6:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <4C89906B.1050406@kernel.org>
[not found] ` <201009101210.31942.bjorn.helgaas@hp.com>
2010-09-10 19:20 ` [PATCH] acpi: handle ACPI0007 Device in acpi_early_set_pdc Yinghai Lu
2010-09-10 23:45 ` Andrew Morton
2010-09-11 2:06 ` Yinghai Lu
2010-09-13 17:28 ` Bjorn Helgaas
2010-09-17 7:21 ` Yinghai Lu
2010-09-17 13:56 ` Bjorn Helgaas
2010-09-17 18:37 ` Yinghai Lu
2010-09-18 6:26 ` [PATCH -v4] acpi: Handle " Yinghai Lu
2010-10-01 6:07 ` Len Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome