On Thu, Aug 27, 2015 at 04:04:30PM -0500, Rob Herring wrote: > On Thu, Aug 27, 2015 at 2:42 PM, Tom Rini <[email protected]> wrote: > > In 2dd4632 the check for where a ramdisk is found on an Android image > > was got moved into the "normal" loop here, causing people to have to > > pass the kernel address in the ramdisk address location in order to have > > Android boot still. This changed previous behavior so perform a check > > early in the function to see if we have an Android image and if so use > > that as where to look for the ramdisk (which is what the rest of the > > code here expects). > > > > Cc: Rob Herring <[email protected]> > > Reported-by: Paul Kocialkowski <[email protected]> > > Signed-off-by: Tom Rini <[email protected]> > > --- > > common/image.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/common/image.c b/common/image.c > > index ca721c5..e938bea 100644 > > --- a/common/image.c > > +++ b/common/image.c > > @@ -907,6 +907,15 @@ int boot_get_ramdisk(int argc, char * const argv[], > > bootm_headers_t *images, > > if (argc >= 2) > > select = argv[1]; > > Perhaps this check should come second so you could override the > ramdisk with "bootm <bootimg> <ramdisk>". Then again, maybe people > should have to pick between a bootimg or separate components.
Yeah, no, I'm not convinced there's a good case for "Android image
kernel+ramdisk 1" kernel + "Android image+ramdisk 2" ramdisk where you
wouldn't be cobbling that particular combination together outside of
U-Boot anyhow. Is there really? And we still allow for disabling the
ramdisk. Or of course just loading separate components and booting
Android that way.
> > +#ifdef CONFIG_ANDROID_BOOT_IMAGE
> > + /*
> > + * Look for an Android boot image.
> > + */
> > + buf = map_sysmem(images->os.start, 0);
> > + if (genimg_get_format(buf) == IMAGE_FORMAT_ANDROID)
> > + select = argv[0];
> > +#endif
>
> Tracing code paths in these functions is bad enough. I would do
> something like this to simplify the code path:
>
> buf = map_sysmem(images->os.start, 0);
> if (genimg_get_format(buf) == IMAGE_FORMAT_ANDROID) {
> ret = android_image_get_ramdisk((void *)images->os.start, rd_start,
> &rd_len);
> *rd_end = *rd_start + rd_len;
> return ret;
> }
>
> And then remove the case statement. We loose dataflash copy and some
> debug prints.
But then we're also saying Android images are a special case that needs
to be treated differently than everyone else here.
--
Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
