diff mbox series

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

Message ID 1610557225-3822-2-git-send-email-tharvey@gateworks.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series [1/4] Respect that some compression algos can be enabled separately for SPL | expand

Commit Message

Tim Harvey Jan. 13, 2021, 5 p.m. UTC
From: Frieder Schrempf <frieder.schrempf@kontron.de>

Some compression algorithms currently can be enabled for SPL and
U-Boot proper separately. Therefore we need to use
CONFIG_IS_ENABLED() in these cases and also prevent compiling these
functions in case of a host tool build.

Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.de>
Signed-off-by: Tim Harvey <tharvey@gateworks.com>
---
 common/image.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Stefano Babic Jan. 23, 2021, 12:39 p.m. UTC | #1
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 !

Best regards,
Stefano

On 13.01.21 18:00, Tim Harvey wrote:
> From: Frieder Schrempf <frieder.schrempf@kontron.de>
> 
> Some compression algorithms currently can be enabled for SPL and
> U-Boot proper separately. Therefore we need to use
> CONFIG_IS_ENABLED() in these cases and also prevent compiling these
> functions in case of a host tool build.
> 
> Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.d
> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> ---
>  common/image.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/common/image.c b/common/image.c
> index 451fc68..bda19c0 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -72,6 +72,7 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
>  
>  #include <u-boot/crc.h>
>  #include <imximage.h>
> +#include <linux/kconfig.h>
>  
>  #ifndef CONFIG_SYS_BARGSIZE
>  #define CONFIG_SYS_BARGSIZE 512
> @@ -460,13 +461,13 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
>  		else
>  			ret = -ENOSPC;
>  		break;
> -#ifdef CONFIG_GZIP
> +#if CONFIG_IS_ENABLED(GZIP) && !defined(USE_HOSTCC)
>  	case IH_COMP_GZIP: {
>  		ret = gunzip(load_buf, unc_len, image_buf, &image_len);
>  		break;
>  	}
>  #endif /* CONFIG_GZIP */
> -#ifdef CONFIG_BZIP2
> +#if CONFIG_IS_ENABLED(BZIP2) && !defined(USE_HOSTCC)
>  	case IH_COMP_BZIP2: {
>  		uint size = unc_len;
>  
> @@ -482,7 +483,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
>  		break;
>  	}
>  #endif /* CONFIG_BZIP2 */
> -#ifdef CONFIG_LZMA
> +#if CONFIG_IS_ENABLED(LZMA) && !defined(USE_HOSTCC)
>  	case IH_COMP_LZMA: {
>  		SizeT lzma_len = unc_len;
>  
> @@ -492,7 +493,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
>  		break;
>  	}
>  #endif /* CONFIG_LZMA */
> -#ifdef CONFIG_LZO
> +#if CONFIG_IS_ENABLED(LZO) && !defined(USE_HOSTCC)
>  	case IH_COMP_LZO: {
>  		size_t size = unc_len;
>  
> @@ -501,7 +502,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
>  		break;
>  	}
>  #endif /* CONFIG_LZO */
> -#ifdef CONFIG_LZ4
> +#if CONFIG_IS_ENABLED(LZ4) && !defined(USE_HOSTCC)
>  	case IH_COMP_LZ4: {
>  		size_t size = unc_len;
>  
> @@ -510,7 +511,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
>  		break;
>  	}
>  #endif /* CONFIG_LZ4 */
> -#ifdef CONFIG_ZSTD
> +#if CONFIG_IS_ENABLED(ZSTD) && !defined(USE_HOSTCC)
>  	case IH_COMP_ZSTD: {
>  		size_t size = unc_len;
>  		ZSTD_DStream *dstream;
>
Tim Harvey Jan. 26, 2021, 5:53 p.m. UTC | #2
On Sat, Jan 23, 2021 at 4:39 AM Stefano Babic <sbabic@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?

Tim

> Best regards,
> Stefano
>
> On 13.01.21 18:00, Tim Harvey wrote:
> > From: Frieder Schrempf <frieder.schrempf@kontron.de>
> >
> > Some compression algorithms currently can be enabled for SPL and
> > U-Boot proper separately. Therefore we need to use
> > CONFIG_IS_ENABLED() in these cases and also prevent compiling these
> > functions in case of a host tool build.
> >
> > Signed-off-by: Frieder Schrempf <frieder.schrempf@kontron.d
> > Signed-off-by: Tim Harvey <tharvey@gateworks.com>
> > ---
> >  common/image.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/common/image.c b/common/image.c
> > index 451fc68..bda19c0 100644
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -72,6 +72,7 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
> >
> >  #include <u-boot/crc.h>
> >  #include <imximage.h>
> > +#include <linux/kconfig.h>
> >
> >  #ifndef CONFIG_SYS_BARGSIZE
> >  #define CONFIG_SYS_BARGSIZE 512
> > @@ -460,13 +461,13 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >               else
> >                       ret = -ENOSPC;
> >               break;
> > -#ifdef CONFIG_GZIP
> > +#if CONFIG_IS_ENABLED(GZIP) && !defined(USE_HOSTCC)
> >       case IH_COMP_GZIP: {
> >               ret = gunzip(load_buf, unc_len, image_buf, &image_len);
> >               break;
> >       }
> >  #endif /* CONFIG_GZIP */
> > -#ifdef CONFIG_BZIP2
> > +#if CONFIG_IS_ENABLED(BZIP2) && !defined(USE_HOSTCC)
> >       case IH_COMP_BZIP2: {
> >               uint size = unc_len;
> >
> > @@ -482,7 +483,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >               break;
> >       }
> >  #endif /* CONFIG_BZIP2 */
> > -#ifdef CONFIG_LZMA
> > +#if CONFIG_IS_ENABLED(LZMA) && !defined(USE_HOSTCC)
> >       case IH_COMP_LZMA: {
> >               SizeT lzma_len = unc_len;
> >
> > @@ -492,7 +493,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >               break;
> >       }
> >  #endif /* CONFIG_LZMA */
> > -#ifdef CONFIG_LZO
> > +#if CONFIG_IS_ENABLED(LZO) && !defined(USE_HOSTCC)
> >       case IH_COMP_LZO: {
> >               size_t size = unc_len;
> >
> > @@ -501,7 +502,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >               break;
> >       }
> >  #endif /* CONFIG_LZO */
> > -#ifdef CONFIG_LZ4
> > +#if CONFIG_IS_ENABLED(LZ4) && !defined(USE_HOSTCC)
> >       case IH_COMP_LZ4: {
> >               size_t size = unc_len;
> >
> > @@ -510,7 +511,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >               break;
> >       }
> >  #endif /* CONFIG_LZ4 */
> > -#ifdef CONFIG_ZSTD
> > +#if CONFIG_IS_ENABLED(ZSTD) && !defined(USE_HOSTCC)
> >       case IH_COMP_ZSTD: {
> >               size_t size = unc_len;
> >               ZSTD_DStream *dstream;
> >
>
>
> --
> =====================================================================
> 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@denx.de
> =====================================================================
Frieder Schrempf Jan. 27, 2021, 7:57 a.m. UTC | #3
On 26.01.21 18:53, Tim Harvey wrote:
> On Sat, Jan 23, 2021 at 4:39 AM Stefano Babic <sbabic@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."
Stefano Babic Jan. 27, 2021, 1:19 p.m. UTC | #4
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@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.

Regards,
Stefano
Stefano Babic Jan. 30, 2021, 9:22 p.m. UTC | #5
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@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 ?

Best regards,
Stefano
Tim Harvey Feb. 4, 2021, 5:31 p.m. UTC | #6
On Sat, Jan 30, 2021 at 1:23 PM Stefano Babic <sbabic@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@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).

Shall I resubmit with your suggested change around
GZIP/BZIP2/LZMA/LZO/LZ4/ZSTD in image_decomp? 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?

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

Best regards,

Tim
Stefano Babic Feb. 4, 2021, 8:49 p.m. UTC | #7
Hallo Tim,

On 04.02.21 18:31, Tim Harvey wrote:
> On Sat, Jan 30, 2021 at 1:23 PM Stefano Babic <sbabic@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@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
Tim Harvey Feb. 5, 2021, 1:37 a.m. UTC | #8
On Thu, Feb 4, 2021 at 12:49 PM Stefano Babic <sbabic@denx.de> wrote:
>
> Hallo Tim,
>
> On 04.02.21 18:31, Tim Harvey wrote:
> > On Sat, Jan 30, 2021 at 1:23 PM Stefano Babic <sbabic@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@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.
>

Ok, I'll submit a v2 with the changes here. The pmic driver already
got applied so the series will drop that patch.

Thanks,

Tim
diff mbox series

Patch

diff --git a/common/image.c b/common/image.c
index 451fc68..bda19c0 100644
--- a/common/image.c
+++ b/common/image.c
@@ -72,6 +72,7 @@  static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
 
 #include <u-boot/crc.h>
 #include <imximage.h>
+#include <linux/kconfig.h>
 
 #ifndef CONFIG_SYS_BARGSIZE
 #define CONFIG_SYS_BARGSIZE 512
@@ -460,13 +461,13 @@  int image_decomp(int comp, ulong load, ulong image_start, int type,
 		else
 			ret = -ENOSPC;
 		break;
-#ifdef CONFIG_GZIP
+#if CONFIG_IS_ENABLED(GZIP) && !defined(USE_HOSTCC)
 	case IH_COMP_GZIP: {
 		ret = gunzip(load_buf, unc_len, image_buf, &image_len);
 		break;
 	}
 #endif /* CONFIG_GZIP */
-#ifdef CONFIG_BZIP2
+#if CONFIG_IS_ENABLED(BZIP2) && !defined(USE_HOSTCC)
 	case IH_COMP_BZIP2: {
 		uint size = unc_len;
 
@@ -482,7 +483,7 @@  int image_decomp(int comp, ulong load, ulong image_start, int type,
 		break;
 	}
 #endif /* CONFIG_BZIP2 */
-#ifdef CONFIG_LZMA
+#if CONFIG_IS_ENABLED(LZMA) && !defined(USE_HOSTCC)
 	case IH_COMP_LZMA: {
 		SizeT lzma_len = unc_len;
 
@@ -492,7 +493,7 @@  int image_decomp(int comp, ulong load, ulong image_start, int type,
 		break;
 	}
 #endif /* CONFIG_LZMA */
-#ifdef CONFIG_LZO
+#if CONFIG_IS_ENABLED(LZO) && !defined(USE_HOSTCC)
 	case IH_COMP_LZO: {
 		size_t size = unc_len;
 
@@ -501,7 +502,7 @@  int image_decomp(int comp, ulong load, ulong image_start, int type,
 		break;
 	}
 #endif /* CONFIG_LZO */
-#ifdef CONFIG_LZ4
+#if CONFIG_IS_ENABLED(LZ4) && !defined(USE_HOSTCC)
 	case IH_COMP_LZ4: {
 		size_t size = unc_len;
 
@@ -510,7 +511,7 @@  int image_decomp(int comp, ulong load, ulong image_start, int type,
 		break;
 	}
 #endif /* CONFIG_LZ4 */
-#ifdef CONFIG_ZSTD
+#if CONFIG_IS_ENABLED(ZSTD) && !defined(USE_HOSTCC)
 	case IH_COMP_ZSTD: {
 		size_t size = unc_len;
 		ZSTD_DStream *dstream;