Hi
-----Original Message----- From: linaro-acpi-bounces@lists.linaro.org [mailto:linaro-acpi- bounces@lists.linaro.org] On Behalf Of Graeme Gregory Sent: 13 March 2014 08:46 To: Ashwin Chaugule Cc: linaro-acpi@lists.linaro.org Subject: Re: [Linaro-acpi] [PATCH] Restructure code to get static PSCI function ID's
On Thu, Mar 13, 2014 at 12:27:29AM -0400, Ashwin Chaugule wrote:
The PSCI Function ID's are well defined for ARM32 and ARM64. See PSCI spec v0.2. The initial version of the PSCI ACPI spec will only have provisions for discovering PSCI and the conduit. (i.e. SMC or HVC)
Reformat code in preparation for patches that discover PSCI and the conduit via FACP flags.
Signed-off-by: Ashwin Chaugule ashwin.chaugule@linaro.org
arch/arm/include/asm/psci.h | 13 ++++++++++++ arch/arm64/include/asm/psci.h | 13 ++++++++++++ arch/arm64/kernel/psci.c | 47 +++++++++++++++++-------------------------- 3 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h index c4ae171..901de0f 100644 --- a/arch/arm/include/asm/psci.h +++ b/arch/arm/include/asm/psci.h @@ -34,6 +34,19 @@ struct psci_operations { extern struct psci_operations psci_ops; extern struct smp_operations psci_smp_ops;
+/* PSCI Function ID's for ARM32 as per PSCI spec v0.2 */
+#define PSCI_ID_VERSION 0x84000000 +#define PSCI_ID_CPU_SUSPEND 0x84000001 +#define PSCI_ID_CPU_OFF 0x84000002 +#define PSCI_ID_CPU_ON 0x84000003 +#define PSCI_ID_AFFINITY_INFO 0x84000004 +#define PSCI_ID_CPU_MIGRATE 0x84000005 +#define PSCI_ID_MIGRATE_INFO_TYPE 0x84000006 +#define PSCI_ID_MIGRATE_INFO_UP_CPU 0x84000007 +#define PSCI_ID_SYSTEM_OFF 0x84000008 +#define PSCI_ID_SYSTEM_RESET 0x84000009
I get the feeling this is actually generic stuff and shows an error in DTB bingings in that the bindings themselves should not define these states.
DT bindings have caused quite a lot of debate and I am not even sure that's closed. There we allowed the option to override. However I think what would make sense from a spec POV would be that there are defaults that may be overridden. Ccing Mark for opinion question on this whole thing.
#ifdef CONFIG_ARM_PSCI void psci_init(void); bool psci_smp_available(void); diff --git a/arch/arm64/include/asm/psci.h b/arch/arm64/include/asm/psci.h index e5312ea..3bde75f 100644 --- a/arch/arm64/include/asm/psci.h +++ b/arch/arm64/include/asm/psci.h @@ -16,4 +16,17 @@
int psci_init(void);
+/* PSCI Function ID's for ARM64 as per PSCI spec v0.2 */
+#define PSCI_ID_VERSION 0x84000000 +#define PSCI_ID_CPU_SUSPEND 0xc4000001 +#define PSCI_ID_CPU_OFF 0x84000002 +#define PSCI_ID_CPU_ON 0xc4000003 +#define PSCI_ID_AFFINITY_INFO 0xc4000004 +#define PSCI_ID_CPU_MIGRATE 0xc4000005 +#define PSCI_ID_MIGRATE_INFO_TYPE 0x84000006 +#define PSCI_ID_MIGRATE_INFO_UP_CPU 0xc4000007 +#define PSCI_ID_SYSTEM_OFF 0x84000008 +#define PSCI_ID_SYSTEM_RESET 0x84000009
Same here, it looks odd that ACPI uses these value but DTB does not, should this patch be broken into a few core cleanups first then apply the ACPI stuff? The core cleanups of DTB can go upstream immediately.
#endif /* __ASM_PSCI_H */ diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index abaf7db..0cad34a 100644 --- a/arch/arm64/kernel/psci.c +++ b/arch/arm64/kernel/psci.c @@ -176,17 +176,12 @@ static const struct of_device_id psci_of_match[]
__initconst = {
{},};
-static int __init psci_of_init(void) +static int __init psci_of_init(struct device_node *np) {
struct device_node *np; const char *method; u32 id; int err = 0;
np = of_find_matching_node(NULL, psci_of_match);
if (!np)
return -ENODEV;pr_info("probing function IDs from device-tree\n");
if (of_property_read_string(np, "method", &method)) {
@@ -230,34 +225,27 @@ out_put_node: return err; }
-#if defined(CONFIG_ACPI) && defined(CONFIG_ARCH_VEXPRESS) +#if defined(CONFIG_ACPI) static int __init psci_acpi_init(void) {
- /*
- FIXME: IDs should be probed from ACPI tables. Due to lack of PSCI
- support in ACPI 5.0, function IDs are hard coded.
- IDs are taken directly from fvp-base-gicv2-psci.dts as we currently
- support FVP Base model. Also, we do not have ID register unique
- for FVP model, thus we make code dependent on
CONFIG_ARCH_VEXPRESS
- too.
- /*TODO: Get conduit method from FACP flags, after
*/
- discovering PSCI. Ret -ENODEV otherwise.
-#define PSCI_FVP_ID_SUSPEND 0xc4000001 -#define PSCI_FVP_ID_OFF 0x84000002 -#define PSCI_FVP_ID_ON 0xc4000003
- pr_info("Get hard coded PSCI function IDs\n");
- pr_info("Get PSCI method.\n"); invoke_psci_fn = __invoke_psci_fn_smc;
- psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_FVP_ID_SUSPEND;
- pr_info("Initializing PSCI Function Id's.\n");
- psci_function_id[PSCI_FN_CPU_SUSPEND] = PSCI_ID_CPU_SUSPEND; psci_ops.cpu_suspend = psci_cpu_suspend;
- psci_function_id[PSCI_FN_CPU_OFF] = PSCI_FVP_ID_OFF;
- psci_function_id[PSCI_FN_CPU_OFF] = PSCI_ID_CPU_OFF; psci_ops.cpu_off = psci_cpu_off;
- psci_function_id[PSCI_FN_CPU_ON] = PSCI_FVP_ID_ON;
psci_function_id[PSCI_FN_CPU_ON] = PSCI_ID_CPU_ON; psci_ops.cpu_on = psci_cpu_on;
psci_function_id[PSCI_FN_MIGRATE] = PSCI_ID_CPU_MIGRATE;
psci_ops.migrate = psci_migrate; return 0;
Again I wonder if these are actually defined by spec why these values are not just const in the code. I think we are working around incorrect DTB implementation here.
} #else @@ -269,13 +257,14 @@ static int __init psci_acpi_init(void)
int __init psci_init(void) {
- int status;
- status = psci_of_init();
- if (!status)
return status;
- struct device_node *np;
- return psci_acpi_init();
- /* Check if there is a valid DT entry first. */
- np = of_find_matching_node(NULL, psci_of_match);
- if (np)
return psci_of_init(np);- else
return psci_acpi_init();}
#ifdef CONFIG_SMP
It does look to me like the DTB implementation is wrong so we are working around that in ACPI. I guess some attempt should be made to fix the DTB version first.
Graeme
Linaro-acpi mailing list Linaro-acpi@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-acpi
-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782