[U-Boot] [PATCH v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

[U-Boot] [PATCH v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

[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(+)

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;
 }
 
-- 
1.8.3.1

[U-Boot] [PATCH v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

[Hans de Goede]

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;
>  }
>  
> 

[U-Boot] [PATCH v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

[Simon Glass]

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

[U-Boot] [PATCH v1] fdt: Allow non-FDT kernels to boot when CONFIG_OF_LIBFDT is defined

[Suriyan Ramasami]

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