[PATCH] ARM: imx6q_logic: Fix broken booting by moving fdt_addr_r address

[Adam Ford]

On Thu, Aug 20, 2020 at 7:17 AM Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Aug 19, 2020 at 02:11:08PM -0500, Adam Ford wrote:
>
> > The loading address is too close to the kernel address, so newer kernels
> > may overlap memory space, so loading the device tree may corrupt zImage.
> >
> > This patch moves the fdt_addr_r to 0x18000000 which is also consistent
> > with some other i.MX6Q boards.
> >
> > Signed-off-by: Adam Ford <aford173 at gmail.com>
> >
> > diff --git a/include/configs/imx6_logic.h b/include/configs/imx6_logic.h
> > index 63662dd18d..58862e4c49 100644
> > --- a/include/configs/imx6_logic.h
> > +++ b/include/configs/imx6_logic.h
> > @@ -34,7 +34,7 @@
> >       "script=boot.scr\0" \
> >       "image=zImage\0" \
> >       "bootm_size=0x10000000\0" \
> > -     "fdt_addr_r=0x13000000\0" \
> > +     "fdt_addr_r=0x18000000\0" \
> >       "ramdisk_addr_r=0x14000000\0" \
> >       "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \
> >       "ramdisk_file=rootfs.cpio.uboot\0" \
>
> Having the fdt above ramdisk is a bad idea.  To quote myself from
> include/configs/ti_armv7_common.h:

I didn't look at the TIstuff, because I'm running iMX6 from NXP.  I
wonder if we could move this to a more obvious place.

>  * We setup defaults based on constraints from the Linux kernel, which
>  * should also be safe elsewhere.  We have the default load at 32MB into
>  * DDR (for the kernel), FDT above 128MB (the maximum location for the
>  * end of the kernel), and the ramdisk 512KB above that (allowing for
>  * hopefully never seen large trees).  We say all of this must be within
>  * the first 256MB as that will normally be within the kernel lowmem and
>  * thus visible via bootm_size and we only run on platforms with 256MB
>  * or more of memory.

This makes a lot of sense to me.

I'll run some tests with
loadaddr = 0x12000000 (same address as before)
fdt_addr_r = 0x14000000 (32MB after loadaddr)
ramdisk_addr_r 0x14080000  (512MB after fdt_addr_r)

If everythiing looks good, and the board boots again, I'll submit a V2 patch.

adam
>
> With the fdt being placed above the ramdisk, it will get broken by a
> sufficiently large distro ramdisk and that ends up being a very annoying
> problem to debug.
>
> And as Linus Walleij's article the other week[1] noted, the zImage will
> relocate itself if there's overlap but the layout above avoids that.

That's an interesting read.  My problem is that reading the fdt blows
away part of the zImage before the relocation and decompression phase,
so I need to allocate more space to the kernel.

adam
>
> [1]: https://people.kernel.org/linusw/how-the-arm32-linux-kernel-decompresses
>
> --
> Tom