On 06/07/2018 10:27 PM, Simon Glass wrote:

Hi Marex,

On 31 May 2018 at 07:59, Marek Vasut marex@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@denx.de Cc: Pantelis Antoniou pantelis.antoniou@konsulko.com Cc: Simon Glass sjg@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