diff mbox

ARM: EXYNOS: remove CONFIG_MACH_EXYNOS[4, 5]_DT config options

Message ID 3809234.0NkWBIjRC9@amdc1032 (mailing list archive)
State New, archived
Headers show

Commit Message

Bartlomiej Zolnierkiewicz Sept. 13, 2013, 9:28 a.m. UTC 
EXYNOS is now Device Tree (DT) only platform so it makes no sense to have
config options responsible for enabling platform specific DT support.

Moreover the kernel image won't even link if neither CONFIG_MACH_EXYNOS4_DT
nor CONFIG_MACH_EXYNOS5_DT config option is enabled (linker fails with "no
machine record defined" error).

Remove CONFIG_MACH_EXYNOS[4,5]_DT config options and just use the standard
CONFIG_ARCH_EXYNOS[4,5] ones instead.

Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 arch/arm/configs/exynos_defconfig |  1 -
 arch/arm/mach-exynos/Kconfig      | 38 +++++++++-----------------------------
 arch/arm/mach-exynos/Makefile     |  4 ++--
 3 files changed, 11 insertions(+), 32 deletions(-)

Comments

Tomasz Figa Sept. 17, 2013, 10:45 a.m. UTC | #1 
Hi Bart,

On Friday 13 of September 2013 11:28:31 Bartlomiej Zolnierkiewicz wrote:
> EXYNOS is now Device Tree (DT) only platform so it makes no sense to have
> config options responsible for enabling platform specific DT support.
> 
> Moreover the kernel image won't even link if neither
> CONFIG_MACH_EXYNOS4_DT nor CONFIG_MACH_EXYNOS5_DT config option is
> enabled (linker fails with "no machine record defined" error).
> 
> Remove CONFIG_MACH_EXYNOS[4,5]_DT config options and just use the
> standard CONFIG_ARCH_EXYNOS[4,5] ones instead.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/configs/exynos_defconfig |  1 -
>  arch/arm/mach-exynos/Kconfig      | 38
> +++++++++----------------------------- arch/arm/mach-exynos/Makefile    
> |  4 ++--
>  3 files changed, 11 insertions(+), 32 deletions(-)

Generally I see this as a nice cleanup, but I'd go even further than this. 
See below.

> diff --git a/arch/arm/configs/exynos_defconfig
> b/arch/arm/configs/exynos_defconfig index ad7dfbb..5181d0d 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -11,7 +11,6 @@ CONFIG_ARCH_EXYNOS=y
>  CONFIG_S3C_LOWLEVEL_UART_PORT=3
>  CONFIG_S3C24XX_PWM=y
>  CONFIG_ARCH_EXYNOS5=y
> -CONFIG_MACH_EXYNOS4_DT=y
>  CONFIG_SMP=y
>  CONFIG_NR_CPUS=2
>  CONFIG_PREEMPT=y
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index cb36807..b810f2f 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -14,19 +14,28 @@ menu "SAMSUNG EXYNOS SoCs Support"
>  config ARCH_EXYNOS4
>  	bool "SAMSUNG EXYNOS4"
>  	default y
> +	select ARM_AMBA
> +	select CLKSRC_OF
> +	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
> +	select CPU_EXYNOS4210

The line above shows the brokenness of current layout of Exynos Kconfig 
options. You need to select some SoC in SAMSUNG EXYNOS4, because otherwise 
the build will fail if user doesn't select at least one of Exynos4 SoCs.

Instead, I would hide the ARCH_EXYNOS4 and ARCH_EXYNOS4 options and select 
them from entries of particular SoCs, i.e. ARCH_EXYNOS4 would be selected 
by CPU_EXYNOS4210, SOC_EXYNOS4212 and SOC_EXYNOS4412, ARCH_EXYNOS5 by 
SOC_EXYNOS5250 and SOC_EXYNOS5420 and so on.

Best regards,
Tomasz
Bartlomiej Zolnierkiewicz Sept. 23, 2013, 10:23 a.m. UTC | #2 
Hi Tomek,

On Tuesday, September 17, 2013 12:45:21 PM Tomasz Figa wrote:
> Hi Bart,
> 
> On Friday 13 of September 2013 11:28:31 Bartlomiej Zolnierkiewicz wrote:
> > EXYNOS is now Device Tree (DT) only platform so it makes no sense to have
> > config options responsible for enabling platform specific DT support.
> > 
> > Moreover the kernel image won't even link if neither
> > CONFIG_MACH_EXYNOS4_DT nor CONFIG_MACH_EXYNOS5_DT config option is
> > enabled (linker fails with "no machine record defined" error).
> > 
> > Remove CONFIG_MACH_EXYNOS[4,5]_DT config options and just use the
> > standard CONFIG_ARCH_EXYNOS[4,5] ones instead.
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  arch/arm/configs/exynos_defconfig |  1 -
> >  arch/arm/mach-exynos/Kconfig      | 38
> > +++++++++----------------------------- arch/arm/mach-exynos/Makefile    
> > |  4 ++--
> >  3 files changed, 11 insertions(+), 32 deletions(-)
> 
> Generally I see this as a nice cleanup, but I'd go even further than this. 
> See below.
> 
> > diff --git a/arch/arm/configs/exynos_defconfig
> > b/arch/arm/configs/exynos_defconfig index ad7dfbb..5181d0d 100644
> > --- a/arch/arm/configs/exynos_defconfig
> > +++ b/arch/arm/configs/exynos_defconfig
> > @@ -11,7 +11,6 @@ CONFIG_ARCH_EXYNOS=y
> >  CONFIG_S3C_LOWLEVEL_UART_PORT=3
> >  CONFIG_S3C24XX_PWM=y
> >  CONFIG_ARCH_EXYNOS5=y
> > -CONFIG_MACH_EXYNOS4_DT=y
> >  CONFIG_SMP=y
> >  CONFIG_NR_CPUS=2
> >  CONFIG_PREEMPT=y
> > diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> > index cb36807..b810f2f 100644
> > --- a/arch/arm/mach-exynos/Kconfig
> > +++ b/arch/arm/mach-exynos/Kconfig
> > @@ -14,19 +14,28 @@ menu "SAMSUNG EXYNOS SoCs Support"
> >  config ARCH_EXYNOS4
> >  	bool "SAMSUNG EXYNOS4"
> >  	default y
> > +	select ARM_AMBA
> > +	select CLKSRC_OF
> > +	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
> > +	select CPU_EXYNOS4210
> 
> The line above shows the brokenness of current layout of Exynos Kconfig 
> options. You need to select some SoC in SAMSUNG EXYNOS4, because otherwise 
> the build will fail if user doesn't select at least one of Exynos4 SoCs.
> 
> Instead, I would hide the ARCH_EXYNOS4 and ARCH_EXYNOS4 options and select 
> them from entries of particular SoCs, i.e. ARCH_EXYNOS4 would be selected 
> by CPU_EXYNOS4210, SOC_EXYNOS4212 and SOC_EXYNOS4412, ARCH_EXYNOS5 by 
> SOC_EXYNOS5250 and SOC_EXYNOS5420 and so on.

This is a good idea but I think that it is better to address it later in
an incremental patch (which I will also do unless someone beats me to it).

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Tomasz Figa Sept. 23, 2013, 12:34 p.m. UTC | #3 
On Monday 23 of September 2013 12:23:51 Bartlomiej Zolnierkiewicz wrote:
> Hi Tomek,
> 
> On Tuesday, September 17, 2013 12:45:21 PM Tomasz Figa wrote:
> > Hi Bart,
[snip]
> > > +	select ARM_AMBA
> > > +	select CLKSRC_OF
> > > +	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
> > > +	select CPU_EXYNOS4210
> > 
> > The line above shows the brokenness of current layout of Exynos Kconfig
> > options. You need to select some SoC in SAMSUNG EXYNOS4, because
> > otherwise the build will fail if user doesn't select at least one of
> > Exynos4 SoCs.
> > 
> > Instead, I would hide the ARCH_EXYNOS4 and ARCH_EXYNOS4 options and
> > select them from entries of particular SoCs, i.e. ARCH_EXYNOS4 would
> > be selected by CPU_EXYNOS4210, SOC_EXYNOS4212 and SOC_EXYNOS4412,
> > ARCH_EXYNOS5 by SOC_EXYNOS5250 and SOC_EXYNOS5420 and so on.
> 
> This is a good idea but I think that it is better to address it later in
> an incremental patch (which I will also do unless someone beats me to
> it).

OK, sounds good. I take your word for it. ;)

Acked-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz
Kim Kukjin Sept. 26, 2013, 4:09 a.m. UTC | #4 
Bartlomiej Zolnierkiewicz wrote:
> 
> EXYNOS is now Device Tree (DT) only platform so it makes no sense to have
> config options responsible for enabling platform specific DT support.
> 
> Moreover the kernel image won't even link if neither
> CONFIG_MACH_EXYNOS4_DT
> nor CONFIG_MACH_EXYNOS5_DT config option is enabled (linker fails with "no
> machine record defined" error).
> 
> Remove CONFIG_MACH_EXYNOS[4,5]_DT config options and just use the standard
> CONFIG_ARCH_EXYNOS[4,5] ones instead.
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  arch/arm/configs/exynos_defconfig |  1 -
>  arch/arm/mach-exynos/Kconfig      | 38
+++++++++--------------------------
> ---
>  arch/arm/mach-exynos/Makefile     |  4 ++--
>  3 files changed, 11 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/arm/configs/exynos_defconfig
> b/arch/arm/configs/exynos_defconfig
> index ad7dfbb..5181d0d 100644
> --- a/arch/arm/configs/exynos_defconfig
> +++ b/arch/arm/configs/exynos_defconfig
> @@ -11,7 +11,6 @@ CONFIG_ARCH_EXYNOS=y
>  CONFIG_S3C_LOWLEVEL_UART_PORT=3
>  CONFIG_S3C24XX_PWM=y
>  CONFIG_ARCH_EXYNOS5=y
> -CONFIG_MACH_EXYNOS4_DT=y
>  CONFIG_SMP=y
>  CONFIG_NR_CPUS=2
>  CONFIG_PREEMPT=y
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index cb36807..b810f2f 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -14,19 +14,28 @@ menu "SAMSUNG EXYNOS SoCs Support"
>  config ARCH_EXYNOS4
>  	bool "SAMSUNG EXYNOS4"
>  	default y
> +	select ARM_AMBA
> +	select CLKSRC_OF
> +	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
> +	select CPU_EXYNOS4210
>  	select GIC_NON_BANKED
> +	select KEYBOARD_SAMSUNG if INPUT_KEYBOARD
>  	select HAVE_ARM_SCU if SMP
>  	select HAVE_SMP
>  	select MIGHT_HAVE_CACHE_L2X0
>  	select PINCTRL
> +	select S5P_DEV_MFC
>  	help
>  	  Samsung EXYNOS4 SoCs based systems
> 
>  config ARCH_EXYNOS5
>  	bool "SAMSUNG EXYNOS5"
> +	select ARM_AMBA
> +	select CLKSRC_OF
>  	select HAVE_ARM_SCU if SMP
>  	select HAVE_SMP
>  	select PINCTRL
> +	select USB_ARCH_HAS_XHCI
>  	help
>  	  Samsung EXYNOS5 (Cortex-A15) SoC based systems
> 
> @@ -110,35 +119,6 @@ config SOC_EXYNOS5440
>  	help
>  	  Enable EXYNOS5440 SoC support
> 
> -comment "Flattened Device Tree based board for EXYNOS SoCs"
> -
> -config MACH_EXYNOS4_DT
> -	bool "Samsung Exynos4 Machine using device tree"
> -	default y
> -	depends on ARCH_EXYNOS4
> -	select ARM_AMBA
> -	select CLKSRC_OF
> -	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
> -	select CPU_EXYNOS4210
> -	select KEYBOARD_SAMSUNG if INPUT_KEYBOARD
> -	select S5P_DEV_MFC
> -	help
> -	  Machine support for Samsung Exynos4 machine with device tree
> enabled.
> -	  Select this if a fdt blob is available for the Exynos4 SoC based
> board.
> -	  Note: This is under development and not all peripherals can be
> supported
> -	  with this machine file.
> -
> -config MACH_EXYNOS5_DT
> -	bool "SAMSUNG EXYNOS5 Machine using device tree"
> -	default y
> -	depends on ARCH_EXYNOS5
> -	select ARM_AMBA
> -	select CLKSRC_OF
> -	select USB_ARCH_HAS_XHCI
> -	help
> -	  Machine support for Samsung EXYNOS5 machine with device tree
> enabled.
> -	  Select this if a fdt blob is available for the EXYNOS5 SoC based
> board.
> -
>  endmenu
> 
>  endif
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 5369615..8930b66 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -32,5 +32,5 @@ AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-
> a$(plus_sec)
> 
>  # machine support
> 
> -obj-$(CONFIG_MACH_EXYNOS4_DT)		+= mach-exynos4-dt.o
> -obj-$(CONFIG_MACH_EXYNOS5_DT)		+= mach-exynos5-dt.o
> +obj-$(CONFIG_ARCH_EXYNOS4)	+= mach-exynos4-dt.o
> +obj-$(CONFIG_ARCH_EXYNOS5)	+= mach-exynos5-dt.o
> --
> 1.8.2.3

OK, this cleanup looks good to me but updating defconfig is not required in
this patch because it should be updated separately according to updating
other stuff...

Will apply except updating defconfig.

Thanks,
Kukjin
diff mbox

Patch

diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig
index ad7dfbb..5181d0d 100644
--- a/arch/arm/configs/exynos_defconfig
+++ b/arch/arm/configs/exynos_defconfig
@@ -11,7 +11,6 @@  CONFIG_ARCH_EXYNOS=y
 CONFIG_S3C_LOWLEVEL_UART_PORT=3
 CONFIG_S3C24XX_PWM=y
 CONFIG_ARCH_EXYNOS5=y
-CONFIG_MACH_EXYNOS4_DT=y
 CONFIG_SMP=y
 CONFIG_NR_CPUS=2
 CONFIG_PREEMPT=y
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index cb36807..b810f2f 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -14,19 +14,28 @@  menu "SAMSUNG EXYNOS SoCs Support"
 config ARCH_EXYNOS4
 	bool "SAMSUNG EXYNOS4"
 	default y
+	select ARM_AMBA
+	select CLKSRC_OF
+	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
+	select CPU_EXYNOS4210
 	select GIC_NON_BANKED
+	select KEYBOARD_SAMSUNG if INPUT_KEYBOARD
 	select HAVE_ARM_SCU if SMP
 	select HAVE_SMP
 	select MIGHT_HAVE_CACHE_L2X0
 	select PINCTRL
+	select S5P_DEV_MFC
 	help
 	  Samsung EXYNOS4 SoCs based systems
 
 config ARCH_EXYNOS5
 	bool "SAMSUNG EXYNOS5"
+	select ARM_AMBA
+	select CLKSRC_OF
 	select HAVE_ARM_SCU if SMP
 	select HAVE_SMP
 	select PINCTRL
+	select USB_ARCH_HAS_XHCI
 	help
 	  Samsung EXYNOS5 (Cortex-A15) SoC based systems
 
@@ -110,35 +119,6 @@  config SOC_EXYNOS5440
 	help
 	  Enable EXYNOS5440 SoC support
 
-comment "Flattened Device Tree based board for EXYNOS SoCs"
-
-config MACH_EXYNOS4_DT
-	bool "Samsung Exynos4 Machine using device tree"
-	default y
-	depends on ARCH_EXYNOS4
-	select ARM_AMBA
-	select CLKSRC_OF
-	select CLKSRC_SAMSUNG_PWM if CPU_EXYNOS4210
-	select CPU_EXYNOS4210
-	select KEYBOARD_SAMSUNG if INPUT_KEYBOARD
-	select S5P_DEV_MFC
-	help
-	  Machine support for Samsung Exynos4 machine with device tree enabled.
-	  Select this if a fdt blob is available for the Exynos4 SoC based board.
-	  Note: This is under development and not all peripherals can be supported
-	  with this machine file.
-
-config MACH_EXYNOS5_DT
-	bool "SAMSUNG EXYNOS5 Machine using device tree"
-	default y
-	depends on ARCH_EXYNOS5
-	select ARM_AMBA
-	select CLKSRC_OF
-	select USB_ARCH_HAS_XHCI
-	help
-	  Machine support for Samsung EXYNOS5 machine with device tree enabled.
-	  Select this if a fdt blob is available for the EXYNOS5 SoC based board.
-
 endmenu
 
 endif
diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 5369615..8930b66 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -32,5 +32,5 @@  AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-a$(plus_sec)
 
 # machine support
 
-obj-$(CONFIG_MACH_EXYNOS4_DT)		+= mach-exynos4-dt.o
-obj-$(CONFIG_MACH_EXYNOS5_DT)		+= mach-exynos5-dt.o
+obj-$(CONFIG_ARCH_EXYNOS4)	+= mach-exynos4-dt.o
+obj-$(CONFIG_ARCH_EXYNOS5)	+= mach-exynos5-dt.o