Re: [PATCH 1/1] ARM: EXYNOS: Consolidate CPU init code

[Tomasz Figa <t.figa@samsung.com>]

Hi Sachin,

On 22.01.2014 12:49, Sachin Kamat wrote:
> cpu_table was used to distinguish between different Exynos4 and 5 SoCs
> and assign the the initialization and io mapping pointers based on type.
> exynos_init is dummy and no longer needed as we do a DT based booting.
> By having a common io mapping function we can get rid of the whole
> table and avoid populating it for every SoC.
>
> Tested on Exynos4210, 5250 and 5420 based boards.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
>   arch/arm/mach-exynos/common.c |   93 ++++++++---------------------------------
>   1 file changed, 17 insertions(+), 76 deletions(-)

On Exynos4412-based Trats2 board:

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

However, please see one minor comment inline.

> diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
> index f18be40e5b21..02d0aaa7af59 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -48,55 +48,7 @@
>   #define L2_AUX_VAL 0x7C470001
>   #define L2_AUX_MASK 0xC200ffff
>
> -static const char name_exynos4210[] = "EXYNOS4210";
> -static const char name_exynos4212[] = "EXYNOS4212";
> -static const char name_exynos4412[] = "EXYNOS4412";
> -static const char name_exynos5250[] = "EXYNOS5250";
> -static const char name_exynos5420[] = "EXYNOS5420";
> -static const char name_exynos5440[] = "EXYNOS5440";
> -
> -static void exynos4_map_io(void);
> -static void exynos5_map_io(void);
> -static int exynos_init(void);
> -
> -static struct cpu_table cpu_ids[] __initdata = {
> -	{
> -		.idcode		= EXYNOS4210_CPU_ID,
> -		.idmask		= EXYNOS4_CPU_MASK,
> -		.map_io		= exynos4_map_io,
> -		.init		= exynos_init,
> -		.name		= name_exynos4210,
> -	}, {
> -		.idcode		= EXYNOS4212_CPU_ID,
> -		.idmask		= EXYNOS4_CPU_MASK,
> -		.map_io		= exynos4_map_io,
> -		.init		= exynos_init,
> -		.name		= name_exynos4212,
> -	}, {
> -		.idcode		= EXYNOS4412_CPU_ID,
> -		.idmask		= EXYNOS4_CPU_MASK,
> -		.map_io		= exynos4_map_io,
> -		.init		= exynos_init,
> -		.name		= name_exynos4412,
> -	}, {
> -		.idcode		= EXYNOS5250_SOC_ID,
> -		.idmask		= EXYNOS5_SOC_MASK,
> -		.map_io		= exynos5_map_io,
> -		.init		= exynos_init,
> -		.name		= name_exynos5250,
> -	}, {
> -		.idcode		= EXYNOS5420_SOC_ID,
> -		.idmask		= EXYNOS5_SOC_MASK,
> -		.map_io		= exynos5_map_io,
> -		.init		= exynos_init,
> -		.name		= name_exynos5420,
> -	}, {
> -		.idcode		= EXYNOS5440_SOC_ID,
> -		.idmask		= EXYNOS5_SOC_MASK,
> -		.init		= exynos_init,
> -		.name		= name_exynos5440,
> -	},
> -};
> +static void exynos_map_io(void);

cosmetic: Instead of adding forward declaration, the resulting 
exynos_map_io() function could be simply merged with exynos_init_io().

Best regards,
Tomasz