On Tue, Apr 19, 2016 at 11:13:42AM +0100, Peter Griffin wrote:
[...]
If so, then later if we need reserve more memory regions then we need
> So I think if the memory has to be reserved in this way (via the memory
> node),
> the correct (although not pretty) way to do it will be for U-Boot to
> represent the dram banks
> as they are currently being described in your DT patch. That way the memory
> DT
> node will be correctly re-generated based on U-Boots view of the dram banks
> when booting the kernel. Something like this: -
>
> diff --git a/board/hisilicon/hikey/hikey.c b/board/hisilicon/hikey/hikey.c
> index 3b484a9..8570972 100644
> --- a/board/hisilicon/hikey/hikey.c
> +++ b/board/hisilicon/hikey/hikey.c
> @@ -71,6 +71,8 @@ U_BOOT_DEVICES(hi6220_gpios) = {
>
> DECLARE_GLOBAL_DATA_PTR;
>
> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
> +
> static const struct pl01x_serial_platdata serial_platdata = {
> #if CONFIG_CONS_INDEX == 1
> .base = HI6220_UART0_BASE,
> @@ -87,6 +89,7 @@ U_BOOT_DEVICE(hikey_seriala) = {
> .name = "serial_pl01x",
> .platdata = &serial_platdata,
> };
> +#endif
>
> static struct mm_region hikey_mem_map[] = {
> {
> @@ -407,8 +410,35 @@ int dram_init(void)
>
> void dram_init_banksize(void)
> {
> + /*
> + * Reserve below regions from DT memory node (which gets generated
> + * by U-Boot from the dram banks before booting the kernel.
> + *
> + * 0x05e0,0000 - 0x05ef,ffff: MCU firmware runtime using
> + * 0x05f0,1000 - 0x05f0,1fff: Reboot reason
> + * 0x06df,f000 - 0x06df,ffff: Mailbox message data
> + * 0x0740,f000 - 0x0740,ffff: MCU firmware section
> + * 0x21f0,0000 - 0x21ff,ffff: pstore/ramoops buffer
> + * 0x3e00,0000 - 0x3fff,ffff: OP-TEE
> + */
> +
> gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> - gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
> + gd->bd->bi_dram[0].size = 0x05e00000;
> +
> + gd->bd->bi_dram[1].start = 0x05f00000;
> + gd->bd->bi_dram[1].size = 0x00001000;
> +
> + gd->bd->bi_dram[2].start = 0x05f02000;
> + gd->bd->bi_dram[2].size = 0x00efd000;
> +
> + gd->bd->bi_dram[3].start = 0x06e00000;
> + gd->bd->bi_dram[3].size = 0x0060f000;
> +
> + gd->bd->bi_dram[4].start = 0x07410000;
> + gd->bd->bi_dram[4].size = 0x1aaf0000;
> +
> + gd->bd->bi_dram[5].start = 0x22000000;
> + gd->bd->bi_dram[5].size = 0x1c000000;
> }
maintain in 3 places: kernel's dts, uboot and uefi :(
> I have a couple of questions though: -
>
> 1) Why can't we use memreserve or reserved-memory properties for some of
> these
> (especially for things like reboot-reason / mbox buf)? I can understand
By using memreserve, kernel will map the memory node as usual but just
don't allocte it from memory block. So if later other driver module
maps it, then that means mapping the reserved region twice. Mark
reminded this will introduce cache alias issue [1].
For reserved-memory, actually EPAPR has not defined this node.
[1] http://archive.arm.linux.org.uk/lurker/message/20160118.154259.eebaf8cd.en.html
[2] http://archive.arm.linux.org.uk/lurker/message/20151105.161315.cc3c99f5.en.html
> there maybe reasons for reserving op-tee memory via the memory node, but
> I'm
> not so sure about some of the other reservations.
Due Hi6220 there have some hardcode to reserve some memory regions
which used by MCU firmware, which is usually is used by power
management related stuffs.
> FYI U-Boot already correctly
> handles memreserve regions found in the DT (e.g. it won't place a initrd,
> or fdt blob
> in a reserved region).
Thanks for sharing this.
>
> 2) If we do have to have some/all memory carveout regions represented in
> the memory node,
> do we have to have these reserved regions spread throughout the memory map?
> It would be much tidier to have one chunk for example 64Mb which is
> reserved at the end of RAM. I guess
> the mcuimage is statically linked by hisilicon at the address currently
> reserved for it, but
> others like mbox-buf, reboot reason etc presumably could be moved?
mbox-buf also is used by MCU :)
MCU will reserve memory regions for itself code and data sections, but
it also reserve mbox-buf which used to communicate message between CA53
and MCU.
> This would be similar to how tegra U-Boot is operating (which is also
> taking the same
> approach as I have described here for overcoming this problem), and has
> some memory
> reserved for the secure monitor which is not described to the kernel DT
> memory node.
>
> See
>
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-tegra/board2.c;h=ac274e17e8bd7deb7e3d9c7e392a7a6bbfeeab89;hb=HEAD#l282
>
> and in particular the comment here
>
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/mach-tegra/board2.c;h=ac274e17e8bd7deb7e3d9c7e392a7a6bbfeeab89;hb=HEAD#l337
>
>
>
> > > This should also help with detecting if we are a 1gb/2gb board etc.
> >
> > Could do this by setting cmdline's mem=XXX, so can remain intact for
> > DTB? Finally we can only maintain DTB in only one place (in kernel).
> >
>
> I can certainly see why that would be preferable, but the U-Boot project
> seems to import copies of the DT from the kernel into the U-Boot tree.
> Also there can be some variation, as my HiKey U-Boot DT patches show
> for example the "arm,pl011" UART needs an extra <clock> DT property
> in U-Boot which isn't present in the kernel DT to work. So in short I'm not
> sure it is possible to only have it in one place.
So does U-boot also need use DT to initialize uart? If so, could we
change DT documentation in kernel so can describe hardware properly?
> This is one reason for wanting the DT to merge in the upstream kernel
> before importing and converting U-Boot to be configured from DT. That
> way we can ensure the DT bindings will match exactly for the hi6220
> SoC devices.
From my understanding, DTB is a description for hardware. So it should
be consistent for one specific platform.
>From ARMv8 acutally we have good chance to unify booting sequence (ARMv7
is hard to unify booting sequence due hardware have not been unified,
some SoCs can directly run in secure world but some will switch to normal
world).
So I checked Juno's code in U-boot; it uses your suggested method to
carve out reserved memory from memory bank but not really use dtb's
memory node.