[U-Boot] [PATCH 7/9] arm: exynos: realign the code to allow support for newer 64-bit platforms

Thomas Abraham
2016-04-18T14:11:29Z

Hi Mr. Kang,

On Mon, Apr 18, 2016 at 4:39 PM, Minkyu Kang <mk7.kang@samsung.com> wrote:
> Dear Thomas Abraham,
>
> On 13/04/16 19:43, Thomas Abraham wrote:
>> From: Thomas Abraham <thomas.ab@samsung.com>
>>
>> The existing Exynos 32-bit platform support needs to be realigned in
>> order to support newer 64-bit Exynos platforms. The driver model will
>> be utlized for drivers on the 64-bit Exynos platforms and so some of
>> the older platform support code would not be required for the newer
>> 64-bit Exynos platforms.
>>
>> Cc: Minkyu Kang <mk7.kang@samsung.com>
>> Signed-off-by: Thomas Abraham <thomas.ab@samsung.com>
>> ---
>> arch/arm/Kconfig | 1 -
>> arch/arm/mach-exynos/Kconfig | 14 ++++++++++++++
>> arch/arm/mach-exynos/Makefile | 7 +++++--
>> arch/arm/mach-exynos/include/mach/cpu.h | 2 +-
>> arch/arm/mach-exynos/include/mach/gpio.h | 2 +-
>> arch/arm/mach-exynos/soc.c | 2 ++
>> 6 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index b82ec18..ee22a3c 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -426,7 +426,6 @@ config TARGET_BCMNSP
>>
>> config ARCH_EXYNOS
>> bool "Samsung EXYNOS"
>> - select CPU_V7
>> select DM
>> select DM_SPI_FLASH
>> select DM_SERIAL
>> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
>> index a6a7597..acab947 100644
>> --- a/arch/arm/mach-exynos/Kconfig
>> +++ b/arch/arm/mach-exynos/Kconfig
>> @@ -7,30 +7,38 @@ choice
>> config TARGET_SMDKV310
>> select SUPPORT_SPL
>> bool "Exynos4210 SMDKV310 board"
>> + select CPU_V7
>> select OF_CONTROL
>>
>> config TARGET_TRATS
>> bool "Exynos4210 Trats board"
>> + select CPU_V7
>>
>> config TARGET_S5PC210_UNIVERSAL
>> bool "EXYNOS4210 Universal C210 board"
>> + select CPU_V7
>>
>> config TARGET_ORIGEN
>> bool "Exynos4412 Origen board"
>> + select CPU_V7
>> select SUPPORT_SPL
>>
>> config TARGET_TRATS2
>> bool "Exynos4412 Trat2 board"
>> + select CPU_V7
>>
>> config TARGET_ODROID
>> bool "Exynos4412 Odroid board"
>> + select CPU_V7
>>
>> config TARGET_ODROID_XU3
>> bool "Exynos5422 Odroid board"
>> + select CPU_V7
>> select OF_CONTROL
>>
>> config TARGET_ARNDALE
>> bool "Exynos5250 Arndale board"
>> + select CPU_V7
>> select CPU_V7_HAS_NONSEC
>> select CPU_V7_HAS_VIRT
>> select SUPPORT_SPL
>> @@ -38,32 +46,38 @@ config TARGET_ARNDALE
>>
>> config TARGET_SMDK5250
>> bool "SMDK5250 board"
>> + select CPU_V7
>> select SUPPORT_SPL
>> select OF_CONTROL
>>
>> config TARGET_SNOW
>> bool "Snow board"
>> + select CPU_V7
>> select SUPPORT_SPL
>> select OF_CONTROL
>>
>> config TARGET_SPRING
>> bool "Spring board"
>> + select CPU_V7
>> select SUPPORT_SPL
>> select OF_CONTROL
>> select SPL_DISABLE_OF_CONTROL
>>
>> config TARGET_SMDK5420
>> bool "SMDK5420 board"
>> + select CPU_V7
>> select SUPPORT_SPL
>> select OF_CONTROL
>>
>> config TARGET_PEACH_PI
>> bool "Peach Pi board"
>> + select CPU_V7
>> select SUPPORT_SPL
>> select OF_CONTROL
>>
>> config TARGET_PEACH_PIT
>> bool "Peach Pit board"
>> + select CPU_V7
>> select SUPPORT_SPL
>> select OF_CONTROL
>
> I think it's better to split to new architecture type for 64bit exynos platform - ARCH_EXYNOS64?
> What do you think?

I was infact thinking to avoid adding a new ARCH type as much as
possible and reuse ARCH_EXYNOS for 64-bit as well. Eventually, the
code in mach-exynos has to move into respective driver folders
(atleast for ARM64 platforms) and have as little as possible in
mach-exynos directory.

>
>>
>> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
>> index 8542f89..be5912e 100644
>> --- a/arch/arm/mach-exynos/Makefile
>> +++ b/arch/arm/mach-exynos/Makefile
>> @@ -5,7 +5,8 @@
>> # SPDX-License-Identifier: GPL-2.0+
>> #
>>
>> -obj-y += clock.o power.o soc.o system.o pinmux.o tzpc.o
>> +obj-y += soc.o
>> +obj-$(CONFIG_CPU_V7) += clock.o pinmux.o power.o soc.o system.o
>
> soc.o, duplicated?

Right, will fix and resend.

>
>>
>> obj-$(CONFIG_EXYNOS5420) += sec_boot.o
>>
>> @@ -13,6 +14,6 @@ ifdef CONFIG_SPL_BUILD
>> obj-$(CONFIG_EXYNOS5) += clock_init_exynos5.o
>> obj-$(CONFIG_EXYNOS5) += dmc_common.o dmc_init_ddr3.o
>> obj-$(CONFIG_EXYNOS4210)+= dmc_init_exynos4.o clock_init_exynos4.o
>> -obj-y += spl_boot.o
>> +obj-y += spl_boot.o tzpc.o
>> obj-y += lowlevel_init.o
>> endif
>> diff --git a/arch/arm/mach-exynos/include/mach/cpu.h b/arch/arm/mach-exynos/include/mach/cpu.h
>> index 14a1692..f12e3d6 100644
>> --- a/arch/arm/mach-exynos/include/mach/cpu.h
>> +++ b/arch/arm/mach-exynos/include/mach/cpu.h
>> @@ -270,7 +270,7 @@ IS_EXYNOS_TYPE(exynos5420, 0x5420)
>> IS_EXYNOS_TYPE(exynos5422, 0x5422)
>>
>> #define SAMSUNG_BASE(device, base) \
>> -static inline unsigned int __attribute__((no_instrument_function)) \
>> +static inline unsigned long __attribute__((no_instrument_function)) \
>> samsung_get_base_##device(void) \
>> { \
>> if (cpu_is_exynos4()) { \
>> diff --git a/arch/arm/mach-exynos/include/mach/gpio.h b/arch/arm/mach-exynos/include/mach/gpio.h
>> index 7fc8e61..81363bd 100644
>> --- a/arch/arm/mach-exynos/include/mach/gpio.h
>> +++ b/arch/arm/mach-exynos/include/mach/gpio.h
>> @@ -1349,7 +1349,7 @@ enum exynos5420_gpio_pin {
>> };
>>
>> struct gpio_info {
>> - unsigned int reg_addr; /* Address of register for this part */
>> + unsigned long reg_addr; /* Address of register for this part */
>> unsigned int max_gpio; /* Maximum GPIO in this part */
>> };
>>
>> diff --git a/arch/arm/mach-exynos/soc.c b/arch/arm/mach-exynos/soc.c
>> index 0f116b1..5cea5ed 100644
>> --- a/arch/arm/mach-exynos/soc.c
>> +++ b/arch/arm/mach-exynos/soc.c
>> @@ -11,7 +11,9 @@
>>
>> void reset_cpu(ulong addr)
>> {
>> +#ifndef CONFIG_OF_CONTROL
>> writel(0x1, samsung_get_base_swreset());
>> +#endif
>
> Is it related change?

Yes, it is but I now realize that this is incorrect since some of the
exynos ARMv7 platforms are OF based. The intent was to not have this
executed for Exynos 64-bit platforms but instead use the reset-uclass
driver (by adding a reset driver for exynos sometime later). Probably,
this has to be changed to "#ifdef CONFIG_CPU_V7" for now.

Thanks for your review.

Regards,
Thomas.

>
>> }
>>
>> #ifndef CONFIG_SYS_DCACHE_OFF
>>
>
> Thanks,
> Minkyu Kang.