Re: [PATCH] imx8m{m,n}_venice: update env memory layout

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3752 bytes --]

On Thu, Mar 10, 2022 at 07:59:30AM -0800, Tim Harvey wrote:
> On Sun, Feb 27, 2022 at 4:22 AM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Tim,
> >
> > [Adding Tom on Cc]
> >
> > On Sat, Feb 26, 2022 at 6:37 PM Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Sat, Feb 26, 2022 at 5:15 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > On Fri, Feb 18, 2022 at 8:20 PM Tim Harvey <tharvey@gateworks.com> wrote:
> > > > >
> > > > > Update distro config env memory layout:
> > > > >  - loadaddr=0x48200000 allows for 130MB area for uncompressing (ie FIT
> > > > >    images, kernel_comp_addr_r)
> > > > >  - fdt_addr_r = loadaddr + 128MB - allows for 128MB kernel
> > > > >  - scriptaddr = fdt_addr_r + 512KB - allows for 512KB fdt
> > > > >  - ramdisk_addr_r = scriptaddr + 512KB - allows for 512KB script
> > > > >
> > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > > >
> > > > What about following the suggestion from Heiko at:
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20220216122514.1659879-2-heiko.thiery@gmail.com/
> > >
> > > Fabio,
> > >
> > > I don't understand why include/configs/ti_armv7_common.h is always
> > > recommended when it comes to address maps. The comment there doesn't
> > > read that well and there are several addresses that I believe are
> > > outdated or uncommon (fdtaddr,rdaddr are outdated and it seems
> > > loadaddr could be used for dtboaddr)
> > >
> > > In my opinion what is relevant when choosing an addressing scheme is
> > > how much space you want to allow for kernel, fdt, scripts, ramdisk.
> > >
> > > ti_armv7_common.h allows for
> > > kernel/loadaddr:96M (too small in my opinion on modern boards)
> > > fdt:512M (makes sense)
> > > script:32M (excessive, and I don't understand why this is below
> > > loadaddr vs stacked on top of loadaddr like fdt)
> > > ramdisk: depends on mem size as its the highest addr (makes sense)
> > >
> > > For my more modern boards with typically 1-4GiB of dram, I wanted to
> > > allow for more than 96M for a kernel (kernel+ramdisk, fit image etc)
> > > as I commonly exceed 96M when using a buildroot kernel+rootfs that has
> > > something like gstreamer support for imx6/imx8 which is why I came up
> > > with a different scheme which I document well in the commit log as
> > >  - loadaddr=0x48200000 allows for 130MB area for uncompressing (ie FIT
> > >    images, kernel_comp_addr_r)
> > >  - fdt_addr_r = loadaddr + 128MB - allows for 128MB kernel
> > >  - scriptaddr = fdt_addr_r + 512KB - allows for 512KB fdt
> > >  - ramdisk_addr_r = scriptaddr + 512KB - allows for 512KB script
> >
> > You bring good points and maybe we could use
> > include/configs/imx8mm_venice.h as a good standard to follow :-)
> >
> > Reviewed-by: Fabio Estevam <festevam@denx.de>
> 
> Tom,
> 
> DId you have any feedback on this conversation about memory addresses?
> I'm also wondering if anyone knows how we can define them to make it a
> bit clearer about their sizes (some sort of macro foo that allows us
> to defined them as sizes on top of a previous address).

Alright, so, yeah, at this point it's true that ti_armv7_common.h could
use some re-evaluation since it's been a while.  What's probably the
best path forward is something under doc/develop/ that explains what the
important environment variables are, general max sizes and links to
where those constraints come from and then an example or two.
Especially since 32 and 64bit ARM have some different constraints (I
want to say that the 96MB kernel size was a practical limitation, but
would need to re-read the kernel docs and maybe check some notes again).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]