| Message ID | 20190305032557.19788-4-marek.vasut+renesas@gmail.com |
|---|---|
| State | Accepted |
| Commit | 175f5027345c7feaa41e8f4201778814bf72fe37 |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [U-Boot,1/4] lib: fdt: Split fdtdec_setup_mem_size_base() | expand |
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. > > 1. 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 > 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? 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. > > > > 1. 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. >>> >>> 1. 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. > >>> > >>> 1. 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. >>>>> >>>>> 1. 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!
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; }
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(-)