From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hanjun Guo Subject: Re: [PATCH v3 part1 01/11] ACPI / processor: Rework _PDC related stuff to make it more arch-independent Date: Sun, 04 May 2014 16:56:28 +0800 Message-ID: <536600BC.3060205@linaro.org> References: <1398432017-8506-1-git-send-email-hanjun.guo@linaro.org> <1398432017-8506-2-git-send-email-hanjun.guo@linaro.org> <20140429093637.75573C4095B@trevor.secretlab.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20140429093637.75573C4095B@trevor.secretlab.ca> Sender: linux-ia64-owner@vger.kernel.org To: Grant Likely , linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org Cc: "Rafael J. Wysocki" , Catalin Marinas , Arnd Bergmann , Mark Brown , Sudeep Holla , Olof Johansson , Mark Rutland , Rob Herring , Will Deacon , Charles.Garcia-Tobin@arm.com, linaro-acpi@lists.linaro.org, Tony Luck , Fenghua Yu , Thomas Gleixner , "H. Peter Anvin" , x86@kernel.org, linux-ia64@vger.kernel.org, Graeme Gregory List-Id: linux-acpi@vger.kernel.org Hi Grant, On 2014-4-29 17:36, Grant Likely wrote: > On Fri, 25 Apr 2014 21:20:07 +0800, Hanjun Guo wrote: >> _PDC related stuff in processor_core.c is little bit X86/IA64 >> dependent, macros of ACPI_PDC_* are _PDC bit definitions for >> Intel processors, if we use these macros in processor_core.c, >> we will meet compile error when ACPI is enabled on ARM64. >> >> This patch reworks the code to make it more arch-independent, >> moving Intel related _PDC bits into architecture directory, >> no functional change. >> >> Cc: Tony Luck >> Cc: Fenghua Yu >> Cc: Thomas Gleixner >> Cc: "H. Peter Anvin" >> Cc: x86@kernel.org >> Cc: linux-ia64@vger.kernel.org >> Signed-off-by: Hanjun Guo >> Signed-off-by: Graeme Gregory >> --- >> arch/ia64/include/asm/acpi.h | 5 +---- >> arch/ia64/kernel/acpi.c | 15 +++++++++++++++ >> arch/x86/include/asm/acpi.h | 19 +------------------ >> arch/x86/kernel/acpi/cstate.c | 27 +++++++++++++++++++++++++++ >> drivers/acpi/processor_core.c | 19 +------------------ >> 5 files changed, 45 insertions(+), 40 deletions(-) >> >> diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/ac= pi.h >> index d651102..d2b8b9d 100644 >> --- a/arch/ia64/include/asm/acpi.h >> +++ b/arch/ia64/include/asm/acpi.h >> @@ -152,10 +152,7 @@ extern int __initdata nid_to_pxm_map[MAX_NUMNOD= ES]; >> #endif >> =20 >> static inline bool arch_has_acpi_pdc(void) { return true; } >> -static inline void arch_acpi_set_pdc_bits(u32 *buf) >> -{ >> - buf[2] |=3D ACPI_PDC_EST_CAPABILITY_SMP; >> -} >> +extern void arch_acpi_set_pdc_bits(u32 *buf); >> =20 >> #define acpi_unlazy_tlb(x) >> =20 >> diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c >> index 0d407b3..23e9b40 100644 >> --- a/arch/ia64/kernel/acpi.c >> +++ b/arch/ia64/kernel/acpi.c >> @@ -996,3 +996,18 @@ EXPORT_SYMBOL(acpi_unregister_ioapic); >> * TBD when when IA64 starts to support suspend... >> */ >> int acpi_suspend_lowlevel(void) { return 0; } >> + >> +void arch_acpi_set_pdc_bits(u32 *buf) >> +{ >> + /* Enable coordination with firmware's _TSD info */ >> + buf[2] |=3D ACPI_PDC_SMP_T_SWCOORD | ACPI_PDC_EST_CAPABILITY_SMP; >> + >> + if (boot_option_idle_override =3D=3D IDLE_NOMWAIT) { >> + /* >> + * If mwait is disabled for CPU C-states, the C2C3_FFH access >> + * mode will be disabled in the parameter of _PDC object. >> + * Of course C1_FFH access mode will also be disabled. >> + */ >> + buf[2] &=3D ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH); >> + } >> +} >=20 > The commit text makes no comment about why this function is being mov= ed > from a static inline to an extern in the acpi.c file. I assume it is > because it needs access to the boot_option_idle_override global > variable, but it isn't immediately obvious, and should be described i= n > the commit text. Ok, we will update the commit text as you suggested. >=20 > Otherwise, the patch looks sane. >=20 > Reviewed-by: Grant Likely Thanks=EF=BC=81 Hanjun -- To unsubscribe from this list: send the line "unsubscribe linux-ia64" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hanjun Guo Date: Sun, 04 May 2014 08:56:28 +0000 Subject: Re: [PATCH v3 part1 01/11] ACPI / processor: Rework _PDC related stuff to make it more arch-independ Message-Id: <536600BC.3060205@linaro.org> List-Id: References: <1398432017-8506-1-git-send-email-hanjun.guo@linaro.org> <1398432017-8506-2-git-send-email-hanjun.guo@linaro.org> <20140429093637.75573C4095B@trevor.secretlab.ca> In-Reply-To: <20140429093637.75573C4095B@trevor.secretlab.ca> MIME-Version: 1.0 Content-Type: text/plain; charset="macroman" Content-Transfer-Encoding: base64 To: Grant Likely , linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org Cc: "Rafael J. Wysocki" , Catalin Marinas , Arnd Bergmann , Mark Brown , Sudeep Holla , Olof Johansson , Mark Rutland , Rob Herring , Will Deacon , Charles.Garcia-Tobin@arm.com, linaro-acpi@lists.linaro.org, Tony Luck , Fenghua Yu , Thomas Gleixner , "H. Peter Anvin" , x86@kernel.org, linux-ia64@vger.kernel.org, Graeme Gregory SGkgR3JhbnQsCgpPbiAyMDE0LTQtMjkgMTc6MzYsIEdyYW50IExpa2VseSB3cm90ZToKPiBPbiBG cmksIDI1IEFwciAyMDE0IDIxOjIwOjA3ICswODAwLCBIYW5qdW4gR3VvIDxoYW5qdW4uZ3VvQGxp bmFyby5vcmc+IHdyb3RlOgo+PiBfUERDIHJlbGF0ZWQgc3R1ZmYgaW4gcHJvY2Vzc29yX2NvcmUu YyBpcyBsaXR0bGUgYml0IFg4Ni9JQTY0Cj4+IGRlcGVuZGVudCwgbWFjcm9zIG9mIEFDUElfUERD XyogYXJlIF9QREMgYml0IGRlZmluaXRpb25zIGZvcgo+PiBJbnRlbCBwcm9jZXNzb3JzLCBpZiB3 ZSB1c2UgdGhlc2UgbWFjcm9zIGluIHByb2Nlc3Nvcl9jb3JlLmMsCj4+IHdlIHdpbGwgbWVldCBj b21waWxlIGVycm9yIHdoZW4gQUNQSSBpcyBlbmFibGVkIG9uIEFSTTY0Lgo+Pgo+PiBUaGlzIHBh dGNoIHJld29ya3MgdGhlIGNvZGUgdG8gbWFrZSBpdCBtb3JlIGFyY2gtaW5kZXBlbmRlbnQsCj4+ IG1vdmluZyBJbnRlbCByZWxhdGVkIF9QREMgYml0cyBpbnRvIGFyY2hpdGVjdHVyZSBkaXJlY3Rv cnksCj4+IG5vIGZ1bmN0aW9uYWwgY2hhbmdlLgo+Pgo+PiBDYzogVG9ueSBMdWNrIDx0b255Lmx1 Y2tAaW50ZWwuY29tPgo+PiBDYzogRmVuZ2h1YSBZdSA8ZmVuZ2h1YS55dUBpbnRlbC5jb20+Cj4+ IENjOiBUaG9tYXMgR2xlaXhuZXIgPHRnbHhAbGludXRyb25peC5kZT4KPj4gQ2M6ICJILiBQZXRl ciBBbnZpbiIgPGhwYUB6eXRvci5jb20+Cj4+IENjOiB4ODZAa2VybmVsLm9yZwo+PiBDYzogbGlu dXgtaWE2NEB2Z2VyLmtlcm5lbC5vcmcKPj4gU2lnbmVkLW9mZi1ieTogSGFuanVuIEd1byA8aGFu anVuLmd1b0BsaW5hcm8ub3JnPgo+PiBTaWduZWQtb2ZmLWJ5OiBHcmFlbWUgR3JlZ29yeSA8Z3Jh ZW1lLmdyZWdvcnlAbGluYXJvLm9yZz4KPj4gLS0tCj4+ICBhcmNoL2lhNjQvaW5jbHVkZS9hc20v YWNwaS5oICB8ICAgIDUgKy0tLS0KPj4gIGFyY2gvaWE2NC9rZXJuZWwvYWNwaS5jICAgICAgIHwg ICAxNSArKysrKysrKysrKysrKysKPj4gIGFyY2gveDg2L2luY2x1ZGUvYXNtL2FjcGkuaCAgIHwg ICAxOSArLS0tLS0tLS0tLS0tLS0tLS0tCj4+ICBhcmNoL3g4Ni9rZXJuZWwvYWNwaS9jc3RhdGUu YyB8ICAgMjcgKysrKysrKysrKysrKysrKysrKysrKysrKysrCj4+ICBkcml2ZXJzL2FjcGkvcHJv Y2Vzc29yX2NvcmUuYyB8ICAgMTkgKy0tLS0tLS0tLS0tLS0tLS0tLQo+PiAgNSBmaWxlcyBjaGFu Z2VkLCA0NSBpbnNlcnRpb25zKCspLCA0MCBkZWxldGlvbnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBh L2FyY2gvaWE2NC9pbmNsdWRlL2FzbS9hY3BpLmggYi9hcmNoL2lhNjQvaW5jbHVkZS9hc20vYWNw aS5oCj4+IGluZGV4IGQ2NTExMDIuLmQyYjhiOWQgMTAwNjQ0Cj4+IC0tLSBhL2FyY2gvaWE2NC9p bmNsdWRlL2FzbS9hY3BpLmgKPj4gKysrIGIvYXJjaC9pYTY0L2luY2x1ZGUvYXNtL2FjcGkuaAo+ PiBAQCAtMTUyLDEwICsxNTIsNyBAQCBleHRlcm4gaW50IF9faW5pdGRhdGEgbmlkX3RvX3B4bV9t YXBbTUFYX05VTU5PREVTXTsKPj4gICNlbmRpZgo+PiAgCj4+ICBzdGF0aWMgaW5saW5lIGJvb2wg YXJjaF9oYXNfYWNwaV9wZGModm9pZCkgeyByZXR1cm4gdHJ1ZTsgfQo+PiAtc3RhdGljIGlubGlu ZSB2b2lkIGFyY2hfYWNwaV9zZXRfcGRjX2JpdHModTMyICpidWYpCj4+IC17Cj4+IC0JYnVmWzJd IHw9IEFDUElfUERDX0VTVF9DQVBBQklMSVRZX1NNUDsKPj4gLX0KPj4gK2V4dGVybiB2b2lkIGFy Y2hfYWNwaV9zZXRfcGRjX2JpdHModTMyICpidWYpOwo+PiAgCj4+ICAjZGVmaW5lIGFjcGlfdW5s YXp5X3RsYih4KQo+PiAgCj4+IGRpZmYgLS1naXQgYS9hcmNoL2lhNjQva2VybmVsL2FjcGkuYyBi L2FyY2gvaWE2NC9rZXJuZWwvYWNwaS5jCj4+IGluZGV4IDBkNDA3YjMuLjIzZTliNDAgMTAwNjQ0 Cj4+IC0tLSBhL2FyY2gvaWE2NC9rZXJuZWwvYWNwaS5jCj4+ICsrKyBiL2FyY2gvaWE2NC9rZXJu ZWwvYWNwaS5jCj4+IEBAIC05OTYsMyArOTk2LDE4IEBAIEVYUE9SVF9TWU1CT0woYWNwaV91bnJl Z2lzdGVyX2lvYXBpYyk7Cj4+ICAgKiBUQkQgd2hlbiB3aGVuIElBNjQgc3RhcnRzIHRvIHN1cHBv cnQgc3VzcGVuZC4uLgo+PiAgICovCj4+ICBpbnQgYWNwaV9zdXNwZW5kX2xvd2xldmVsKHZvaWQp IHsgcmV0dXJuIDA7IH0KPj4gKwo+PiArdm9pZCBhcmNoX2FjcGlfc2V0X3BkY19iaXRzKHUzMiAq YnVmKQo+PiArewo+PiArCS8qIEVuYWJsZSBjb29yZGluYXRpb24gd2l0aCBmaXJtd2FyZSdzIF9U U0QgaW5mbyAqLwo+PiArCWJ1ZlsyXSB8PSBBQ1BJX1BEQ19TTVBfVF9TV0NPT1JEIHwgQUNQSV9Q RENfRVNUX0NBUEFCSUxJVFlfU01QOwo+PiArCj4+ICsJaWYgKGJvb3Rfb3B0aW9uX2lkbGVfb3Zl cnJpZGUgPSBJRExFX05PTVdBSVQpIHsKPj4gKwkJLyoKPj4gKwkJICogSWYgbXdhaXQgaXMgZGlz YWJsZWQgZm9yIENQVSBDLXN0YXRlcywgdGhlIEMyQzNfRkZIIGFjY2Vzcwo+PiArCQkgKiBtb2Rl IHdpbGwgYmUgZGlzYWJsZWQgaW4gdGhlIHBhcmFtZXRlciBvZiBfUERDIG9iamVjdC4KPj4gKwkJ ICogT2YgY291cnNlIEMxX0ZGSCBhY2Nlc3MgbW9kZSB3aWxsIGFsc28gYmUgZGlzYWJsZWQuCj4+ ICsJCSAqLwo+PiArCQlidWZbMl0gJj0gfihBQ1BJX1BEQ19DX0MyQzNfRkZIIHwgQUNQSV9QRENf Q19DMV9GRkgpOwo+PiArCX0KPj4gK30KPiAKPiBUaGUgY29tbWl0IHRleHQgbWFrZXMgbm8gY29t bWVudCBhYm91dCB3aHkgdGhpcyBmdW5jdGlvbiBpcyBiZWluZyBtb3ZlZAo+IGZyb20gYSBzdGF0 aWMgaW5saW5lIHRvIGFuIGV4dGVybiBpbiB0aGUgYWNwaS5jIGZpbGUuIEkgYXNzdW1lIGl0IGlz Cj4gYmVjYXVzZSBpdCBuZWVkcyBhY2Nlc3MgdG8gdGhlIGJvb3Rfb3B0aW9uX2lkbGVfb3ZlcnJp ZGUgZ2xvYmFsCj4gdmFyaWFibGUsIGJ1dCBpdCBpc24ndCBpbW1lZGlhdGVseSBvYnZpb3VzLCBh bmQgc2hvdWxkIGJlIGRlc2NyaWJlZCBpbgo+IHRoZSBjb21taXQgdGV4dC4KCk9rLCB3ZSB3aWxs IHVwZGF0ZSB0aGUgY29tbWl0IHRleHQgYXMgeW91IHN1Z2dlc3RlZC4KCj4gCj4gT3RoZXJ3aXNl LCB0aGUgcGF0Y2ggbG9va3Mgc2FuZS4KPiAKPiBSZXZpZXdlZC1ieTogR3JhbnQgTGlrZWx5IDxn cmFudC5saWtlbHlAbGluYXJvLm9yZz4KClRoYW5rc++8gQoKSGFuanVuCgotLQpUbyB1bnN1YnNj cmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtaWE2 NCIgaW4KdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcK TW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8t aW5mby5odG1s From mboxrd@z Thu Jan 1 00:00:00 1970 From: hanjun.guo@linaro.org (Hanjun Guo) Date: Sun, 04 May 2014 16:56:28 +0800 Subject: [PATCH v3 part1 01/11] ACPI / processor: Rework _PDC related stuff to make it more arch-independent In-Reply-To: <20140429093637.75573C4095B@trevor.secretlab.ca> References: <1398432017-8506-1-git-send-email-hanjun.guo@linaro.org> <1398432017-8506-2-git-send-email-hanjun.guo@linaro.org> <20140429093637.75573C4095B@trevor.secretlab.ca> Message-ID: <536600BC.3060205@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Grant, On 2014-4-29 17:36, Grant Likely wrote: > On Fri, 25 Apr 2014 21:20:07 +0800, Hanjun Guo wrote: >> _PDC related stuff in processor_core.c is little bit X86/IA64 >> dependent, macros of ACPI_PDC_* are _PDC bit definitions for >> Intel processors, if we use these macros in processor_core.c, >> we will meet compile error when ACPI is enabled on ARM64. >> >> This patch reworks the code to make it more arch-independent, >> moving Intel related _PDC bits into architecture directory, >> no functional change. >> >> Cc: Tony Luck >> Cc: Fenghua Yu >> Cc: Thomas Gleixner >> Cc: "H. Peter Anvin" >> Cc: x86 at kernel.org >> Cc: linux-ia64 at vger.kernel.org >> Signed-off-by: Hanjun Guo >> Signed-off-by: Graeme Gregory >> --- >> arch/ia64/include/asm/acpi.h | 5 +---- >> arch/ia64/kernel/acpi.c | 15 +++++++++++++++ >> arch/x86/include/asm/acpi.h | 19 +------------------ >> arch/x86/kernel/acpi/cstate.c | 27 +++++++++++++++++++++++++++ >> drivers/acpi/processor_core.c | 19 +------------------ >> 5 files changed, 45 insertions(+), 40 deletions(-) >> >> diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h >> index d651102..d2b8b9d 100644 >> --- a/arch/ia64/include/asm/acpi.h >> +++ b/arch/ia64/include/asm/acpi.h >> @@ -152,10 +152,7 @@ extern int __initdata nid_to_pxm_map[MAX_NUMNODES]; >> #endif >> >> static inline bool arch_has_acpi_pdc(void) { return true; } >> -static inline void arch_acpi_set_pdc_bits(u32 *buf) >> -{ >> - buf[2] |= ACPI_PDC_EST_CAPABILITY_SMP; >> -} >> +extern void arch_acpi_set_pdc_bits(u32 *buf); >> >> #define acpi_unlazy_tlb(x) >> >> diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c >> index 0d407b3..23e9b40 100644 >> --- a/arch/ia64/kernel/acpi.c >> +++ b/arch/ia64/kernel/acpi.c >> @@ -996,3 +996,18 @@ EXPORT_SYMBOL(acpi_unregister_ioapic); >> * TBD when when IA64 starts to support suspend... >> */ >> int acpi_suspend_lowlevel(void) { return 0; } >> + >> +void arch_acpi_set_pdc_bits(u32 *buf) >> +{ >> + /* Enable coordination with firmware's _TSD info */ >> + buf[2] |= ACPI_PDC_SMP_T_SWCOORD | ACPI_PDC_EST_CAPABILITY_SMP; >> + >> + if (boot_option_idle_override == IDLE_NOMWAIT) { >> + /* >> + * If mwait is disabled for CPU C-states, the C2C3_FFH access >> + * mode will be disabled in the parameter of _PDC object. >> + * Of course C1_FFH access mode will also be disabled. >> + */ >> + buf[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH); >> + } >> +} > > The commit text makes no comment about why this function is being moved > from a static inline to an extern in the acpi.c file. I assume it is > because it needs access to the boot_option_idle_override global > variable, but it isn't immediately obvious, and should be described in > the commit text. Ok, we will update the commit text as you suggested. > > Otherwise, the patch looks sane. > > Reviewed-by: Grant Likely Thanks? Hanjun