On 11/22/16 16:54, Mike Looijmans wrote: > On 22-11-16 12:00, Michal Simek wrote: >> On 21.11.2016 09:30, Mike Looijmans wrote: >>> Miami boards can have memory sizes of 256M, 512M or 1GB. To >>> prevent requiring separate bootloaders for each variant, just >>> detect the RAM size at boot time instead of relying on the >>> devicetree information. >>> >>> Signed-off-by: Mike Looijmans <mike.looijm...@topic.nl> --- >>> board/topic/zynq/board.c | 39 >>> +++++++++++++++++++++++++++++++++++++++ >>> configs/topic_miami_defconfig | 1 + >>> configs/topic_miamiplus_defconfig | 1 + 3 files changed, 41 >>> insertions(+) >>> >>> diff --git a/board/topic/zynq/board.c b/board/topic/zynq/board.c >>> index a95c9d1..8a5765e 100644 --- a/board/topic/zynq/board.c +++ >>> b/board/topic/zynq/board.c @@ -1 +1,40 @@ +/* + * (C) Copyright >>> 2016 Topic Embedded Products + * + * SPDX-License-Identifier: >>> GPL-2.0+ + */ + +/* + * Miami boards can have memory sizes of >>> 256M, 512M or 1GB. To prevent needing + * separate bootloaders >>> for each variant, just detect the RAM size at boot time + * >>> instead of relying on the devicetree information. + */ +#define >>> CONFIG_SYS_SDRAM_BASE 0 +#define CONFIG_SYS_SDRAM_SIZE >>> topic_get_sdram_size() >> >> I am not happy with this but I see where you go. > > Of the several options I tried to "overload" the memory init > functions, this was basically the lesser evil... > > Maybe it would be possible to make some changes to the generic Zynq > board.c file to facilitate overlaying the code? > > Note that the memory detection would work on any board, not just the > Topic boards. I agree that devicetree is considered the best thing > since frozen pizza, but for the Zynq it looks as though detection is > much simpler than static configuration. > >> >>> +#define CONFIG_SYS_SDRAM_SIZE_MAX 0x40000000u + +static unsigned >>> int topic_get_sdram_size(void); + #include >>> "../../xilinx/zynq/board.c" + +#include <fdt_support.h> + +int >>> ft_board_setup(void *blob, bd_t *bd) +{ + >>> fdt_fixup_memory(blob, (u64)CONFIG_SYS_SDRAM_BASE, >>> (u64)gd->ram_size); + + return 0; +} >> >> This action is taken at arch_fixup_fdt(). Can you please confirm >> that this is really needed? And it is not done there? That you >> don't duplicate stuff here. > > If I omitted this step, the Linux devicetree would not be adjusted > (during bootm) and would still report its default. I needed both > arch_fixup_fdt and this extra line to properly adjust the Linux > devicetree for booting. > > >> >>> + +void dram_init_banksize(void) +{ + gd->bd->bi_dram[0].start >>> = CONFIG_SYS_SDRAM_BASE; + gd->bd->bi_dram[0].size = >>> gd->ram_size; +} >> >> This should be fine. >> > > A note here, if you compile for Zynq with CONFIG_SYS_SDRAM_BASE and > CONFIG_SYS_SDRAM_SIZE set, there will be no implementation of > 'dram_init_banksize' and the system will fail to boot. > > It might be wise to put this snippet into the zynq/board.c in the > "#else" clause. I'll send a separate patch for that if you agree. > > >>> + +unsigned int topic_get_sdram_size(void) +{ + /* Detect and >>> fix ram size */ + return get_ram_size((void >>> *)CONFIG_SYS_SDRAM_BASE, + >>> CONFIG_SYS_SDRAM_SIZE_MAX); >> >> In the first look I didn't know that get_ram_size is u-boot >> function for memsize detection. > > Indeed, the name "get_ram_size" doesn't really match what it actually > does, but the function's documentation is quite good. > > I found "get_ram_size" and "ft_board_setup" while browsing the code > of other boards to see how they handled dynamic configuration.
+1 That's how things works in U-Boot for years... -- Regards, Igor. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot