From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Thu, 27 Aug 2015 17:47:35 -0400 Subject: [U-Boot] [PATCH] common/image.c: Make boot_get_ramdisk() perform a check for Android images In-Reply-To: References: <1440704561-17015-1-git-send-email-trini@konsulko.com> Message-ID: <20150827214735.GE25532@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Thu, Aug 27, 2015 at 04:04:30PM -0500, Rob Herring wrote: > On Thu, Aug 27, 2015 at 2:42 PM, Tom Rini 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 > > Reported-by: Paul Kocialkowski > > Signed-off-by: Tom Rini > > --- > > 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 ". 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: