| Message ID | 20210719195002.6753-2-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tricore fixes | expand |
On Mon, 19 Jul 2021 at 20:52, Alex Bennée <alex.bennee@linaro.org> wrote: > > We inadvertently added a symbol clash causing the build not to include > the testboard needed for check-tcg. > > Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > configs/devices/tricore-softmmu/default.mak | 1 + > hw/tricore/Kconfig | 3 +-- > hw/tricore/meson.build | 4 ++-- > 3 files changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Peter Maydell <peter.maydell@linaro.org> as far as this fix goes (though maybe CONFIG_TRICORE_TESTBOARD would be better?) But I still don't understand and would like to know: (1) why doesn't CONFIG_TRICORE get set by Kconfig anyway, as f4063f9c31 claims to be doing? (2) what are the CONFIG_$ARCH flags for? Apart from this, we don't seem to be using any of them, as demonstrated by the fact that nothing else broke :-) thanks -- PMM
On 7/19/21 9:50 PM, Alex Bennée wrote: > We inadvertently added a symbol clash causing the build not to include > the testboard needed for check-tcg. > > Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") Sorry for the mess, I remember having tested this carefully. OK, so my patch was sent/tested first: commit f4063f9c319e3924b0c6d09dfe43e94d01253ee0 Message-Id: <20210131111316.232778-6-f4bug@amsat.org> but got merged *after* Bastian's series: commit 582079c9d27fc8cfff9f495072300416e0e4aafe Message-Id: <20210305170045.869437-4-kbastian@mail.uni-paderborn.de> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > --- > configs/devices/tricore-softmmu/default.mak | 1 + > hw/tricore/Kconfig | 3 +-- > hw/tricore/meson.build | 4 ++-- > 3 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/configs/devices/tricore-softmmu/default.mak b/configs/devices/tricore-softmmu/default.mak > index 5cc91cebce..724cb85de7 100644 > --- a/configs/devices/tricore-softmmu/default.mak > +++ b/configs/devices/tricore-softmmu/default.mak > @@ -1 +1,2 @@ > +CONFIG_TRITEST=y > CONFIG_TRIBOARD=y > diff --git a/hw/tricore/Kconfig b/hw/tricore/Kconfig > index 506e6183c1..a1b2438d99 100644 > --- a/hw/tricore/Kconfig > +++ b/hw/tricore/Kconfig > @@ -1,9 +1,8 @@ > -config TRICORE > +config TRITEST > bool > > config TRIBOARD > bool > - select TRICORE > select TC27X_SOC > > config TC27X_SOC > diff --git a/hw/tricore/meson.build b/hw/tricore/meson.build > index 47e36bb077..692a4708ba 100644 > --- a/hw/tricore/meson.build > +++ b/hw/tricore/meson.build > @@ -1,6 +1,6 @@ > tricore_ss = ss.source_set() > -tricore_ss.add(when: 'CONFIG_TRICORE', if_true: files('tricore_testboard.c')) > -tricore_ss.add(when: 'CONFIG_TRICORE', if_true: files('tricore_testdevice.c')) > +tricore_ss.add(when: 'CONFIG_TRITEST', if_true: files('tricore_testboard.c')) > +tricore_ss.add(when: 'CONFIG_TRITEST', if_true: files('tricore_testdevice.c')) > tricore_ss.add(when: 'CONFIG_TRIBOARD', if_true: files('triboard.c')) > tricore_ss.add(when: 'CONFIG_TC27X_SOC', if_true: files('tc27x_soc.c')) > >
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 19 Jul 2021 at 20:52, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> We inadvertently added a symbol clash causing the build not to include >> the testboard needed for check-tcg. >> >> Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> --- >> configs/devices/tricore-softmmu/default.mak | 1 + >> hw/tricore/Kconfig | 3 +-- >> hw/tricore/meson.build | 4 ++-- >> 3 files changed, 4 insertions(+), 4 deletions(-) > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > as far as this fix goes (though maybe CONFIG_TRICORE_TESTBOARD would be better?) > > But I still don't understand and would like to know: > (1) why doesn't CONFIG_TRICORE get set by Kconfig anyway, as > f4063f9c31 claims to be doing? It does (or should) thanks to meson: 'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y' > (2) what are the CONFIG_$ARCH flags for? Apart from this, we > don't seem to be using any of them, as demonstrated by the fact > that nothing else broke :-) They need to be declared in Kconfig otherwise minikconf complains about them not being defined when you pass it in. This is part of minikconf's sanity checking code. > > thanks > -- PMM -- Alex Bennée
On Tue, 20 Jul 2021 at 10:47, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Peter Maydell <peter.maydell@linaro.org> writes: > > > On Mon, 19 Jul 2021 at 20:52, Alex Bennée <alex.bennee@linaro.org> wrote: > >> > >> We inadvertently added a symbol clash causing the build not to include > >> the testboard needed for check-tcg. > >> > >> Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") > >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > >> --- > >> configs/devices/tricore-softmmu/default.mak | 1 + > >> hw/tricore/Kconfig | 3 +-- > >> hw/tricore/meson.build | 4 ++-- > >> 3 files changed, 4 insertions(+), 4 deletions(-) > > > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > as far as this fix goes (though maybe CONFIG_TRICORE_TESTBOARD would be better?) > > > > But I still don't understand and would like to know: > > (1) why doesn't CONFIG_TRICORE get set by Kconfig anyway, as > > f4063f9c31 claims to be doing? > > It does (or should) thanks to meson: > > 'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y' Yeah, but it doesn't, as you can see if you look at the meson build log: we do pass CONFIG_TRICORE=y on the minikconf command line, but it doesn't appear in minikconf's output! > > (2) what are the CONFIG_$ARCH flags for? Apart from this, we > > don't seem to be using any of them, as demonstrated by the fact > > that nothing else broke :-) > > They need to be declared in Kconfig otherwise minikconf complains about > them not being defined when you pass it in. This is part of minikconf's > sanity checking code. No, I mean, if nothing anywhere in the build system is conditional on these flags, why do we have them at all ? We know we don't have anything that cares about them, because right now we have a bug where they're never set... -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On Tue, 20 Jul 2021 at 10:47, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> >> Peter Maydell <peter.maydell@linaro.org> writes: >> >> > On Mon, 19 Jul 2021 at 20:52, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> >> >> We inadvertently added a symbol clash causing the build not to include >> >> the testboard needed for check-tcg. >> >> >> >> Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") >> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> >> --- >> >> configs/devices/tricore-softmmu/default.mak | 1 + >> >> hw/tricore/Kconfig | 3 +-- >> >> hw/tricore/meson.build | 4 ++-- >> >> 3 files changed, 4 insertions(+), 4 deletions(-) >> > >> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >> > as far as this fix goes (though maybe CONFIG_TRICORE_TESTBOARD would be better?) >> > >> > But I still don't understand and would like to know: >> > (1) why doesn't CONFIG_TRICORE get set by Kconfig anyway, as >> > f4063f9c31 claims to be doing? >> >> It does (or should) thanks to meson: >> >> 'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y' > > Yeah, but it doesn't, as you can see if you look at the meson build > log: we do pass CONFIG_TRICORE=y on the minikconf command line, > but it doesn't appear in minikconf's output! > >> > (2) what are the CONFIG_$ARCH flags for? Apart from this, we >> > don't seem to be using any of them, as demonstrated by the fact >> > that nothing else broke :-) >> >> They need to be declared in Kconfig otherwise minikconf complains about >> them not being defined when you pass it in. This is part of minikconf's >> sanity checking code. > > No, I mean, if nothing anywhere in the build system is conditional > on these flags, why do we have them at all ? We know we don't > have anything that cares about them, because right now we have > a bug where they're never set... Well we have one place at the moment to ensure v7m gets included even if you don't include the various M profile boards: default y if TCG && (ARM || AARCH64) which is because translate.c still currently has a dependency on those bits. Without that you'll get a linker failure with the following build: '../../configure' '--without-default-features' '--target-list=arm-softmmu,aarch64-softmmu' '--with-devices-aarch64=minimal' I thought I'd added that to the build matrix but I can't find it now. > > -- PMM -- Alex Bennée
Alex Bennée <alex.bennee@linaro.org> writes: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On Tue, 20 Jul 2021 at 10:47, Alex Bennée <alex.bennee@linaro.org> wrote: >>> >>> >>> Peter Maydell <peter.maydell@linaro.org> writes: >>> >>> > On Mon, 19 Jul 2021 at 20:52, Alex Bennée <alex.bennee@linaro.org> wrote: >>> >> >>> >> We inadvertently added a symbol clash causing the build not to include >>> >> the testboard needed for check-tcg. >>> >> >>> >> Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") >>> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> >> --- >>> >> configs/devices/tricore-softmmu/default.mak | 1 + >>> >> hw/tricore/Kconfig | 3 +-- >>> >> hw/tricore/meson.build | 4 ++-- >>> >> 3 files changed, 4 insertions(+), 4 deletions(-) >>> > >>> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >>> > as far as this fix goes (though maybe CONFIG_TRICORE_TESTBOARD would be better?) >>> > >>> > But I still don't understand and would like to know: >>> > (1) why doesn't CONFIG_TRICORE get set by Kconfig anyway, as >>> > f4063f9c31 claims to be doing? >>> >>> It does (or should) thanks to meson: >>> >>> 'CONFIG_' + config_target['TARGET_ARCH'].to_upper() + '=y' >> >> Yeah, but it doesn't, as you can see if you look at the meson build >> log: we do pass CONFIG_TRICORE=y on the minikconf command line, >> but it doesn't appear in minikconf's output! >> >>> > (2) what are the CONFIG_$ARCH flags for? Apart from this, we >>> > don't seem to be using any of them, as demonstrated by the fact >>> > that nothing else broke :-) >>> >>> They need to be declared in Kconfig otherwise minikconf complains about >>> them not being defined when you pass it in. This is part of minikconf's >>> sanity checking code. >> >> No, I mean, if nothing anywhere in the build system is conditional >> on these flags, why do we have them at all ? We know we don't >> have anything that cares about them, because right now we have >> a bug where they're never set... > > Well we have one place at the moment to ensure v7m gets included even if > you don't include the various M profile boards: > > default y if TCG && (ARM || AARCH64) > > which is because translate.c still currently has a dependency on those > bits. Without that you'll get a linker failure with the following build: > > '../../configure' '--without-default-features' '--target-list=arm-softmmu,aarch64-softmmu' '--with-devices-aarch64=minimal' > > I thought I'd added that to the build matrix but I can't find it now. Ahh still part of the larger series: Subject: [PATCH v16 99/99] gitlab: defend the new stripped down arm64 configs Date: Fri, 4 Jun 2021 16:53:12 +0100 Message-Id: <20210604155312.15902-100-alex.bennee@linaro.org> > >> >> -- PMM -- Alex Bennée
diff --git a/configs/devices/tricore-softmmu/default.mak b/configs/devices/tricore-softmmu/default.mak index 5cc91cebce..724cb85de7 100644 --- a/configs/devices/tricore-softmmu/default.mak +++ b/configs/devices/tricore-softmmu/default.mak @@ -1 +1,2 @@ +CONFIG_TRITEST=y CONFIG_TRIBOARD=y diff --git a/hw/tricore/Kconfig b/hw/tricore/Kconfig index 506e6183c1..a1b2438d99 100644 --- a/hw/tricore/Kconfig +++ b/hw/tricore/Kconfig @@ -1,9 +1,8 @@ -config TRICORE +config TRITEST bool config TRIBOARD bool - select TRICORE select TC27X_SOC config TC27X_SOC diff --git a/hw/tricore/meson.build b/hw/tricore/meson.build index 47e36bb077..692a4708ba 100644 --- a/hw/tricore/meson.build +++ b/hw/tricore/meson.build @@ -1,6 +1,6 @@ tricore_ss = ss.source_set() -tricore_ss.add(when: 'CONFIG_TRICORE', if_true: files('tricore_testboard.c')) -tricore_ss.add(when: 'CONFIG_TRICORE', if_true: files('tricore_testdevice.c')) +tricore_ss.add(when: 'CONFIG_TRITEST', if_true: files('tricore_testboard.c')) +tricore_ss.add(when: 'CONFIG_TRITEST', if_true: files('tricore_testdevice.c')) tricore_ss.add(when: 'CONFIG_TRIBOARD', if_true: files('triboard.c')) tricore_ss.add(when: 'CONFIG_TC27X_SOC', if_true: files('tc27x_soc.c'))
We inadvertently added a symbol clash causing the build not to include the testboard needed for check-tcg. Fixes: f4063f9c31 ("meson: Introduce target-specific Kconfig") Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- configs/devices/tricore-softmmu/default.mak | 1 + hw/tricore/Kconfig | 3 +-- hw/tricore/meson.build | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) -- 2.32.0.264.g75ae10bc75