Re: [U-Boot] [PATCH 1/2] spl: Fix redundant image of uboot
Hi
On Wed, Jul 4, 2018 at 5:10 PM, Lothar Waßmann wrote:
> Michael Nazzareno Trimarchi wrote:
>
>> Hi
>>
>> On Wed, Jul 4, 2018 at 4:19 PM, Lothar Waßmann
>> 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
>> >> ---
>> >> 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, , 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 | i...@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 |
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] spl: Fix redundant image of uboot
Michael Nazzareno Trimarchi wrote:
> Hi
>
> On Wed, Jul 4, 2018 at 4:19 PM, Lothar Waßmann
> 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
> >> ---
> >> 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, , 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.
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 | i...@karo-electronics.de
___
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] spl: Fix redundant image of uboot
Hi
On Wed, Jul 4, 2018 at 4:19 PM, Lothar Waßmann 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
>> ---
>> 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, , 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 ;)
Michael
>
>> + debug("Image checksum failed\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> }
>>
>> static int spl_nand_load_image(struct spl_image_info *spl_image,
>
>
> Lothar Waßmann
--
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO - Founder Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
| [`as] http://www.amarulasolutions.com |
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] spl: Fix redundant image of uboot
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
> ---
> 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, , 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.
> + debug("Image checksum failed\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> }
>
> static int spl_nand_load_image(struct spl_image_info *spl_image,
Lothar Waßmann
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
