| Message ID | 1416527749-3290-1-git-send-email-suriyan.r@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
Hello Suriyan Ramasami, Am 21.11.2014 00:55, schrieb Suriyan Ramasami: > The boot commands - bootz/bootm mandate a third argument which is the > address to the FDT blob. In cases where this argument is not specified, > boot fails with a message indicating a missing FDT. > > This causes non-FDT kernels to fail to boot. This patch allows both FDT > and non-FDT kernels to boot by making the third parameter to the bootm/bootz > optional. > > Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com> > --- > > Changes in v1: > - First try > > common/image-fdt.c | 4 ++++ > 1 file changed, 4 insertions(+) Thanks! Acked-by: Heiko Schocher <hs@denx.de> tested on an at91sam9g20 based board (not mainlined yet), so: Tested-by: Heiko Schocher <hs@denx.de> bye, Heiko
Hi, On 11/21/2014 12:55 AM, Suriyan Ramasami wrote: > The boot commands - bootz/bootm mandate a third argument which is the > address to the FDT blob. In cases where this argument is not specified, > boot fails with a message indicating a missing FDT. > > This causes non-FDT kernels to fail to boot. This patch allows both FDT > and non-FDT kernels to boot by making the third parameter to the bootm/bootz > optional. > > Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com> Looks good, and works for my case (booting old linux-sunxi 3.4 kernels) too) : Tested-by: Hans de Goede <hdegoede@redhat.com> Acked-by: Hans de Goede <hdegoede@redhat.com> Thanks & Regards, Hans > --- > > Changes in v1: > - First try > > common/image-fdt.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index a39ae1b..1a02166 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, > error: > *of_flat_tree = NULL; > *of_size = 0; > + if (argc <= 2) { > + debug("Continuing to boot without FDT\n"); > + return 0; > + } > return 1; > } > >
Hi Suriyan, On 20 November 2014 at 16:55, Suriyan Ramasami <suriyan.r@gmail.com> wrote: > The boot commands - bootz/bootm mandate a third argument which is the > address to the FDT blob. In cases where this argument is not specified, > boot fails with a message indicating a missing FDT. > > This causes non-FDT kernels to fail to boot. This patch allows both FDT > and non-FDT kernels to boot by making the third parameter to the bootm/bootz > optional. > > Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com> > --- > > Changes in v1: > - First try > > common/image-fdt.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/image-fdt.c b/common/image-fdt.c > index a39ae1b..1a02166 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, > error: > *of_flat_tree = NULL; > *of_size = 0; > + if (argc <= 2) { if (!select) is better I think since that holds the selected device tree. If it NULL when there is none. > + debug("Continuing to boot without FDT\n"); > + return 0; > + } > return 1; > } I think everyone is happy with the approach so I'd like to merge this. But I'm not keen on the error handling. Some of the cases are genuine errors, viz.: if ((load < image_end) && (load_end > image_start)) { fdt_error("fdt overwritten"); goto error; } if (fdt_check_header(fdt_blob) != 0) { fdt_error("image is not a fdt"); goto error; } if (fdt_totalsize(fdt_blob) != fdt_len) { fdt_error("fdt size != image size"); goto error; } So how about leaving error: alone and adding a new label above it like no_fdt: Then you can change the other goto statements to 'goto no_fdt' which can do your check and either return, or fall through to errror:. Regards, Simon
Hello Simon, Thanks for the review! and happy Thanksgiving :-) On Thu, Nov 27, 2014 at 7:58 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Suriyan, > > On 20 November 2014 at 16:55, Suriyan Ramasami <suriyan.r@gmail.com> wrote: >> The boot commands - bootz/bootm mandate a third argument which is the >> address to the FDT blob. In cases where this argument is not specified, >> boot fails with a message indicating a missing FDT. >> >> This causes non-FDT kernels to fail to boot. This patch allows both FDT >> and non-FDT kernels to boot by making the third parameter to the bootm/bootz >> optional. >> >> Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com> >> --- >> >> Changes in v1: >> - First try >> >> common/image-fdt.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/common/image-fdt.c b/common/image-fdt.c >> index a39ae1b..1a02166 100644 >> --- a/common/image-fdt.c >> +++ b/common/image-fdt.c >> @@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, >> error: >> *of_flat_tree = NULL; >> *of_size = 0; >> + if (argc <= 2) { > > if (!select) > > is better I think since that holds the selected device tree. If it > NULL when there is none. > >> + debug("Continuing to boot without FDT\n"); >> + return 0; >> + } >> return 1; >> } > > I think everyone is happy with the approach so I'd like to merge this. > But I'm not keen on the error handling. Some of the cases are genuine > errors, viz.: > > if ((load < image_end) && (load_end > image_start)) { > fdt_error("fdt overwritten"); > goto error; > } > > if (fdt_check_header(fdt_blob) != 0) { > fdt_error("image is not a fdt"); > goto error; > } > > if (fdt_totalsize(fdt_blob) != fdt_len) { > fdt_error("fdt size != image size"); > goto error; > } > > So how about leaving error: alone and adding a new label above it like no_fdt: > > Then you can change the other goto statements to 'goto no_fdt' which > can do your check and either return, or fall through to errror:. > I shall incorporate your suggestions in my next patch. Thanks - Suriyan > Regards, > Simon
diff --git a/common/image-fdt.c b/common/image-fdt.c index a39ae1b..1a02166 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -430,6 +430,10 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, error: *of_flat_tree = NULL; *of_size = 0; + if (argc <= 2) { + debug("Continuing to boot without FDT\n"); + return 0; + } return 1; }
The boot commands - bootz/bootm mandate a third argument which is the address to the FDT blob. In cases where this argument is not specified, boot fails with a message indicating a missing FDT. This causes non-FDT kernels to fail to boot. This patch allows both FDT and non-FDT kernels to boot by making the third parameter to the bootm/bootz optional. Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com> --- Changes in v1: - First try common/image-fdt.c | 4 ++++ 1 file changed, 4 insertions(+)