[U-Boot] [PATCH 2/3] arm/PSCI: Fixed the backward compatiblity issue

[york sun]

On 07/29/2016 03:37 AM, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
>
> Appended the compatible strings of old version PSCI to the latest version
> supported. And there are some psci functions' property must be added to DT
> only for psci version 0.1, such as 'cpu_on' 'cpu_off' etc.
>
> Note:
> The PSCI version 0.1 isn't supported by ARMv8 Secure Firmware Framework.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> ---

You missed version number and change log.

>  arch/arm/include/asm/psci.h |  3 +++
>  arch/arm/lib/psci-dt.c      | 61 ++++++++++++++++++++++++++-------------------
>  2 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm/include/asm/psci.h b/arch/arm/include/asm/psci.h
> index 8aefaa7..5b8ce4d 100644
> --- a/arch/arm/include/asm/psci.h
> +++ b/arch/arm/include/asm/psci.h
> @@ -18,6 +18,9 @@
>  #ifndef __ARM_PSCI_H__
>  #define __ARM_PSCI_H__
>
> +#define ARM_PSCI_VER_1_0		(0x00010000)
> +#define ARM_PSCI_VER_0_2		(0x00000002)
> +
>  /* PSCI 0.1 interface */
>  #define ARM_PSCI_FN_BASE		0x95c1ba5e
>  #define ARM_PSCI_FN(n)			(ARM_PSCI_FN_BASE + (n))
> diff --git a/arch/arm/lib/psci-dt.c b/arch/arm/lib/psci-dt.c
> index bcd92e7..af49c24 100644
> --- a/arch/arm/lib/psci-dt.c
> +++ b/arch/arm/lib/psci-dt.c
> @@ -19,7 +19,6 @@ int fdt_psci(void *fdt)
>  #if defined(CONFIG_ARMV8_PSCI) || defined(CONFIG_ARMV7_PSCI)
>  	int nodeoff;
>  	unsigned int psci_ver = 0;
> -	char *psci_compt;
>  	int tmp;
>
>  	nodeoff = fdt_path_offset(fdt, "/cpus");
> @@ -68,39 +67,49 @@ init_psci_node:
>  	psci_ver = sec_firmware_support_psci_version();
>  #endif
>  	switch (psci_ver) {
> -	case 0x00010000:
> -		psci_compt = "arm,psci-1.0";
> -		break;
> -	case 0x00000002:
> -		psci_compt = "arm,psci-0.2";
> -		break;
> +	case ARM_PSCI_VER_1_0:
> +		tmp = fdt_setprop_string(fdt, nodeoff,
> +				"compatible", "arm,psci-1.0");
> +		if (tmp)
> +			return tmp;

Add a comment "fall through".

> +	case ARM_PSCI_VER_0_2:
> +		tmp = fdt_appendprop_string(fdt, nodeoff,
> +				"compatible", "arm,psci-0.2");
> +		if (tmp)
> +			return tmp;

Add a comment "fall through".

>  	default:
> -		psci_compt = "arm,psci";
> +	/*
> +	 * The Secure firmware framework isn't able to support PSCI version 0.1.
> +	 */
> +#ifndef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
> +		tmp = fdt_appendprop_string(fdt, nodeoff,
> +				"compatible", "arm,psci");
> +		if (tmp)
> +			return tmp;
> +		tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_suspend",
> +				ARM_PSCI_FN_CPU_SUSPEND);
> +		if (tmp)
> +			return tmp;
> +		tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_off",
> +				ARM_PSCI_FN_CPU_OFF);
> +		if (tmp)
> +			return tmp;
> +		tmp = fdt_setprop_u32(fdt, nodeoff, "cpu_on",
> +				ARM_PSCI_FN_CPU_ON);
> +		if (tmp)
> +			return tmp;
> +		tmp = fdt_setprop_u32(fdt, nodeoff, "migrate",
> +				ARM_PSCI_FN_MIGRATE);
> +		if (tmp)
> +			return tmp;

This may not be a real concern, but I am curious what would happen if 
one of the above fdt_setprop_u32 fails? Would it be better to set 
"arm,psci" last?

York