Wolfgang Denk wrote at Friday, October 07, 2011 12:51 AM: > Dear Stephen Warren, > > In message <1317940584-19528-1-git-send-email-swar...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
