[U-Boot] [PATCH] spl: fat: Support full fitImage handling

[Marek Vasut]

On 06/07/2018 10:27 PM, Simon Glass wrote:
> Hi Marex,
> 
> On 31 May 2018 at 07:59, Marek Vasut <marex at denx.de> wrote:
>> Handle the case where the full fitImage support is enabled. In this
>> case, the whole fitImage must be loaded up front as some parts of the
>> fitImage code require memory-mapped access to the entire fitImage.
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Pantelis Antoniou <pantelis.antoniou at konsulko.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> ---
>>  common/spl/spl_fat.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
>> index 87dd553210..0403016bb4 100644
>> --- a/common/spl/spl_fat.c
>> +++ b/common/spl/spl_fat.c
>> @@ -70,7 +70,18 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
>>         if (err <= 0)
>>                 goto end;
>>
>> -       if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
>> +       if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
>> +           image_get_magic(header) == FDT_MAGIC) {
>> +               err = file_fat_read(filename, (void *)CONFIG_SYS_LOAD_ADDR, 0);
>> +               if (err <= 0)
>> +                       goto end;
>> +               err = spl_parse_image_header(spl_image,
>> +                               (struct image_header *)CONFIG_SYS_LOAD_ADDR);
>> +               if (err == -EAGAIN)
>> +                       return err;
>> +               if (err == 0)
>> +                       err = 1;
> 
> The error handling here is too confusing.
> 
> spl_load_image_fat() has no comment indicating what the return value
> means. So is 0 success?

Yes, 0 is success, EAGAIN is retry, anything else is an error.

The SPL loader expects I think positive return value to indicate success
though.

> spl_parse_image_header() seems to return 0 on success, which is good,
> but your code appears to turn that into 1. Why?
> 
> Can you please add comments and consider moving towards using 0 to
> mean success everywhere?
> 
>> +       } else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
>>             image_get_magic(header) == FDT_MAGIC) {
>>                 struct spl_load_info load;
>>
>> --
>> 2.16.2
>>
> 
> Regards,
> Simon
> 


-- 
Best regards,
Marek Vasut