diff mbox series

[v2,1/2] board: samsung: Fix SYS_CONFIG_NAME configs in axy17lte Kconfig

Message ID 20231109201313.29791-1-semen.protsenko@linaro.org
State New
Headers show
Series [v2,1/2] board: samsung: Fix SYS_CONFIG_NAME configs in axy17lte Kconfig | expand

Commit Message

Sam Protsenko Nov. 9, 2023, 8:13 p.m. UTC 
There is a couple of issues related to SYS_CONFIG_NAME config in
axy17lte Kconfig.

1. 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.

2. 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 a build error, as the generated include file has a double '.h'
   extension.

3. Each target in axy17lte/Kconfig defines its own SYS_CONFIG_NAME. But
   all of those in fact incorrect, as corresponding
   include/configs/<CONFIG_SYS_CONFIG_NAME>.h header files don't exist.

4. The global SYS_CONFIG_NAME pretty much repeats the help description
   from arch/Kconfig and doc/README.kconfig.

Corresponding defconfig files (a*y17lte_defconfig) fix above issues by
overriding SYS_CONFIG_NAME and correctly setting it to
"exynos78x0-common".

Fix all mentioned issues by removing the incorrect global
SYS_CONFIG_NAME and instead specifying it (correctly) in SYS_CONFIG_NAME
options for each target instead.

Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
Fixes: 3e2095e960b4 ("board: samsung: add support for Galaxy A series of 2017 (a5y17lte)")
---
Changes in v2:
  - Don't just remove global SYS_CONFIG_NAME, also fix SYS_CONFIG_NAME
    for each target in Kconfig

 board/samsung/axy17lte/Kconfig | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Sam Protsenko Nov. 21, 2023, 5:38 p.m. UTC | #1 
Hi Tom, Minkyu,

On Thu, Nov 9, 2023 at 2:13 PM Sam Protsenko <semen.protsenko@linaro.org> wrote:
>
> There is a couple of issues related to SYS_CONFIG_NAME config in
> axy17lte Kconfig.
>
> 1. 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.
>
> 2. 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 a build error, as the generated include file has a double '.h'
>    extension.
>
> 3. Each target in axy17lte/Kconfig defines its own SYS_CONFIG_NAME. But
>    all of those in fact incorrect, as corresponding
>    include/configs/<CONFIG_SYS_CONFIG_NAME>.h header files don't exist.
>
> 4. The global SYS_CONFIG_NAME pretty much repeats the help description
>    from arch/Kconfig and doc/README.kconfig.
>
> Corresponding defconfig files (a*y17lte_defconfig) fix above issues by
> overriding SYS_CONFIG_NAME and correctly setting it to
> "exynos78x0-common".
>
> Fix all mentioned issues by removing the incorrect global
> SYS_CONFIG_NAME and instead specifying it (correctly) in SYS_CONFIG_NAME
> options for each target instead.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> Fixes: 3e2095e960b4 ("board: samsung: add support for Galaxy A series of 2017 (a5y17lte)")
> ---

If this series (v2) looks good to you, can you please apply it? It's
needed for the new Exynos dev board port I'm working on right now, and
expect to submit the patches soon.

Thanks!

> Changes in v2:
>   - Don't just remove global SYS_CONFIG_NAME, also fix SYS_CONFIG_NAME
>     for each target in Kconfig
>
>  board/samsung/axy17lte/Kconfig | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/board/samsung/axy17lte/Kconfig b/board/samsung/axy17lte/Kconfig
> index a018547ff5d4..64a4ffa7e673 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"
> @@ -16,7 +8,7 @@ config SYS_VENDOR
>         default "samsung"
>
>  config SYS_CONFIG_NAME
> -       default "a5y17lte"
> +       default "exynos78x0-common"
>
>  config EXYNOS7880
>      bool "Exynos 7880 SOC support"
> @@ -33,7 +25,7 @@ config SYS_VENDOR
>         default "samsung"
>
>  config SYS_CONFIG_NAME
> -       default "a5y17lte"
> +       default "exynos78x0-common"
>
>  config EXYNOS7880
>      bool "Exynos 7880 SOC support"
> @@ -50,7 +42,7 @@ config SYS_VENDOR
>         default "samsung"
>
>  config SYS_CONFIG_NAME
> -       default "a3y17lte"
> +       default "exynos78x0-common"
>
>  config EXYNOS7870
>      bool "Exynos 7870 SOC support"
> --
> 2.39.2
>
Minkyu Kang Nov. 28, 2023, 3:02 a.m. UTC | #2 
Hi!


2023년 11월 22일 (수) 02:38, Sam Protsenko <semen.protsenko@linaro.org>님이 작성:

> Hi Tom, Minkyu,
>
> On Thu, Nov 9, 2023 at 2:13 PM Sam Protsenko <semen.protsenko@linaro.org>
> wrote:
> >
> > There is a couple of issues related to SYS_CONFIG_NAME config in
> > axy17lte Kconfig.
> >
> > 1. 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.
> >
> > 2. 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 a build error, as the generated include file has a double '.h'
> >    extension.
> >
> > 3. Each target in axy17lte/Kconfig defines its own SYS_CONFIG_NAME. But
> >    all of those in fact incorrect, as corresponding
> >    include/configs/<CONFIG_SYS_CONFIG_NAME>.h header files don't exist.
> >
> > 4. The global SYS_CONFIG_NAME pretty much repeats the help description
> >    from arch/Kconfig and doc/README.kconfig.
> >
> > Corresponding defconfig files (a*y17lte_defconfig) fix above issues by
> > overriding SYS_CONFIG_NAME and correctly setting it to
> > "exynos78x0-common".
> >
> > Fix all mentioned issues by removing the incorrect global
> > SYS_CONFIG_NAME and instead specifying it (correctly) in SYS_CONFIG_NAME
> > options for each target instead.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > Fixes: 3e2095e960b4 ("board: samsung: add support for Galaxy A series of
> 2017 (a5y17lte)")
> > ---
>
> If this series (v2) looks good to you, can you please apply it? It's
> needed for the new Exynos dev board port I'm working on right now, and
> expect to submit the patches soon.
>
> Thanks!
>
> > Changes in v2:
> >   - Don't just remove global SYS_CONFIG_NAME, also fix SYS_CONFIG_NAME
> >     for each target in Kconfig
> >
> >  board/samsung/axy17lte/Kconfig | 14 +++-----------
> >  1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/board/samsung/axy17lte/Kconfig
> b/board/samsung/axy17lte/Kconfig
> > index a018547ff5d4..64a4ffa7e673 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"
> > @@ -16,7 +8,7 @@ config SYS_VENDOR
> >         default "samsung"
> >
> >  config SYS_CONFIG_NAME
> > -       default "a5y17lte"
> > +       default "exynos78x0-common"
> >
> >  config EXYNOS7880
> >      bool "Exynos 7880 SOC support"
> > @@ -33,7 +25,7 @@ config SYS_VENDOR
> >         default "samsung"
> >
> >  config SYS_CONFIG_NAME
> > -       default "a5y17lte"
> > +       default "exynos78x0-common"
> >
> >  config EXYNOS7880
> >      bool "Exynos 7880 SOC support"
> > @@ -50,7 +42,7 @@ config SYS_VENDOR
> >         default "samsung"
> >
> >  config SYS_CONFIG_NAME
> > -       default "a3y17lte"
> > +       default "exynos78x0-common"
> >
> >  config EXYNOS7870
> >      bool "Exynos 7870 SOC support"
> > --
> > 2.39.2
> >


applied to u-boot-samsung.

Thanks.
Minkyu Kang.
diff mbox series

Patch

diff --git a/board/samsung/axy17lte/Kconfig b/board/samsung/axy17lte/Kconfig
index a018547ff5d4..64a4ffa7e673 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"
@@ -16,7 +8,7 @@  config SYS_VENDOR
 	default "samsung"
 
 config SYS_CONFIG_NAME
-	default "a5y17lte"
+	default "exynos78x0-common"
 
 config EXYNOS7880
     bool "Exynos 7880 SOC support"
@@ -33,7 +25,7 @@  config SYS_VENDOR
 	default "samsung"
 
 config SYS_CONFIG_NAME
-	default "a5y17lte"
+	default "exynos78x0-common"
 
 config EXYNOS7880
     bool "Exynos 7880 SOC support"
@@ -50,7 +42,7 @@  config SYS_VENDOR
 	default "samsung"
 
 config SYS_CONFIG_NAME
-	default "a3y17lte"
+	default "exynos78x0-common"
 
 config EXYNOS7870
     bool "Exynos 7870 SOC support"