On 30/12/2019 02.21, Simon Glass wrote:
> Hi Rasmus,
> 
> On Wed, 11 Dec 2019 at 04:03, Rasmus Villemoes
> <rasmus.villem...@prevas.dk> wrote:
>>
>> From: "Klaus H. Sorensen" <k...@prevas.dk>
>>
>> Allow reading compressed content from fit image, even if
>> CONFIG_SPL_OS_BOOT is not set.
>>
>> This allow booting compressed 2nd stage u-boot from fit image.
>>
>> Additionally, do not print warning message if compression node is not
>> found, since it simply implies the content is uncompressed.
>>
>> Signed-off-by: Klaus H. Sorensen <k...@prevas.dk>
>> Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk>
>> ---
>>  common/spl/spl_fit.c | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index b3e3ccd5a2..98271eb878 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -195,11 +195,9 @@ static int spl_load_fit_image(struct spl_load_info 
>> *info, ulong sector,
>>                         debug("%s ", genimg_get_type_name(type));
>>         }
>>
>> -       if (IS_ENABLED(CONFIG_SPL_OS_BOOT) && IS_ENABLED(CONFIG_SPL_GZIP)) {
>> -               if (fit_image_get_comp(fit, node, &image_comp))
>> -                       puts("Cannot get image compression format.\n");
> 
> You dropped this error, which seems necessary.

How so? There's no change in control flow, and the code proceeds exactly
as if 'compression = "none"' was specified (or, in fact, for some
reason, 'compression = "bzip2"' or anything else at all also ends up
with the image just being memcpy'ed to its loadaddr).

If a 'compression = "foo"' property is supposed to be mandatory (even to
explicitly specify "none"), I'm fine leaving it in. But it seems that
what's really needed is an error message (and actually erroring out) if
the compression property specifies something other than "none" or "gzip".

Rasmus

Reply via email to