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

2016-02-19 Thread Simon Glass 
+Masahiro

On 2 February 2016 at 00:13, Jeffy Chen  wrote:
> Hi Tom,
>
> Sorry for being late..
>
>
> On 2016-1-26 3:07, Tom Rini wrote:
>>
>> On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
>>>
>>> Hi Tom,
>>>
>>> On 2016-1-15 8:59, Tom Rini wrote:

 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 
> Acked-by: Simon Glass 
> ---
>
> 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.
>>>
>>> Yep, the default bootm flow would not contain ramdisk relocate state:
>>>
>>> vi common/cmd_bootm.c
>>>  return do_bootm_states(cmdtp, flag, argc, argv,
>>> BOOTM_STATE_START |
>>>  BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
>>>  BOOTM_STATE_LOADOS |
>>> #if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
>>>  BOOTM_STATE_OS_CMDLINE |
>>> #endif
>>>  BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
>>>  BOOTM_STATE_OS_GO, , 1);
>>>
>>> But i'm not sure if it's ok to add it here...
>>
>> So, I've poked aruond a bit more on some of my TI platforms at least.
>> Can you look at the following things on yours?
>> 1) Do you still see the problem on top of tree?

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

2016-02-01 Thread Jeffy Chen 

Hi Tom,

Sorry for being late..

On 2016-1-26 3:07, Tom Rini wrote:

On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:

Hi Tom,

On 2016-1-15 8:59, Tom Rini wrote:

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 
Acked-by: Simon Glass 
---

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.

Yep, the default bootm flow would not contain ramdisk relocate state:

vi common/cmd_bootm.c
 return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
 BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
 BOOTM_STATE_LOADOS |
#if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
 BOOTM_STATE_OS_CMDLINE |
#endif
 BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
 BOOTM_STATE_OS_GO, , 1);

But i'm not sure if it's ok to add it here...

So, I've poked aruond a bit more on some of my TI platforms at least.
Can you look at the following things on yours?
1) Do you still see the problem on top of tree? Masahiro fixed at least
one problem just before v2016.01
Yes, after rebase to tag "v2016.01-rc4", i could still repro it with the 
normal bootm cmd.


It seems we would call boot_ramdisk_high if fdt enabled in the prep stage:

 vi common/image.c

#ifdef CONFIG_LMB
int image_setup_linux(bootm_headers_t *images)
{
...
if (IMAGE_ENABLE_RAMDISK_HIGH) {
rd_len = images->rd_end - images->rd_start;
ret = boot_ramdisk_high(lmb, images->rd_start, rd_len,
initrd_start, initrd_end);
if (ret)
return ret;
}

 vi arch/arm/lib/bootm.c

/* Subcommand: PREP */
static void boot_prep_linux(bootm_headers_t *images)

if (IMAGE_ENABLE_OF_LIBFDT && images->ft_len) {
#ifdef CONFIG_OF_LIBFDT
debug("using: FDT\n");
if (image_setup_linux(images)) {
printf("FDT creation failed! hanging...");
hang();

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

2016-01-25 Thread Tom Rini 
On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
> Hi Tom,
> 
> On 2016-1-15 8:59, Tom Rini wrote:
> >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 
> >>Acked-by: Simon Glass 
> >>---
> >>
> >>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.
> Yep, the default bootm flow would not contain ramdisk relocate state:
> 
> vi common/cmd_bootm.c
> return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
> BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
> BOOTM_STATE_LOADOS |
> #if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
> BOOTM_STATE_OS_CMDLINE |
> #endif
> BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
> BOOTM_STATE_OS_GO, , 1);
> 
> But i'm not sure if it's ok to add it here...

So, I've poked aruond a bit more on some of my TI platforms at least.
Can you look at the following things on yours?
1) Do you still see the problem on top of tree? Masahiro fixed at least
one problem just before v2016.01
2) Instead of fdt_high / initrd_high can you look at bootm_low /
bootm_size ?  include/configs/ti_armv7_common.h has these along with a
big comment about what the overall constraints are.

Thanks!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-01-25 Thread Tom Rini 
On Fri, Jan 15, 2016 at 04:42:21PM +0100, Daniel Schwierzeck wrote:
> Am Freitag, den 15.01.2016, 09:42 -0500 schrieb Tom Rini:
> > On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
> > > Hi Tom,
> > > 
> > > On 2016-1-15 8:59, Tom Rini wrote:
> > > > 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 
> > > > > > > > > Acked-by: Simon Glass 
> > > > > > > > > ---
> > > > > > > > > 
> > > > > > > > > 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.
> > > Yep, the default bootm flow would not contain ramdisk

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

2016-01-21 Thread Simon Glass 
Hi,

On 15 January 2016 at 18:18, Simon Glass  wrote:
> Hi,
>
> On 15 January 2016 at 08:42, Daniel Schwierzeck
>  wrote:
>> Am Freitag, den 15.01.2016, 09:42 -0500 schrieb Tom Rini:
>>> On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
>>> > Hi Tom,
>>> >
>>> > On 2016-1-15 8:59, Tom Rini wrote:
>>> > > 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 
>>> > > > > > > > Acked-by: Simon Glass 
>>> > > > > > > > ---
>>> > > > > > > >
>>> > > > > > > > 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
>>> > >

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

2016-01-15 Thread Tom Rini 
On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
> Hi Tom,
> 
> On 2016-1-15 8:59, Tom Rini wrote:
> >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 
> >>Acked-by: Simon Glass 
> >>---
> >>
> >>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.
> Yep, the default bootm flow would not contain ramdisk relocate state:
> 
> vi common/cmd_bootm.c
> return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
> BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
> BOOTM_STATE_LOADOS |
> #if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
> BOOTM_STATE_OS_CMDLINE |
> #endif
> BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
> BOOTM_STATE_OS_GO, , 1);
> 
> But i'm not sure if it's ok to add it here...

Actually, maybe so.  This made me do some digging again back into
Simon's series that refactored the bootm code.  In the long long ago,
nearly every architecture would call bootm_ramdisk_high to relocate the
ramdisk and set the environment (and poke the DT).  But it wasn't always
clear when / why it would do this.  And it got consolidated.  And here's
where it's tricky.  It looks correct today, still, to make sure that we
relocate the ramdisk.  image_setup_linux() checks
IMAGE_ENABLE_RAMDISK_HIGH which is toggled by
CONFIG_SYS_BOOT_RAMDISK_HIGH.  But for example, MIPS whacked _back_ in a
local call to make sure the ramdisk was being relocated because it
wasn't

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

2016-01-15 Thread Daniel Schwierzeck 
Am Freitag, den 15.01.2016, 09:42 -0500 schrieb Tom Rini:
> On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
> > Hi Tom,
> > 
> > On 2016-1-15 8:59, Tom Rini wrote:
> > > 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 
> > > > > > > > Acked-by: Simon Glass 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > 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.
> > Yep, the default bootm flow would not contain ramdisk relocate
> > state:
> > 
> > vi common/cmd_bootm.c
> > return do_bootm_states(cmdtp, flag, argc, argv,
> > BOOTM_STATE_START |
> > BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
> > BOOTM_STATE_LOADOS |
> > #if defined(CONFIG_PPC)

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

2016-01-15 Thread Simon Glass 
Hi,

On 15 January 2016 at 08:42, Daniel Schwierzeck
 wrote:
> Am Freitag, den 15.01.2016, 09:42 -0500 schrieb Tom Rini:
>> On Fri, Jan 15, 2016 at 10:20:43AM +0800, Jeffy Chen wrote:
>> > Hi Tom,
>> >
>> > On 2016-1-15 8:59, Tom Rini wrote:
>> > > 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 
>> > > > > > > > Acked-by: Simon Glass 
>> > > > > > > > ---
>> > > > > > > >
>> > > > > > > > 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.
>> > Yep, the default bootm flow would not contain ramdisk relocate
>> > state:
>> >
>> > vi common/cmd_bootm.c
>> > return

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

2016-01-14 Thread Tom Rini 
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 
> >>Acked-by: Simon Glass 
> >>---
> >>
> >>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) ?

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-01-14 Thread Jeffy Chen 

Hi Tom,

On 2016-1-15 8:59, Tom Rini wrote:

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 
Acked-by: Simon Glass 
---

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.

Yep, the default bootm flow would not contain ramdisk relocate state:

vi common/cmd_bootm.c
return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
BOOTM_STATE_LOADOS |
#if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
BOOTM_STATE_OS_CMDLINE |
#endif
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
BOOTM_STATE_OS_GO, , 1);

But i'm not sure if it's ok to add it here...

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

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

2016-01-14 Thread Tom Rini 
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 
> Acked-by: Simon Glass 
> ---
> 
> 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


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2016-01-14 Thread Jeffy Chen 

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 
Acked-by: Simon Glass 
---

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).


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

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

2016-01-13 Thread Jeffy Chen 

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 
Acked-by: Simon Glass 
---

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 :)


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

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

2016-01-13 Thread Tom Rini 
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 
> Acked-by: Simon Glass 
> ---
> 
> 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.

> +#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!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot