[U-Boot] [RFC] armv8: layerscape: Add support of u-boot device tree fix-up
There is a requirement of u-boot dts fix-up before it is being consumed by u-boot.
NXP's SoC LS2085A, LS2080A and LS2088A are almost same except variation in ARM core where LS2085A/LS2080A has A57 and LS2088A has A72. These SoCs will be placed on common LS2085ARDB platform.
So instead of maintaining defferent device tree per SoC, fix-up dts before being used by u-boot.
Signed-off-by: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com --- arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 53 ++++++++++++++++++++++++++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 4 ++ include/common.h | 2 + include/configs/ls2080a_common.h | 1 + lib/fdtdec.c | 10 +++++ 5 files changed, 70 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c index 4e4861d..cbdeef3 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c @@ -5,11 +5,13 @@ */
#include <common.h> +#include <asm/io.h> #include <libfdt.h> #include <fdt_support.h> #include <phy.h> #ifdef CONFIG_FSL_LSCH3 #include <asm/arch/fdt.h> +#include <asm/arch/soc.h> #endif #ifdef CONFIG_FSL_ESDHC #include <fsl_esdhc.h> @@ -18,6 +20,8 @@ #include <asm/arch/mp.h> #endif
+DECLARE_GLOBAL_DATA_PTR; + int fdt_fixup_phy_connection(void *blob, int offset, phy_interface_t phyc) { return fdt_setprop_string(blob, offset, "phy-connection-type", @@ -205,3 +209,52 @@ void ft_cpu_setup(void *blob, bd_t *bd) fdt_fixup_smmu(blob); #endif } + +void ft_early_fixup_cpu(void *blob) +{ + int off; + u32 svr, ver; + struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR); + + off = fdt_path_offset(blob, "/cpus"); + if (off < 0) { + puts("couldn't find /cpus node\n"); + return; + } + + off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu", 4); + svr = gur_in32(&gur->svr); + ver = SVR_SOC_VER(svr); + + while (off != -FDT_ERR_NOTFOUND) { + switch(ver) { + case SVR_LS2080: + case SVR_LS2085: + case SVR_LS2045: + case SVR_LS2040: + fdt_setprop_string(blob, off, "compatible", + "arm,cortex-a57"); + break; + case SVR_LS2088: + case SVR_LS2048: + case SVR_LS2084: + case SVR_LS2028: + fdt_setprop_string(blob, off, "compatible", + "arm,cortex-a72"); + break; + } + + off = fdt_node_offset_by_prop_value(blob, off, "device_type", + "cpu", 4); + } +} + +void ft_early_cpu_setup(void **blob) +{ + fdt_move(*blob, (void *)CONFIG_SYS_DTS_ADDR, fdt_totalsize(blob)); + + *blob = (void *)CONFIG_SYS_DTS_ADDR; + + ft_early_fixup_cpu((void *) *blob); + +} diff --git a/arch/arm/include/asm/arch-fsl-layerscape/soc.h b/arch/arm/include/asm/arch-fsl-layerscape/soc.h index ea78e15..a6cff40 100644 --- a/arch/arm/include/asm/arch-fsl-layerscape/soc.h +++ b/arch/arm/include/asm/arch-fsl-layerscape/soc.h @@ -46,6 +46,10 @@ struct cpu_type { #define SVR_LS2080 0x870110 #define SVR_LS2085 0x870100 #define SVR_LS2040 0x870130 +#define SVR_LS2088 0x870901 +#define SVR_LS2048 0x870921 +#define SVR_LS2084 0x870911 +#define SVR_LS2028 0x870922
#define SVR_MAJ(svr) (((svr) >> 4) & 0xf) #define SVR_MIN(svr) (((svr) >> 0) & 0xf) diff --git a/include/common.h b/include/common.h index 1563d64..6dc8a7f 100644 --- a/include/common.h +++ b/include/common.h @@ -603,6 +603,8 @@ void ft_pci_setup(void *blob, bd_t *bd); #endif #endif
+void ft_early_cpu_setup(void **); + void smp_set_core_boot_addr(unsigned long addr, int corenr); void smp_kick_all_cpus(void);
diff --git a/include/configs/ls2080a_common.h b/include/configs/ls2080a_common.h index def0a6f..aa5ace9 100644 --- a/include/configs/ls2080a_common.h +++ b/include/configs/ls2080a_common.h @@ -24,6 +24,7 @@
/* Link Definitions */ #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_FSL_OCRAM_BASE + 0xfff0) +#define CONFIG_SYS_DTS_ADDR (CONFIG_SYS_FSL_OCRAM_BASE + 0xfff0)
/* We need architecture specific misc initializations */ #define CONFIG_ARCH_MISC_INIT diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1b1ca02..fc200cf 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -76,6 +76,14 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(COMPAT_INTEL_IVYBRIDGE_FSP, "intel,ivybridge-fsp"), };
+void __ft_early_cpu_setup(void **blob) +{ + return; +} +void ft_early_cpu_setup(void **blob) + __attribute__((weak, alias("__ft_early_cpu_setup"))); + + const char *fdtdec_get_compatible(enum fdt_compat_id id) { /* We allow reading of the 'unknown' ID for testing purposes */ @@ -605,6 +613,8 @@ int fdtdec_prepare_fdt(void) #endif return -1; } + + ft_early_cpu_setup((void *)&gd->fdt_blob); return 0; }
On Mon, 2016-02-22 at 16:05 +0530, Prabhakar Kushwaha wrote:
There is a requirement of u-boot dts fix-up before it is being consumed by u-boot.
You might want to explain the reason *why* we have this requirement -- that the board takes a socketed SoC, and we don't want to have to reflash the board if one SoC is unplugged and another plugged in.
NXP's SoC LS2085A, LS2080A and LS2088A are almost same except variation in ARM core where LS2085A/LS2080A has A57 and LS2088A has A72. These SoCs will be placed on common LS2085ARDB platform.
So instead of maintaining defferent device tree per SoC, fix-up dts before being used by u-boot.
Signed-off-by: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com
And what happens when the next socketed board can support chips with device trees that are significantly more different? There should be a mechanism for having multiple device trees present, and choosing one based on platform code. That would also avoid any problems of trying to modify a device tree before relocation.
arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 53 ++++++++++++++++++++++++++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 4 ++ include/common.h | 2 + include/configs/ls2080a_common.h | 1 + lib/fdtdec.c | 10 +++++ 5 files changed, 70 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c index 4e4861d..cbdeef3 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c @@ -5,11 +5,13 @@ */
#include <common.h> +#include <asm/io.h> #include <libfdt.h> #include <fdt_support.h> #include <phy.h> #ifdef CONFIG_FSL_LSCH3 #include <asm/arch/fdt.h> +#include <asm/arch/soc.h> #endif #ifdef CONFIG_FSL_ESDHC #include <fsl_esdhc.h> @@ -18,6 +20,8 @@ #include <asm/arch/mp.h> #endif
+DECLARE_GLOBAL_DATA_PTR;
int fdt_fixup_phy_connection(void *blob, int offset, phy_interface_t phyc) { return fdt_setprop_string(blob, offset, "phy-connection-type", @@ -205,3 +209,52 @@ void ft_cpu_setup(void *blob, bd_t *bd) fdt_fixup_smmu(blob); #endif }
+void ft_early_fixup_cpu(void *blob) +{
- int off;
- u32 svr, ver;
- struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
- off = fdt_path_offset(blob, "/cpus");
- if (off < 0) {
puts("couldn't find /cpus node\n");return;- }
- off = fdt_node_offset_by_prop_value(blob, -1, "device_type", "cpu",
4);
- svr = gur_in32(&gur->svr);
- ver = SVR_SOC_VER(svr);
- while (off != -FDT_ERR_NOTFOUND) {
switch(ver) {case SVR_LS2080:case SVR_LS2085:case SVR_LS2045:case SVR_LS2040:fdt_setprop_string(blob, off, "compatible","arm,cortex-a57");break;case SVR_LS2088:case SVR_LS2048:case SVR_LS2084:case SVR_LS2028:fdt_setprop_string(blob, off, "compatible","arm,cortex-a72");break;}off = fdt_node_offset_by_prop_value(blob, off,"device_type",
"cpu", 4);- }
+}
+void ft_early_cpu_setup(void **blob) +{
- fdt_move(*blob, (void *)CONFIG_SYS_DTS_ADDR, fdt_totalsize(blob));
- *blob = (void *)CONFIG_SYS_DTS_ADDR;
- ft_early_fixup_cpu((void *) *blob);
+}
This is hard to follow. Eliminate unnecessary casts, and s/DTS/DTB/ -- we're not dealing with the source code here. Why do you need to do *blob instead of just referencing gd directly?
diff --git a/include/common.h b/include/common.h index 1563d64..6dc8a7f 100644 --- a/include/common.h +++ b/include/common.h @@ -603,6 +603,8 @@ void ft_pci_setup(void *blob, bd_t *bd); #endif #endif
+void ft_early_cpu_setup(void **);
Arguments should have names.
diff --git a/include/configs/ls2080a_common.h b/include/configs/ls2080a_common.h index def0a6f..aa5ace9 100644 --- a/include/configs/ls2080a_common.h +++ b/include/configs/ls2080a_common.h @@ -24,6 +24,7 @@
/* Link Definitions */ #define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_FSL_OCRAM_BASE + 0xfff0) +#define CONFIG_SYS_DTS_ADDR (CONFIG_SYS_FSL_OCRAM_BASE + 0xfff0)
Why 0xfff0, and not, say, 0x10000 (or rather, why is INIT_SP_ADDR 0xfff0 and not 0x10000 if there is no need to reserve some space above the initial SP addr (e.g. to indicate the end of a traceback))? Is there anywhere that documents how various pieces of OCRAM are used?
BTW, arch/arm/include/asm/arch-fsl-layerscape/config.h says OCRAM 2 MiB but the RM says it's 128 KiB.
Where do you check that the device tree fits in OCRAM? What about when SPL is occupying OCRAM? Does the device tree get used with SPL (I don't think we were using FDT control at all the last time I worked with SPL on these chips)?
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1b1ca02..fc200cf 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -76,6 +76,14 @@ static const char * const compat_names[COMPAT_COUNT] = { COMPAT(COMPAT_INTEL_IVYBRIDGE_FSP, "intel,ivybridge-fsp"), };
+void __ft_early_cpu_setup(void **blob) +{
- return;
+} +void ft_early_cpu_setup(void **blob)
- __attribute__((weak, alias("__ft_early_cpu_setup")));
Why is common code getting infrastructure that assumes CPU is some special consideration? If we allow fixups for this, why not for other things?
const char *fdtdec_get_compatible(enum fdt_compat_id id) { /* We allow reading of the 'unknown' ID for testing purposes */ @@ -605,6 +613,8 @@ int fdtdec_prepare_fdt(void) #endif return -1; }
- ft_early_cpu_setup((void *)&gd->fdt_blob);
Unnecessary cast (or, it would be if you added the appropriate const in fdt_early_cpu_setup).
-Scott
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 6:52 AM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
On Mon, 2016-02-22 at 16:05 +0530, Prabhakar Kushwaha wrote:
There is a requirement of u-boot dts fix-up before it is being consumed by u-boot.
You might want to explain the reason *why* we have this requirement -- that the board takes a socketed SoC, and we don't want to have to reflash the board if one SoC is unplugged and another plugged in.
NXP's SoC LS2085A, LS2080A and LS2088A are almost same except variation in ARM core where LS2085A/LS2080A has A57 and LS2088A has
A72.
These SoCs will be placed on common LS2085ARDB platform.
So instead of maintaining defferent device tree per SoC, fix-up dts before being used by u-boot.
Signed-off-by: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com
And what happens when the next socketed board can support chips with device trees that are significantly more different?
Usually next revision of board should not have major change in terms of interface. If it is **different** new device tree, defconfig, <board>_config.h needs to be used.
There should be a mechanism for having multiple device trees present, and choosing one based on platform code. That would also avoid any problems of trying to modify a device tree before relocation.
I agree. But it may have following problems - Increasing bootloader size (u-boot + dtb(s)). - Need to maintain different dts per combination like LS2080 + LS2085ARDB , LS2088 + LS2085ARDB, LS2085A + LS2085ARDB Here board is common and SoC getting change
Assumption: SoC does not have major change.
arch/arm/cpu/armv8/fsl-layerscape/fdt.c | 53 ++++++++++++++++++++++++++ arch/arm/include/asm/arch-fsl-layerscape/soc.h | 4 ++ include/common.h | 2 + include/configs/ls2080a_common.h | 1 + lib/fdtdec.c | 10 +++++ 5 files changed, 70 insertions(+)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c index 4e4861d..cbdeef3 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c @@ -5,11 +5,13 @@ */
#include <common.h> +#include <asm/io.h> #include <libfdt.h> #include <fdt_support.h> #include <phy.h> #ifdef CONFIG_FSL_LSCH3 #include <asm/arch/fdt.h> +#include <asm/arch/soc.h> #endif #ifdef CONFIG_FSL_ESDHC #include <fsl_esdhc.h> @@ -18,6 +20,8 @@ #include <asm/arch/mp.h> #endif
+DECLARE_GLOBAL_DATA_PTR;
int fdt_fixup_phy_connection(void *blob, int offset, phy_interface_t phyc) { return fdt_setprop_string(blob, offset, "phy-connection-type", @@ -205,3 +209,52 @@ void ft_cpu_setup(void *blob, bd_t *bd) fdt_fixup_smmu(blob); #endif }
+void ft_early_fixup_cpu(void *blob) +{
- int off;
- u32 svr, ver;
- struct ccsr_gur __iomem *gur = (void
*)(CONFIG_SYS_FSL_GUTS_ADDR);
- off = fdt_path_offset(blob, "/cpus");
- if (off < 0) {
puts("couldn't find /cpus node\n");return;- }
- off = fdt_node_offset_by_prop_value(blob, -1, "device_type",
"cpu",
4);
- svr = gur_in32(&gur->svr);
- ver = SVR_SOC_VER(svr);
- while (off != -FDT_ERR_NOTFOUND) {
switch(ver) {case SVR_LS2080:case SVR_LS2085:case SVR_LS2045:case SVR_LS2040:fdt_setprop_string(blob, off, "compatible","arm,cortex-a57");break;case SVR_LS2088:case SVR_LS2048:case SVR_LS2084:case SVR_LS2028:fdt_setprop_string(blob, off, "compatible","arm,cortex-a72");break;}off = fdt_node_offset_by_prop_value(blob, off,"device_type",
"cpu", 4);- }
+}
+void ft_early_cpu_setup(void **blob) +{
- fdt_move(*blob, (void *)CONFIG_SYS_DTS_ADDR,
fdt_totalsize(blob));
- *blob = (void *)CONFIG_SYS_DTS_ADDR;
- ft_early_fixup_cpu((void *) *blob);
+}
This is hard to follow. Eliminate unnecessary casts, and s/DTS/DTB/ -- we're not dealing with the source code here.
Sure, I will fix it.
Why do you need to do *blob instead of just referencing gd directly?
Then the code will be like below
fdt_move(gd->blob, (void *)CONFIG_SYS_DTS_ADDR, fdt_totalsize(blob)); gd->blob = (void *)CONFIG_SYS_DTS_ADDR ft_early_fixup_cpu(gd->blob); --> As gd is global variable. May be avoid passing it as function parameter
diff --git a/include/common.h b/include/common.h index 1563d64..6dc8a7f 100644 --- a/include/common.h +++ b/include/common.h @@ -603,6 +603,8 @@ void ft_pci_setup(void *blob, bd_t *bd); #endif #endif
+void ft_early_cpu_setup(void **);
Arguments should have names.
diff --git a/include/configs/ls2080a_common.h b/include/configs/ls2080a_common.h index def0a6f..aa5ace9 100644 --- a/include/configs/ls2080a_common.h +++ b/include/configs/ls2080a_common.h @@ -24,6 +24,7 @@
/* Link Definitions */ #define CONFIG_SYS_INIT_SP_ADDR
(CONFIG_SYS_FSL_OCRAM_BASE +
0xfff0) +#define CONFIG_SYS_DTS_ADDR
(CONFIG_SYS_FSL_OCRAM_BASE +
0xfff0)
Why 0xfff0, and not, say, 0x10000 (or rather, why is INIT_SP_ADDR 0xfff0 and not 0x10000 if there is no need to reserve some space above the initial SP addr (e.g. to indicate the end of a traceback))?
I used the 0xfff0 thinking STACK moving from 0xfff0 to 0x0000. So better put dts starting from 0xfff0 to high address.
Is there anywhere that documents how various pieces of OCRAM are used?
IIUC there is no such document available. Need to create one. Let me work on this.
BTW, arch/arm/include/asm/arch-fsl-layerscape/config.h says OCRAM 2 MiB but the RM says it's 128 KiB.
Thanks for pointing out. Yes, It is wrong. I don’t have reason for this. May be just to have generic define.
Where do you check that the device tree fits in OCRAM? What about when SPL is occupying OCRAM? Does the device tree get used with SPL (I don't think we were using FDT control at all the last time I worked with SPL on these chips)?
I checked ls2085ardb dtb size in Linux. It is ~30K. So from 0xfff0 to 20000 there is enough space.
For SPL. Assumption is SPL never use dts. It will only be used by u-boot. Once control is transferred to u-boot, things work as it is like NOR boot.
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1b1ca02..fc200cf 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -76,6 +76,14 @@ static const char * const
compat_names[COMPAT_COUNT] = {
COMPAT(COMPAT_INTEL_IVYBRIDGE_FSP, "intel,ivybridge-fsp"), };
+void __ft_early_cpu_setup(void **blob) {
- return;
+} +void ft_early_cpu_setup(void **blob)
- __attribute__((weak, alias("__ft_early_cpu_setup")));
Why is common code getting infrastructure that assumes CPU is some special consideration? If we allow fixups for this, why not for other things?
fl_early_cpu_setup has been creating similar to ft_cpu_setup which calls all necessary fix-up including cpu.
const char *fdtdec_get_compatible(enum fdt_compat_id id) { /* We allow reading of the 'unknown' ID for testing purposes */ @@ -605,6 +613,8 @@ int fdtdec_prepare_fdt(void) #endif return -1; }
- ft_early_cpu_setup((void *)&gd->fdt_blob);
Unnecessary cast (or, it would be if you added the appropriate const in fdt_early_cpu_setup).
Sure.
--prabhakar
On Tue, 2016-02-23 at 04:09 +0000, Prabhakar Kushwaha wrote:
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 6:52 AM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
On Mon, 2016-02-22 at 16:05 +0530, Prabhakar Kushwaha wrote:
There is a requirement of u-boot dts fix-up before it is being consumed by u-boot.
You might want to explain the reason *why* we have this requirement -- that the board takes a socketed SoC, and we don't want to have to reflash the board if one SoC is unplugged and another plugged in.
NXP's SoC LS2085A, LS2080A and LS2088A are almost same except variation in ARM core where LS2085A/LS2080A has A57 and LS2088A has
A72.
These SoCs will be placed on common LS2085ARDB platform.
So instead of maintaining defferent device tree per SoC, fix-up dts before being used by u-boot.
Signed-off-by: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com
And what happens when the next socketed board can support chips with device trees that are significantly more different?
Usually next revision of board should not have major change in terms of interface. If it is **different** new device tree, defconfig, <board>_config.h needs to be used.
This has nothing to do with different revisions of a board. It has to do with the set of chips that can be plugged into the *same* board.
There should be a mechanism for having multiple device trees present, and choosing one based on platform code. That would also avoid any problems of trying to modify a device tree before relocation.
I agree. But it may have following problems
- Increasing bootloader size (u-boot + dtb(s)).
Not by much. If some boot sources are so close to the limit that this presents a problem, then we can deal with it, but this should not be a problem with NOR flash at least.
- Need to maintain different dts per combination like LS2080 + LS2085ARDB , LS2088 + LS2085ARDB, LS2085A + LS2085ARDB
So? Includes make it easy to maintain variants.
Here board is common and SoC getting change
Assumption: SoC does not have major change.
I don't think that's a good assumption -- and what constitutes a major change? A different core could be considered a major change. In any case, you need specialized fixup code for every type of difference between the SoCs.
Choosing from multiple device trees is a cleaner solution that does not place limits on the type of change from SoC to SoC, and does not consume OCRAM (among other benefits, this makes the mechanism transferable to other types of devices that may not have enough pre-relocation RAM to hold a device tree).
diff --git a/include/configs/ls2080a_common.h b/include/configs/ls2080a_common.h index def0a6f..aa5ace9 100644 --- a/include/configs/ls2080a_common.h +++ b/include/configs/ls2080a_common.h @@ -24,6 +24,7 @@
/* Link Definitions */ #define CONFIG_SYS_INIT_SP_ADDR
(CONFIG_SYS_FSL_OCRAM_BASE +
0xfff0) +#define CONFIG_SYS_DTS_ADDR
(CONFIG_SYS_FSL_OCRAM_BASE +
0xfff0)
Why 0xfff0, and not, say, 0x10000 (or rather, why is INIT_SP_ADDR 0xfff0 and not 0x10000 if there is no need to reserve some space above the initial SP addr (e.g. to indicate the end of a traceback))?
I used the 0xfff0 thinking STACK moving from 0xfff0 to 0x0000. So better put dts starting from 0xfff0 to high address.
My point is that there is likely a reason that the stack ends at an odd address like 0xfff0 rather than 0x10000, and most likely that reason is to reserve a location at the end of the stack that contains zeroes, to terminate a stack traceback. So please don't put something else there.
Where do you check that the device tree fits in OCRAM? What about when SPL is occupying OCRAM? Does the device tree get used with SPL (I don't think we were using FDT control at all the last time I worked with SPL on these chips)?
I checked ls2085ardb dtb size in Linux. It is ~30K. So from 0xfff0 to 20000 there is enough space.
What about when we have PPA or similar? How much space will that take up?
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1b1ca02..fc200cf 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -76,6 +76,14 @@ static const char * const
compat_names[COMPAT_COUNT] = {
COMPAT(COMPAT_INTEL_IVYBRIDGE_FSP, "intel,ivybridge-fsp"), };
+void __ft_early_cpu_setup(void **blob) {
- return;
+} +void ft_early_cpu_setup(void **blob)
- __attribute__((weak, alias("__ft_early_cpu_setup")));
Why is common code getting infrastructure that assumes CPU is some special consideration? If we allow fixups for this, why not for other things?
fl_early_cpu_setup has been creating similar to ft_cpu_setup which calls all necessary fix-up including cpu.
I see ft_cpu_setup() doing a bunch of things that aren't related to a cpu, so that's not exactly a good pattern to follow.
-Scott
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 12:14 PM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
On Tue, 2016-02-23 at 04:09 +0000, Prabhakar Kushwaha wrote:
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 6:52 AM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
On Mon, 2016-02-22 at 16:05 +0530, Prabhakar Kushwaha wrote:
There is a requirement of u-boot dts fix-up before it is being consumed by u-boot.
You might want to explain the reason *why* we have this requirement -- that the board takes a socketed SoC, and we don't want to have to reflash the board if one SoC is unplugged and another plugged in.
NXP's SoC LS2085A, LS2080A and LS2088A are almost same except variation in ARM core where LS2085A/LS2080A has A57 and LS2088A has
A72.
These SoCs will be placed on common LS2085ARDB platform.
So instead of maintaining defferent device tree per SoC, fix-up dts before being used by u-boot.
Signed-off-by: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com
And what happens when the next socketed board can support chips with device trees that are significantly more different?
Usually next revision of board should not have major change in terms of interface. If it is **different** new device tree, defconfig, <board>_config.h needs to be used.
This has nothing to do with different revisions of a board. It has to do with the set of chips that can be plugged into the *same* board.
There should be a mechanism for having multiple device trees present, and choosing one based on platform code. That would also avoid any problems of trying to modify a device tree before relocation.
I agree. But it may have following problems
- Increasing bootloader size (u-boot + dtb(s)).
Not by much. If some boot sources are so close to the limit that this presents a problem, then we can deal with it, but this should not be a problem with NOR flash at least.
I agree. But we may not be looking to increase u-boot size too much.
- Need to maintain different dts per combination like LS2080 + LS2085ARDB
,
LS2088 + LS2085ARDB, LS2085A + LS2085ARDB
So? Includes make it easy to maintain variants.
I agree. It will provide better dts understanding.
Same can be achieved by "fdt print" to get u-boot fixed dts.
Here board is common and SoC getting change
Assumption: SoC does not have major change.
I don't think that's a good assumption -- and what constitutes a major change? A different core could be considered a major change. In any case, you need specialized fixup code for every type of difference between the SoCs.
Ideally dts fix-up approach should only be applied to new Revision of SoCs. And one should be very careful of using this approach for new SoC.
Choosing from multiple device trees is a cleaner solution that does not place limits on the type of change from SoC to SoC, and does not consume OCRAM (among other benefits, this makes the mechanism transferable to other types of devices that may not have enough pre-relocation RAM to hold a device tree).
Choosing from multiple device tree is cleaner approach. I agree with you.
But same should be applied for Linux also. We should not have different approach for u-boot dts and Linux dts.
I will suggest to have both a) dts fix-up b) multiple dts
SoC with small change can follow "a"
SoC with major change can follow "b" assuming same approach has been accepted in Linux.
diff --git a/include/configs/ls2080a_common.h b/include/configs/ls2080a_common.h index def0a6f..aa5ace9 100644 --- a/include/configs/ls2080a_common.h +++ b/include/configs/ls2080a_common.h @@ -24,6 +24,7 @@
/* Link Definitions */ #define CONFIG_SYS_INIT_SP_ADDR
(CONFIG_SYS_FSL_OCRAM_BASE +
0xfff0) +#define CONFIG_SYS_DTS_ADDR
(CONFIG_SYS_FSL_OCRAM_BASE +
0xfff0)
Why 0xfff0, and not, say, 0x10000 (or rather, why is INIT_SP_ADDR 0xfff0 and not 0x10000 if there is no need to reserve some space above the initial SP addr (e.g. to indicate the end of a traceback))?
I used the 0xfff0 thinking STACK moving from 0xfff0 to 0x0000. So better put dts starting from 0xfff0 to high address.
My point is that there is likely a reason that the stack ends at an odd address like 0xfff0 rather than 0x10000, and most likely that reason is to reserve a location at the end of the stack that contains zeroes, to terminate a stack traceback. So please don't put something else there.
Dtb can be put 128K - 32K location 17FF0. I hope dts will not be more than 32K. Current Linux dts is ~27K It is platform dependent define.
Where do you check that the device tree fits in OCRAM? What about when SPL is occupying OCRAM? Does the device tree get used with SPL (I don't think we were using FDT control at all the last time I worked with SPL on these chips)?
I checked ls2085ardb dtb size in Linux. It is ~30K. So from 0xfff0 to 20000 there is enough space.
What about when we have PPA or similar? How much space will that take up?
PPA will not be using OCRAM. If my understanding is correct PPA will be loaded directly in DDR. There is 2 MB secure DDR for the same.
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 1b1ca02..fc200cf 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -76,6 +76,14 @@ static const char * const
compat_names[COMPAT_COUNT] = {
COMPAT(COMPAT_INTEL_IVYBRIDGE_FSP, "intel,ivybridge-fsp"), };
+void __ft_early_cpu_setup(void **blob) {
- return;
+} +void ft_early_cpu_setup(void **blob)
- __attribute__((weak, alias("__ft_early_cpu_setup")));
Why is common code getting infrastructure that assumes CPU is some special consideration? If we allow fixups for this, why not for other
things?
fl_early_cpu_setup has been creating similar to ft_cpu_setup which calls all necessary fix-up including cpu.
I see ft_cpu_setup() doing a bunch of things that aren't related to a cpu, so that's not exactly a good pattern to follow.
I will update it.
--prabhakar
On Wed, 2016-02-24 at 02:50 +0000, Prabhakar Kushwaha wrote:
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 12:14 PM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
On Tue, 2016-02-23 at 04:09 +0000, Prabhakar Kushwaha wrote:
There should be a mechanism for having multiple device trees present, and choosing one based on platform code. That would also avoid any problems of trying to modify a device tree before relocation.
I agree. But it may have following problems
- Increasing bootloader size (u-boot + dtb(s)).
Not by much. If some boot sources are so close to the limit that this presents a problem, then we can deal with it, but this should not be a problem with NOR flash at least.
I agree. But we may not be looking to increase u-boot size too much.
I don't think it's a problem for an evaluation board. Developers of actual embedded systems can of course install only the device tree they care about. We could also look into why the device tree is as big as it is -- there's probably stuff in there U-Boot doesn't need.
Here board is common and SoC getting change
Assumption: SoC does not have major change.
I don't think that's a good assumption -- and what constitutes a major change? A different core could be considered a major change. In any case, you need specialized fixup code for every type of difference between the SoCs.
Ideally dts fix-up approach should only be applied to new Revision of SoCs. And one should be very careful of using this approach for new SoC.
No. Again, it's not for new revisions of an SoC. It's for any SoC that is user-pluggable into the board.
Choosing from multiple device trees is a cleaner solution that does not place limits on the type of change from SoC to SoC, and does not consume OCRAM (among other benefits, this makes the mechanism transferable to other types of devices that may not have enough pre-relocation RAM to hold a device tree).
Choosing from multiple device tree is cleaner approach. I agree with you.
But same should be applied for Linux also. We should not have different approach for u-boot dts and Linux dts.
It would be nice to do so, and there's been some discussion of this before: http://lists.denx.de/pipermail/u-boot/2013-January/143669.html
However, I don't think it's necessarily true that one implies the other. It's much less of a burden to change a U-Boot environment variable to get the right dtb to pass to the OS, versus having to reflash U-Boot itself.
I will suggest to have both a) dts fix-up b) multiple dts
SoC with small change can follow "a"
SoC with major change can follow "b" assuming same approach has been accepted in Linux.
What do you mean by "accepted in Linux"? Even for the dtb that gets passed to the OS, we're still talking about U-Boot mechanisms, not anything that Linux sees.
Why would we want two mechanisms when we could have one, especially if one of the benefits of "b" is not having to deal with the possibility of modifying the device tree before relocation?
-Scott
On Tue, 2016-02-23 at 04:09 +0000, Prabhakar Kushwaha wrote:
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 6:52 AM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
Where do you check that the device tree fits in OCRAM? What about when SPL is occupying OCRAM? Does the device tree get used with SPL (I don't think we were using FDT control at all the last time I worked with SPL on these chips)?
I checked ls2085ardb dtb size in Linux. It is ~30K. So from 0xfff0 to 20000 there is enough space.
For SPL. Assumption is SPL never use dts. It will only be used by u-boot. Once control is transferred to u-boot, things work as it is like NOR boot.
I don't think that's a good assumption, given that I just saw a patch posted regarding SPL_OF_CONTROL.
-Scott
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Wednesday, March 09, 2016 1:02 AM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
On Tue, 2016-02-23 at 04:09 +0000, Prabhakar Kushwaha wrote:
-----Original Message----- From: Scott Wood [mailto:oss@buserror.net] Sent: Tuesday, February 23, 2016 6:52 AM To: Prabhakar Kushwaha prabhakar.kushwaha@nxp.com; u- boot@lists.denx.de Cc: york sun york.sun@nxp.com; Priyanka Jain priyanka.jain@nxp.com Subject: Re: [RFC] armv8: layerscape: Add support of u-boot device tree fix- up
Where do you check that the device tree fits in OCRAM? What about when SPL is occupying OCRAM? Does the device tree get used with SPL (I don't think we were using FDT control at all the last time I worked with SPL on these chips)?
I checked ls2085ardb dtb size in Linux. It is ~30K. So from 0xfff0 to 20000 there is enough space.
For SPL. Assumption is SPL never use dts. It will only be used by u-boot. Once control is transferred to u-boot, things work as it is like NOR boot.
I don't think that's a good assumption, given that I just saw a patch posted regarding SPL_OF_CONTROL.
Thanks Scott for pointing out.
This means SPL may also be have dtb. Will it not increase size of SPL binary?
--prabhakar
participants (2)
-
Prabhakar Kushwaha -
Scott Wood