* [PATCH v4 0/6] rockchip: kylin: Boot with android boot image
@ 2016-01-13 8:53 ` Jeffy Chen
0 siblings, 0 replies; 54+ messages in thread
From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw)
To: u-boot-0aAXYlwwYIKGBzrmiIFOJg
Cc: Albert Aribaud, huang lin, Joe Hershberger, Jan Kiszka,
sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, Paul Kocialkowski,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Peng Fan,
Tom Warren, Max Krummenacher, York Sun
We are porting android to kylin board now.
This series could let it boot up with android's boot image.
Changes in v4:
Remove unused reboot mode definitions.
Changes in v3:
Use rockchip's legacy reboot mode definitions.
Changes in v2:
Add comments.
Jeffy Chen (6):
common/image-fdt.c: Make boot_get_fdt() perform a check for Android
images
ARM: bootm: Try to use relocated ramdisk
rockchip: rk3036: Bind GPIO banks
rockchip: kylin: Add default gpt partition table
rockchip: kylin: Enable boot with android boot image
rockchip: kylin: Check fastboot request
arch/arm/lib/bootm.c | 12 ++++++-
board/kylin/kylin_rk3036/kylin_rk3036.c | 32 ++++++++++++++++++
common/image-fdt.c | 4 +++
drivers/pinctrl/rockchip/pinctrl_rk3036.c | 8 +++++
include/configs/kylin_rk3036.h | 55 +++++++++++++++++++++++++++++++
5 files changed, 110 insertions(+), 1 deletion(-)
--
2.1.4
^ permalink raw reply [flat|nested] 54+ messages in thread* [U-Boot] [PATCH v4 0/6] rockchip: kylin: Boot with android boot image @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot We are porting android to kylin board now. This series could let it boot up with android's boot image. Changes in v4: Remove unused reboot mode definitions. Changes in v3: Use rockchip's legacy reboot mode definitions. Changes in v2: Add comments. Jeffy Chen (6): common/image-fdt.c: Make boot_get_fdt() perform a check for Android images ARM: bootm: Try to use relocated ramdisk rockchip: rk3036: Bind GPIO banks rockchip: kylin: Add default gpt partition table rockchip: kylin: Enable boot with android boot image rockchip: kylin: Check fastboot request arch/arm/lib/bootm.c | 12 ++++++- board/kylin/kylin_rk3036/kylin_rk3036.c | 32 ++++++++++++++++++ common/image-fdt.c | 4 +++ drivers/pinctrl/rockchip/pinctrl_rk3036.c | 8 +++++ include/configs/kylin_rk3036.h | 55 +++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 1 deletion(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 54+ messages in thread
[parent not found: <1452675200-15941-1-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 8:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot-0aAXYlwwYIKGBzrmiIFOJg Cc: Joe Hershberger, sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, Paul Kocialkowski, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Peng Fan, Max Krummenacher Android images don't have a fdt. Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> --- Changes in v4: None Changes in v3: None Changes in v2: None common/image-fdt.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/image-fdt.c b/common/image-fdt.c index 5e4e5bd..41aaa0d 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -379,6 +379,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, (long)fdt_addr); } break; +#ifdef CONFIG_ANDROID_BOOT_IMAGE + case IMAGE_FORMAT_ANDROID: + goto no_fdt; +#endif default: puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); goto no_fdt; -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot Android images don't have a fdt. 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 common/image-fdt.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/image-fdt.c b/common/image-fdt.c index 5e4e5bd..41aaa0d 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -379,6 +379,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, (long)fdt_addr); } break; +#ifdef CONFIG_ANDROID_BOOT_IMAGE + case IMAGE_FORMAT_ANDROID: + goto no_fdt; +#endif default: puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); goto no_fdt; -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 15:21 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:21 UTC (permalink / raw) To: Jeffy Chen, Rob Herring, Paul Kocialkowski Cc: u-boot, linux-rockchip, Max Krummenacher, Joe Hershberger [-- Attachment #1.1: Type: text/plain, Size: 1054 bytes --] On Wed, Jan 13, 2016 at 04:53:15PM +0800, Jeffy Chen wrote: > Android images don't have a fdt. > > 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 > > common/image-fdt.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index 5e4e5bd..41aaa0d 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -379,6 +379,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, > (long)fdt_addr); > } > break; > +#ifdef CONFIG_ANDROID_BOOT_IMAGE > + case IMAGE_FORMAT_ANDROID: > + goto no_fdt; > +#endif > default: > puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); > goto no_fdt; Hang on, this doesn't seem right. Rob or Paul can you comment more here? I know we've gone through some issues in the past with respect to booting Android and FDT. Thanks! -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images @ 2016-01-13 15:21 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:21 UTC (permalink / raw) To: u-boot On Wed, Jan 13, 2016 at 04:53:15PM +0800, Jeffy Chen wrote: > Android images don't have a fdt. > > 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 > > common/image-fdt.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index 5e4e5bd..41aaa0d 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -379,6 +379,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, > (long)fdt_addr); > } > break; > +#ifdef CONFIG_ANDROID_BOOT_IMAGE > + case IMAGE_FORMAT_ANDROID: > + goto no_fdt; > +#endif > default: > puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); > goto no_fdt; Hang on, this doesn't seem right. Rob or Paul can you comment more here? I know we've gone through some issues in the past with respect to booting Android and FDT. Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160113/f80d1ba1/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [U-Boot] [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images 2016-01-13 15:21 ` [U-Boot] " Tom Rini @ 2016-01-14 1:47 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-14 1:47 UTC (permalink / raw) To: Tom Rini, Rob Herring, Paul Kocialkowski Cc: u-boot-0aAXYlwwYIKGBzrmiIFOJg, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Max Krummenacher, Joe Hershberger Hi Tom, On 2016-1-13 23:21, Tom Rini wrote: > On Wed, Jan 13, 2016 at 04:53:15PM +0800, Jeffy Chen wrote: > >> Android images don't have a fdt. >> >> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> >> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> >> --- >> >> Changes in v4: None >> Changes in v3: None >> Changes in v2: None >> >> common/image-fdt.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/common/image-fdt.c b/common/image-fdt.c >> index 5e4e5bd..41aaa0d 100644 >> --- a/common/image-fdt.c >> +++ b/common/image-fdt.c >> @@ -379,6 +379,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, >> (long)fdt_addr); >> } >> break; >> +#ifdef CONFIG_ANDROID_BOOT_IMAGE >> + case IMAGE_FORMAT_ANDROID: >> + goto no_fdt; >> +#endif >> default: >> puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); >> goto no_fdt; > Hang on, this doesn't seem right. Rob or Paul can you comment more > here? I know we've gone through some issues in the past with respect to > booting Android and FDT. Thanks! Oh! How careless i was...Seems we don't need this patch, the default case is enough :) ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images @ 2016-01-14 1:47 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-14 1:47 UTC (permalink / raw) To: u-boot Hi Tom, On 2016-1-13 23:21, Tom Rini wrote: > On Wed, Jan 13, 2016 at 04:53:15PM +0800, Jeffy Chen wrote: > >> Android images don't have a fdt. >> >> 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 >> >> common/image-fdt.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/common/image-fdt.c b/common/image-fdt.c >> index 5e4e5bd..41aaa0d 100644 >> --- a/common/image-fdt.c >> +++ b/common/image-fdt.c >> @@ -379,6 +379,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, >> (long)fdt_addr); >> } >> break; >> +#ifdef CONFIG_ANDROID_BOOT_IMAGE >> + case IMAGE_FORMAT_ANDROID: >> + goto no_fdt; >> +#endif >> default: >> puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); >> goto no_fdt; > Hang on, this doesn't seem right. Rob or Paul can you comment more > here? I know we've gone through some issues in the past with respect to > booting Android and FDT. Thanks! Oh! How careless i was...Seems we don't need this patch, the default case is enough :) ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v4 2/6] ARM: bootm: Try to use relocated ramdisk 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 8:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot-0aAXYlwwYIKGBzrmiIFOJg Cc: Albert Aribaud, Joe Hershberger, Jan Kiszka, sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Peng Fan, Tom Warren, York Sun After boot_ramdisk_high(), ramdisk would be relocated to initrd_start & initrd_end, so use them instead of rd_start & rd_end. Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> --- Changes in v4: None Changes in v3: None Changes in v2: Add comments. arch/arm/lib/bootm.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index a477cae..0838d89 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -225,7 +225,17 @@ static void boot_prep_linux(bootm_headers_t *images) if (BOOTM_ENABLE_MEMORY_TAGS) setup_memory_tags(gd->bd); if (BOOTM_ENABLE_INITRD_TAG) { - if (images->rd_start && images->rd_end) { + /* + * In boot_ramdisk_high(), it may relocate ramdisk to + * a specified location. And set images->initrd_start & + * images->initrd_end to relocated ramdisk's start/end + * addresses. So use them instead of images->rd_start & + * images->rd_end when possible. + */ + if (images->initrd_start && images->initrd_end) { + setup_initrd_tag(gd->bd, images->initrd_start, + images->initrd_end); + } else if (images->rd_start && images->rd_end) { setup_initrd_tag(gd->bd, images->rd_start, images->rd_end); } -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 2/6] ARM: bootm: Try to use relocated ramdisk @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot After boot_ramdisk_high(), ramdisk would be relocated to initrd_start & initrd_end, so use them instead of rd_start & rd_end. 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: Add comments. arch/arm/lib/bootm.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index a477cae..0838d89 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -225,7 +225,17 @@ static void boot_prep_linux(bootm_headers_t *images) if (BOOTM_ENABLE_MEMORY_TAGS) setup_memory_tags(gd->bd); if (BOOTM_ENABLE_INITRD_TAG) { - if (images->rd_start && images->rd_end) { + /* + * In boot_ramdisk_high(), it may relocate ramdisk to + * a specified location. And set images->initrd_start & + * images->initrd_end to relocated ramdisk's start/end + * addresses. So use them instead of images->rd_start & + * images->rd_end when possible. + */ + if (images->initrd_start && images->initrd_end) { + setup_initrd_tag(gd->bd, images->initrd_start, + images->initrd_end); + } else if (images->rd_start && images->rd_end) { setup_initrd_tag(gd->bd, images->rd_start, images->rd_end); } -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v4 2/6] ARM: bootm: Try to use relocated ramdisk 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 15:27 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:27 UTC (permalink / raw) To: Jeffy Chen Cc: linux-rockchip, Joe Hershberger, Jan Kiszka, u-boot, Tom Warren, York Sun [-- Attachment #1.1: Type: text/plain, Size: 362 bytes --] On Wed, Jan 13, 2016 at 04:53:16PM +0800, Jeffy Chen wrote: > After boot_ramdisk_high(), ramdisk would be relocated to > initrd_start & initrd_end, so use them instead of rd_start & rd_end. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Acked-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Tom Rini <trini@konsulko.com> -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 2/6] ARM: bootm: Try to use relocated ramdisk @ 2016-01-13 15:27 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:27 UTC (permalink / raw) To: u-boot On Wed, Jan 13, 2016 at 04:53:16PM +0800, Jeffy Chen wrote: > After boot_ramdisk_high(), ramdisk would be relocated to > initrd_start & initrd_end, so use them instead of rd_start & rd_end. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Acked-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160113/27299b8b/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v4 3/6] rockchip: rk3036: Bind GPIO banks 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 8:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot-0aAXYlwwYIKGBzrmiIFOJg Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, huang lin Call dm_scan_fdt_node() in rk3036 pinctrl uclass binding. Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> --- Changes in v4: None Changes in v3: None Changes in v2: None drivers/pinctrl/rockchip/pinctrl_rk3036.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3036.c b/drivers/pinctrl/rockchip/pinctrl_rk3036.c index 581b096..1f78bf8 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3036.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3036.c @@ -15,6 +15,7 @@ #include <asm/arch/hardware.h> #include <asm/arch/periph.h> #include <dm/pinctrl.h> +#include <dm/root.h> DECLARE_GLOBAL_DATA_PTR; @@ -252,6 +253,12 @@ static struct pinctrl_ops rk3036_pinctrl_ops = { .get_periph_id = rk3036_pinctrl_get_periph_id, }; +static int rk3036_pinctrl_bind(struct udevice *dev) +{ + /* scan child GPIO banks */ + return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); +} + static int rk3036_pinctrl_probe(struct udevice *dev) { struct rk3036_pinctrl_priv *priv = dev_get_priv(dev); @@ -272,5 +279,6 @@ U_BOOT_DRIVER(pinctrl_rk3036) = { .of_match = rk3036_pinctrl_ids, .priv_auto_alloc_size = sizeof(struct rk3036_pinctrl_priv), .ops = &rk3036_pinctrl_ops, + .bind = rk3036_pinctrl_bind, .probe = rk3036_pinctrl_probe, }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 3/6] rockchip: rk3036: Bind GPIO banks @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot Call dm_scan_fdt_node() in rk3036 pinctrl uclass binding. 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 drivers/pinctrl/rockchip/pinctrl_rk3036.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3036.c b/drivers/pinctrl/rockchip/pinctrl_rk3036.c index 581b096..1f78bf8 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3036.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3036.c @@ -15,6 +15,7 @@ #include <asm/arch/hardware.h> #include <asm/arch/periph.h> #include <dm/pinctrl.h> +#include <dm/root.h> DECLARE_GLOBAL_DATA_PTR; @@ -252,6 +253,12 @@ static struct pinctrl_ops rk3036_pinctrl_ops = { .get_periph_id = rk3036_pinctrl_get_periph_id, }; +static int rk3036_pinctrl_bind(struct udevice *dev) +{ + /* scan child GPIO banks */ + return dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset, false); +} + static int rk3036_pinctrl_probe(struct udevice *dev) { struct rk3036_pinctrl_priv *priv = dev_get_priv(dev); @@ -272,5 +279,6 @@ U_BOOT_DRIVER(pinctrl_rk3036) = { .of_match = rk3036_pinctrl_ids, .priv_auto_alloc_size = sizeof(struct rk3036_pinctrl_priv), .ops = &rk3036_pinctrl_ops, + .bind = rk3036_pinctrl_bind, .probe = rk3036_pinctrl_probe, }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v4 3/6] rockchip: rk3036: Bind GPIO banks 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 15:28 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: Jeffy Chen; +Cc: u-boot, linux-rockchip [-- Attachment #1.1: Type: text/plain, Size: 287 bytes --] On Wed, Jan 13, 2016 at 04:53:17PM +0800, Jeffy Chen wrote: > Call dm_scan_fdt_node() in rk3036 pinctrl uclass binding. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Acked-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com> -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 3/6] rockchip: rk3036: Bind GPIO banks @ 2016-01-13 15:28 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: u-boot On Wed, Jan 13, 2016 at 04:53:17PM +0800, Jeffy Chen wrote: > Call dm_scan_fdt_node() in rk3036 pinctrl uclass binding. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Acked-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160113/b98b0fe5/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v4 4/6] rockchip: kylin: Add default gpt partition table 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 8:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot-0aAXYlwwYIKGBzrmiIFOJg Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, huang lin Add default android gpt partition table for kylin board. Use "gpt write mmc 0 $partitions" to apply. Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> --- Changes in v4: None Changes in v3: None Changes in v2: None include/configs/kylin_rk3036.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h index aa07889..b750b26 100644 --- a/include/configs/kylin_rk3036.h +++ b/include/configs/kylin_rk3036.h @@ -9,4 +9,33 @@ #include <configs/rk3036_common.h> +#ifndef CONFIG_SPL_BUILD + +/* Enable gpt partition table */ +#define CONFIG_CMD_GPT +#define CONFIG_RANDOM_UUID +#define CONFIG_EFI_PARTITION +#define PARTS_DEFAULT \ + "uuid_disk=${uuid_gpt_disk};" \ + "name=loader,start=32K,size=4000K,uuid=${uuid_gpt_loader};" \ + "name=reserved,size=64K,uuid=${uuid_gpt_reserved};" \ + "name=misc,size=4M,uuid=${uuid_gpt_misc};" \ + "name=recovery,size=32M,uuid=${uuid_gpt_recovery};" \ + "name=boot_a,size=32M,uuid=${uuid_gpt_boot_a};" \ + "name=boot_b,size=32M,uuid=${uuid_gpt_boot_b};" \ + "name=system_a,size=818M,uuid=${uuid_gpt_system_a};" \ + "name=system_b,size=818M,uuid=${uuid_gpt_system_b};" \ + "name=vendor_a,size=50M,uuid=${uuid_gpt_vendor_a};" \ + "name=vendor_b,size=50M,uuid=${uuid_gpt_vendor_b};" \ + "name=cache,size=100M,uuid=${uuid_gpt_cache};" \ + "name=metadata,size=16M,uuid=${uuid_gpt_metadata};" \ + "name=persist,size=4M,uuid=${uuid_gpt_persist};" \ + "name=userdata,size=-,uuid=${uuid_gpt_userdata};\0" \ + +#undef CONFIG_EXTRA_ENV_SETTINGS +#define CONFIG_EXTRA_ENV_SETTINGS \ + "partitions=" PARTS_DEFAULT \ + +#endif + #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 4/6] rockchip: kylin: Add default gpt partition table @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot Add default android gpt partition table for kylin board. Use "gpt write mmc 0 $partitions" to apply. 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 | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h index aa07889..b750b26 100644 --- a/include/configs/kylin_rk3036.h +++ b/include/configs/kylin_rk3036.h @@ -9,4 +9,33 @@ #include <configs/rk3036_common.h> +#ifndef CONFIG_SPL_BUILD + +/* Enable gpt partition table */ +#define CONFIG_CMD_GPT +#define CONFIG_RANDOM_UUID +#define CONFIG_EFI_PARTITION +#define PARTS_DEFAULT \ + "uuid_disk=${uuid_gpt_disk};" \ + "name=loader,start=32K,size=4000K,uuid=${uuid_gpt_loader};" \ + "name=reserved,size=64K,uuid=${uuid_gpt_reserved};" \ + "name=misc,size=4M,uuid=${uuid_gpt_misc};" \ + "name=recovery,size=32M,uuid=${uuid_gpt_recovery};" \ + "name=boot_a,size=32M,uuid=${uuid_gpt_boot_a};" \ + "name=boot_b,size=32M,uuid=${uuid_gpt_boot_b};" \ + "name=system_a,size=818M,uuid=${uuid_gpt_system_a};" \ + "name=system_b,size=818M,uuid=${uuid_gpt_system_b};" \ + "name=vendor_a,size=50M,uuid=${uuid_gpt_vendor_a};" \ + "name=vendor_b,size=50M,uuid=${uuid_gpt_vendor_b};" \ + "name=cache,size=100M,uuid=${uuid_gpt_cache};" \ + "name=metadata,size=16M,uuid=${uuid_gpt_metadata};" \ + "name=persist,size=4M,uuid=${uuid_gpt_persist};" \ + "name=userdata,size=-,uuid=${uuid_gpt_userdata};\0" \ + +#undef CONFIG_EXTRA_ENV_SETTINGS +#define CONFIG_EXTRA_ENV_SETTINGS \ + "partitions=" PARTS_DEFAULT \ + +#endif + #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v4 4/6] rockchip: kylin: Add default gpt partition table 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 15:28 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: Jeffy Chen; +Cc: u-boot, linux-rockchip [-- Attachment #1.1: Type: text/plain, Size: 337 bytes --] On Wed, Jan 13, 2016 at 04:53:18PM +0800, Jeffy Chen wrote: > Add default android gpt partition table for kylin board. > > Use "gpt write mmc 0 $partitions" to apply. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Acked-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com> -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 4/6] rockchip: kylin: Add default gpt partition table @ 2016-01-13 15:28 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: u-boot On Wed, Jan 13, 2016 at 04:53:18PM +0800, Jeffy Chen wrote: > Add default android gpt partition table for kylin board. > > Use "gpt write mmc 0 $partitions" to apply. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Acked-by: Simon Glass <sjg@chromium.org> Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160113/f8ce9c9e/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 8:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot-0aAXYlwwYIKGBzrmiIFOJg Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, huang lin 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-TNX95d0MmH7DzftRWevZcw@public.gmane.org> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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 +#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 #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot 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 +#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 #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 15:28 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: Jeffy Chen; +Cc: u-boot, linux-rockchip [-- Attachment #1.1: Type: text/plain, Size: 1872 bytes --] 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. > +#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 [-- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-13 15:28 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: u-boot 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. > +#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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160113/71fd91f8/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-13 15:28 ` [U-Boot] " Tom Rini @ 2016-01-14 2:31 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-14 2:31 UTC (permalink / raw) To: Tom Rini Cc: u-boot-0aAXYlwwYIKGBzrmiIFOJg, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r 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-TNX95d0MmH7DzftRWevZcw@public.gmane.org> >> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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 :) ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-14 2:31 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-14 2:31 UTC (permalink / raw) To: u-boot 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 :) ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-14 2:31 ` Jeffy Chen @ 2016-01-14 16:22 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-14 16:22 UTC (permalink / raw) To: Jeffy Chen; +Cc: u-boot, linux-rockchip [-- Attachment #1.1: Type: text/plain, Size: 2600 bytes --] 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) ? -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-14 16:22 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-14 16:22 UTC (permalink / raw) To: u-boot 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) ? -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160114/f5d09f05/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-14 16:22 ` [U-Boot] " Tom Rini @ 2016-01-15 0:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-15 0:53 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot, linux-rockchip 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). ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-15 0:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-15 0:53 UTC (permalink / raw) To: u-boot 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). ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 0:53 ` [U-Boot] " Jeffy Chen @ 2016-01-15 0:59 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-15 0:59 UTC (permalink / raw) To: Jeffy Chen; +Cc: u-boot, linux-rockchip [-- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-15 0:59 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-15 0:59 UTC (permalink / raw) To: u-boot 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160114/f48c07b8/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 0:59 ` [U-Boot] " Tom Rini @ 2016-01-15 2:20 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-15 2:20 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot, linux-rockchip 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 <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. 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, &images, 1); But i'm not sure if it's ok to add it here... ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-15 2:20 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-15 2:20 UTC (permalink / raw) To: u-boot 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 <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. 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, &images, 1); But i'm not sure if it's ok to add it here... ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 2:20 ` [U-Boot] " Jeffy Chen @ 2016-01-15 14:42 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-15 14:42 UTC (permalink / raw) To: Jeffy Chen, Simon Glass, Masahiro Yamada, Daniel Schwierzeck Cc: u-boot, linux-rockchip [-- Attachment #1.1: Type: text/plain, Size: 5321 bytes --] 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 <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. > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to some of the initrd rated things Masahiro has found recently. So something is wrong down in this core code. Jeffy, can you try and debug this a bit since you have a failing case in front of you? Otherwise I'll put it on my TODO list. Thanks! -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-15 14:42 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-15 14:42 UTC (permalink / raw) To: u-boot 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 <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. > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to some of the initrd rated things Masahiro has found recently. So something is wrong down in this core code. Jeffy, can you try and debug this a bit since you have a failing case in front of you? Otherwise I'll put it on my TODO list. Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160115/daec7b95/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 14:42 ` [U-Boot] " Tom Rini @ 2016-01-15 15:42 ` Daniel Schwierzeck -1 siblings, 0 replies; 54+ messages in thread From: Daniel Schwierzeck @ 2016-01-15 15:42 UTC (permalink / raw) To: Tom Rini, Jeffy Chen, Simon Glass, Masahiro Yamada; +Cc: u-boot, linux-rockchip 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 <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. > > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to > some of the initrd rated things Masahiro has found recently. So > something is wrong down in this core code. one problem is the different behaviour of bootm between legacy uImages and FIT uImages. In case of legacy uImages, the core code automatically relocates initramfs and DTB. In case of FIT uImages, the arch-specific do_bootm_linux() still has to do that. This was only somehow addressed by adding the helper function image_setup_linux() and calling it from do_bootm_linux(). But you can't use that function with legacy uImages, because initramfs and DTB would be relocated twice because the states BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of that image_setup_linux() also leads to a different behaviour when calling bootm as a single command and calling bootm with sub-steps. So the current bootm implementation on MIPS is the only way for me to make bootm working with all possible combinations of legacy/FIT uImage, with/without initramfs, with/without DTB and single/multiple bootm calls. I suspect that the current implementation on ARM does not work with all possible combinations. > Jeffy, can you try and debug > this a bit since you have a failing case in front of you? Otherwise > I'll put it on my TODO list. Thanks! > Some ideas for a possible refactoring of the bootm core code: - remove arch-specific do_bootm_linux() and image_setup_linux() - rename functions like boot_prep_linux() and boot_jump_linux() to arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as __weak and add a default implementation, the arch code can overwrite them - refactor the core code to call those functions at the appropriate locations -- - Daniel ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-15 15:42 ` Daniel Schwierzeck 0 siblings, 0 replies; 54+ messages in thread From: Daniel Schwierzeck @ 2016-01-15 15:42 UTC (permalink / raw) To: u-boot 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 <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. > > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to > some of the initrd rated things Masahiro has found recently. So > something is wrong down in this core code. one problem is the different behaviour of bootm between legacy uImages and FIT uImages. In case of legacy uImages, the core code automatically relocates initramfs and DTB. In case of FIT uImages, the arch-specific do_bootm_linux() still has to do that. This was only somehow addressed by adding the helper function image_setup_linux() and calling it from do_bootm_linux(). But you can't use that function with legacy uImages, because initramfs and DTB would be relocated twice because the states BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of that image_setup_linux() also leads to a different behaviour when calling bootm as a single command and calling bootm with sub-steps. So the current bootm implementation on MIPS is the only way for me to make bootm working with all possible combinations of legacy/FIT uImage, with/without initramfs, with/without DTB and single/multiple bootm calls. I suspect that the current implementation on ARM does not work with all possible combinations. > Jeffy, can you try and debug > this a bit since you have a failing case in front of you? Otherwise > I'll put it on my TODO list. Thanks! > Some ideas for a possible refactoring of the bootm core code: - remove arch-specific do_bootm_linux() and image_setup_linux() - rename functions like boot_prep_linux() and boot_jump_linux() to arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as __weak and add a default implementation, the arch code can overwrite them - refactor the core code to call those functions at the appropriate locations -- - Daniel ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 15:42 ` [U-Boot] " Daniel Schwierzeck @ 2016-01-16 1:18 ` Simon Glass -1 siblings, 0 replies; 54+ messages in thread From: Simon Glass @ 2016-01-16 1:18 UTC (permalink / raw) To: Daniel Schwierzeck Cc: U-Boot Mailing List, Tom Rini, linux-rockchip, Jeffy Chen Hi, On 15 January 2016 at 08:42, Daniel Schwierzeck <daniel.schwierzeck@gmail.com> 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 <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. >> > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to >> some of the initrd rated things Masahiro has found recently. So >> something is wrong down in this core code. > > one problem is the different behaviour of bootm between legacy uImages > and FIT uImages. In case of legacy uImages, the core code automatically > relocates initramfs and DTB. In case of FIT uImages, the arch-specific > do_bootm_linux() still has to do that. This was only somehow addressed > by adding the helper function image_setup_linux() and calling it from > do_bootm_linux(). But you can't use that function with legacy uImages, > because initramfs and DTB would be relocated twice because the states > BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of > that image_setup_linux() also leads to a different behaviour when > calling bootm as a single command and calling bootm with sub-steps. > > So the current bootm implementation on MIPS is the only way for me to > make bootm working with all possible combinations of legacy/FIT uImage, > with/without initramfs, with/without DTB and single/multiple bootm > calls. I suspect that the current implementation on ARM does not work > with all possible combinations. > >> Jeffy, can you try and debug >> this a bit since you have a failing case in front of you? Otherwise >> I'll put it on my TODO list. Thanks! >> > > Some ideas for a possible refactoring of the bootm core code: > - remove arch-specific do_bootm_linux() and image_setup_linux() > - rename functions like boot_prep_linux() and boot_jump_linux() to > arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as > __weak and add a default implementation, the arch code can overwrite > them > - refactor the core code to call those functions at the appropriate > locations Perhaps we need a more formal API between bootm and the arch-specific implementation of the various pieces. Also it would be great to add more tests for bootm. I made a start based on what I could figure out about the behaviour (tests/image/test-fit.py), but we should do more. Regards, Simon ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-16 1:18 ` Simon Glass 0 siblings, 0 replies; 54+ messages in thread From: Simon Glass @ 2016-01-16 1:18 UTC (permalink / raw) To: u-boot Hi, On 15 January 2016 at 08:42, Daniel Schwierzeck <daniel.schwierzeck@gmail.com> 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 <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. >> > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to >> some of the initrd rated things Masahiro has found recently. So >> something is wrong down in this core code. > > one problem is the different behaviour of bootm between legacy uImages > and FIT uImages. In case of legacy uImages, the core code automatically > relocates initramfs and DTB. In case of FIT uImages, the arch-specific > do_bootm_linux() still has to do that. This was only somehow addressed > by adding the helper function image_setup_linux() and calling it from > do_bootm_linux(). But you can't use that function with legacy uImages, > because initramfs and DTB would be relocated twice because the states > BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of > that image_setup_linux() also leads to a different behaviour when > calling bootm as a single command and calling bootm with sub-steps. > > So the current bootm implementation on MIPS is the only way for me to > make bootm working with all possible combinations of legacy/FIT uImage, > with/without initramfs, with/without DTB and single/multiple bootm > calls. I suspect that the current implementation on ARM does not work > with all possible combinations. > >> Jeffy, can you try and debug >> this a bit since you have a failing case in front of you? Otherwise >> I'll put it on my TODO list. Thanks! >> > > Some ideas for a possible refactoring of the bootm core code: > - remove arch-specific do_bootm_linux() and image_setup_linux() > - rename functions like boot_prep_linux() and boot_jump_linux() to > arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as > __weak and add a default implementation, the arch code can overwrite > them > - refactor the core code to call those functions at the appropriate > locations Perhaps we need a more formal API between bootm and the arch-specific implementation of the various pieces. Also it would be great to add more tests for bootm. I made a start based on what I could figure out about the behaviour (tests/image/test-fit.py), but we should do more. Regards, Simon ^ permalink raw reply [flat|nested] 54+ messages in thread
[parent not found: <CAPnjgZ1SUJDVPFVXgJjEYgom7tEdaORUAjQ8QfQ8jCpw=NtcJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-16 1:18 ` [U-Boot] " Simon Glass @ 2016-01-22 3:13 ` Simon Glass -1 siblings, 0 replies; 54+ messages in thread From: Simon Glass @ 2016-01-22 3:13 UTC (permalink / raw) To: Daniel Schwierzeck Cc: U-Boot Mailing List, Tom Rini, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jeffy Chen, Masahiro Yamada Hi, On 15 January 2016 at 18:18, Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote: > Hi, > > On 15 January 2016 at 08:42, Daniel Schwierzeck > <daniel.schwierzeck-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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 <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> >>> > > > > > > > Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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. >>> > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to >>> some of the initrd rated things Masahiro has found recently. So >>> something is wrong down in this core code. >> >> one problem is the different behaviour of bootm between legacy uImages >> and FIT uImages. In case of legacy uImages, the core code automatically >> relocates initramfs and DTB. In case of FIT uImages, the arch-specific >> do_bootm_linux() still has to do that. This was only somehow addressed >> by adding the helper function image_setup_linux() and calling it from >> do_bootm_linux(). But you can't use that function with legacy uImages, >> because initramfs and DTB would be relocated twice because the states >> BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of >> that image_setup_linux() also leads to a different behaviour when >> calling bootm as a single command and calling bootm with sub-steps. >> >> So the current bootm implementation on MIPS is the only way for me to >> make bootm working with all possible combinations of legacy/FIT uImage, >> with/without initramfs, with/without DTB and single/multiple bootm >> calls. I suspect that the current implementation on ARM does not work >> with all possible combinations. >> >>> Jeffy, can you try and debug >>> this a bit since you have a failing case in front of you? Otherwise >>> I'll put it on my TODO list. Thanks! >>> >> >> Some ideas for a possible refactoring of the bootm core code: >> - remove arch-specific do_bootm_linux() and image_setup_linux() >> - rename functions like boot_prep_linux() and boot_jump_linux() to >> arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as >> __weak and add a default implementation, the arch code can overwrite >> them >> - refactor the core code to call those functions at the appropriate >> locations > > Perhaps we need a more formal API between bootm and the arch-specific > implementation of the various pieces. > > Also it would be great to add more tests for bootm. I made a start > based on what I could figure out about the behaviour > (tests/image/test-fit.py), but we should do more. > > Regards, > Simon I'm going to hold off on this patch as there seems to be some other root cause. Regards, Simon ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-22 3:13 ` Simon Glass 0 siblings, 0 replies; 54+ messages in thread From: Simon Glass @ 2016-01-22 3:13 UTC (permalink / raw) To: u-boot Hi, On 15 January 2016 at 18:18, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 15 January 2016 at 08:42, Daniel Schwierzeck > <daniel.schwierzeck@gmail.com> 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 <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. >>> > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to >>> some of the initrd rated things Masahiro has found recently. So >>> something is wrong down in this core code. >> >> one problem is the different behaviour of bootm between legacy uImages >> and FIT uImages. In case of legacy uImages, the core code automatically >> relocates initramfs and DTB. In case of FIT uImages, the arch-specific >> do_bootm_linux() still has to do that. This was only somehow addressed >> by adding the helper function image_setup_linux() and calling it from >> do_bootm_linux(). But you can't use that function with legacy uImages, >> because initramfs and DTB would be relocated twice because the states >> BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of >> that image_setup_linux() also leads to a different behaviour when >> calling bootm as a single command and calling bootm with sub-steps. >> >> So the current bootm implementation on MIPS is the only way for me to >> make bootm working with all possible combinations of legacy/FIT uImage, >> with/without initramfs, with/without DTB and single/multiple bootm >> calls. I suspect that the current implementation on ARM does not work >> with all possible combinations. >> >>> Jeffy, can you try and debug >>> this a bit since you have a failing case in front of you? Otherwise >>> I'll put it on my TODO list. Thanks! >>> >> >> Some ideas for a possible refactoring of the bootm core code: >> - remove arch-specific do_bootm_linux() and image_setup_linux() >> - rename functions like boot_prep_linux() and boot_jump_linux() to >> arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as >> __weak and add a default implementation, the arch code can overwrite >> them >> - refactor the core code to call those functions at the appropriate >> locations > > Perhaps we need a more formal API between bootm and the arch-specific > implementation of the various pieces. > > Also it would be great to add more tests for bootm. I made a start > based on what I could figure out about the behaviour > (tests/image/test-fit.py), but we should do more. > > Regards, > Simon I'm going to hold off on this patch as there seems to be some other root cause. Regards, Simon ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 15:42 ` [U-Boot] " Daniel Schwierzeck @ 2016-01-25 16:57 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-25 16:57 UTC (permalink / raw) To: Daniel Schwierzeck; +Cc: linux-rockchip, Jeffy Chen, u-boot [-- Attachment #1.1: Type: text/plain, Size: 8599 bytes --] 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 <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. > > > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to > > some of the initrd rated things Masahiro has found recently. So > > something is wrong down in this core code. > > one problem is the different behaviour of bootm between legacy uImages > and FIT uImages. In case of legacy uImages, the core code automatically > relocates initramfs and DTB. In case of FIT uImages, the arch-specific > do_bootm_linux() still has to do that. This was only somehow addressed > by adding the helper function image_setup_linux() and calling it from > do_bootm_linux(). But you can't use that function with legacy uImages, > because initramfs and DTB would be relocated twice because the states > BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of > that image_setup_linux() also leads to a different behaviour when > calling bootm as a single command and calling bootm with sub-steps. > > So the current bootm implementation on MIPS is the only way for me to > make bootm working with all possible combinations of legacy/FIT uImage, > with/without initramfs, with/without DTB and single/multiple bootm > calls. I suspect that the current implementation on ARM does not work > with all possible combinations. Yes, I bet ARM and PowerPC and SPARC and m68k are broken now. > > Jeffy, can you try and debug > > this a bit since you have a failing case in front of you? Otherwise > > I'll put it on my TODO list. Thanks! > > > > Some ideas for a possible refactoring of the bootm core code: > - remove arch-specific do_bootm_linux() and image_setup_linux() > - rename functions like boot_prep_linux() and boot_jump_linux() to > arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as > __weak and add a default implementation, the arch code can overwrite > them > - refactor the core code to call those functions at the appropriate > locations There's certainly room to make the whole bootm sequence shared better. But I also see now that in the cleanup we moved from always calling bootm_ramdisk_high to, well, not always calling it in all cases as you found (but it looks like it should have been, but isn't). I'm trying to wrap my head around how to fix this. Thanks! -- 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 ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-25 16:57 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-25 16:57 UTC (permalink / raw) To: u-boot 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 <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. > > > 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, &images, 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 back in 2014 into boot_prep_linux. It may even be related to > > some of the initrd rated things Masahiro has found recently. So > > something is wrong down in this core code. > > one problem is the different behaviour of bootm between legacy uImages > and FIT uImages. In case of legacy uImages, the core code automatically > relocates initramfs and DTB. In case of FIT uImages, the arch-specific > do_bootm_linux() still has to do that. This was only somehow addressed > by adding the helper function image_setup_linux() and calling it from > do_bootm_linux(). But you can't use that function with legacy uImages, > because initramfs and DTB would be relocated twice because the states > BOOTM_STATE_RAMDISK and BOOTM_STATE_FDT are not checked. Because of > that image_setup_linux() also leads to a different behaviour when > calling bootm as a single command and calling bootm with sub-steps. > > So the current bootm implementation on MIPS is the only way for me to > make bootm working with all possible combinations of legacy/FIT uImage, > with/without initramfs, with/without DTB and single/multiple bootm > calls. I suspect that the current implementation on ARM does not work > with all possible combinations. Yes, I bet ARM and PowerPC and SPARC and m68k are broken now. > > Jeffy, can you try and debug > > this a bit since you have a failing case in front of you? Otherwise > > I'll put it on my TODO list. Thanks! > > > > Some ideas for a possible refactoring of the bootm core code: > - remove arch-specific do_bootm_linux() and image_setup_linux() > - rename functions like boot_prep_linux() and boot_jump_linux() to > arch_boot_prep_linux() and arch_boot_jump_linux() and declare them as > __weak and add a default implementation, the arch code can overwrite > them > - refactor the core code to call those functions at the appropriate > locations There's certainly room to make the whole bootm sequence shared better. But I also see now that in the cleanup we moved from always calling bootm_ramdisk_high to, well, not always calling it in all cases as you found (but it looks like it should have been, but isn't). I'm trying to wrap my head around how to fix this. Thanks! -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160125/1dce85db/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
[parent not found: <5698577B.8040209-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-15 2:20 ` [U-Boot] " Jeffy Chen @ 2016-01-25 19:07 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-25 19:07 UTC (permalink / raw) To: Jeffy Chen Cc: u-boot-0aAXYlwwYIKGBzrmiIFOJg, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [-- Attachment #1.1: Type: text/plain, Size: 4810 bytes --] 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 <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > >>>>>>Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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. > 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, &images, 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 [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 200 bytes --] _______________________________________________ Linux-rockchip mailing list Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-01-25 19:07 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-25 19:07 UTC (permalink / raw) To: u-boot 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 <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. > 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, &images, 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160125/e7bb3b24/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-01-25 19:07 ` Tom Rini @ 2016-02-02 7:13 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-02-02 7:13 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot, linux-rockchip 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 <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. >> 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, &images, 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(); } #endif And as Daniel said, these would "leads to a different behaviour when calling bootm as a single command and calling bootm with sub-steps." If IMAGE_ENABLE_OF_LIBFDT is enabled & images->ft_len is not zero, boot_ramdisk_high would be called twice with splited bootm cmds. And in our kylin case (IMAGE_ENABLE_OF_LIBFDT is disabled), the boot_ramdisk_high would not be called with normal bootm cmd :( > 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. On current kylin board, we're not using fdt_high or initrd_high. The bootm_low / bootm_size would be CONFIG_SYS_SDRAM_BASE / gd->bd->bi_dram[0].size by default(if not define those env), and arm arch will reserve 4K at the end for stack, so the ramdisk would be relocated to around (dram_end - 4K) :) > Thanks! > ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-02-02 7:13 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-02-02 7:13 UTC (permalink / raw) To: u-boot 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 <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. >> 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, &images, 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(); } #endif And as Daniel said, these would "leads to a different behaviour when calling bootm as a single command and calling bootm with sub-steps." If IMAGE_ENABLE_OF_LIBFDT is enabled & images->ft_len is not zero, boot_ramdisk_high would be called twice with splited bootm cmds. And in our kylin case (IMAGE_ENABLE_OF_LIBFDT is disabled), the boot_ramdisk_high would not be called with normal bootm cmd :( > 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. On current kylin board, we're not using fdt_high or initrd_high. The bootm_low / bootm_size would be CONFIG_SYS_SDRAM_BASE / gd->bd->bi_dram[0].size by default(if not define those env), and arm arch will reserve 4K at the end for stack, so the ramdisk would be relocated to around (dram_end - 4K) :) > Thanks! > ^ permalink raw reply [flat|nested] 54+ messages in thread
[parent not found: <56B056FD.70006-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image 2016-02-02 7:13 ` [U-Boot] " Jeffy Chen @ 2016-02-19 20:55 ` Simon Glass -1 siblings, 0 replies; 54+ messages in thread From: Simon Glass @ 2016-02-19 20:55 UTC (permalink / raw) To: Jeffy Chen Cc: Masahiro Yamada, Tom Rini, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, U-Boot Mailing List +Masahiro On 2 February 2016 at 00:13, Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 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 <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> >>>>>>>>> Acked-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.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. >>> >>> 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, &images, 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(); > } > #endif > > > And as Daniel said, these would "leads to a different behaviour when calling > bootm as a single command and calling bootm with sub-steps." > > If IMAGE_ENABLE_OF_LIBFDT is enabled & images->ft_len is not zero, > boot_ramdisk_high would be called twice with splited bootm cmds. And in our > kylin case (IMAGE_ENABLE_OF_LIBFDT is disabled), the boot_ramdisk_high would > not be called with normal bootm cmd :( > >> 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. > > On current kylin board, we're not using fdt_high or initrd_high. > The bootm_low / bootm_size would be CONFIG_SYS_SDRAM_BASE / > gd->bd->bi_dram[0].size by default(if not define those env), and arm arch > will reserve 4K at the end for stack, so the ramdisk would be relocated to > around (dram_end - 4K) :) >> >> Thanks! >> > > > _______________________________________________ > U-Boot mailing list > U-Boot-0aAXYlwwYIKGBzrmiIFOJg@public.gmane.org > http://lists.denx.de/mailman/listinfo/u-boot ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image @ 2016-02-19 20:55 ` Simon Glass 0 siblings, 0 replies; 54+ messages in thread From: Simon Glass @ 2016-02-19 20:55 UTC (permalink / raw) To: u-boot +Masahiro On 2 February 2016 at 00:13, Jeffy Chen <jeffy.chen@rock-chips.com> 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 <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. >>> >>> 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, &images, 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(); > } > #endif > > > And as Daniel said, these would "leads to a different behaviour when calling > bootm as a single command and calling bootm with sub-steps." > > If IMAGE_ENABLE_OF_LIBFDT is enabled & images->ft_len is not zero, > boot_ramdisk_high would be called twice with splited bootm cmds. And in our > kylin case (IMAGE_ENABLE_OF_LIBFDT is disabled), the boot_ramdisk_high would > not be called with normal bootm cmd :( > >> 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. > > On current kylin board, we're not using fdt_high or initrd_high. > The bootm_low / bootm_size would be CONFIG_SYS_SDRAM_BASE / > gd->bd->bi_dram[0].size by default(if not define those env), and arm arch > will reserve 4K at the end for stack, so the ramdisk would be relocated to > around (dram_end - 4K) :) >> >> Thanks! >> > > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot ^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v4 6/6] rockchip: kylin: Check fastboot request 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 8:53 ` Jeffy Chen -1 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot-0aAXYlwwYIKGBzrmiIFOJg Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, sjg-F7+t8E8rja9g9hUCZPvPmw, Jeffy Chen, huang lin We will save boot mode flag in grf's os_reg[4], if fastboot requested or fastboot key pressed, try to enter fastboot mode at preboot stage. Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> --- Changes in v4: Remove unused reboot mode definitions. Changes in v3: Use rockchip's legacy reboot mode definitions. Changes in v2: None board/kylin/kylin_rk3036/kylin_rk3036.c | 32 ++++++++++++++++++++++++++++++++ include/configs/kylin_rk3036.h | 3 +++ 2 files changed, 35 insertions(+) diff --git a/board/kylin/kylin_rk3036/kylin_rk3036.c b/board/kylin/kylin_rk3036/kylin_rk3036.c index 40d6b52..a730d7c 100644 --- a/board/kylin/kylin_rk3036/kylin_rk3036.c +++ b/board/kylin/kylin_rk3036/kylin_rk3036.c @@ -8,10 +8,15 @@ #include <dm.h> #include <asm/io.h> #include <asm/arch/uart.h> +#include <asm/arch-rockchip/grf_rk3036.h> #include <asm/arch/sdram_rk3036.h> +#include <asm/gpio.h> DECLARE_GLOBAL_DATA_PTR; +#define GRF_BASE 0x20008000 +static struct rk3036_grf * const grf = (void *)GRF_BASE; + void get_ddr_config(struct rk3036_ddr_config *config) { /* K4B4G1646Q config */ @@ -28,6 +33,33 @@ void get_ddr_config(struct rk3036_ddr_config *config) config->bw = 1; } +#define FASTBOOT_KEY_GPIO 93 + +int fastboot_key_pressed(void) +{ + gpio_request(FASTBOOT_KEY_GPIO, "fastboot_key"); + gpio_direction_input(FASTBOOT_KEY_GPIO); + return !gpio_get_value(FASTBOOT_KEY_GPIO); +} + +#define ROCKCHIP_BOOT_MODE_FASTBOOT 0x5242C309 + +int board_late_init(void) +{ + int boot_mode = readl(&grf->os_reg[4]); + + /* Clear boot mode */ + writel(0, &grf->os_reg[4]); + + if (boot_mode == ROCKCHIP_BOOT_MODE_FASTBOOT || + fastboot_key_pressed()) { + printf("enter fastboot!\n"); + setenv("preboot", "setenv preboot; fastboot usb0"); + } + + return 0; +} + int board_init(void) { return 0; diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h index 49997ec..424e81b 100644 --- a/include/configs/kylin_rk3036.h +++ b/include/configs/kylin_rk3036.h @@ -39,6 +39,9 @@ "mmcpart=5\0" \ "loadaddr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \ +#define CONFIG_BOARD_LATE_INIT +#define CONFIG_PREBOOT + #define CONFIG_ANDROID_BOOT_IMAGE #define CONFIG_SYS_BOOT_RAMDISK_HIGH #define CONFIG_SYS_HUSH_PARSER -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 6/6] rockchip: kylin: Check fastboot request @ 2016-01-13 8:53 ` Jeffy Chen 0 siblings, 0 replies; 54+ messages in thread From: Jeffy Chen @ 2016-01-13 8:53 UTC (permalink / raw) To: u-boot We will save boot mode flag in grf's os_reg[4], if fastboot requested or fastboot key pressed, try to enter fastboot mode at preboot stage. Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> --- Changes in v4: Remove unused reboot mode definitions. Changes in v3: Use rockchip's legacy reboot mode definitions. Changes in v2: None board/kylin/kylin_rk3036/kylin_rk3036.c | 32 ++++++++++++++++++++++++++++++++ include/configs/kylin_rk3036.h | 3 +++ 2 files changed, 35 insertions(+) diff --git a/board/kylin/kylin_rk3036/kylin_rk3036.c b/board/kylin/kylin_rk3036/kylin_rk3036.c index 40d6b52..a730d7c 100644 --- a/board/kylin/kylin_rk3036/kylin_rk3036.c +++ b/board/kylin/kylin_rk3036/kylin_rk3036.c @@ -8,10 +8,15 @@ #include <dm.h> #include <asm/io.h> #include <asm/arch/uart.h> +#include <asm/arch-rockchip/grf_rk3036.h> #include <asm/arch/sdram_rk3036.h> +#include <asm/gpio.h> DECLARE_GLOBAL_DATA_PTR; +#define GRF_BASE 0x20008000 +static struct rk3036_grf * const grf = (void *)GRF_BASE; + void get_ddr_config(struct rk3036_ddr_config *config) { /* K4B4G1646Q config */ @@ -28,6 +33,33 @@ void get_ddr_config(struct rk3036_ddr_config *config) config->bw = 1; } +#define FASTBOOT_KEY_GPIO 93 + +int fastboot_key_pressed(void) +{ + gpio_request(FASTBOOT_KEY_GPIO, "fastboot_key"); + gpio_direction_input(FASTBOOT_KEY_GPIO); + return !gpio_get_value(FASTBOOT_KEY_GPIO); +} + +#define ROCKCHIP_BOOT_MODE_FASTBOOT 0x5242C309 + +int board_late_init(void) +{ + int boot_mode = readl(&grf->os_reg[4]); + + /* Clear boot mode */ + writel(0, &grf->os_reg[4]); + + if (boot_mode == ROCKCHIP_BOOT_MODE_FASTBOOT || + fastboot_key_pressed()) { + printf("enter fastboot!\n"); + setenv("preboot", "setenv preboot; fastboot usb0"); + } + + return 0; +} + int board_init(void) { return 0; diff --git a/include/configs/kylin_rk3036.h b/include/configs/kylin_rk3036.h index 49997ec..424e81b 100644 --- a/include/configs/kylin_rk3036.h +++ b/include/configs/kylin_rk3036.h @@ -39,6 +39,9 @@ "mmcpart=5\0" \ "loadaddr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \ +#define CONFIG_BOARD_LATE_INIT +#define CONFIG_PREBOOT + #define CONFIG_ANDROID_BOOT_IMAGE #define CONFIG_SYS_BOOT_RAMDISK_HIGH #define CONFIG_SYS_HUSH_PARSER -- 2.1.4 ^ permalink raw reply related [flat|nested] 54+ messages in thread
[parent not found: <1452675200-15941-7-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]
* Re: [U-Boot] [PATCH v4 6/6] rockchip: kylin: Check fastboot request 2016-01-13 8:53 ` [U-Boot] " Jeffy Chen @ 2016-01-13 15:28 ` Tom Rini -1 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: Jeffy Chen Cc: u-boot-0aAXYlwwYIKGBzrmiIFOJg, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r [-- Attachment #1.1: Type: text/plain, Size: 387 bytes --] On Wed, Jan 13, 2016 at 04:53:20PM +0800, Jeffy Chen wrote: > We will save boot mode flag in grf's os_reg[4], if fastboot > requested or fastboot key pressed, try to enter fastboot mode > at preboot stage. > > Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> > Reviewed-by: Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> -- Tom [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 200 bytes --] _______________________________________________ Linux-rockchip mailing list Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 54+ messages in thread
* [U-Boot] [PATCH v4 6/6] rockchip: kylin: Check fastboot request @ 2016-01-13 15:28 ` Tom Rini 0 siblings, 0 replies; 54+ messages in thread From: Tom Rini @ 2016-01-13 15:28 UTC (permalink / raw) To: u-boot On Wed, Jan 13, 2016 at 04:53:20PM +0800, Jeffy Chen wrote: > We will save boot mode flag in grf's os_reg[4], if fastboot > requested or fastboot key pressed, try to enter fastboot mode > at preboot stage. > > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com> > Reviewed-by: Tom Rini <trini@konsulko.com> -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160113/6c8e0da9/attachment.sig> ^ permalink raw reply [flat|nested] 54+ messages in thread
end of thread, other threads:[~2016-02-19 20:55 UTC | newest]
Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-13 8:53 [PATCH v4 0/6] rockchip: kylin: Boot with android boot image Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
[not found] ` <1452675200-15941-1-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-01-13 8:53 ` [PATCH v4 1/6] common/image-fdt.c: Make boot_get_fdt() perform a check for Android images Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
2016-01-13 15:21 ` Tom Rini
2016-01-13 15:21 ` [U-Boot] " Tom Rini
2016-01-14 1:47 ` Jeffy Chen
2016-01-14 1:47 ` Jeffy Chen
2016-01-13 8:53 ` [PATCH v4 2/6] ARM: bootm: Try to use relocated ramdisk Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
2016-01-13 15:27 ` Tom Rini
2016-01-13 15:27 ` [U-Boot] " Tom Rini
2016-01-13 8:53 ` [PATCH v4 3/6] rockchip: rk3036: Bind GPIO banks Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
2016-01-13 15:28 ` Tom Rini
2016-01-13 15:28 ` [U-Boot] " Tom Rini
2016-01-13 8:53 ` [PATCH v4 4/6] rockchip: kylin: Add default gpt partition table Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
2016-01-13 15:28 ` Tom Rini
2016-01-13 15:28 ` [U-Boot] " Tom Rini
2016-01-13 8:53 ` [PATCH v4 5/6] rockchip: kylin: Enable boot with android boot image Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
2016-01-13 15:28 ` Tom Rini
2016-01-13 15:28 ` [U-Boot] " Tom Rini
2016-01-14 2:31 ` Jeffy Chen
2016-01-14 2:31 ` Jeffy Chen
2016-01-14 16:22 ` Tom Rini
2016-01-14 16:22 ` [U-Boot] " Tom Rini
2016-01-15 0:53 ` Jeffy Chen
2016-01-15 0:53 ` [U-Boot] " Jeffy Chen
2016-01-15 0:59 ` Tom Rini
2016-01-15 0:59 ` [U-Boot] " Tom Rini
2016-01-15 2:20 ` Jeffy Chen
2016-01-15 2:20 ` [U-Boot] " Jeffy Chen
2016-01-15 14:42 ` Tom Rini
2016-01-15 14:42 ` [U-Boot] " Tom Rini
2016-01-15 15:42 ` Daniel Schwierzeck
2016-01-15 15:42 ` [U-Boot] " Daniel Schwierzeck
2016-01-16 1:18 ` Simon Glass
2016-01-16 1:18 ` [U-Boot] " Simon Glass
[not found] ` <CAPnjgZ1SUJDVPFVXgJjEYgom7tEdaORUAjQ8QfQ8jCpw=NtcJQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-01-22 3:13 ` Simon Glass
2016-01-22 3:13 ` Simon Glass
2016-01-25 16:57 ` Tom Rini
2016-01-25 16:57 ` [U-Boot] " Tom Rini
[not found] ` <5698577B.8040209-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-01-25 19:07 ` Tom Rini
2016-01-25 19:07 ` Tom Rini
2016-02-02 7:13 ` Jeffy Chen
2016-02-02 7:13 ` [U-Boot] " Jeffy Chen
[not found] ` <56B056FD.70006-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-02-19 20:55 ` Simon Glass
2016-02-19 20:55 ` Simon Glass
2016-01-13 8:53 ` [PATCH v4 6/6] rockchip: kylin: Check fastboot request Jeffy Chen
2016-01-13 8:53 ` [U-Boot] " Jeffy Chen
[not found] ` <1452675200-15941-7-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-01-13 15:28 ` Tom Rini
2016-01-13 15:28 ` Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.