Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3488 bytes --]

On Fri, Jan 15, 2016 at 08:53:06AM +0800, Jeffy Chen wrote:
> Hi Tom,
> 
> On 2016-1-15 0:22, Tom Rini wrote:
> >On Thu, Jan 14, 2016 at 10:31:34AM +0800, Jeffy Chen wrote:
> >>Hi Tom,
> >>
> >>On 2016-1-13 23:28, Tom Rini wrote:
> >>>On Wed, Jan 13, 2016 at 04:53:19PM +0800, Jeffy Chen wrote:
> >>>
> >>>>The android kernel is using appended dtb by default, and store
> >>>>ramdisk right after kernel & dtb.
> >>>>So we needs to relocate ramdisk, and use atags to pass params.
> >>>>
> >>>>Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> >>>>Acked-by: Simon Glass <sjg@chromium.org>
> >>>>---
> >>>>
> >>>>Changes in v4: None
> >>>>Changes in v3: None
> >>>>Changes in v2: None
> >>>>
> >>>>  include/configs/kylin_rk3036.h | 23 +++++++++++++++++++++++
> >>>>  1 file changed, 23 insertions(+)
> >>>>
> >>>>diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h
> >>>>index b750b26..49997ec 100644
> >>>>--- a/include/configs/kylin_rk3036.h
> >>>>+++ b/include/configs/kylin_rk3036.h
> >>>>@@ -35,6 +35,29 @@
> >>>>  #undef CONFIG_EXTRA_ENV_SETTINGS
> >>>>  #define CONFIG_EXTRA_ENV_SETTINGS \
> >>>>  	"partitions=" PARTS_DEFAULT \
> >>>>+	"mmcdev=0\0" \
> >>>>+	"mmcpart=5\0" \
> >>>>+	"loadaddr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> >>>>+
> >>>>+#define CONFIG_ANDROID_BOOT_IMAGE
> >>>>+#define CONFIG_SYS_BOOT_RAMDISK_HIGH
> >>>This should already be set.
> >>Right, i'll remove it...
> >>>>+#define CONFIG_SYS_HUSH_PARSER
> >>>>+
> >>>>+#undef CONFIG_BOOTCOMMAND
> >>>>+#define CONFIG_BOOTCOMMAND \
> >>>>+	"mmc dev ${mmcdev}; if mmc rescan; then " \
> >>>>+		"part start mmc ${mmcdev} ${mmcpart} boot_start;" \
> >>>>+		"part size mmc ${mmcdev} ${mmcpart} boot_size;" \
> >>>>+		"mmc read ${loadaddr} ${boot_start} ${boot_size};" \
> >>>>+		"bootm start ${loadaddr}; bootm ramdisk;" \
> >>>>+		"bootm prep; bootm go;" \
> >>>>+	"fi;" \
> >>>>+
> >>>>+/* Enable atags */
> >>>>+#define CONFIG_SYS_BOOTPARAMS_LEN	(64*1024)
> >>>>+#define CONFIG_INITRD_TAG
> >>>>+#define CONFIG_SETUP_MEMORY_TAGS
> >>>>+#define CONFIG_CMDLINE_TAG
> >>>But I'm confused as to what exactly is going on here.  Appended dtb is
> >>>not the same as ATAGS.  And you shouldn't need to split up bootm like
> >>>that.  Can you please explain a bit more?  Thanks!
> >>The u-boot will pass atags to kernel, and kernel will merge those
> >>atags into the appended dtb(fdt).
> >>
> >>The default bootm flow would not pass ramdisk state, but we need it,
> >>so we should add this state into default flow, or just use split
> >>bootm cmds :)
> >That seems very strange.  Is the ramdisk concatenated with the kernel
> >and dtb as well (and that's why bootm ramdisk somehow finds it but
> >normal bootm doesn't as you aren't passing in a ramdisk address) ?
> Yes, the ramdisk concatenated with the kernel and dtb as
> well(u-boot/include/android_image.h: struct andr_img_hdr).
> 
> And the normal bootm cmd would find it by parsing andr_img_hdr struct.
> But we still need bootm ramdisk state, because it will call
> boot_ramdisk_high to relocate ramdisk area :)
> 
> I found if not relocate it to somewhere else, it would be corrupted
> after kernel's decompressing(during update fdt area).

So 'bootm $loadaddr' of an Android image sees, but does not relocate the
ramdisk that is included in the image, but bootm ramdisk does?  That
sounds like a bug in the regular bootm handling.

-- 
Tom

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 134 bytes --]

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot