All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] spl: fat: Support full fitImage handling
@ 2018-05-31 15:59 Marek Vasut
  2018-06-07 20:27 ` Simon Glass
  2018-07-11 12:41 ` [U-Boot] " Tom Rini
  0 siblings, 2 replies; 4+ messages in thread
From: Marek Vasut @ 2018-05-31 15:59 UTC (permalink / raw)
  To: u-boot

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;
+	} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
 	    image_get_magic(header) == FDT_MAGIC) {
 		struct spl_load_info load;
 
-- 
2.16.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH] spl: fat: Support full fitImage handling
  2018-05-31 15:59 [U-Boot] [PATCH] spl: fat: Support full fitImage handling Marek Vasut
@ 2018-06-07 20:27 ` Simon Glass
  2018-06-07 20:55   ` Marek Vasut
  2018-07-11 12:41 ` [U-Boot] " Tom Rini
  1 sibling, 1 reply; 4+ messages in thread
From: Simon Glass @ 2018-06-07 20:27 UTC (permalink / raw)
  To: u-boot

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?

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH] spl: fat: Support full fitImage handling
  2018-06-07 20:27 ` Simon Glass
@ 2018-06-07 20:55   ` Marek Vasut
  0 siblings, 0 replies; 4+ messages in thread
From: Marek Vasut @ 2018-06-07 20:55 UTC (permalink / raw)
  To: u-boot

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
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] spl: fat: Support full fitImage handling
  2018-05-31 15:59 [U-Boot] [PATCH] spl: fat: Support full fitImage handling Marek Vasut
  2018-06-07 20:27 ` Simon Glass
@ 2018-07-11 12:41 ` Tom Rini
  1 sibling, 0 replies; 4+ messages in thread
From: Tom Rini @ 2018-07-11 12:41 UTC (permalink / raw)
  To: u-boot

On Thu, May 31, 2018 at 05:59:19PM +0200, Marek Vasut 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>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180711/d4aeab67/attachment.sig>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-07-11 12:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-31 15:59 [U-Boot] [PATCH] spl: fat: Support full fitImage handling Marek Vasut
2018-06-07 20:27 ` Simon Glass
2018-06-07 20:55   ` Marek Vasut
2018-07-11 12:41 ` [U-Boot] " Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.