Some of the ACPI code is arch-dependent and make the code can't be compiled on !x86 or !ia64, the first two patches just do some rework on the idle_boot_override and _PDC related stuff to make the ACPI code more arch-independent.
The third patch just introduce map_gic_id() for ACPI processor core followed the ACPI 5.0 spec. Last two patches are just some cleanups.
I have compiled the kernel successfully after appling this patch set on x86 and ia64.
Changs for v5: Realign the additional lines to the open parenthesis pr_info(xxxx); xxxx;
Changes for v4: a) remove preprocessor directives in function bodies; b) add another two patches for cleanups.
Changes for v3: Fix the issues pointed out by Lan Tianyu and add the Reviewed-by for first patch.
Changes for v2: a) add #if defined(CONFIG_X86) || defined(CONFIG_IA64) for idle_boot_override related code; b) Rebased on 3.14-rc1.
Changes since the RFC version: a) Remove the RFC tag; b) Move idle_boot_override out of the arch directory suggested by Alan; c) Make these 3 patches as a separate patch set since there are not not related to the ARM/ARM64 platform.
Hanjun Guo (5): ACPI / idle: Make idle_boot_override depend on x86 and ia64 ACPI / processor_core: Rework _PDC related stuff to make it more arch-independent ACPI / processor: Introduce map_gic_id() to get apic id from MADT or _MAT method ACPI: Move BAD_MADT_ENTRY() to linux/acpi.h ACPI: Replace printk with pr_* in tables.c
arch/ia64/include/asm/acpi.h | 5 +- arch/ia64/kernel/acpi.c | 18 ++++-- arch/x86/include/asm/acpi.h | 19 +------ arch/x86/kernel/acpi/boot.c | 4 -- arch/x86/kernel/acpi/cstate.c | 29 ++++++++++ drivers/acpi/processor_core.c | 92 ++++++++++++++++++------------ drivers/acpi/tables.c | 124 ++++++++++++++++++----------------------- include/linux/acpi.h | 4 ++ 8 files changed, 158 insertions(+), 137 deletions(-)
idle_boot_override depends on x86 and ia64 now, and we can not foresee it will be used on ARM or ARM64, so move the code into CONFIG_X86 and CONFIG_IA64 #ifdefs to make processor_core.c can be compiled on ARM64.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/processor_core.c | 47 ++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 19 deletions(-)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 7c50a4c..4d91b32 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -19,24 +19,6 @@ #define _COMPONENT ACPI_PROCESSOR_COMPONENT ACPI_MODULE_NAME("processor_core");
-static int __init set_no_mwait(const struct dmi_system_id *id) -{ - printk(KERN_NOTICE PREFIX "%s detected - " - "disabling mwait for CPU C-states\n", id->ident); - boot_option_idle_override = IDLE_NOMWAIT; - return 0; -} - -static struct dmi_system_id processor_idle_dmi_table[] __initdata = { - { - set_no_mwait, "Extensa 5220", { - DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"), - DMI_MATCH(DMI_SYS_VENDOR, "Acer"), - DMI_MATCH(DMI_PRODUCT_VERSION, "0100"), - DMI_MATCH(DMI_BOARD_NAME, "Columbia") }, NULL}, - {}, -}; - static int map_lapic_id(struct acpi_subtable_header *entry, u32 acpi_id, int *apic_id) { @@ -379,13 +361,40 @@ early_init_pdc(acpi_handle handle, u32 lvl, void *context, void **rv) return AE_OK; }
-void __init acpi_early_processor_set_pdc(void) +#if defined(CONFIG_X86) || defined(CONFIG_IA64) +static int __init set_no_mwait(const struct dmi_system_id *id) +{ + pr_notice(PREFIX "%s detected - disabling mwait for CPU C-states\n", + id->ident); + boot_option_idle_override = IDLE_NOMWAIT; + return 0; +} + +static struct dmi_system_id processor_idle_dmi_table[] __initdata = { + { + set_no_mwait, "Extensa 5220", { + DMI_MATCH(DMI_BIOS_VENDOR, "Phoenix Technologies LTD"), + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), + DMI_MATCH(DMI_PRODUCT_VERSION, "0100"), + DMI_MATCH(DMI_BOARD_NAME, "Columbia") }, NULL}, + {}, +}; + +static void __init processor_dmi_check(void) { /* * Check whether the system is DMI table. If yes, OSPM * should not use mwait for CPU-states. */ dmi_check_system(processor_idle_dmi_table); +} +#else +static inline void processor_dmi_check(void) {} +#endif + +void __init acpi_early_processor_set_pdc(void) +{ + processor_dmi_check();
acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX,
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org --- arch/ia64/include/asm/acpi.h | 5 +---- arch/ia64/kernel/acpi.c | 14 ++++++++++++++ arch/x86/include/asm/acpi.h | 19 +------------------ arch/x86/kernel/acpi/cstate.c | 29 +++++++++++++++++++++++++++++ drivers/acpi/processor_core.c | 19 +------------------ 5 files changed, 46 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 07d209c..af9d9e4 100644 --- a/arch/ia64/kernel/acpi.c +++ b/arch/ia64/kernel/acpi.c @@ -1014,3 +1014,17 @@ 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); + } +} diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index c8c1e70..e9f71bc 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -147,24 +147,7 @@ static inline bool arch_has_acpi_pdc(void) c->x86_vendor == X86_VENDOR_CENTAUR); }
-static inline void arch_acpi_set_pdc_bits(u32 *buf) -{ - struct cpuinfo_x86 *c = &cpu_data(0); - - buf[2] |= ACPI_PDC_C_CAPABILITY_SMP; - - if (cpu_has(c, X86_FEATURE_EST)) - buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP; - - if (cpu_has(c, X86_FEATURE_ACPI)) - buf[2] |= ACPI_PDC_T_FFH; - - /* - * If mwait/monitor is unsupported, C2/C3_FFH will be disabled - */ - if (!cpu_has(c, X86_FEATURE_MWAIT)) - buf[2] &= ~(ACPI_PDC_C_C2C3_FFH); -} +extern void arch_acpi_set_pdc_bits(u32 *buf);
#else /* !CONFIG_ACPI */
diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c index e69182f..a36638f 100644 --- a/arch/x86/kernel/acpi/cstate.c +++ b/arch/x86/kernel/acpi/cstate.c @@ -16,6 +16,35 @@ #include <asm/mwait.h> #include <asm/special_insns.h>
+void arch_acpi_set_pdc_bits(u32 *buf) +{ + struct cpuinfo_x86 *c = &cpu_data(0); + + /* Enable coordination with firmware's _TSD info */ + buf[2] |= ACPI_PDC_SMP_T_SWCOORD | ACPI_PDC_C_CAPABILITY_SMP; + + if (cpu_has(c, X86_FEATURE_EST)) + buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP; + + if (cpu_has(c, X86_FEATURE_ACPI)) + buf[2] |= ACPI_PDC_T_FFH; + + /* + * If mwait/monitor is unsupported, C2/C3_FFH will be disabled + */ + if (!cpu_has(c, X86_FEATURE_MWAIT)) + buf[2] &= ~(ACPI_PDC_C_C2C3_FFH); + + 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); + } +} + /* * Initialize bm_flags based on the CPU cache properties * On SMP it depends on cache configuration diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4d91b32..4dcf776 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -255,9 +255,6 @@ static void acpi_set_pdc_bits(u32 *buf) buf[0] = ACPI_PDC_REVISION_ID; buf[1] = 1;
- /* Enable coordination with firmware's _TSD info */ - buf[2] = ACPI_PDC_SMP_T_SWCOORD; - /* Twiddle arch-specific bits needed for _PDC */ arch_acpi_set_pdc_bits(buf); } @@ -282,7 +279,7 @@ static struct acpi_object_list *acpi_processor_alloc_pdc(void) return NULL; }
- buf = kmalloc(12, GFP_KERNEL); + buf = kzalloc(12, GFP_KERNEL); if (!buf) { printk(KERN_ERR "Memory allocation error\n"); kfree(obj); @@ -310,20 +307,6 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in) { acpi_status status = AE_OK;
- 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. - */ - union acpi_object *obj; - u32 *buffer = NULL; - - obj = pdc_in->pointer; - buffer = (u32 *)(obj->buffer.pointer); - buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH); - - } status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
if (ACPI_FAILURE(status))
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks!
arch/ia64/include/asm/acpi.h | 5 +---- arch/ia64/kernel/acpi.c | 14 ++++++++++++++ arch/x86/include/asm/acpi.h | 19 +------------------ arch/x86/kernel/acpi/cstate.c | 29 +++++++++++++++++++++++++++++ drivers/acpi/processor_core.c | 19 +------------------ 5 files changed, 46 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 07d209c..af9d9e4 100644 --- a/arch/ia64/kernel/acpi.c +++ b/arch/ia64/kernel/acpi.c @@ -1014,3 +1014,17 @@ 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);- }
+} diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index c8c1e70..e9f71bc 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -147,24 +147,7 @@ static inline bool arch_has_acpi_pdc(void) c->x86_vendor == X86_VENDOR_CENTAUR); } -static inline void arch_acpi_set_pdc_bits(u32 *buf) -{
- struct cpuinfo_x86 *c = &cpu_data(0);
- buf[2] |= ACPI_PDC_C_CAPABILITY_SMP;
- if (cpu_has(c, X86_FEATURE_EST))
buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP;- if (cpu_has(c, X86_FEATURE_ACPI))
buf[2] |= ACPI_PDC_T_FFH;- /*
* If mwait/monitor is unsupported, C2/C3_FFH will be disabled*/- if (!cpu_has(c, X86_FEATURE_MWAIT))
buf[2] &= ~(ACPI_PDC_C_C2C3_FFH);-} +extern void arch_acpi_set_pdc_bits(u32 *buf); #else /* !CONFIG_ACPI */ diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c index e69182f..a36638f 100644 --- a/arch/x86/kernel/acpi/cstate.c +++ b/arch/x86/kernel/acpi/cstate.c @@ -16,6 +16,35 @@ #include <asm/mwait.h> #include <asm/special_insns.h> +void arch_acpi_set_pdc_bits(u32 *buf) +{
- struct cpuinfo_x86 *c = &cpu_data(0);
- /* Enable coordination with firmware's _TSD info */
- buf[2] |= ACPI_PDC_SMP_T_SWCOORD | ACPI_PDC_C_CAPABILITY_SMP;
- if (cpu_has(c, X86_FEATURE_EST))
buf[2] |= ACPI_PDC_EST_CAPABILITY_SWSMP;- if (cpu_has(c, X86_FEATURE_ACPI))
buf[2] |= ACPI_PDC_T_FFH;- /*
* If mwait/monitor is unsupported, C2/C3_FFH will be disabled*/- if (!cpu_has(c, X86_FEATURE_MWAIT))
buf[2] &= ~(ACPI_PDC_C_C2C3_FFH);- 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);- }
+}
/*
- Initialize bm_flags based on the CPU cache properties
- On SMP it depends on cache configuration
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4d91b32..4dcf776 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -255,9 +255,6 @@ static void acpi_set_pdc_bits(u32 *buf) buf[0] = ACPI_PDC_REVISION_ID; buf[1] = 1;
- /* Enable coordination with firmware's _TSD info */
- buf[2] = ACPI_PDC_SMP_T_SWCOORD;
- /* Twiddle arch-specific bits needed for _PDC */ arch_acpi_set_pdc_bits(buf);
} @@ -282,7 +279,7 @@ static struct acpi_object_list *acpi_processor_alloc_pdc(void) return NULL; }
- buf = kmalloc(12, GFP_KERNEL);
- buf = kzalloc(12, GFP_KERNEL); if (!buf) { printk(KERN_ERR "Memory allocation error\n"); kfree(obj);
@@ -310,20 +307,6 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in) { acpi_status status = AE_OK;
- 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.*/union acpi_object *obj;u32 *buffer = NULL;obj = pdc_in->pointer;buffer = (u32 *)(obj->buffer.pointer);buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);- } status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
if (ACPI_FAILURE(status))
On 2014年02月19日 08:50, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks! I will resend this patch with Tony and Thomas in cc list, so is there any chance for this patch to be merged into 3.15 if I get acked from them?
Best regards Hanjun
On Wednesday, February 19, 2014 10:16:16 AM Hanjun Guo wrote:
On 2014年02月19日 08:50, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks! I will resend this patch with Tony and Thomas in cc list, so is there any chance for this patch to be merged into 3.15 if I get acked from them?
If they ack it, then yes, it can.
Thanks!
Hi Rafael,
On Wed, Feb 19, 2014 at 01:50:22AM +0100, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks for taking these patches. I would however hold onto patch 3/5 as this is still under discussion. Basically for patches specific to ARM ACPI I would really like to see more acks before being merged as that's a new thing for us.
Thanks.
On Friday, February 21, 2014 06:24:24 PM Catalin Marinas wrote:
Hi Rafael,
On Wed, Feb 19, 2014 at 01:50:22AM +0100, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks for taking these patches. I would however hold onto patch 3/5 as this is still under discussion. Basically for patches specific to ARM ACPI I would really like to see more acks before being merged as that's a new thing for us.
OK, I'll drop [3/5] for now, then.
I'm wondering, though, whose ACKs I should be waiting for before applying those patches?
Rafael
On 21 Feb 2014, at 23:35, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, February 21, 2014 06:24:24 PM Catalin Marinas wrote:
On Wed, Feb 19, 2014 at 01:50:22AM +0100, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks for taking these patches. I would however hold onto patch 3/5 as this is still under discussion. Basically for patches specific to ARM ACPI I would really like to see more acks before being merged as that's a new thing for us.
OK, I'll drop [3/5] for now, then.
Thanks (it’s only temporary ;)).
I'm wondering, though, whose ACKs I should be waiting for before applying those patches?
Good question ;). In this particular case, there is an ongoing discussion between Hanjun and Sudeep. While there isn’t anything major, I would like to see some agreement and potentially an Ack from the other party involved in the discussion (Sudeep).
There are other patches that are not specific to ARM, so it’s really your decision. As for the general ARM(64) ACPI case, I don’t think we have anyone in charge with deciding what’s correct or not (BTW, who are the people active both in the _ARM_ Linux kernel community and the ACPI standardisation forum?).
In the mean-time, I would like to see at least an ack from the arm-soc team (Arnd and Olof) or them collecting those ARM-specific patches and sending pull request to you. My biggest worry is how the ACPI standard is interpreted by vendors and how (non-standard) features get into the Linux kernel.
Anyway, I’ll meet the Linaro guys in a week time and we’ll agree on a way forward.
Thanks,
Catalin
Catalin, Sudeep,
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
We would welcome all your early comments, as already mentioned months ago.
At Connect we will have ACPI sessions on power management, clocks and regulators and upstreaming.
I expect that by the end of Connect we have a clear well defined list of who you want to ack, who has already ack'ed, who not and why. I also want to agree on who owns ACPI on ARM, as you have raised this question already few times. I do not care who, I care that we do not raise such topics in front of Rafael, as Intel loves just too much to read that the ARM camp is in total confusion.
Thanks Andrea
On Saturday, February 22, 2014, Catalin Marinas catalin.marinas@arm.com wrote:
On 21 Feb 2014, at 23:35, Rafael J. Wysocki <rjw@rjwysocki.netjavascript:;> wrote:
On Friday, February 21, 2014 06:24:24 PM Catalin Marinas wrote:
On Wed, Feb 19, 2014 at 01:50:22AM +0100, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64
dependent,
rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org javascript:;> Signed-off-by: Graeme Gregory <graeme.gregory@linaro.orgjavascript:;
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo
changelog
modifications), but this one should be CCed to the x86 and ia64
maintainers.
Thanks for taking these patches. I would however hold onto patch 3/5 as this is still under discussion. Basically for patches specific to ARM ACPI I would really like to see more acks before being merged as that's a new thing for us.
OK, I'll drop [3/5] for now, then.
Thanks (it's only temporary ;)).
I'm wondering, though, whose ACKs I should be waiting for before
applying those
patches?
Good question ;). In this particular case, there is an ongoing discussion between Hanjun and Sudeep. While there isn't anything major, I would like to see some agreement and potentially an Ack from the other party involved in the discussion (Sudeep).
There are other patches that are not specific to ARM, so it's really your decision. As for the general ARM(64) ACPI case, I don't think we have anyone in charge with deciding what's correct or not (BTW, who are the people active both in the _ARM_ Linux kernel community and the ACPI standardisation forum?).
In the mean-time, I would like to see at least an ack from the arm-soc team (Arnd and Olof) or them collecting those ARM-specific patches and sending pull request to you. My biggest worry is how the ACPI standard is interpreted by vendors and how (non-standard) features get into the Linux kernel.
Anyway, I'll meet the Linaro guys in a week time and we'll agree on a way forward.
Thanks,
Catalin _______________________________________________ Linaro-acpi mailing list Linaro-acpi@lists.linaro.org javascript:; http://lists.linaro.org/mailman/listinfo/linaro-acpi
Hi Andrea,
On 22 Feb 2014, at 17:42, Andrea Gallo andrea.gallo@linaro.org wrote:
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Which is great, though more Reviewed-by tags on such patches would be even better for something of such importance.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
I don’t mind which list, can be LKML. My point is that such patches have been sent to Rafael but none of these patches have a Reviewed-by tag. I’m ok with the generic patches which is pretty much Rafael’s decision to merge but those related to the ARM IP I would really like more reviews/acks.
Really sorry to ask Rafael for a revert of 3/5 but we shouldn’t send him patches until fully reviewed. AFAIK, Sudeep raised the same issues on previous version of the patch.
Thanks,
Catalin
Hi Catalin,
On 2014-2-23 2:27, Catalin Marinas wrote:
Hi Andrea,
On 22 Feb 2014, at 17:42, Andrea Gallo andrea.gallo@linaro.org wrote:
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Which is great, though more Reviewed-by tags on such patches would be even better for something of such importance.
I will add Reviewed-by tag in next version, thanks for the reminding.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
I don’t mind which list, can be LKML. My point is that such patches have been sent to Rafael but none of these patches have a Reviewed-by tag. I’m ok with the generic patches which is pretty much Rafael’s decision to merge but those related to the ARM IP I would really like more reviews/acks.
Really sorry to ask Rafael for a revert of 3/5 but we shouldn’t send him patches until fully reviewed. AFAIK, Sudeep raised the same issues on previous version of the patch.
It is ok to me. Since GIC ID and UID for ACPI 5.0 are not clear, and the code will have impact on the implementation of firmware, we must be very careful.
Thanks Hanjun
Hi Andrea,
On 22/02/14 17:42, Andrea Gallo wrote:
Catalin, Sudeep,
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Yes I have subscribed to that list and even interacted couple of times. Only problem I see there is that the patches posted there are mostly based on Linaro tree which has whole lot of other changes rather than mainline.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
Yes, but these reworked patches were not posted just on linaro-acpi list. I have no problem reviewing on any list as long as its based on mainline. I have started looking at the linaro tree, but it has several other changes, so bit hard for me to follow now as it has several months of ACPI work by Linaro.
We would welcome all your early comments, as already mentioned months ago.
No problem, I have several months of work to follow, will take some time. I can easily understand small changes based on mainline and hence reviewing them.
At Connect we will have ACPI sessions on power management, clocks and regulators and upstreaming.
Yes I will make sure to attend them so that I get complete picture and easy for me to ramp up.
I expect that by the end of Connect we have a clear well defined list of who you want to ack, who has already ack'ed, who not and why. I also want to agree on who owns ACPI on ARM, as you have raised this question already few times. I do not care who, I care that we do not raise such topics in front of Rafael, as Intel loves just too much to read that the ARM camp is in total confusion.
Ok, agreed.
Regards, Sudeep
On Mon, Feb 24, 2014 at 10:29:51AM +0000, Sudeep Holla wrote:
Hi Andrea,
On 22/02/14 17:42, Andrea Gallo wrote:
Catalin, Sudeep,
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Yes I have subscribed to that list and even interacted couple of times. Only problem I see there is that the patches posted there are mostly based on Linaro tree which has whole lot of other changes rather than mainline.
It did while we were prototyping, it no longer does.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
Yes, but these reworked patches were not posted just on linaro-acpi list. I have no problem reviewing on any list as long as its based on mainline. I have started looking at the linaro tree, but it has several other changes, so bit hard for me to follow now as it has several months of ACPI work by Linaro.
The construction of the linaro branches is as follows.
mainline->acpi-mainline-core->acpi-core->acpi-drivers
acpi-mainline-core is the patch series as it is sent by Hanjun acpi-core is stuff we are currently working on which has not been sent yet acpi-drivers is the drivers needed to boot on model, which are probably never destined for mainline.
I thought I had sent the note detailing this to linaro-acpi but I might have messed up, Ill check and resend later.
Graeme
On 24/02/14 10:57, Graeme Gregory wrote:
On Mon, Feb 24, 2014 at 10:29:51AM +0000, Sudeep Holla wrote:
Hi Andrea,
On 22/02/14 17:42, Andrea Gallo wrote:
Catalin, Sudeep,
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Yes I have subscribed to that list and even interacted couple of times. Only problem I see there is that the patches posted there are mostly based on Linaro tree which has whole lot of other changes rather than mainline.
It did while we were prototyping, it no longer does.
I see, that's good.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
Yes, but these reworked patches were not posted just on linaro-acpi list. I have no problem reviewing on any list as long as its based on mainline. I have started looking at the linaro tree, but it has several other changes, so bit hard for me to follow now as it has several months of ACPI work by Linaro.
The construction of the linaro branches is as follows.
mainline->acpi-mainline-core->acpi-core->acpi-drivers
acpi-mainline-core is the patch series as it is sent by Hanjun acpi-core is stuff we are currently working on which has not been sent yet acpi-drivers is the drivers needed to boot on model, which are probably never destined for mainline.
Thanks for this info that's useful, I started initially to follow these branches on similar thoughts based on the names, but then started following another tree after seeing the mail "leg-kernel tag leg-20140217.0 released", mainly because I wanted to get the similar set-up at my end before LCA14 so that I have some understanding on what's done so far. That may be the reason why I saw lot of other changes in the tree, so for that.
I thought I had sent the note detailing this to linaro-acpi but I might have messed up, Ill check and resend later.
Hmm, I don't remember seeing such a mail, may be I might have missed or accidentally deleted it.
Regards, Sudeep
On Mon, Feb 24, 2014 at 11:30:18AM +0000, Sudeep Holla wrote:
On 24/02/14 10:57, Graeme Gregory wrote:
On Mon, Feb 24, 2014 at 10:29:51AM +0000, Sudeep Holla wrote:
Hi Andrea,
On 22/02/14 17:42, Andrea Gallo wrote:
Catalin, Sudeep,
Hanjun, Al, Graeme and the whole team discuss and review all such patches on the linaro-acpi list before sending them out.
Yes I have subscribed to that list and even interacted couple of times. Only problem I see there is that the patches posted there are mostly based on Linaro tree which has whole lot of other changes rather than mainline.
It did while we were prototyping, it no longer does.
I see, that's good.
Would it make sense to discuss all these points on such list instead of creating such confusion with the broader linux-acpi or lkml? Even more as Sudeep seems working deeply on ACPI now, it would be very beneficial.
Yes, but these reworked patches were not posted just on linaro-acpi list. I have no problem reviewing on any list as long as its based on mainline. I have started looking at the linaro tree, but it has several other changes, so bit hard for me to follow now as it has several months of ACPI work by Linaro.
The construction of the linaro branches is as follows.
mainline->acpi-mainline-core->acpi-core->acpi-drivers
acpi-mainline-core is the patch series as it is sent by Hanjun acpi-core is stuff we are currently working on which has not been sent yet acpi-drivers is the drivers needed to boot on model, which are probably never destined for mainline.
Thanks for this info that's useful, I started initially to follow these branches on similar thoughts based on the names, but then started following another tree after seeing the mail "leg-kernel tag leg-20140217.0 released", mainly because I wanted to get the similar set-up at my end before LCA14 so that I have some understanding on what's done so far. That may be the reason why I saw lot of other changes in the tree, so for that.
leg-kernel combines the ACPI work with UEFI work and LLCT so it can be fed through the CI infrastructure for testing. Unfortuneately there is no method to pass ACPI tables without UEFI patches.
I thought I had sent the note detailing this to linaro-acpi but I might have messed up, Ill check and resend later.
Hmm, I don't remember seeing such a mail, may be I might have missed or accidentally deleted it.
No, I checked, I sent it to team internally for review but forgot to send it out to list. I have now sent one to list. Sorry for the confusion I caused by missing this essential info.
Graeme
On 24/02/14 11:38, Graeme Gregory wrote:
On Mon, Feb 24, 2014 at 11:30:18AM +0000, Sudeep Holla wrote:
[...]
Thanks for this info that's useful, I started initially to follow these branches on similar thoughts based on the names, but then started following another tree after seeing the mail "leg-kernel tag leg-20140217.0 released", mainly because I wanted to get the similar set-up at my end before LCA14 so that I have some understanding on what's done so far. That may be the reason why I saw lot of other changes in the tree, so for that.
leg-kernel combines the ACPI work with UEFI work and LLCT so it can be fed through the CI infrastructure for testing. Unfortuneately there is no method to pass ACPI tables without UEFI patches.
Thanks for the confirmation, that was my understanding too and hence I picked up that tree.
I thought I had sent the note detailing this to linaro-acpi but I might have messed up, Ill check and resend later.
Hmm, I don't remember seeing such a mail, may be I might have missed or accidentally deleted it.
No, I checked, I sent it to team internally for review but forgot to send it out to list. I have now sent one to list. Sorry for the confusion I caused by missing this essential info.
No problem, got that mail now. Thanks for all the info.
Regards, Sudeep
On 2014-2-22 18:33, Catalin Marinas wrote:
On 21 Feb 2014, at 23:35, Rafael J. Wysocki rjw@rjwysocki.net wrote:
On Friday, February 21, 2014 06:24:24 PM Catalin Marinas wrote:
On Wed, Feb 19, 2014 at 01:50:22AM +0100, Rafael J. Wysocki wrote:
On Wednesday, February 19, 2014 12:23:55 AM Hanjun Guo wrote:
_PDC related stuff in processor_core.c is little bit X86/IA64 dependent, rework the code to make it more arch-independent, no functional change in this patch.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org Signed-off-by: Graeme Gregory graeme.gregory@linaro.org
I've queued up patches [1,3-5/5] from this series for 3.15 (modulo changelog modifications), but this one should be CCed to the x86 and ia64 maintainers.
Thanks for taking these patches. I would however hold onto patch 3/5 as this is still under discussion. Basically for patches specific to ARM ACPI I would really like to see more acks before being merged as that's a new thing for us.
OK, I'll drop [3/5] for now, then.
Thanks (it’s only temporary ;)).
I'm wondering, though, whose ACKs I should be waiting for before applying those patches?
Good question ;). In this particular case, there is an ongoing discussion between Hanjun and Sudeep. While there isn’t anything major, I would like to see some agreement and potentially an Ack from the other party involved in the discussion (Sudeep).
There are other patches that are not specific to ARM, so it’s really your decision. As for the general ARM(64) ACPI case, I don’t think we have anyone in charge with deciding what’s correct or not (BTW, who are the people active both in the _ARM_ Linux kernel community and the ACPI standardisation forum?).
I'm in ASWG (ACPI spec working group) under UEFI, and Al Stone and Charles (+cc Charles) are also in this forum.
Thanks Hanjun
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; }
+static int map_gic_id(struct acpi_subtable_header *entry, + int device_declaration, u32 acpi_id, int *apic_id) +{ + struct acpi_madt_generic_interrupt *gic = + (struct acpi_madt_generic_interrupt *)entry; + + if (!(gic->flags & ACPI_MADT_ENABLED)) + return -ENODEV; + + /* In the GIC interrupt model, logical processors are + * required to have a Processor Device object in the DSDT, + * so we should check device_declaration here + */ + if (device_declaration && (gic->uid == acpi_id)) { + *apic_id = gic->gic_id; + return 0; + } + + return -EINVAL; +} + static int map_madt_entry(int type, u32 acpi_id) { unsigned long madt_end, entry; @@ -106,6 +127,9 @@ static int map_madt_entry(int type, u32 acpi_id) } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { if (!map_lsapic_id(header, type, acpi_id, &apic_id)) break; + } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) { + if (!map_gic_id(header, type, acpi_id, &apic_id)) + break; } entry += header->length; } @@ -136,6 +160,8 @@ static int map_mat_entry(acpi_handle handle, int type, u32 acpi_id) map_lapic_id(header, acpi_id, &apic_id); } else if (header->type == ACPI_MADT_TYPE_LOCAL_SAPIC) { map_lsapic_id(header, type, acpi_id, &apic_id); + } else if (header->type == ACPI_MADT_TYPE_GENERIC_INTERRUPT) { + map_gic_id(header, type, acpi_id, &apic_id); }
exit:
Hi Hanjun,
On 18/02/14 16:23, Hanjun Guo wrote:
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } +static int map_gic_id(struct acpi_subtable_header *entry,
int device_declaration, u32 acpi_id, int *apic_id)+{
- struct acpi_madt_generic_interrupt *gic =
(struct acpi_madt_generic_interrupt *)entry;- if (!(gic->flags & ACPI_MADT_ENABLED))
return -ENODEV;- /* In the GIC interrupt model, logical processors are
* required to have a Processor Device object in the DSDT,* so we should check device_declaration here*/- if (device_declaration && (gic->uid == acpi_id)) {
*apic_id = gic->gic_id;
I have mentioned this earlier, it's not clear yet to me how does this work ? It needs more clarity in the form of comment here at-least as the ACPIv5.0 is also not so clear or explicit on how to handle this.
Here you are expecting gic->uid = acpi_id which is fine, while acpi_map_cpuid matches apic_id with cpu_physical_id(which must be MPIDR in ARM{32,64}). The latter imposes restriction that gic->gic_id has to be MPIDR. Does that mean we are imposing restriction on GIC ID to be MPIDR ? If so please document it here and please explain the reason behind that choice.
I would expect _UID to be MPIDR rather than GIC ID but you may have some reasons for this choice.
Regards, Sudeep
Hi Sudeep,
Thanks for your comments, please refer to the replies below. :)
On 2014年02月19日 22:33, Sudeep Holla wrote:
Hi Hanjun,
On 18/02/14 16:23, Hanjun Guo wrote:
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } +static int map_gic_id(struct acpi_subtable_header *entry,
int device_declaration, u32 acpi_id, int *apic_id)+{
- struct acpi_madt_generic_interrupt *gic =
(struct acpi_madt_generic_interrupt *)entry;- if (!(gic->flags & ACPI_MADT_ENABLED))
return -ENODEV;- /* In the GIC interrupt model, logical processors are
* required to have a Processor Device object in the DSDT,* so we should check device_declaration here*/- if (device_declaration && (gic->uid == acpi_id)) {
*apic_id = gic->gic_id;I have mentioned this earlier, it's not clear yet to me how does this work ? It needs more clarity in the form of comment here at-least as the ACPIv5.0 is also not so clear or explicit on how to handle this.
Yes, I noticed your comments and had a reply for that, after a long consideration for this, I would withdraw my previous comments before, please refer to the comments below.
Here you are expecting gic->uid = acpi_id which is fine, while acpi_map_cpuid matches apic_id with cpu_physical_id(which must be MPIDR in ARM{32,64}). The latter imposes restriction that gic->gic_id has to be MPIDR. Does that mean we are imposing restriction on GIC ID to be MPIDR ? If so please document it here and please explain the reason behind that choice.
On x86 and IA64, APIC/SAPIC ID is the hardware id of the logical processor, and UID is just a unique ID to identify the processor in DSDT, it can be any value, and even can be strings defined in ASL if I remember that correctly.
The processor driver also follows that guidance now, and GIC structure in MADT actually represents a processor (GICC) in the system, so I would let gic->gic_id as MPIDR(processor hardware ID) for now to avoid confusions and keep consistency with current ACPI driver.
UID can be any value and it depends on implementation, so it can be MPIDR also, it will not conflict with GIC ID and can works fine in acpi processor driver now.
I would expect _UID to be MPIDR rather than GIC ID but you may have some reasons for this choice.
I think they both can be MPIDR for now, is this ok to you?
we can update the code when the new ACPI spec is coming out if there is a clear explanation for GIC ID and UID in GIC structures, does this make sense to you?
Thanks Hanjun
Hi Hanjun,
(Adding MarcZ for his views on GIC)
On 20/02/14 03:59, Hanjun Guo wrote:
Hi Sudeep,
Thanks for your comments, please refer to the replies below. :)
On 2014年02月19日 22:33, Sudeep Holla wrote:
Hi Hanjun,
On 18/02/14 16:23, Hanjun Guo wrote:
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } +static int map_gic_id(struct acpi_subtable_header *entry,
int device_declaration, u32 acpi_id, int *apic_id)+{
- struct acpi_madt_generic_interrupt *gic =
(struct acpi_madt_generic_interrupt *)entry;- if (!(gic->flags & ACPI_MADT_ENABLED))
return -ENODEV;- /* In the GIC interrupt model, logical processors are
* required to have a Processor Device object in the DSDT,* so we should check device_declaration here*/- if (device_declaration && (gic->uid == acpi_id)) {
*apic_id = gic->gic_id;I have mentioned this earlier, it's not clear yet to me how does this work ? It needs more clarity in the form of comment here at-least as the ACPIv5.0 is also not so clear or explicit on how to handle this.
Yes, I noticed your comments and had a reply for that, after a long consideration for this, I would withdraw my previous comments before, please refer to the comments below.
Here you are expecting gic->uid = acpi_id which is fine, while acpi_map_cpuid matches apic_id with cpu_physical_id(which must be MPIDR in ARM{32,64}). The latter imposes restriction that gic->gic_id has to be MPIDR. Does that mean we are imposing restriction on GIC ID to be MPIDR ? If so please document it here and please explain the reason behind that choice.
On x86 and IA64, APIC/SAPIC ID is the hardware id of the logical processor, and UID is just a unique ID to identify the processor in DSDT, it can be any value, and even can be strings defined in ASL if I remember that correctly.
OK, but that's not the case on ARM{32,64}. My main concern here is if we don't make this definitions clear enough, the vendors might produce ACPI tables with whatever suits them and we may end up supporting them. Since we are starting with clean slate, we can avoid getting into such situations. I will be to be more elaborate this time.
The GIC ID is referred as the local GIC’s hardware ID in ACPIv5.0. IIUC, since GICC is per-cpu entry, it has to GIC CPU interface ID.
Now how does it differ from MPIDR ? e.g. ARM TC2(multi cluster system) GIC ID MPIDR Comment 0 0x000 CA15_0 1 0x001 CA15_1 2 0x100 CA7_0 3 0x101 CA7_1 4 0x102 CA7_2
The processor driver also follows that guidance now, and GIC structure in MADT actually represents a processor (GICC) in the system, so I would let gic->gic_id as MPIDR(processor hardware ID) for now to avoid confusions and keep consistency with current ACPI driver.
Yes that's my worry "... so I would let gic->gic_id as MPIDR(processor hardware ID)" as it contradicts the definition.
1. Yes today GIC CPU ID is enumerated in the code and we don't need it, but that doesn't mean the GICC contain whatever(of vendor choice). IMO it should be CPU interface ID as per the definition.
2. It's better you post this patch as part of ARM64 port for easier understanding on how this gets used as its ARM specific anyway. That gives better/complete picture at least for review purposes.
UID can be any value and it depends on implementation, so it can be MPIDR also, it will not conflict with GIC ID and can works fine in acpi processor driver now.
Agreed UID can be anything, but your initial ARM64 APCI patches were assuming it to be MPIDR. Since MPIDR is not specified anywhere else in the ACPI tables and it is required for some boot methods like PSCI and may be others, I am for mandating _UID in processor object to be MPIDR. If you agree please make it explicit in the comments.
I would expect _UID to be MPIDR rather than GIC ID but you may have some reasons for this choice.
I think they both can be MPIDR for now, is this ok to you?
Yes for _UID and no for GIC ID, they need to represent the hardware. We can see how to manage in this in ACPI. Initially I went by the name "cpu_physical_id" in acpi_map_cpuid but it looks like it's cpuid->apicid mapping on x86. So that should not be problem.
we can update the code when the new ACPI spec is coming out if there is a clear explanation for GIC ID and UID in GIC structures, does this make sense to you?
Yes, that's for next version of ACPI but we don't know that yet. IIUC you are trying to implement ACPI port for ARM64 based on ACPIv5.0 through these patches right ? Hence I pushing you to be more explicit through comments as ACPIv5.0 is not clear enough.
Regards, Sudeep
On 2014-2-21 20:37, Sudeep Holla wrote:
Hi Hanjun,
(Adding MarcZ for his views on GIC)
On 20/02/14 03:59, Hanjun Guo wrote:
Hi Sudeep,
Thanks for your comments, please refer to the replies below. :)
On 2014年02月19日 22:33, Sudeep Holla wrote:
Hi Hanjun,
On 18/02/14 16:23, Hanjun Guo wrote:
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; } +static int map_gic_id(struct acpi_subtable_header *entry,
int device_declaration, u32 acpi_id, int *apic_id)+{
- struct acpi_madt_generic_interrupt *gic =
(struct acpi_madt_generic_interrupt *)entry;- if (!(gic->flags & ACPI_MADT_ENABLED))
return -ENODEV;- /* In the GIC interrupt model, logical processors are
* required to have a Processor Device object in the DSDT,* so we should check device_declaration here*/- if (device_declaration && (gic->uid == acpi_id)) {
*apic_id = gic->gic_id;I have mentioned this earlier, it's not clear yet to me how does this work ? It needs more clarity in the form of comment here at-least as the ACPIv5.0 is also not so clear or explicit on how to handle this.
Yes, I noticed your comments and had a reply for that, after a long consideration for this, I would withdraw my previous comments before, please refer to the comments below.
Here you are expecting gic->uid = acpi_id which is fine, while acpi_map_cpuid matches apic_id with cpu_physical_id(which must be MPIDR in ARM{32,64}). The latter imposes restriction that gic->gic_id has to be MPIDR. Does that mean we are imposing restriction on GIC ID to be MPIDR ? If so please document it here and please explain the reason behind that choice.
On x86 and IA64, APIC/SAPIC ID is the hardware id of the logical processor, and UID is just a unique ID to identify the processor in DSDT, it can be any value, and even can be strings defined in ASL if I remember that correctly.
OK, but that's not the case on ARM{32,64}. My main concern here is if we don't make this definitions clear enough, the vendors might produce ACPI tables with whatever suits them and we may end up supporting them. Since we are starting with clean slate, we can avoid getting into such situations. I will be to be more elaborate this time.
I agree.
The GIC ID is referred as the local GIC’s hardware ID in ACPIv5.0. IIUC, since GICC is per-cpu entry, it has to GIC CPU interface ID.
Now how does it differ from MPIDR ? e.g. ARM TC2(multi cluster system) GIC ID MPIDR Comment 0 0x000 CA15_0 1 0x001 CA15_1 2 0x100 CA7_0 3 0x101 CA7_1 4 0x102 CA7_2
Yes, obvious different. I know GIC ID can matche the bit index of the associated processor in the distributor's GICD_ITARGETSR register, and it a clear statement in GICv1/GICv2, my question is that is this consistent in GICv3/v4 too? this will have some impact on the code implementation.
The processor driver also follows that guidance now, and GIC structure in MADT actually represents a processor (GICC) in the system, so I would let gic->gic_id as MPIDR(processor hardware ID) for now to avoid confusions and keep consistency with current ACPI driver.
Yes that's my worry "... so I would let gic->gic_id as MPIDR(processor hardware ID)" as it contradicts the definition.
Yes today GIC CPU ID is enumerated in the code and we don't need it, but that doesn't mean the GICC contain whatever(of vendor choice). IMO it should be CPU interface ID as per the definition.
It's better you post this patch as part of ARM64 port for easier understanding on how this gets used as its ARM specific anyway. That gives better/complete picture at least for review purposes.
It make sense to me, since Rafael had dropped this patch, I will resend it in ARM64 ACPI core patche set.
UID can be any value and it depends on implementation, so it can be MPIDR also, it will not conflict with GIC ID and can works fine in acpi processor driver now.
Agreed UID can be anything, but your initial ARM64 APCI patches were assuming it to be MPIDR. Since MPIDR is not specified anywhere else in the ACPI tables and it is required for some boot methods like PSCI and may be others, I am for mandating _UID in processor object to be MPIDR. If you agree please make it explicit in the comments.
I would expect _UID to be MPIDR rather than GIC ID but you may have some reasons for this choice.
I think they both can be MPIDR for now, is this ok to you?
Yes for _UID and no for GIC ID, they need to represent the hardware. We can see how to manage in this in ACPI. Initially I went by the name "cpu_physical_id" in acpi_map_cpuid but it looks like it's cpuid->apicid mapping on x86. So that should not be problem.
Yes, from the ACPI side, it is ok. but we still need the mappings from logical processor id to MPIDR value for SMP initialization, just as cpu_logical_map() in ARM64 did, so I will address your comments and figure out a better solution.
we can update the code when the new ACPI spec is coming out if there is a clear explanation for GIC ID and UID in GIC structures, does this make sense to you?
Yes, that's for next version of ACPI but we don't know that yet. IIUC you are trying to implement ACPI port for ARM64 based on ACPIv5.0 through these patches right ? Hence I pushing you to be more explicit through comments as ACPIv5.0 is not clear enough.
I agree. I will resend this patch as part of ARM64 ACPI core patch set (it is still need some time though), please help me to review the code and let's discuss it at that time, is that make sense to you?
Thanks Hanjun
On 2014-02-22 10:21, Hanjun Guo wrote:
On 2014-2-21 20:37, Sudeep Holla wrote:
Hi Hanjun,
(Adding MarcZ for his views on GIC)
On 20/02/14 03:59, Hanjun Guo wrote:
Hi Sudeep,
Thanks for your comments, please refer to the replies below. :)
On 2014年02月19日 22:33, Sudeep Holla wrote:
Hi Hanjun,
On 18/02/14 16:23, Hanjun Guo wrote:
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; }
+static int map_gic_id(struct acpi_subtable_header *entry,
int device_declaration, u32 acpi_id, int *apic_id)+{
- struct acpi_madt_generic_interrupt *gic =
(struct acpi_madt_generic_interrupt *)entry;- if (!(gic->flags & ACPI_MADT_ENABLED))
return -ENODEV;- /* In the GIC interrupt model, logical processors are
* required to have a Processor Device object in the DSDT,* so we should check device_declaration here*/- if (device_declaration && (gic->uid == acpi_id)) {
*apic_id = gic->gic_id;I have mentioned this earlier, it's not clear yet to me how does this work ? It needs more clarity in the form of comment here at-least as the ACPIv5.0 is also not so clear or explicit on how to handle this.
Yes, I noticed your comments and had a reply for that, after a long consideration for this, I would withdraw my previous comments before, please refer to the comments below.
Here you are expecting gic->uid = acpi_id which is fine, while acpi_map_cpuid matches apic_id with cpu_physical_id(which must be MPIDR in ARM{32,64}). The latter imposes restriction that gic->gic_id has to be MPIDR. Does that mean we are imposing restriction on GIC ID to be MPIDR ? If so please document it here and please explain the reason behind that choice.
On x86 and IA64, APIC/SAPIC ID is the hardware id of the logical processor, and UID is just a unique ID to identify the processor in DSDT, it can be any value, and even can be strings defined in ASL if I remember that correctly.
OK, but that's not the case on ARM{32,64}. My main concern here is if we don't make this definitions clear enough, the vendors might produce ACPI tables with whatever suits them and we may end up supporting them. Since we are starting with clean slate, we can avoid getting into such situations. I will be to be more elaborate this time.
I agree.
The GIC ID is referred as the local GIC’s hardware ID in ACPIv5.0. IIUC, since GICC is per-cpu entry, it has to GIC CPU interface ID.
Now how does it differ from MPIDR ? e.g. ARM TC2(multi cluster system) GIC ID MPIDR Comment 0 0x000 CA15_0 1 0x001 CA15_1 2 0x100 CA7_0 3 0x101 CA7_1 4 0x102 CA7_2
Yes, obvious different. I know GIC ID can matche the bit index of the associated processor in the distributor's GICD_ITARGETSR register, and it a clear statement in GICv1/GICv2, my question is that is this consistent in GICv3/v4 too? this will have some impact on the code implementation.
For GICv3/v4, the only way you can match a CPU with its local redistributor is by using the CPU MPIDR. The GIC CPU ID is an implementation choice that may not exist (it doesn't in a distributed implementation), so anything that relies on a GIC CPU ID is broken for GICv3.
M.
On 2014-2-22 19:30, Marc Zyngier wrote:
On 2014-02-22 10:21, Hanjun Guo wrote:
On 2014-2-21 20:37, Sudeep Holla wrote:
Hi Hanjun,
(Adding MarcZ for his views on GIC)
On 20/02/14 03:59, Hanjun Guo wrote:
Hi Sudeep,
Thanks for your comments, please refer to the replies below. :)
On 2014年02月19日 22:33, Sudeep Holla wrote:
Hi Hanjun,
On 18/02/14 16:23, Hanjun Guo wrote:
Get apic id from MADT or _MAT method is not implemented on arm/arm64, and ACPI 5.0 introduces GIC Structure for it, so this patch introduces map_gic_id() to get apic id followed the ACPI 5.0 spec.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org
drivers/acpi/processor_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c index 4dcf776..d316d9b 100644 --- a/drivers/acpi/processor_core.c +++ b/drivers/acpi/processor_core.c @@ -71,6 +71,27 @@ static int map_lsapic_id(struct acpi_subtable_header *entry, return 0; }
+static int map_gic_id(struct acpi_subtable_header *entry,
int device_declaration, u32 acpi_id, int *apic_id)+{
- struct acpi_madt_generic_interrupt *gic =
(struct acpi_madt_generic_interrupt *)entry;- if (!(gic->flags & ACPI_MADT_ENABLED))
return -ENODEV;- /* In the GIC interrupt model, logical processors are
* required to have a Processor Device object in the DSDT,* so we should check device_declaration here*/- if (device_declaration && (gic->uid == acpi_id)) {
*apic_id = gic->gic_id;I have mentioned this earlier, it's not clear yet to me how does this work ? It needs more clarity in the form of comment here at-least as the ACPIv5.0 is also not so clear or explicit on how to handle this.
Yes, I noticed your comments and had a reply for that, after a long consideration for this, I would withdraw my previous comments before, please refer to the comments below.
Here you are expecting gic->uid = acpi_id which is fine, while acpi_map_cpuid matches apic_id with cpu_physical_id(which must be MPIDR in ARM{32,64}). The latter imposes restriction that gic->gic_id has to be MPIDR. Does that mean we are imposing restriction on GIC ID to be MPIDR ? If so please document it here and please explain the reason behind that choice.
On x86 and IA64, APIC/SAPIC ID is the hardware id of the logical processor, and UID is just a unique ID to identify the processor in DSDT, it can be any value, and even can be strings defined in ASL if I remember that correctly.
OK, but that's not the case on ARM{32,64}. My main concern here is if we don't make this definitions clear enough, the vendors might produce ACPI tables with whatever suits them and we may end up supporting them. Since we are starting with clean slate, we can avoid getting into such situations. I will be to be more elaborate this time.
I agree.
The GIC ID is referred as the local GIC’s hardware ID in ACPIv5.0. IIUC, since GICC is per-cpu entry, it has to GIC CPU interface ID.
Now how does it differ from MPIDR ? e.g. ARM TC2(multi cluster system) GIC ID MPIDR Comment 0 0x000 CA15_0 1 0x001 CA15_1 2 0x100 CA7_0 3 0x101 CA7_1 4 0x102 CA7_2
Yes, obvious different. I know GIC ID can matche the bit index of the associated processor in the distributor's GICD_ITARGETSR register, and it a clear statement in GICv1/GICv2, my question is that is this consistent in GICv3/v4 too? this will have some impact on the code implementation.
For GICv3/v4, the only way you can match a CPU with its local redistributor is by using the CPU MPIDR. The GIC CPU ID is an implementation choice that may not exist (it doesn't in a distributed implementation), so anything that relies on a GIC CPU ID is broken for GICv3.
So that's the broken part, it seems that GIC ID is useless both in GICv1/v2 and GICv3/v4 if it represents GIC CPU interface ID, the only reliable one is the MPIDR.
I agree UID should be MPIDR for future usage, so how about this solution:
+static int map_gic_id(struct acpi_subtable_header *entry, + int device_declaration, u32 acpi_id, int *apic_id) +{ + struct acpi_madt_generic_interrupt *gic = + (struct acpi_madt_generic_interrupt *)entry; + + if (!(gic->flags & ACPI_MADT_ENABLED)) + return -ENODEV; + + /* In the GIC interrupt model, logical processors are + * required to have a Processor Device object in the DSDT, + * so we should check device_declaration here + */ + if (device_declaration && (gic->uid == acpi_id)) { /* * this is the special case for ARM/ARM64, gic->uid * should be MPIDR which represents the CPU hardware * id, so use gic->uid instead of gic->gic_id here * to get the physical processor ID. (...) */ + *apic_id = gic->uid; + return 0; + } + + return -EINVAL; +} +
Thanks Hanjun
BAD_MADT_ENTRY() is arch independent and will be used for all archs which parsing MADT table, so move it to linux/acpi.h to simply the code.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- arch/ia64/kernel/acpi.c | 4 ---- arch/x86/kernel/acpi/boot.c | 4 ---- include/linux/acpi.h | 4 ++++ 3 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c index af9d9e4..f7bd074 100644 --- a/arch/ia64/kernel/acpi.c +++ b/arch/ia64/kernel/acpi.c @@ -54,10 +54,6 @@ #include <asm/sal.h> #include <asm/cyclone.h>
-#define BAD_MADT_ENTRY(entry, end) ( \ - (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ - ((struct acpi_subtable_header *)entry)->length < sizeof(*entry)) - #define PREFIX "ACPI: "
unsigned int acpi_cpei_override; diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c index 1dac942..123f9e3 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -53,10 +53,6 @@ EXPORT_SYMBOL(acpi_disabled); # include <asm/proto.h> #endif /* X86 */
-#define BAD_MADT_ENTRY(entry, end) ( \ - (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ - ((struct acpi_subtable_header *)entry)->length < sizeof(*entry)) - #define PREFIX "ACPI: "
int acpi_noirq; /* skip ACPI IRQ initialization */ diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 1151a1d..6a15ddd 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -108,6 +108,10 @@ static inline void acpi_initrd_override(void *data, size_t size) } #endif
+#define BAD_MADT_ENTRY(entry, end) ( \ + (!entry) || (unsigned long)entry + sizeof(*entry) > end || \ + ((struct acpi_subtable_header *)entry)->length < sizeof(*entry)) + char * __acpi_map_table (unsigned long phys_addr, unsigned long size); void __acpi_unmap_table(char *map, unsigned long size); int early_acpi_boot_init(void);
This patch just do some clean up to replace printk with pr_*, no functional change.
Signed-off-by: Hanjun Guo hanjun.guo@linaro.org --- drivers/acpi/tables.c | 124 +++++++++++++++++++++---------------------------- 1 file changed, 54 insertions(+), 70 deletions(-)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index 5837f85..fd95a76 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -55,10 +55,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_apic *p = (struct acpi_madt_local_apic *)header; - printk(KERN_INFO PREFIX - "LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n", - p->processor_id, p->id, - (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled"); + pr_info(PREFIX "LAPIC (acpi_id[0x%02x] lapic_id[0x%02x] %s)\n", + p->processor_id, p->id, + (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled"); } break;
@@ -66,11 +65,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_x2apic *p = (struct acpi_madt_local_x2apic *)header; - printk(KERN_INFO PREFIX - "X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n", - p->local_apic_id, p->uid, - (p->lapic_flags & ACPI_MADT_ENABLED) ? - "enabled" : "disabled"); + pr_info(PREFIX "X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n", + p->local_apic_id, p->uid, + (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled"); } break;
@@ -78,9 +75,8 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_io_apic *p = (struct acpi_madt_io_apic *)header; - printk(KERN_INFO PREFIX - "IOAPIC (id[0x%02x] address[0x%08x] gsi_base[%d])\n", - p->id, p->address, p->global_irq_base); + pr_info(PREFIX "IOAPIC (id[0x%02x] address[0x%08x] gsi_base[%d])\n", + p->id, p->address, p->global_irq_base); } break;
@@ -88,18 +84,15 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_interrupt_override *p = (struct acpi_madt_interrupt_override *)header; - printk(KERN_INFO PREFIX - "INT_SRC_OVR (bus %d bus_irq %d global_irq %d %s %s)\n", - p->bus, p->source_irq, p->global_irq, - mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK], - mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2]); + pr_info(PREFIX "INT_SRC_OVR (bus %d bus_irq %d global_irq %d %s %s)\n", + p->bus, p->source_irq, p->global_irq, + mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK], + mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2]); if (p->inti_flags & ~(ACPI_MADT_POLARITY_MASK | ACPI_MADT_TRIGGER_MASK)) - printk(KERN_INFO PREFIX - "INT_SRC_OVR unexpected reserved flags: 0x%x\n", - p->inti_flags & + pr_info(PREFIX "INT_SRC_OVR unexpected reserved flags: 0x%x\n", + p->inti_flags & ~(ACPI_MADT_POLARITY_MASK | ACPI_MADT_TRIGGER_MASK)); - } break;
@@ -107,11 +100,10 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_nmi_source *p = (struct acpi_madt_nmi_source *)header; - printk(KERN_INFO PREFIX - "NMI_SRC (%s %s global_irq %d)\n", - mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK], - mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2], - p->global_irq); + pr_info(PREFIX "NMI_SRC (%s %s global_irq %d)\n", + mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK], + mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2], + p->global_irq); } break;
@@ -119,12 +111,11 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_apic_nmi *p = (struct acpi_madt_local_apic_nmi *)header; - printk(KERN_INFO PREFIX - "LAPIC_NMI (acpi_id[0x%02x] %s %s lint[0x%x])\n", - p->processor_id, - mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK ], - mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2], - p->lint); + pr_info(PREFIX "LAPIC_NMI (acpi_id[0x%02x] %s %s lint[0x%x])\n", + p->processor_id, + mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK ], + mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2], + p->lint); } break;
@@ -137,12 +128,11 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) polarity = p->inti_flags & ACPI_MADT_POLARITY_MASK; trigger = (p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2;
- printk(KERN_INFO PREFIX - "X2APIC_NMI (uid[0x%02x] %s %s lint[0x%x])\n", - p->uid, - mps_inti_flags_polarity[polarity], - mps_inti_flags_trigger[trigger], - p->lint); + pr_info(PREFIX "X2APIC_NMI (uid[0x%02x] %s %s lint[0x%x])\n", + p->uid, + mps_inti_flags_polarity[polarity], + mps_inti_flags_trigger[trigger], + p->lint); } break;
@@ -150,9 +140,8 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_apic_override *p = (struct acpi_madt_local_apic_override *)header; - printk(KERN_INFO PREFIX - "LAPIC_ADDR_OVR (address[%p])\n", - (void *)(unsigned long)p->address); + pr_info(PREFIX "LAPIC_ADDR_OVR (address[%p])\n", + (void *)(unsigned long)p->address); } break;
@@ -160,10 +149,9 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_io_sapic *p = (struct acpi_madt_io_sapic *)header; - printk(KERN_INFO PREFIX - "IOSAPIC (id[0x%x] address[%p] gsi_base[%d])\n", - p->id, (void *)(unsigned long)p->address, - p->global_irq_base); + pr_info(PREFIX "IOSAPIC (id[0x%x] address[%p] gsi_base[%d])\n", + p->id, (void *)(unsigned long)p->address, + p->global_irq_base); } break;
@@ -171,10 +159,10 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_local_sapic *p = (struct acpi_madt_local_sapic *)header; - printk(KERN_INFO PREFIX - "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n", - p->processor_id, p->id, p->eid, - (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled"); + pr_info(PREFIX + "LSAPIC (acpi_id[0x%02x] lsapic_id[0x%02x] lsapic_eid[0x%02x] %s)\n", + p->processor_id, p->id, p->eid, + (p->lapic_flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled"); } break;
@@ -182,19 +170,18 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) { struct acpi_madt_interrupt_source *p = (struct acpi_madt_interrupt_source *)header; - printk(KERN_INFO PREFIX - "PLAT_INT_SRC (%s %s type[0x%x] id[0x%04x] eid[0x%x] iosapic_vector[0x%x] global_irq[0x%x]\n", - mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK], - mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2], - p->type, p->id, p->eid, p->io_sapic_vector, - p->global_irq); + pr_info(PREFIX + "PLAT_INT_SRC (%s %s type[0x%x] id[0x%04x] eid[0x%x] iosapic_vector[0x%x] global_irq[0x%x]\n", + mps_inti_flags_polarity[p->inti_flags & ACPI_MADT_POLARITY_MASK], + mps_inti_flags_trigger[(p->inti_flags & ACPI_MADT_TRIGGER_MASK) >> 2], + p->type, p->id, p->eid, p->io_sapic_vector, + p->global_irq); } break;
default: - printk(KERN_WARNING PREFIX - "Found unsupported MADT entry (type = 0x%x)\n", - header->type); + pr_warn(PREFIX "Found unsupported MADT entry (type = 0x%x)\n", + header->type); break; } } @@ -225,7 +212,7 @@ acpi_table_parse_entries(char *id, acpi_get_table_with_size(id, 0, &table_header, &tbl_size);
if (!table_header) { - printk(KERN_WARNING PREFIX "%4.4s not present\n", id); + pr_warn(PREFIX "%4.4s not present\n", id); return -ENODEV; }
@@ -256,8 +243,8 @@ acpi_table_parse_entries(char *id, ((unsigned long)entry + entry->length); } if (max_entries && count > max_entries) { - printk(KERN_WARNING PREFIX "[%4.4s:0x%02x] ignored %i entries of " - "%i found\n", id, entry_id, count - max_entries, count); + pr_warn(PREFIX "[%4.4s:0x%02x] ignored %i entries of %i found\n", + id, entry_id, count - max_entries, count); }
early_acpi_os_unmap_memory((char *)table_header, tbl_size); @@ -322,13 +309,11 @@ static void __init check_multiple_madt(void)
acpi_get_table_with_size(ACPI_SIG_MADT, 2, &table, &tbl_size); if (table) { - printk(KERN_WARNING PREFIX - "BIOS bug: multiple APIC/MADT found," - " using %d\n", acpi_apic_instance); - printk(KERN_WARNING PREFIX - "If "acpi_apic_instance=%d" works better, " - "notify linux-acpi@vger.kernel.org\n", - acpi_apic_instance ? 0 : 2); + pr_warn(PREFIX "BIOS bug: multiple APIC/MADT found, using %d\n", + acpi_apic_instance); + pr_warn(PREFIX "If "acpi_apic_instance=%d" works better, " + "notify linux-acpi@vger.kernel.org\n", + acpi_apic_instance ? 0 : 2); early_acpi_os_unmap_memory(table, tbl_size);
} else @@ -365,8 +350,7 @@ static int __init acpi_parse_apic_instance(char *str)
acpi_apic_instance = simple_strtoul(str, NULL, 0);
- printk(KERN_NOTICE PREFIX "Shall use APIC/MADT table %d\n", - acpi_apic_instance); + pr_notice(PREFIX "Shall use APIC/MADT table %d\n", acpi_apic_instance);
return 0; }
-
Andrea Gallo -
Catalin Marinas -
Graeme Gregory -
Hanjun Guo -
Marc Zyngier -
Rafael J. Wysocki -
Sudeep Holla