On Thu, Feb 4, 2021 at 12:49 PM Stefano Babic <sba...@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 <sba...@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 <sba...@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