[U-Boot] [PATCH 1/4] lib: fdt: Split fdtdec_setup_mem_size_base()
Split fdtdec_setup_mem_size_base() into fdtdec_setup_mem_size_base_fdt(), which allows the caller to pass custom blob into the function and the original fdtdec_setup_mem_size_base(), which uses the gd->fdt_blob. This is useful when configuring the DRAM properties from a FDT blob fragment passed in by the firmware.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Nobuhiro Iwamatsu iwamatsu@nigauri.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- include/fdtdec.h | 20 ++++++++++++++++++++ lib/fdtdec.c | 11 ++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index b7e35cd87c..3e681099a8 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -917,6 +917,26 @@ struct display_timing { int fdtdec_decode_display_timing(const void *blob, int node, int index, struct display_timing *config);
+/** + * fdtdec_setup_mem_size_base_fdt() - decode and setup gd->ram_size and + * gd->ram_start + * + * Decode the /memory 'reg' property to determine the size and start of the + * first memory bank, populate the global data with the size and start of the + * first bank of memory. + * + * This function should be called from a boards dram_init(). This helper + * function allows for boards to query the device tree for DRAM size and start + * address instead of hard coding the value in the case where the memory size + * and start address cannot be detected automatically. + * + * @param blob FDT blob + * + * @return 0 if OK, -EINVAL if the /memory node or reg property is missing or + * invalid + */ +int fdtdec_setup_mem_size_base_fdt(const void *blob); + /** * fdtdec_setup_mem_size_base() - decode and setup gd->ram_size and * gd->ram_start diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 09a7e133a5..3f29e9d647 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1088,18 +1088,18 @@ int fdtdec_decode_display_timing(const void *blob, int parent, int index, return ret; }
-int fdtdec_setup_mem_size_base(void) +int fdtdec_setup_mem_size_base_fdt(const void *blob) { int ret, mem; struct fdt_resource res;
- mem = fdt_path_offset(gd->fdt_blob, "/memory"); + mem = fdt_path_offset(blob, "/memory"); if (mem < 0) { debug("%s: Missing /memory node\n", __func__); return -EINVAL; }
- ret = fdt_get_resource(gd->fdt_blob, mem, "reg", 0, &res); + ret = fdt_get_resource(blob, mem, "reg", 0, &res); if (ret != 0) { debug("%s: Unable to decode first memory bank\n", __func__); return -EINVAL; @@ -1113,6 +1113,11 @@ int fdtdec_setup_mem_size_base(void) return 0; }
+int fdtdec_setup_mem_size_base(void) +{ + return fdtdec_setup_mem_size_base_fdt(gd->fdt_blob); +} + #if defined(CONFIG_NR_DRAM_BANKS)
static int get_next_memory_node(const void *blob, int mem)
Split fdtdec_setup_memory_banksize() into fdtdec_setup_memory_banksize_fdt(), which allows the caller to pass custom blob into the function and the original fdtdec_setup_memory_banksize(), which uses the gd->fdt_blob. This is useful when configuring the DRAM properties from a FDT blob fragment passed in by the firmware.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Nobuhiro Iwamatsu iwamatsu@nigauri.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com --- include/fdtdec.h | 19 +++++++++++++++++++ lib/fdtdec.c | 18 ++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/include/fdtdec.h b/include/fdtdec.h index 3e681099a8..ad00f79f20 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -955,6 +955,25 @@ int fdtdec_setup_mem_size_base_fdt(const void *blob); */ int fdtdec_setup_mem_size_base(void);
+/** + * fdtdec_setup_memory_banksize_fdt() - decode and populate gd->bd->bi_dram + * + * Decode the /memory 'reg' property to determine the address and size of the + * memory banks. Use this data to populate the global data board info with the + * phys address and size of memory banks. + * + * This function should be called from a boards dram_init_banksize(). This + * helper function allows for boards to query the device tree for memory bank + * information instead of hard coding the information in cases where it cannot + * be detected automatically. + * + * @param blob FDT blob + * + * @return 0 if OK, -EINVAL if the /memory node or reg property is missing or + * invalid + */ +int fdtdec_setup_memory_banksize_fdt(const void *blob); + /** * fdtdec_setup_memory_banksize() - decode and populate gd->bd->bi_dram * diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 3f29e9d647..79915b59bd 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1123,33 +1123,33 @@ int fdtdec_setup_mem_size_base(void) static int get_next_memory_node(const void *blob, int mem) { do { - mem = fdt_node_offset_by_prop_value(gd->fdt_blob, mem, + mem = fdt_node_offset_by_prop_value(blob, mem, "device_type", "memory", 7); } while (!fdtdec_get_is_enabled(blob, mem));
return mem; }
-int fdtdec_setup_memory_banksize(void) +int fdtdec_setup_memory_banksize_fdt(const void *blob) { int bank, ret, mem, reg = 0; struct fdt_resource res;
- mem = get_next_memory_node(gd->fdt_blob, -1); + mem = get_next_memory_node(blob, -1); if (mem < 0) { debug("%s: Missing /memory node\n", __func__); return -EINVAL; }
for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) { - ret = fdt_get_resource(gd->fdt_blob, mem, "reg", reg++, &res); + ret = fdt_get_resource(blob, mem, "reg", reg++, &res); if (ret == -FDT_ERR_NOTFOUND) { reg = 0; - mem = get_next_memory_node(gd->fdt_blob, mem); + mem = get_next_memory_node(blob, mem); if (mem == -FDT_ERR_NOTFOUND) break;
- ret = fdt_get_resource(gd->fdt_blob, mem, "reg", reg++, &res); + ret = fdt_get_resource(blob, mem, "reg", reg++, &res); if (ret == -FDT_ERR_NOTFOUND) break; } @@ -1169,6 +1169,12 @@ int fdtdec_setup_memory_banksize(void)
return 0; } + +int fdtdec_setup_memory_banksize(void) +{ + return fdtdec_setup_memory_banksize_fdt(gd->fdt_blob); + +} #endif
#if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
On Mon, 4 Mar 2019 at 20:26, Marek Vasut marek.vasut@gmail.com wrote:
Split fdtdec_setup_memory_banksize() into fdtdec_setup_memory_banksize_fdt(), which allows the caller to pass custom blob into the function and the original fdtdec_setup_memory_banksize(), which uses the gd->fdt_blob. This is useful when configuring the DRAM properties from a FDT blob fragment passed in by the firmware.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Nobuhiro Iwamatsu iwamatsu@nigauri.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
include/fdtdec.h | 19 +++++++++++++++++++ lib/fdtdec.c | 18 ++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Save these registers for future use.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Nobuhiro Iwamatsu iwamatsu@nigauri.org --- arch/arm/mach-rmobile/lowlevel_init_gen3.S | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-rmobile/lowlevel_init_gen3.S b/arch/arm/mach-rmobile/lowlevel_init_gen3.S index f42b53fd88..213ec143e2 100644 --- a/arch/arm/mach-rmobile/lowlevel_init_gen3.S +++ b/arch/arm/mach-rmobile/lowlevel_init_gen3.S @@ -16,6 +16,21 @@ #include <linux/linkage.h> #include <asm/macro.h>
+.align 8 +.globl rcar_atf_boot_args +rcar_atf_boot_args: + .dword 0 + .dword 0 + .dword 0 + .dword 0 + +ENTRY(save_boot_params) + adr x8, rcar_atf_boot_args + stp x0, x1, [x8], #16 + stp x2, x3, [x8], #16 + b save_boot_params_ret +ENDPROC(save_boot_params) + ENTRY(lowlevel_init) mov x29, lr /* Save LR */
Hi Marek,
On Tue, Mar 5, 2019 at 4:41 AM Marek Vasut marek.vasut@gmail.com wrote: [..]
+.align 8 +.globl rcar_atf_boot_args +rcar_atf_boot_args:
.dword 0.dword 0.dword 0.dword 0+ENTRY(save_boot_params)
adr x8, rcar_atf_boot_argsstp x0, x1, [x8], #16stp x2, x3, [x8], #16b save_boot_params_ret+ENDPROC(save_boot_params)
What about relocating the function to C like in [1] and passing the 4 arguments to it from ASM like in [2]? This would allow to: - reduce asm operations to minimum (mov and bl) and make code transparent. - avoid custom globals and store the ATF information in some C-defined struct.
[1] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7... [2] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7...
Best regards, Eugeniu.
On Tue, Mar 12, 2019 at 07:59:40PM +0100, Eugeniu Rosca wrote:
Hi Marek,
On Tue, Mar 5, 2019 at 4:41 AM Marek Vasut marek.vasut@gmail.com wrote: [..]
+.align 8 +.globl rcar_atf_boot_args +rcar_atf_boot_args:
.dword 0.dword 0.dword 0.dword 0+ENTRY(save_boot_params)
adr x8, rcar_atf_boot_argsstp x0, x1, [x8], #16stp x2, x3, [x8], #16b save_boot_params_ret+ENDPROC(save_boot_params)
What about relocating the function to C like in [1] and passing the 4 arguments to it from ASM like in [2]? This would allow to:
- reduce asm operations to minimum (mov and bl) and make code transparent.
- avoid custom globals and store the ATF information in some C-defined struct.
[1] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7... [2] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7...
This is super early in the boot process and I'm really not a fan in general of writing and calling C before we have things setup for C calls to actually work normally, even more so for small things that can be commented to be obvious as to what's going on.
Hi Tom,
On Tue, Mar 12, 2019 at 03:42:24PM -0400, Tom Rini wrote:
On Tue, Mar 12, 2019 at 07:59:40PM +0100, Eugeniu Rosca wrote:
Hi Marek,
On Tue, Mar 5, 2019 at 4:41 AM Marek Vasut marek.vasut@gmail.com wrote: [..]
+.align 8 +.globl rcar_atf_boot_args +rcar_atf_boot_args:
.dword 0.dword 0.dword 0.dword 0+ENTRY(save_boot_params)
adr x8, rcar_atf_boot_argsstp x0, x1, [x8], #16stp x2, x3, [x8], #16b save_boot_params_ret+ENDPROC(save_boot_params)
What about relocating the function to C like in [1] and passing the 4 arguments to it from ASM like in [2]? This would allow to:
- reduce asm operations to minimum (mov and bl) and make code transparent.
- avoid custom globals and store the ATF information in some C-defined struct.
[1] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7... [2] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7...
This is super early in the boot process and I'm really not a fan in general of writing and calling C before we have things setup for C calls to actually work normally, even more so for small things that can be commented to be obvious as to what's going on.
I don't have a strong preference about that. It is just my experience that Renesas once rewrote the elaborate Bosch-contributed BL2 routine of enabling the cntfrq_el0 system counter from ASM [1] to C [2]. This is at the BL1-BL2 boundary, while we are still in EL3, so much earlier in the boot process. This implementation currently runs on the targets of our customers.
I am fine with any working solution. Thanks for commenting!
[1] https://github.com/renesas-rcar/arm-trusted-firmware/commit/e23524689abb637b... [2] https://github.com/renesas-rcar/arm-trusted-firmware/commit/867d84191319#dif...
-- Tom
Best regards, Eugeniu.
On 3/12/19 9:29 PM, Eugeniu Rosca wrote:
Hi Tom,
Hi,
On Tue, Mar 12, 2019 at 03:42:24PM -0400, Tom Rini wrote:
On Tue, Mar 12, 2019 at 07:59:40PM +0100, Eugeniu Rosca wrote:
Hi Marek,
On Tue, Mar 5, 2019 at 4:41 AM Marek Vasut marek.vasut@gmail.com wrote: [..]
+.align 8 +.globl rcar_atf_boot_args +rcar_atf_boot_args:
.dword 0.dword 0.dword 0.dword 0+ENTRY(save_boot_params)
adr x8, rcar_atf_boot_argsstp x0, x1, [x8], #16stp x2, x3, [x8], #16b save_boot_params_ret+ENDPROC(save_boot_params)
What about relocating the function to C like in [1] and passing the 4 arguments to it from ASM like in [2]? This would allow to:
- reduce asm operations to minimum (mov and bl) and make code transparent.
- avoid custom globals and store the ATF information in some C-defined struct.
[1] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7... [2] https://github.com/ARM-software/arm-trusted-firmware/blob/c48d02bade88b07fa7...
This is super early in the boot process and I'm really not a fan in general of writing and calling C before we have things setup for C calls to actually work normally, even more so for small things that can be commented to be obvious as to what's going on.
I don't have a strong preference about that. It is just my experience that Renesas once rewrote the elaborate Bosch-contributed BL2 routine of enabling the cntfrq_el0 system counter from ASM [1] to C [2].
At that point, BL2 already has a C runtime set up.
This code is instead running without the C runtime and we want to make sure the correct registers get stored at the correct location, without the compiler interfering with it in any way. Keep in mind this code runs right at the beginning of U-Boot execution to make sure registers used to pass parameters stay intact (which I believe is what Tom meant by "early in the boot process").
This is at the BL1-BL2 boundary, while we are still in EL3, so much earlier in the boot process. This implementation currently runs on the targets of our customers.
I am fine with any working solution. Thanks for commenting!
[1] https://github.com/renesas-rcar/arm-trusted-firmware/commit/e23524689abb637b... [2] https://github.com/renesas-rcar/arm-trusted-firmware/commit/867d84191319#dif...
-- Tom
Best regards, Eugeniu.
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Nobuhiro Iwamatsu iwamatsu@nigauri.org --- board/renesas/ebisu/ebisu.c | 28 +++++++++++++++++++++++---- board/renesas/salvator-x/salvator-x.c | 28 +++++++++++++++++++++++---- board/renesas/ulcb/ulcb.c | 28 +++++++++++++++++++++++---- 3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/board/renesas/ebisu/ebisu.c b/board/renesas/ebisu/ebisu.c index 5d8b79eee3..60429e4529 100644 --- a/board/renesas/ebisu/ebisu.c +++ b/board/renesas/ebisu/ebisu.c @@ -43,17 +43,37 @@ int board_init(void) return 0; }
+/* + * If the firmware passed a device tree use it for U-Boot DRAM setup. + */ +extern u64 rcar_atf_boot_args[]; + int dram_init(void) { - if (fdtdec_setup_mem_size_base() != 0) - return -EINVAL; + const void *atf_fdt_blob = (const void *)(rcar_atf_boot_args[1]); + const void *blob;
- return 0; + /* Check if ATF passed us DTB. If not, fall back to builtin DTB. */ + if (fdt_magic(atf_fdt_blob) == FDT_MAGIC) + blob = atf_fdt_blob; + else + blob = gd->fdt_blob; + + return fdtdec_setup_mem_size_base_fdt(blob); }
int dram_init_banksize(void) { - fdtdec_setup_memory_banksize(); + const void *atf_fdt_blob = (const void *)(rcar_atf_boot_args[1]); + const void *blob; + + /* Check if ATF passed us DTB. If not, fall back to builtin DTB. */ + if (fdt_magic(atf_fdt_blob) == FDT_MAGIC) + blob = atf_fdt_blob; + else + blob = gd->fdt_blob; + + fdtdec_setup_memory_banksize_fdt(blob);
return 0; } diff --git a/board/renesas/salvator-x/salvator-x.c b/board/renesas/salvator-x/salvator-x.c index 8f0247e046..1db08fce6a 100644 --- a/board/renesas/salvator-x/salvator-x.c +++ b/board/renesas/salvator-x/salvator-x.c @@ -69,17 +69,37 @@ int board_init(void) return 0; }
+/* + * If the firmware passed a device tree use it for U-Boot DRAM setup. + */ +extern u64 rcar_atf_boot_args[]; + int dram_init(void) { - if (fdtdec_setup_mem_size_base() != 0) - return -EINVAL; + const void *atf_fdt_blob = (const void *)(rcar_atf_boot_args[1]); + const void *blob;
- return 0; + /* Check if ATF passed us DTB. If not, fall back to builtin DTB. */ + if (fdt_magic(atf_fdt_blob) == FDT_MAGIC) + blob = atf_fdt_blob; + else + blob = gd->fdt_blob; + + return fdtdec_setup_mem_size_base_fdt(blob); }
int dram_init_banksize(void) { - fdtdec_setup_memory_banksize(); + const void *atf_fdt_blob = (const void *)(rcar_atf_boot_args[1]); + const void *blob; + + /* Check if ATF passed us DTB. If not, fall back to builtin DTB. */ + if (fdt_magic(atf_fdt_blob) == FDT_MAGIC) + blob = atf_fdt_blob; + else + blob = gd->fdt_blob; + + fdtdec_setup_memory_banksize_fdt(blob);
return 0; } diff --git a/board/renesas/ulcb/ulcb.c b/board/renesas/ulcb/ulcb.c index 81d6f8f6f2..3ed7bfaa88 100644 --- a/board/renesas/ulcb/ulcb.c +++ b/board/renesas/ulcb/ulcb.c @@ -68,17 +68,37 @@ int board_init(void) return 0; }
+/* + * If the firmware passed a device tree use it for U-Boot DRAM setup. + */ +extern u64 rcar_atf_boot_args[]; + int dram_init(void) { - if (fdtdec_setup_mem_size_base() != 0) - return -EINVAL; + const void *atf_fdt_blob = (const void *)(rcar_atf_boot_args[1]); + const void *blob;
- return 0; + /* Check if ATF passed us DTB. If not, fall back to builtin DTB. */ + if (fdt_magic(atf_fdt_blob) == FDT_MAGIC) + blob = atf_fdt_blob; + else + blob = gd->fdt_blob; + + return fdtdec_setup_mem_size_base_fdt(blob); }
int dram_init_banksize(void) { - fdtdec_setup_memory_banksize(); + const void *atf_fdt_blob = (const void *)(rcar_atf_boot_args[1]); + const void *blob; + + /* Check if ATF passed us DTB. If not, fall back to builtin DTB. */ + if (fdt_magic(atf_fdt_blob) == FDT_MAGIC) + blob = atf_fdt_blob; + else + blob = gd->fdt_blob; + + fdtdec_setup_memory_banksize_fdt(blob);
return 0; }
Hi Marek cc: Michael
On Tue, Mar 5, 2019 at 4:37 AM Marek Vasut marek.vasut@gmail.com wrote:
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
1. Do the ATF changes supporting this feature already exist in mainline ATF?
2. I see more DRAM-related compile-time knobs in vanilla ATF: ➜ arm-trusted-firmware (c48d02bade88) $ grep "DRAM.*:" plat/renesas/rcar/platform.mk RCAR_DRAM_SPLIT := 0 RCAR_DRAM_LPDDR4_MEMCONF :=1 RCAR_DRAM_DDR3L_MEMCONF :=1 RCAR_DRAM_DDR3L_MEMDUAL :=1 RCAR_DRAM_CHANNEL :=15
My understanding is that these are local to ATF and U-Boot can stay agnostic about them? IOW, DRAM start and size are totally enough for U-Boot? TIA!
Best regards, Eugeniu.
On 3/12/19 8:30 PM, Eugeniu Rosca wrote:
Hi Marek cc: Michael
Hi,
On Tue, Mar 5, 2019 at 4:37 AM Marek Vasut marek.vasut@gmail.com wrote:
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
- Do the ATF changes supporting this feature already exist in mainline ATF?
Yes, for quite some time, see commit 1d85c4bd5b6291b99feb2409525c96f0c1744328 plat: rcar: Pass DTB with DRAM layout from BL2 to next stages
- I see more DRAM-related compile-time knobs in vanilla ATF:
➜ arm-trusted-firmware (c48d02bade88) $ grep "DRAM.*:" plat/renesas/rcar/platform.mk RCAR_DRAM_SPLIT := 0 RCAR_DRAM_LPDDR4_MEMCONF :=1 RCAR_DRAM_DDR3L_MEMCONF :=1 RCAR_DRAM_DDR3L_MEMDUAL :=1 RCAR_DRAM_CHANNEL :=15
My understanding is that these are local to ATF and U-Boot can stay agnostic about them?
Yes, although the long term goal is to get rid of those and replace them with DT configuration too. So ATF BL2 would then parse supplied DT to figure out the DRAM layout instead of using those knobs.
IOW, DRAM start and size are totally enough for U-Boot?
No, U-Boot needs to know start/size of each bank. That's what's in the DT that is passed in, plus a machine compatible string, so technically, you might be able to build one U-Boot for all boards already, but then you hit a size limit where the BL33 must be below 1 MiB. I'm looking into that one too.
On Tue, Mar 12, 2019 at 10:11:21PM +0100, Marek Vasut wrote:
On 3/12/19 8:30 PM, Eugeniu Rosca wrote:
Hi Marek cc: Michael
Hi,
On Tue, Mar 5, 2019 at 4:37 AM Marek Vasut marek.vasut@gmail.com wrote:
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
- Do the ATF changes supporting this feature already exist in mainline ATF?
Yes, for quite some time, see commit 1d85c4bd5b6291b99feb2409525c96f0c1744328 plat: rcar: Pass DTB with DRAM layout from BL2 to next stages
That's helpful, but I think such information should go by default into commit description.
[..]
Besides the above, I wonder why this patch duplicates code across three board files? Could this code be relocated to some common area like [1]? FWIW building the latter could be re-enabled by reverting the Makefile changes from v2018.01-rc1 commit [2].
To differentiate between the boards which expect/require the ATF args and those which don't, I guess that run-time (IS_ENABLED) checking of a newly created Kconfig symbol, e.g. SUPPORTS_ATF_ARGS or similar, selected on per-board basis, could do the job?
Note that we already face the need to have an area for Android features common between all Gen3 boards, so I think reviving [1] is inevitable.
[1] board/renesas/rcar-common/common.c [2] http://git.denx.de/?p=u-boot.git;a=commitdiff;h=e23eb942ad10 ("ARM: rmobile: Stop using rcar-common/common.c on Gen3")
Best regards, Eugeniu.
On 3/13/19 4:25 PM, Eugeniu Rosca wrote:
On Tue, Mar 12, 2019 at 10:11:21PM +0100, Marek Vasut wrote:
On 3/12/19 8:30 PM, Eugeniu Rosca wrote:
Hi Marek cc: Michael
Hi,
On Tue, Mar 5, 2019 at 4:37 AM Marek Vasut marek.vasut@gmail.com wrote:
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
- Do the ATF changes supporting this feature already exist in mainline ATF?
Yes, for quite some time, see commit 1d85c4bd5b6291b99feb2409525c96f0c1744328 plat: rcar: Pass DTB with DRAM layout from BL2 to next stages
That's helpful, but I think such information should go by default into commit description.
[..]
Besides the above, I wonder why this patch duplicates code across three board files? Could this code be relocated to some common area like [1]?
That's the plan.
FWIW building the latter could be re-enabled by reverting the Makefile changes from v2018.01-rc1 commit [2].
I prefer a more progressive approach, that is applying patches, rather than reverting.
To differentiate between the boards which expect/require the ATF args and those which don't, I guess that run-time (IS_ENABLED) checking of a newly created Kconfig symbol, e.g. SUPPORTS_ATF_ARGS or similar, selected on per-board basis, could do the job?
No, the code should auto-detect whether the ATF supports the DT passing or not.
Note that we already face the need to have an area for Android features common between all Gen3 boards, so I think reviving [1] is inevitable.
[1] board/renesas/rcar-common/common.c [2] http://git.denx.de/?p=u-boot.git;a=commitdiff;h=e23eb942ad10 ("ARM: rmobile: Stop using rcar-common/common.c on Gen3")
Best regards, Eugeniu.
Hi Marek,
On Tue, Apr 02, 2019 at 05:45:29AM +0200, Marek Vasut wrote:
On 3/13/19 4:25 PM, Eugeniu Rosca wrote:
On Tue, Mar 12, 2019 at 10:11:21PM +0100, Marek Vasut wrote:
On 3/12/19 8:30 PM, Eugeniu Rosca wrote:
Hi Marek cc: Michael
Hi,
On Tue, Mar 5, 2019 at 4:37 AM Marek Vasut marek.vasut@gmail.com wrote:
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
- Do the ATF changes supporting this feature already exist in mainline ATF?
Yes, for quite some time, see commit 1d85c4bd5b6291b99feb2409525c96f0c1744328 plat: rcar: Pass DTB with DRAM layout from BL2 to next stages
That's helpful, but I think such information should go by default into commit description.
[..]
Besides the above, I wonder why this patch duplicates code across three board files? Could this code be relocated to some common area like [1]?
That's the plan.
I look forward to seeing v2 then, since this would allow us not duplicating this code over and over again in the board-specific files of the customer R-Car3 targets.
FWIW building the latter could be re-enabled by reverting the Makefile changes from v2018.01-rc1 commit [2].
I prefer a more progressive approach, that is applying patches, rather than reverting.
"reverting" is used here figuratively, suggesting to undo the Makefile changes added by commit [2], in order to create/restore the R-Car3 common/board-independent space. It doesn't ask creating a revert commit.
To differentiate between the boards which expect/require the ATF args and those which don't, I guess that run-time (IS_ENABLED) checking of a newly created Kconfig symbol, e.g. SUPPORTS_ATF_ARGS or similar, selected on per-board basis, could do the job?
No, the code should auto-detect whether the ATF supports the DT passing or not.
Great. Less build-time options the better.
Note that we already face the need to have an area for Android features common between all Gen3 boards, so I think reviving [1] is inevitable.
[1] board/renesas/rcar-common/common.c [2] http://git.denx.de/?p=u-boot.git;a=commitdiff;h=e23eb942ad10 ("ARM: rmobile: Stop using rcar-common/common.c on Gen3")
Best regards, Eugeniu.
-- Best regards, Marek Vasut
I would appreciate being CC-ed in v2, if possible. Thanks!
On 4/4/19 3:45 PM, Eugeniu Rosca wrote:
Hi Marek,
Hi,
On Tue, Apr 02, 2019 at 05:45:29AM +0200, Marek Vasut wrote:
On 3/13/19 4:25 PM, Eugeniu Rosca wrote:
On Tue, Mar 12, 2019 at 10:11:21PM +0100, Marek Vasut wrote:
On 3/12/19 8:30 PM, Eugeniu Rosca wrote:
Hi Marek cc: Michael
Hi,
On Tue, Mar 5, 2019 at 4:37 AM Marek Vasut marek.vasut@gmail.com wrote:
The ATF can pass additional information via the first four registers, x0...x3. The R-Car Gen3 with mainline ATF, register x1 contains pointer to a device tree with platform information. Parse this device tree and extract DRAM size information from it. This is useful on systems where the DRAM size can vary between configurations.
- Do the ATF changes supporting this feature already exist in mainline ATF?
Yes, for quite some time, see commit 1d85c4bd5b6291b99feb2409525c96f0c1744328 plat: rcar: Pass DTB with DRAM layout from BL2 to next stages
That's helpful, but I think such information should go by default into commit description.
[..]
Besides the above, I wonder why this patch duplicates code across three board files? Could this code be relocated to some common area like [1]?
That's the plan.
I look forward to seeing v2 then, since this would allow us not duplicating this code over and over again in the board-specific files of the customer R-Car3 targets.
Should be fixed by http://patchwork.ozlabs.org/patch/1101744/
[...]
Hi Marek,
On Mon, May 20, 2019 at 12:50:07AM +0200, Marek Vasut wrote:
On 4/4/19 3:45 PM, Eugeniu Rosca wrote:
[..]
[..]
Besides the above, I wonder why this patch duplicates code across three board files? Could this code be relocated to some common area like [1]?
That's the plan.
I look forward to seeing v2 then, since this would allow us not duplicating this code over and over again in the board-specific files of the customer R-Car3 targets.
Should be fixed by http://patchwork.ozlabs.org/patch/1101744/
Thank you very much. We'll check it out later and report the results (don't feel blocked however). Thanks again!
On Mon, 4 Mar 2019 at 20:26, Marek Vasut marek.vasut@gmail.com wrote:
Split fdtdec_setup_mem_size_base() into fdtdec_setup_mem_size_base_fdt(), which allows the caller to pass custom blob into the function and the original fdtdec_setup_mem_size_base(), which uses the gd->fdt_blob. This is useful when configuring the DRAM properties from a FDT blob fragment passed in by the firmware.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Nobuhiro Iwamatsu iwamatsu@nigauri.org Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@konsulko.com
include/fdtdec.h | 20 ++++++++++++++++++++ lib/fdtdec.c | 11 ++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (5)
-
Eugeniu Rosca -
Eugeniu Rosca -
Marek Vasut -
Simon Glass -
Tom Rini