[U-Boot] [PATCH 2/2] boards: ls2080: Disable fdt copying by default

[Scott Wood]

On Tue, 2016-03-01 at 16:48 +0000, york sun wrote:
> On 03/01/2016 08:01 AM, Scott Wood wrote:
> > On Tue, 2016-03-01 at 06:03 +0000, Bhupesh Sharma wrote:
> > > > From: york sun
> > > > Sent: Tuesday, March 01, 2016 11:30 AM
> > > > 
> > > > On 02/29/2016 09:20 PM, Bhupesh Sharma wrote:
> > > > > > From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of
> > > > > > Scott
> > > > > > Wood
> > > > > > Sent: Tuesday, March 01, 2016 7:13 AM
> > > > > > 
> > > > > > On Tue, 2016-03-01 at 00:08 +0000, york sun wrote:
> > > > > > > Sorry for top posting. I am on outlook web access.
> > > > > > > 
> > > > > > > There may be some limitation on fdt relocation. Without setting
> > > > > > > fdt_high, u -boot relocates the device tree toward the end of
> > > > > > > useable memory. I haven't got a chance to debug why it doesn't
> > > > > > > work.
> > > > > > > 
> > > > > > > This patch is to disable the relocation by default. A magic
> > > > > > > number
> > > > > > > 0xa0000000 doesn't make much sense here.
> > > > > > 
> > > > > > I agree, if the number is arbitrary.  But if Linux has a
> > > > > > limitation,
> > > > > > as it does on arm32, then it should be expressed here.
> > > > > > 
> > > > > 
> > > > > There are explicit requirements for placement of DTB for aarch64
> > > > > linux.
> > > > > Linux versions prior to v4.2 also require that the DTB be placed
> > > > > within the 512 MB region starting at text_offset bytes below the
> > > > > kernel
> > > > Image:
> > > > > 
> > > > > http://lxr.free-electrons.com/source/Documentation/arm64/booting.txt
> > > > > #L
> > > > > 43
> > > > > 
> > > > > York - have you tested this patch against older kernels like 3.18?
> > > > 
> > > > Bhupesh,
> > > > 
> > > > Thanks for the link. It may explained why Linux doesn't boot if
> > > > fdt_high
> > > > is not set properly on kernel prior 4.2.
> > > > 
> > > > I proposed to disable fdt copying by default. It doesn't mean we
> > > > cannot
> > > > will won't use fdt_high. My point is, setting an arbitrary value
> > > > doesn't
> > > > make much sense.
> > 
> > It's not arbitrary if it comes from a Linux requirement.
> 
> Even kernel requires device tree to be within 512MB from kernel image, it
> doesn't warrant a magic number, because the kernel image location is not
> fixed.

It's not "512MB from kernel image".  It's "within the 512 MB region starting
at text_offset bytes below the kernel Image" which, when combined with "The
Image must be placed text_offset bytes from a 2MB aligned base address near
the start of usable system RAM and called there" means that if RAM starts at
0x80000000, the actual limit is "near" (and no lower than) 0xa0000000, which
makes 0xa0000000 a good default.

> > It could be in overlap with ramdisk, or something else.
> > 
> > How?  It's an upper limit.  It's not a directive to put the fdt at a
> > specific
> > address.  U-Boot knows where the ramdisk is and should avoid overlapping
> > them.
> 
> Yes, it is an upper limit. U-Boot will try to put device tree right under
> that
> limit, if possible, regardless where the kernel image is.

If it's ignoring the kernel image when selecting a location, that's a bug that
should be fixed.

> As for the overlapping, you could load FIT image into memory, near the
> location
> of fdt_high. U-Boot knows the location _after_ uncompressing/copying.

Knows which location after uncompressing/copying what?

In any case, having a good default location for the FIT is why kernel_load
should stay.

> > > > Since we are using FIT image for this board, it is easy to set correct
> > > > load address for kernel/ramdisk/dtb.  
> > 
> > How does FIT make that any easier?  Picking correct addresses is the same
> > challenge as before -- now it's just specified in a different location.
> 
> Picking correct address is the same challenges, but at least not twice.

Twice?

>  And the kernel address is also in FIT image, together with device tree 
> address. It may be less used by us, but the feature is there.

Not sure what you mean here.

> Please note the addresses 0x80_8008_0000, 0x80_a000_0000, 0x80_9000_0000 are
> put
> into the its file by me. I can specify any addresses as far as they meet
> kernel
> requirement. 

This does not meet the kernel requirement unless you consider 0x80_8000_0000
to be "near the start of usable system RAM", with any RAM below that unused by
Linux.

> In this case, the kernel load/entry address dictates where the
> device tree image should be. The best chance to make it right is to specify
> device tree address in the same its file, and use it in place. "fdt_high"
> totally defeats this purpose.

I don't think it's unreasonable for U-Boot to know what the start of system
RAM is supposed to be, and have the environment variables set accordingly. 
 You're trying to leave the start of system RAM up to the user, which is an
unusual situation and shouldn't be used to judge the merits of fdt_high in
general.  I can maybe see disabling relocation for this specific target due to
its problematic memory map, if we decide we need to support both, but that's a
different justification than "this is an arbitrary magic number".  I hope our
future chips have a conforming memory map that avoids the problem.

> > This is true regarding kernel_start/kernel_size, but not kernel_load which
> > tells you what a sane place would be to load an image, rather than
> > describing
> > an image that is already there.
> 
> The kernel_load is used by copying image from NOR flash into memory.

It's a destination for loading an image, regardless of where it's loaded from.

>  It is not
> totally insane, but not necessary. As I explained in above FIT image
> example, if
> the ramdisk load address is set correctly, there is no need to copy the
> image at
> the first place.

What does that have to do with it?

If I'm putting together a FIT image, how do I choose the addresses if I have
no idea where the user is going to load the FIT image itself?

Our ARM targets have too few helpful environment variables, not too many.  I
really miss what our PPC targets provided with nfsboot, etc. (there was room
for improvement there, but getting rid of it entirely just makes more work for
the user).

-Scott