[U-Boot] [PATCH] image: Implement IH_TYPE_KERNEL_REL

Stephen Warren swarren at nvidia.com
Fri Oct 7 18:11:58 CEST 2011


Wolfgang Denk wrote at Friday, October 07, 2011 12:51 AM:
> Dear Stephen Warren,
> 
> In message <1317940584-19528-1-git-send-email-swarren at nvidia.com> you wrote:
> > uImage files contain absolute "load" and "entry" addresses. Such a concept
> > is incompatible with using the same kernel image on multiple SoCs, each with
> > a potentially different SDRAM base. To support that, create a new image type
> > IH_TYPE_KERNEL_REL, which is handled identically to IH_TYPE_KERNEL, except
> > that the "load" and "entry" properties are an offset from the base of SDRAM,
> > rather than an absolute address.
> >
> > "An offset from the base of SDRAM" is specified as:
> > a) CONFIG_SYS_SDRAM_BASE, if set.
> > b) Otherwise, for ARM, the start address of the first bank of SDRAM known
> >    to U-Boot.
> > c) Otherwise, 0.
> 
> I agree with Kumar: it should be sufficient to have this omment in
> the code.

OK. I was trying to make the commit comment standalone so people could
read and understand it without having to go read the whole patch to find
the comment too. I guess I can drop it from the commit description if
you want.

> But please define "first bank" - does it mena the firs one
> initialized, or the lowest start address, or the lowest chip select,
> or ... ?

It's the following currently:

gd->bd->bi_dram[0].start

* How would you describe this; "the first DRAM bank registered with
U-Boot"?

* Is this a good value to use, or should getenv_bootm_low() search through
all banks to find the one with lowest address or something?

> Also, I think you are right, and we should add IH_TYPE_FLATDT_REL as
> well.

OK.

...
> > - * fit_image_get_load - get load address property for a given component image node
> > + * fit_image_get_load_raw - get raw load address property for a given component image node
> 
> Line too long. [Did you run checkpatch?]

Yes, but I though (evidently incorrectly) that the first sentence of
function comments was supposed to be one line for grep'ability.

...
> Hm... this appears to add some additional code.  How much does the
> size grow?

At least when building for Tegra, this adds 576 bytes text, and 136 bytes
rodata. That's about a 0.4% size increase to each section.

> Can we make support for relative images optional?

I guess it's possible. It'll mean a heck of a lot of ifdef's in the
middle of all those multi-test if statements that work on a variety of
image types though. Are the size increases above as large as you feared;
is it worth making it optional?

Thanks for the review.

-- 
nvpublic



More information about the U-Boot mailing list