| Message ID | 20231020184253.21915-1-semen.protsenko@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | board: samsung: Remove incorrect SYS_CONFIG_NAME in axy17lte Kconfig | expand |
Hi! 2023년 10월 21일 (토) 09:10, Sam Protsenko <semen.protsenko@linaro.org>님이 작성: > The global SYS_CONFIG_NAME in axy17lte Kconfig overrides SYS_CONFIG_NAME > for all boards specified after this line in > arch/arm/mach-exynos/Kconfig: > > source "board/samsung/axy17lte/Kconfig" > > Right now it's the last 'source' line there, so the issue is not > reproducible. But once some board is moved or added after this line the > next build error will happen: > > GEN include/autoconf.mk.dep > In file included from ./include/common.h:16: > include/config.h:3:10: fatal error: configs/exynos78x0-common.h.h: > No such file or directory > 3 | #include <configs/exynos78x0-common.h.h> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > compilation terminated. > > That's happening because axy17lte Kconfig defines SYS_CONFIG_NAME option > in global namespace (not guarded with any "if TARGET_..."), so it > basically rewrites the correct SYS_CONFIG_NAME defined in the > hypothetical boards which might appear after axy17lte in mach-exynos > Kconfig. > > Another side of the issue is that SYS_CONFIG_NAME is defined > incorrectly in axy17lte Kconfig: > > config SYS_CONFIG_NAME > default "exynos78x0-common.h" > > The .h extension should not have been specified there. It's leading to > the build error, as the generated include file has double '.h' extension. > > Fix the issue by simply removing the SYS_CONFIG_NAME definition in > axy17lte Kconfig, as all related defconfigs already declare internal > values for SYS_CONFIG_NAME: > > - a5y17lte_defconfig > - a7y17lte_defconfig > - a3y17lte_defconfig > > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> > Fixes: 3e2095e960b4 ("board: samsung: add support for Galaxy A series of > 2017 (a5y17lte)") > --- > board/samsung/axy17lte/Kconfig | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/board/samsung/axy17lte/Kconfig > b/board/samsung/axy17lte/Kconfig > index a018547ff5d4..209394a8a5a8 100644 > --- a/board/samsung/axy17lte/Kconfig > +++ b/board/samsung/axy17lte/Kconfig > @@ -1,11 +1,3 @@ > -config SYS_CONFIG_NAME > - string "Board configuration name" > - default "exynos78x0-common.h" > - help > - This option contains information about board configuration name. > - Based on this option include/configs/<CONFIG_SYS_CONFIG_NAME>.h > header > - will be used for board configuration. > - > if TARGET_A5Y17LTE > config SYS_BOARD > default "axy17lte" > -- > 2.39.2 > This patch makes a build error since axy17lte.h was not exist. Am I miss something? And.. actually I can't catch what you exactly fix with this patch. Thanks. Minkyu Kang.
On Fri, Oct 20, 2023 at 01:42:53PM -0500, Sam Protsenko wrote: > The global SYS_CONFIG_NAME in axy17lte Kconfig overrides SYS_CONFIG_NAME > for all boards specified after this line in > arch/arm/mach-exynos/Kconfig: > > source "board/samsung/axy17lte/Kconfig" > > Right now it's the last 'source' line there, so the issue is not > reproducible. But once some board is moved or added after this line the > next build error will happen: Good spotting on this one. [snip] > diff --git a/board/samsung/axy17lte/Kconfig b/board/samsung/axy17lte/Kconfig > index a018547ff5d4..209394a8a5a8 100644 > --- a/board/samsung/axy17lte/Kconfig > +++ b/board/samsung/axy17lte/Kconfig > @@ -1,11 +1,3 @@ > -config SYS_CONFIG_NAME > - string "Board configuration name" > - default "exynos78x0-common.h" > - help > - This option contains information about board configuration name. > - Based on this option include/configs/<CONFIG_SYS_CONFIG_NAME>.h header > - will be used for board configuration. > - > if TARGET_A5Y17LTE > config SYS_BOARD > default "axy17lte" But this isn't right as Minkyu points out because SYS_CONFIG_NAME isn't set for these platforms now. The fix is to do as board/freescale/lx2160a/Kconfig does for example and have SYS_CONFIG_NAME under each if TARGET_... stanza, even if we're repeating the value. It may or may not look cleaner to do: config SYS_CONFIG_NAME default "exynos78x0-common.h" if TARGET_A5Y17LTE || \ TARGET_A7Y17LTE || TARGET_A3Y17LTE In the file directly, instead.
On Thu, Nov 9, 2023 at 6:53 AM Tom Rini <trini@konsulko.com> wrote: > > On Fri, Oct 20, 2023 at 01:42:53PM -0500, Sam Protsenko wrote: > > > The global SYS_CONFIG_NAME in axy17lte Kconfig overrides SYS_CONFIG_NAME > > for all boards specified after this line in > > arch/arm/mach-exynos/Kconfig: > > > > source "board/samsung/axy17lte/Kconfig" > > > > Right now it's the last 'source' line there, so the issue is not > > reproducible. But once some board is moved or added after this line the > > next build error will happen: > > Good spotting on this one. > > [snip] > > diff --git a/board/samsung/axy17lte/Kconfig b/board/samsung/axy17lte/Kconfig > > index a018547ff5d4..209394a8a5a8 100644 > > --- a/board/samsung/axy17lte/Kconfig > > +++ b/board/samsung/axy17lte/Kconfig > > @@ -1,11 +1,3 @@ > > -config SYS_CONFIG_NAME > > - string "Board configuration name" > > - default "exynos78x0-common.h" > > - help > > - This option contains information about board configuration name. > > - Based on this option include/configs/<CONFIG_SYS_CONFIG_NAME>.h header > > - will be used for board configuration. > > - > > if TARGET_A5Y17LTE > > config SYS_BOARD > > default "axy17lte" > > But this isn't right as Minkyu points out because SYS_CONFIG_NAME isn't > set for these platforms now. The fix is to do as > board/freescale/lx2160a/Kconfig does for example and have > SYS_CONFIG_NAME under each if TARGET_... stanza, even if we're repeating > the value. It may or may not look cleaner to do: > config SYS_CONFIG_NAME > default "exynos78x0-common.h" if TARGET_A5Y17LTE || \ > TARGET_A7Y17LTE || TARGET_A3Y17LTE > > In the file directly, instead. > Thanks for the review! I just sent v2 [1], hopefully addressing your and Minkyu's comments. And also threw in [PATCH v2 2/2] this time, reworking defconfigs accordingly. [1] https://lists.denx.de/pipermail/u-boot/2023-November/536981.html > -- > Tom
diff --git a/board/samsung/axy17lte/Kconfig b/board/samsung/axy17lte/Kconfig index a018547ff5d4..209394a8a5a8 100644 --- a/board/samsung/axy17lte/Kconfig +++ b/board/samsung/axy17lte/Kconfig @@ -1,11 +1,3 @@ -config SYS_CONFIG_NAME - string "Board configuration name" - default "exynos78x0-common.h" - help - This option contains information about board configuration name. - Based on this option include/configs/<CONFIG_SYS_CONFIG_NAME>.h header - will be used for board configuration. - if TARGET_A5Y17LTE config SYS_BOARD default "axy17lte"
The global SYS_CONFIG_NAME in axy17lte Kconfig overrides SYS_CONFIG_NAME for all boards specified after this line in arch/arm/mach-exynos/Kconfig: source "board/samsung/axy17lte/Kconfig" Right now it's the last 'source' line there, so the issue is not reproducible. But once some board is moved or added after this line the next build error will happen: GEN include/autoconf.mk.dep In file included from ./include/common.h:16: include/config.h:3:10: fatal error: configs/exynos78x0-common.h.h: No such file or directory 3 | #include <configs/exynos78x0-common.h.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. That's happening because axy17lte Kconfig defines SYS_CONFIG_NAME option in global namespace (not guarded with any "if TARGET_..."), so it basically rewrites the correct SYS_CONFIG_NAME defined in the hypothetical boards which might appear after axy17lte in mach-exynos Kconfig. Another side of the issue is that SYS_CONFIG_NAME is defined incorrectly in axy17lte Kconfig: config SYS_CONFIG_NAME default "exynos78x0-common.h" The .h extension should not have been specified there. It's leading to the build error, as the generated include file has double '.h' extension. Fix the issue by simply removing the SYS_CONFIG_NAME definition in axy17lte Kconfig, as all related defconfigs already declare internal values for SYS_CONFIG_NAME: - a5y17lte_defconfig - a7y17lte_defconfig - a3y17lte_defconfig Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org> Fixes: 3e2095e960b4 ("board: samsung: add support for Galaxy A series of 2017 (a5y17lte)") --- board/samsung/axy17lte/Kconfig | 8 -------- 1 file changed, 8 deletions(-)