[PATCH 1/4] Respect that some compression algos can be enabled separately for SPL

[Stefano Babic]

Hallo Tim,

On 04.02.21 18:31, Tim Harvey wrote:
> On Sat, Jan 30, 2021 at 1:23 PM Stefano Babic <sbabic at denx.de> wrote:
>>
>> Hi everybody,
>>
>> On 27.01.21 14:19, Stefano Babic wrote:
>>> On 27.01.21 08:57, Frieder Schrempf wrote:
>>>> On 26.01.21 18:53, Tim Harvey wrote:
>>>>> On Sat, Jan 23, 2021 at 4:39 AM Stefano Babic <sbabic at denx.de> wrote:
>>>>>>
>>>>>> Hi Tim,
>>>>>>
>>>>>> there is a weird side effect with this patch, breaking m68k
>>>>>> architecture. For some reason, image.c is not compiled clean because gd
>>>>>> is not declared anymore.
>>>>>>
>>>>>> In file included from tools/common/image.c:1:
>>>>>> ./tools/../common/image.c: In function ‘get_table_entry_id’:
>>>>>> ./tools/../common/image.c:982:41: error: ‘gd’ undeclared (first use in
>>>>>> this function)
>>>>>>      982 |   if (t->sname && strcasecmp(t->sname + gd->reloc_off, name)
>>>>>> == 0)
>>>>>>          |                                         ^~
>>>>>> ./tools/../common/image.c:982:41: note: each undeclared identifier is
>>>>>> reported only once for each functi
>>>>>>
>>>>>> Can you take a look ? Thanks !
>>>>>>
>>>>>
>>>>> Stefano,
>>>>>
>>>>> I have no idea what could cause this... must be something to do with
>>>>> including kconfig.h?
>>>>>
>>>>> What board/target would I build to reproduce this?
>>>>>
>>>>> Frieder, do you have any ideas?
>>>>
>>>> I can't remember if I did a full test on all platforms when I wrote this
>>>> patch. Probably not.
>>>>
>>>> And I have no idea what is causing this, nor do I currently have the
>>>> time to have a closer look, sorry.
>>>>
>>>> BTW, this is a very good example for what I said earlier:
>>>>
>>>> "I failed to do upstreaming work for U-Boot for quite some time. Mainly
>>>> because whenever I try to, I end up fixing/cleaning some weird U-Boot
>>>> code/behavior and spending way more time than I actually have available."
>>>
>>> True, but...we cannot even break other boards, too. I have also get a
>>> short look at the issue, but I could not find myself the reason, too. I
>>> plan to get a deeper look in the week end.
>>
>> It looks like that only for m68k the following statement does not work
>>
>> #if CONFIG_IS_ENABLED(GZIP) && !defined(USE_HOSTCC)
>>
>> But if I replace this with the equivalent one:
>>
>> +#ifndef USE_HOSTCC
>> +#if CONFIG_IS_ENABLED(GZIP)
>>
>> This works - someone else should have discovered this before, because in
>> common/image.c USE_HOSTCC is never anded with another CONFIG.
>>
>> I have not found why this happens just for m68k, no issue with
>> arm/aarch64 or PowerPC. I send a V2 with the changes like above.
>>
>> @Tim: why do we need include/kconfig ?
> 
> Stefano,
> 
> If we switch to your method above we don't need it (it was needed for
> what I submitted).

Ok

> 
> Shall I resubmit with your suggested change around
> GZIP/BZIP2/LZMA/LZO/LZ4/ZSTD in image_decomp?

Yes, please.

> And if so, with these
> changes would I replace Frieder's sign-off with a Cc as the patch has
> changed quite a bit from his original derived work?

No, I do not think so. Original work is from Frieder, and his work 
should be tracked. You add of course your Signed-off-by, but without 
removing Frieder's.

> 
> Have you had any time to review the reset of the patch series yet?

The rest, you mean :-)

It was ok for me and I was already merging the whole series until I got 
this weird error on m68k.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================