[U-Boot] [PATCHv6 5/6] ARMv8/PSCI: Fixup the device tree for PSCI

[Zhiqiang Hou <zhiqiang.hou@nxp.com>]

Hi York,

Thanks for your comments!

> -----Original Message-----
> From: york sun
> Sent: 2016?6?23? 0:52
> To: Zhiqiang Hou <zhiqiang.hou@nxp.com>; u-boot at lists.denx.de;
> albert.u.boot at aribaud.net; scottwood at freescale.com;
> Mingkai.hu at freescale.com; yorksun at freescale.com; leoli at freescale.com;
> prabhakar at freescale.com; bhupesh.sharma at freescale.com
> Subject: Re: [PATCHv6 5/6] ARMv8/PSCI: Fixup the device tree for PSCI
> 
> On 06/21/2016 08:42 PM, Zhiqiang Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> >
> > Set the enable-method in the cpu node to PSCI, and create device node
> > for PSCI, when PSCI was enabled.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
> > ---
> > V6:
> >   - Removed PSCI version 0.1 support.
> >
> > V5:
> >   - Moved the weak func sec_firmware_support_psci_version to sec_firmware.c.
> >   - Correct the PSCI version value in switch-case. The right version format is
> marjor[31:16]:minor[15:0].
> >
> >   arch/arm/cpu/armv8/Makefile |   1 +
> >   arch/arm/cpu/armv8/cpu-dt.c | 122
> ++++++++++++++++++++++++++++++++++++++++++++
> >   arch/arm/lib/bootm-fdt.c    |   2 +-
> >   3 files changed, 124 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/arm/cpu/armv8/cpu-dt.c
> >
> > diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> > index ee9e009..33e6db0 100644
> > --- a/arch/arm/cpu/armv8/Makefile
> > +++ b/arch/arm/cpu/armv8/Makefile
> > @@ -15,6 +15,7 @@ obj-y	+= cache.o
> >   obj-y	+= tlb.o
> >   obj-y	+= transition.o
> >   obj-y	+= fwcall.o
> > +obj-y	+= cpu-dt.o
> >   obj-$(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) += sec_firmware.o
> > sec_firmware_asm.o
> >
> >   obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/ diff --git
> > a/arch/arm/cpu/armv8/cpu-dt.c b/arch/arm/cpu/armv8/cpu-dt.c new file
> > mode 100644 index 0000000..6b9aa77
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv8/cpu-dt.c
> > @@ -0,0 +1,122 @@
> > +/*
> > + * Copyright 2016 NXP Semiconductor, Inc.
> > + *
> > + * SPDX-License-Identifier:	GPL-2.0+
> > + */
> > +
> > +#include <common.h>
> > +#include <libfdt.h>
> > +#include <fdt_support.h>
> > +#include <linux/sizes.h>
> > +#include <linux/kernel.h>
> > +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT #include
> > +<asm/armv8/sec_firmware.h> #endif
> > +
> > +#ifdef CONFIG_MP
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +#if defined(CONFIG_ARMV8_PSCI)
> > +static int cpu_update_dt_psci(void *fdt) {
> > +	int nodeoff;
> > +	unsigned int psci_ver;
> > +	char *psci_compt;
> > +	int tmp;
> > +
> > +	nodeoff = fdt_path_offset(fdt, "/cpus");
> > +	if (nodeoff < 0) {
> > +		printf("couldn't find /cpus\n");
> > +		return nodeoff;
> > +	}
> > +
> > +	/* add 'enable-method = "psci"' to each cpu node */
> > +	for (tmp = fdt_first_subnode(fdt, nodeoff);
> > +	     tmp >= 0;
> > +	     tmp = fdt_next_subnode(fdt, tmp)) {
> > +		const struct fdt_property *prop;
> > +		int len;
> > +
> > +		prop = fdt_get_property(fdt, tmp, "device_type", &len);
> > +		if (!prop)
> > +			continue;
> > +		if (len < 4)
> > +			continue;
> > +		if (strcmp(prop->data, "cpu"))
> > +			continue;
> > +
> > +		/*
> > +		 * Not checking rv here, our approach is to skip over errors in
> > +		 * individual cpu nodes, hopefully some of the nodes are
> > +		 * processed correctly and those will boot
> > +		 */
> > +		fdt_setprop_string(fdt, tmp, "enable-method", "psci");
> > +	}
> > +
> > +	/*
> > +	 * The PSCI node might be called "/psci" or might be called something
> > +	 * else but contain either of the compatible strings
> > +	 * "arm,psci"/"arm,psci-0.2"
> > +	 */
> > +	nodeoff = fdt_path_offset(fdt, "/psci");
> > +	if (nodeoff >= 0)
> > +		goto init_psci_node;
> > +
> > +	nodeoff = fdt_node_offset_by_compatible(fdt, -1, "arm,psci");
> > +	if (nodeoff >= 0)
> > +		goto init_psci_node;
> > +
> > +	nodeoff = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-0.2");
> > +	if (nodeoff >= 0)
> > +		goto init_psci_node;
> > +
> > +	nodeoff = fdt_node_offset_by_compatible(fdt, -1, "arm,psci-1.0");
> > +	if (nodeoff >= 0)
> > +		goto init_psci_node;
> > +
> > +	nodeoff = fdt_path_offset(fdt, "/");
> > +	if (nodeoff < 0)
> > +		return nodeoff;
> > +
> > +	nodeoff = fdt_add_subnode(fdt, nodeoff, "psci");
> > +	if (nodeoff < 0)
> > +		return nodeoff;
> > +
> > +init_psci_node:
> > +#ifdef CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT
> > +	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;
> > +	default:
> > +		psci_compt = "arm,psci-0.2";
> > +		break;
> > +	}
> > +
> > +	tmp = fdt_setprop_string(fdt, nodeoff, "compatible", psci_compt);
> > +	if (tmp)
> > +		return tmp;
> > +
> > +	tmp = fdt_setprop_string(fdt, nodeoff, "method", "smc");
> > +	if (tmp)
> > +		return tmp;
> > +
> > +	return 0;
> > +}
> > +#endif
> > +#endif
> > +
> > +int psci_update_dt(void *fdt)
> > +{
> > +#ifdef CONFIG_MP
> > +#if defined(CONFIG_ARMV8_PSCI)
> > +	cpu_update_dt_psci(fdt);
> > +#endif
> > +#endif
> > +	return 0;
> > +}
> > diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index
> > 7677358..c642ff8 100644
> > --- a/arch/arm/lib/bootm-fdt.c
> > +++ b/arch/arm/lib/bootm-fdt.c
> > @@ -42,7 +42,7 @@ int arch_fixup_fdt(void *blob)
> >   	}
> >
> >   	ret = fdt_fixup_memory_banks(blob, start, size,
> > CONFIG_NR_DRAM_BANKS); -#ifdef CONFIG_ARMV7_NONSEC
> > +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV8_PSCI)
> >   	if (ret)
> >   		return ret;
> >
> >
> 
> As far as CONFIG_ARMV8_PSCI is defined, psci_update_dt(blob) is called.
> Regardless if ppa is running, the psci node is always created and cpu boot method
> is always updated with "psci". Then previous patch (4th in this set) detects psci
> version. If it is 0xffffffff, psci node is removed, and cpu boot method is updated
> again with spin-table. Do I understand your flow correctly?
> 
> If my understand is correct, I suggest to add a check of psci_version before calling
> cpu_update_dt_psci(fdt). It avoids unnecessary setting and helps to understand the
> flow.

Your understand is correct, and will take your suggestion.

Thanks,
Zhiqiang