Message ID | 0ac50506-75ff-cb0c-55da-6b93febc8fe5@ti.com |
---|---|
State | RFC |
Delegated to: | Tom Rini |
Headers | show |
On 11.1.2017 06:19, Lokesh Vutla wrote: > > > On Tuesday 10 January 2017 06:28 PM, Michal Simek wrote: >> U-Boot configured via DTB can use the same DTB for booting the kernel. >> When OF_CONTROL is used fdtcontroladdr is setup and can be use for boot. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> Didn't check if there is any side effect or not but it looks weird when >> you have DT driver u-boot that you have to load dtb again. >> >> --- >> arch/arm/lib/bootm.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c >> index 43cc83ec95b6..9740045b0094 100644 >> --- a/arch/arm/lib/bootm.c >> +++ b/arch/arm/lib/bootm.c >> @@ -245,6 +245,20 @@ static void boot_prep_linux(bootm_headers_t *images) >> } >> setup_board_tags(¶ms); >> setup_end_tag(gd->bd); >> + } else if (IS_ENABLED(CONFIG_OF_CONTROL)) { >> +#ifdef CONFIG_OF_LIBFDT >> + images->ft_addr = (char *)getenv_hex("fdtcontroladdr", 0); > > If your intent is to use U-Boot's fdt as fallback, why can't we just use > gd->fdt_blob instead of getting from env? Not an issue to use gd->fdt_block instead but I expect we should copy it to the different location too. > > IMO, this is not the right place to get the fallback fdt. Instead it > should fallback at bootm_get_fdt() so that other parameters like > images->ft_length is also updated. Can you try the following diff? Not a problem to move it to this position. I have tried it and system is not booting. > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index e7540be8d6..f8714b3702 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -238,7 +238,6 @@ int boot_get_fdt(int flag, int argc, char * const > argv[], uint8_t arch, > int fdt_noffset; > #endif > const char *select = NULL; > - int ok_no_fdt = 0; > > *of_flat_tree = NULL; > *of_size = 0; > @@ -311,7 +310,7 @@ int boot_get_fdt(int flag, int argc, char * const > argv[], uint8_t arch, > fdt_addr); > fdt_hdr = image_get_fdt(fdt_addr); > if (!fdt_hdr) > - goto no_fdt; > + goto fallback_fdt; > > /* > * move image data to the load address, > @@ -381,7 +380,7 @@ int boot_get_fdt(int flag, int argc, char * const > argv[], uint8_t arch, > break; > default: > puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); This probably requires some code around not to show this error message because it should be valid boot option. > - goto no_fdt; > + goto fallback_fdt; > } > > printf(" Booting using the fdt blob at %#08lx\n", fdt_addr); > @@ -415,11 +414,17 @@ int boot_get_fdt(int flag, int argc, char * const > argv[], uint8_t arch, > } > } else { > debug("## No Flattened Device Tree\n"); > - goto no_fdt; > + goto fallback_fdt; > } > } else { > debug("## No Flattened Device Tree\n"); > - goto no_fdt; > + goto fallback_fdt; > + } > + > +fallback_fdt: > + if (!fdt_blob) { > + printf("## Using U-boot's fdt to boot kernel\n"); > + fdt_blob = (char *)gd->fdt_blob; > } > > *of_flat_tree = fdt_blob; > @@ -429,12 +434,10 @@ int boot_get_fdt(int flag, int argc, char * const > argv[], uint8_t arch, > > return 0; > > -no_fdt: > - ok_no_fdt = 1; > error: > *of_flat_tree = NULL; > *of_size = 0; > - if (!select && ok_no_fdt) { > + if (!select) { > debug("Continuing to boot without FDT\n"); > return 0; > } > > Thanks and regards, > Lokesh > Thanks, Michal
On Wednesday 11 January 2017 12:50 PM, Michal Simek wrote: > On 11.1.2017 06:19, Lokesh Vutla wrote: >> >> >> On Tuesday 10 January 2017 06:28 PM, Michal Simek wrote: >>> U-Boot configured via DTB can use the same DTB for booting the kernel. >>> When OF_CONTROL is used fdtcontroladdr is setup and can be use for boot. >>> >>> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >>> --- >>> >>> Didn't check if there is any side effect or not but it looks weird when >>> you have DT driver u-boot that you have to load dtb again. >>> >>> --- >>> arch/arm/lib/bootm.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c >>> index 43cc83ec95b6..9740045b0094 100644 >>> --- a/arch/arm/lib/bootm.c >>> +++ b/arch/arm/lib/bootm.c >>> @@ -245,6 +245,20 @@ static void boot_prep_linux(bootm_headers_t *images) >>> } >>> setup_board_tags(¶ms); >>> setup_end_tag(gd->bd); >>> + } else if (IS_ENABLED(CONFIG_OF_CONTROL)) { >>> +#ifdef CONFIG_OF_LIBFDT >>> + images->ft_addr = (char *)getenv_hex("fdtcontroladdr", 0); >> >> If your intent is to use U-Boot's fdt as fallback, why can't we just use >> gd->fdt_blob instead of getting from env? > > Not an issue to use gd->fdt_block instead but I expect we should copy it > to the different location too. > >> >> IMO, this is not the right place to get the fallback fdt. Instead it >> should fallback at bootm_get_fdt() so that other parameters like >> images->ft_length is also updated. Can you try the following diff? > > Not a problem to move it to this position. > > I have tried it and system is not booting. Hmm.. not sure if I am missing something. I am able to boot with this change and not loading any dtb: http://pastebin.ubuntu.com/23780811/ Thanks and regards, Lokesh > >> >> diff --git a/common/image-fdt.c b/common/image-fdt.c >> index e7540be8d6..f8714b3702 100644 >> --- a/common/image-fdt.c >> +++ b/common/image-fdt.c >> @@ -238,7 +238,6 @@ int boot_get_fdt(int flag, int argc, char * const >> argv[], uint8_t arch, >> int fdt_noffset; >> #endif >> const char *select = NULL; >> - int ok_no_fdt = 0; >> >> *of_flat_tree = NULL; >> *of_size = 0; >> @@ -311,7 +310,7 @@ int boot_get_fdt(int flag, int argc, char * const >> argv[], uint8_t arch, >> fdt_addr); >> fdt_hdr = image_get_fdt(fdt_addr); >> if (!fdt_hdr) >> - goto no_fdt; >> + goto fallback_fdt; >> >> /* >> * move image data to the load address, >> @@ -381,7 +380,7 @@ int boot_get_fdt(int flag, int argc, char * const >> argv[], uint8_t arch, >> break; >> default: >> puts("ERROR: Did not find a cmdline Flattened Device Tree\n"); > > > This probably requires some code around not to show this error message > because it should be valid boot option. > >> - goto no_fdt; >> + goto fallback_fdt; >> } >> >> printf(" Booting using the fdt blob at %#08lx\n", fdt_addr); >> @@ -415,11 +414,17 @@ int boot_get_fdt(int flag, int argc, char * const >> argv[], uint8_t arch, >> } >> } else { >> debug("## No Flattened Device Tree\n"); >> - goto no_fdt; >> + goto fallback_fdt; >> } >> } else { >> debug("## No Flattened Device Tree\n"); >> - goto no_fdt; >> + goto fallback_fdt; >> + } >> + >> +fallback_fdt: >> + if (!fdt_blob) { >> + printf("## Using U-boot's fdt to boot kernel\n"); >> + fdt_blob = (char *)gd->fdt_blob; >> } >> >> *of_flat_tree = fdt_blob; >> @@ -429,12 +434,10 @@ int boot_get_fdt(int flag, int argc, char * const >> argv[], uint8_t arch, >> >> return 0; >> >> -no_fdt: >> - ok_no_fdt = 1; >> error: >> *of_flat_tree = NULL; >> *of_size = 0; >> - if (!select && ok_no_fdt) { >> + if (!select) { >> debug("Continuing to boot without FDT\n"); >> return 0; >> } >> >> Thanks and regards, >> Lokesh >> > > Thanks, > Michal >
diff --git a/common/image-fdt.c b/common/image-fdt.c index e7540be8d6..f8714b3702 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -238,7 +238,6 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, int fdt_noffset; #endif const char *select = NULL; - int ok_no_fdt = 0; *of_flat_tree = NULL;