[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 at chromium.org> wrote:
> Hi Suriyan,
>
> On 20 November 2014 at 16:55, Suriyan Ramasami <suriyan.r at 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 at 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