[U-Boot] [PATCH 1/2] spl: Fix redundant image of uboot

[Michael Nazzareno Trimarchi]

Hi

On Wed, Jul 4, 2018 at 5:10 PM, Lothar Waßmann <LW at karo-electronics.de> wrote:
> Michael Nazzareno Trimarchi <michael at amarulasolutions.com> wrote:
>
>> Hi
>>
>> On Wed, Jul 4, 2018 at 4:19 PM, Lothar Waßmann <LW at karo-electronics.de> wrote:
>> > Hi,
>> >
>> > On Wed,  4 Jul 2018 15:53:36 +0200 Michael Trimarchi wrote:
>> >> We need to address the redundat image case and undestand if the
>> >> image is corrupted or not and fallback to the copy. The function
>> >> used before was always return 0 without any evaluation of the
>> >> error. We try to make it work properly
>> >>
>> >> Change-Id: Id6fc221c5cc08934b7324dd5d319b93c56e2e678
>> >> Signed-off-by: Michael Trimarchi <michael at amarulasolutions.com>
>> >> ---
>> >>  common/spl/spl_nand.c | 34 +++++++++++++++++++++++++---------
>> >>  1 file changed, 25 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
>> >> index 9a52500..b96fce2 100644
>> >> --- a/common/spl/spl_nand.c
>> >> +++ b/common/spl/spl_nand.c
>> >> @@ -44,9 +44,7 @@ static int spl_nand_load_element(struct spl_image_info *spl_image,
>> >>  {
>> >>       int err;
>> >>
>> >> -     err = nand_spl_load_image(offset, sizeof(*header), (void *)header);
>> >> -     if (err)
>> >> -             return err;
>> >> +     nand_spl_load_image(offset, sizeof(*header), (void *)header);
>> >>
>> >>       if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
>> >>           image_get_magic(header) == FDT_MAGIC) {
>> >> @@ -59,13 +57,31 @@ static int spl_nand_load_element(struct spl_image_info *spl_image,
>> >>               load.bl_len = 1;
>> >>               load.read = spl_nand_fit_read;
>> >>               return spl_load_simple_fit(spl_image, &load, offset, header);
>> >> -     } else {
>> >> -             err = spl_parse_image_header(spl_image, header);
>> >> -             if (err)
>> >> -                     return err;
>> >> -             return nand_spl_load_image(offset, spl_image->size,
>> >> -                                        (void *)(ulong)spl_image->load_addr);
>> >>       }
>> >> +     err = spl_parse_image_header(spl_image, header);
>> >> +     if (err)
>> >> +             return err;
>> >> +
>> >> +     nand_spl_load_image(offset, spl_image->size,
>> >> +                        (void *)(ulong)spl_image->load_addr);
>> >> +
>> >> +     /*
>> >> +      * Logic of the error is inverted for image_check* functions.
>> >> +      * We want to verify that header is correct and the data are correct
>> >> +      * for LEGACY image type
>> >> +      */
>> >> +     err = image_check_hcrc((const image_header_t *)spl_image->load_addr);
>> >> +     if (!err) {
>> >>
>> > Why not simply:
>> >         if (!image_check_hcrc(...) {
>> >> +             debug("Header checksum failed\n");
>> >> +             return -EINVAL;
>> >> +     }
>> >> +     err = image_check_dcrc((const image_header_t *)spl_image->load_addr);
>> >> +     if (!err) {
>> >>
>> > dto.
>>
>> Just because I want to be in the style of this file ;)
>>
> In other places the 'err' variable is used as return value.
> If you don't need it for that, it's vain to assign a value to it.
>

Let me collect all the comments and I will follow your suggestion on next post

Michael

>
> Lothar Waßmann
> --
> ___________________________________________________________
>
> Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
> Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
> Geschäftsführer: Matthias Kaussen
> Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
>
> www.karo-electronics.de | info at karo-electronics.de
> ___________________________________________________________



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |