Hi Kgene,
Please see my comments inline.
On Saturday 27 of October 2012 02:55:48 Kukjin Kim wrote:
> This patch adds support for EXYNOS5440 SoC which is including
> Cortex-A15 Quad cores.
>
> Signed-off-by: Kukjin Kim <kgene.kim@xxxxxxxxxxx>
> ---
> arch/arm/mach-exynos/Kconfig | 20 ++++++
> arch/arm/mach-exynos/Makefile | 3 +-
> arch/arm/mach-exynos/common.c | 85
> ++++++++++++++++++++++++-- arch/arm/mach-exynos/common.h
> | 2 +
> arch/arm/mach-exynos/dev-uart.c | 14 ++++
> arch/arm/mach-exynos/include/mach/irqs.h | 5 ++
> arch/arm/mach-exynos/include/mach/map.h | 8 +++
> arch/arm/mach-exynos/include/mach/regs-pmu.h | 1 +
> arch/arm/mach-exynos/mach-exynos5-dt.c | 34 ++++++++---
> arch/arm/mach-exynos/mct.c | 15 +++++
> arch/arm/mach-exynos/setup-i2c0.c | 2 +-
> arch/arm/plat-samsung/include/plat/cpu.h | 8 +++
> arch/arm/plat-samsung/include/plat/devs.h | 1 +
> drivers/tty/serial/samsung.c | 3 +-
> 14 files changed, 184 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
> index 1d0d083..c047aba 100644
> --- a/arch/arm/mach-exynos/Kconfig
> +++ b/arch/arm/mach-exynos/Kconfig
> @@ -68,6 +68,16 @@ config SOC_EXYNOS5250
> help
> Enable EXYNOS5250 SoC support
>
> +config SOC_EXYNOS5440
> + bool "SAMSUNG EXYNOS5440"
> + default y
> + depends on ARCH_EXYNOS5
> + select ARM_ARCH_TIMER
> + select AUTO_ZRELADDR
> + select COMMON_CLK
> + help
> + Enable EXYNOS5440 SoC support
> +
> config EXYNOS4_MCT
> bool
> default y
> @@ -426,6 +436,16 @@ config MACH_EXYNOS5_DT
> Machine support for Samsung EXYNOS5 machine with device tree
> enabled. Select this if a fdt blob is available for the EXYNOS5 SoC
> based board.
>
> +config MACH_EXYNOS5440_DT
> + bool "SAMSUNG EXYNOS5440 Machine using device tree"
> + depends on ARCH_EXYNOS5
> + select ARM_AMBA
> + select SOC_EXYNOS5440
> + select USE_OF
> + help
> + Machine support for Samsung EXYNOS5440 machine with device tree
> enabled. + Select this if a fdt blob is available for the EXYNOS5440
> SoC based board. +
> if ARCH_EXYNOS4
>
> comment "Configuration for HSMMC 8-bit bus width"
> diff --git a/arch/arm/mach-exynos/Makefile
> b/arch/arm/mach-exynos/Makefile index ad66c9f4..92df758 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -13,7 +13,7 @@ obj- :=
> # Core
>
> obj-$(CONFIG_ARCH_EXYNOS) += common.o
> -obj-$(CONFIG_ARCH_EXYNOS5) += clock-exynos5.o
> +obj-$(CONFIG_SOC_EXYNOS5250) += clock-exynos5.o
>
> obj-$(CONFIG_PM) += pm.o
> obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
> @@ -41,6 +41,7 @@ obj-$(CONFIG_MACH_SMDK4412) += mach-smdk4x12.o
>
> obj-$(CONFIG_MACH_EXYNOS4_DT) += mach-exynos4-dt.o
> obj-$(CONFIG_MACH_EXYNOS5_DT) += mach-exynos5-dt.o
> +obj-$(CONFIG_MACH_EXYNOS5440_DT) += mach-exynos5-dt.o
>
> # device support
>
> diff --git a/arch/arm/mach-exynos/common.c
> b/arch/arm/mach-exynos/common.c index fea1542..786d8f4 100644
> --- a/arch/arm/mach-exynos/common.c
> +++ b/arch/arm/mach-exynos/common.c
> @@ -57,9 +57,11 @@ 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_exynos5440[] = "EXYNOS5440";
>
> static void exynos4_map_io(void);
> static void exynos5_map_io(void);
> +static void exynos5440_map_io(void);
> static void exynos5_init_clocks(int xtal);
> static void exynos_init_uarts(struct s3c2410_uartcfg *cfg, int no);
> static int exynos_init(void);
> @@ -94,6 +96,13 @@ static struct cpu_table cpu_ids[] __initdata = {
> .init_uarts = exynos_init_uarts,
> .init = exynos_init,
> .name = name_exynos5250,
> + }, {
> + .idcode = EXYNOS5440_SOC_ID,
> + .idmask = EXYNOS5_SOC_MASK,
> + .map_io = exynos5440_map_io,
> + .init_uarts = exynos_init_uarts,
> + .init = exynos_init,
> + .name = name_exynos5440,
> },
> };
>
> @@ -108,6 +117,15 @@ static struct map_desc exynos_iodesc[] __initdata =
> { },
> };
>
> +static struct map_desc exynos5440_iodesc[] __initdata = {
> + {
> + .virtual = (unsigned long)S5P_VA_CHIPID,
> + .pfn = __phys_to_pfn(EXYNOS5440_PA_CHIPID),
> + .length = SZ_4K,
> + .type = MT_DEVICE,
> + },
> +};
> +
> static struct map_desc exynos4_iodesc[] __initdata = {
> {
> .virtual = (unsigned long)S3C_VA_SYS,
> @@ -274,6 +292,25 @@ static struct map_desc exynos5_iodesc[] __initdata
> = { },
> };
>
> +static struct map_desc exynos5440_iodesc0[] __initdata = {
> + {
> + .virtual = (unsigned long)S3C_VA_UART,
> + .pfn = __phys_to_pfn(EXYNOS5440_PA_UART0),
> + .length = SZ_512K,
> + .type = MT_DEVICE,
> + }, {
> + .virtual = (unsigned long)S5P_VA_GIC_CPU,
> + .pfn = __phys_to_pfn(EXYNOS5440_PA_GIC_CPU),
> + .length = SZ_64K,
> + .type = MT_DEVICE,
> + }, {
> + .virtual = (unsigned long)S5P_VA_GIC_DIST,
> + .pfn = __phys_to_pfn(EXYNOS5440_PA_GIC_DIST),
> + .length = SZ_64K,
> + .type = MT_DEVICE,
> + },
> +};
> +
> void exynos4_restart(char mode, const char *cmd)
> {
> __raw_writel(0x1, S5P_SWRESET);
> @@ -281,11 +318,29 @@ void exynos4_restart(char mode, const char *cmd)
>
> void exynos5_restart(char mode, const char *cmd)
> {
> - __raw_writel(0x1, EXYNOS_SWRESET);
> + u32 val;
> + void __iomem *addr;
> +
> + if (of_machine_is_compatible("samsung,exynos5250")) {
> + val = 0x1;
> + addr = EXYNOS_SWRESET;
> + } else if (of_machine_is_compatible("samsung,exynos5440")) {
> + val = (0x10 << 20) | (0x1 << 16);
> + addr = EXYNOS5440_SWRESET;
> + } else {
> + pr_err("%s: cannot support non-DT\n", __func__);
> + return;
> + }
Why soc_is_XXX isn't used here? It should be faster and more correct than of_machine_is_compatible.
I can imagine the same board available with two different SoCs, for which of_machine_is_compatible wouldn't work.
> +
> + __raw_writel(val, addr);
> }
>
> void __init exynos_init_late(void)
> {
> + if (of_machine_is_compatible("samsung,exynos5440"))
> + /* to be supported later */
> + return;
Same here.
> exynos_pm_late_initcall();
> }
>
> @@ -298,7 +353,11 @@ void __init exynos_init_late(void)
> void __init exynos_init_io(struct map_desc *mach_desc, int size)
> {
> /* initialize the io descriptors we need for initialization */
> - iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc));
> + if (of_machine_is_compatible("samsung,exynos5440"))
> + iotable_init(exynos5440_iodesc, ARRAY_SIZE(exynos5440_iodesc));
> + else
> + iotable_init(exynos_iodesc, ARRAY_SIZE(exynos_iodesc));
> +
Same here.
> if (mach_desc)
> iotable_init(mach_desc, size);
>
> @@ -364,6 +423,11 @@ static void __init exynos5_map_io(void)
> s3c_i2c2_setname("s3c2440-i2c");
> }
>
> +static void __init exynos5440_map_io(void)
> +{
> + iotable_init(exynos5440_iodesc0, ARRAY_SIZE(exynos5440_iodesc0));
> +}
> +
> static void __init exynos5_init_clocks(int xtal)
> {
> printk(KERN_DEBUG "%s: initializing clocks\n", __func__);
> @@ -587,6 +651,11 @@ static const struct of_device_id
> exynos4_dt_irq_match[] = { .data = combiner_of_init, },
> {},
> };
> +
> +static const struct of_device_id exynos5440_dt_irq_match[] = {
> + { .compatible = "arm,cortex-a15-gic", .data = gic_of_init, },
> + {},
> +};
> #endif
>
> void __init exynos4_init_irq(void)
> @@ -616,14 +685,18 @@ void __init exynos4_init_irq(void)
> void __init exynos5_init_irq(void)
> {
> #ifdef CONFIG_OF
> - of_irq_init(exynos4_dt_irq_match);
> + if (soc_is_exynos5440())
> + of_irq_init(exynos5440_dt_irq_match);
> + else
> + of_irq_init(exynos4_dt_irq_match);
This looks much better.
> #endif
> /*
> * The parameters of s5p_init_irq() are for VIC init.
> * Theses parameters should be NULL and 0 because EXYNOS4
> * uses GIC instead of VIC.
> */
> - s5p_init_irq(NULL, 0);
> + if (!soc_is_exynos5440())
> + s5p_init_irq(NULL, 0);
> }
>
> struct bus_type exynos_subsys = {
> @@ -646,7 +719,7 @@ static int __init exynos4_l2x0_cache_init(void)
> {
> int ret;
>
> - if (soc_is_exynos5250())
> + if (soc_is_exynos5250() || soc_is_exynos5440())
> return 0;
>
> ret = l2x0_of_init(L2_AUX_VAL, L2_AUX_MASK);
> @@ -714,6 +787,8 @@ static void __init exynos_init_uarts(struct
> s3c2410_uartcfg *cfg, int no)
>
> if (soc_is_exynos5250())
> s3c24xx_init_uartdevs("exynos4210-uart", exynos5_uart_resources, cfg,
> no); + else if (soc_is_exynos5440())
> + s3c24xx_init_uartdevs("exynos4210-uart", exynos5440_uart_resources,
> cfg, no); else
> s3c24xx_init_uartdevs("exynos4210-uart", exynos4_uart_resources, cfg,
> no); }
> diff --git a/arch/arm/mach-exynos/common.h
> b/arch/arm/mach-exynos/common.h index 7a4e0ea..99b88f8 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -13,9 +13,11 @@
> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>
> extern struct sys_timer exynos4_timer;
> +extern struct sys_timer exynos5_timer;
>
> struct map_desc;
> void exynos_init_io(struct map_desc *mach_desc, int size);
> +void exynos5440_init_io(struct map_desc *mach_desc, int size);
> void exynos4_init_irq(void);
> void exynos5_init_irq(void);
> void exynos4_restart(char mode, const char *cmd);
> diff --git a/arch/arm/mach-exynos/dev-uart.c
> b/arch/arm/mach-exynos/dev-uart.c index 2e85c02..95b887f 100644
> --- a/arch/arm/mach-exynos/dev-uart.c
> +++ b/arch/arm/mach-exynos/dev-uart.c
> @@ -76,3 +76,17 @@ struct s3c24xx_uart_resources
> exynos5_uart_resources[] __initdata = { .nr_resources =
> ARRAY_SIZE(exynos5_uart3_resource),
> },
> };
> +
> +EXYNOS_UART_RESOURCE(5440, 0)
> +EXYNOS_UART_RESOURCE(5440, 1)
> +
> +struct s3c24xx_uart_resources exynos5440_uart_resources[] __initdata =
> { + [0] = {
> + .resources = exynos5440_uart0_resource,
> + .nr_resources = ARRAY_SIZE(exynos5440_uart0_resource),
> + },
> + [1] = {
> + .resources = exynos5440_uart1_resource,
> + .nr_resources = ARRAY_SIZE(exynos5440_uart0_resource),
> + },
> +};
> diff --git a/arch/arm/mach-exynos/include/mach/irqs.h
> b/arch/arm/mach-exynos/include/mach/irqs.h index 35bced6..f43a96c
> 100644
> --- a/arch/arm/mach-exynos/include/mach/irqs.h
> +++ b/arch/arm/mach-exynos/include/mach/irqs.h
> @@ -333,6 +333,11 @@
> #define EXYNOS5_IRQ_FIMC_LITE1 IRQ_SPI(126)
> #define EXYNOS5_IRQ_RP_TIMER IRQ_SPI(127)
>
> +/* EXYNOS5440 */
> +
> +#define EXYNOS5440_IRQ_UART0 IRQ_SPI(2)
> +#define EXYNOS5440_IRQ_UART1 IRQ_SPI(3)
> +
> #define EXYNOS5_IRQ_PMU COMBINER_IRQ(1, 2)
>
> #define EXYNOS5_IRQ_SYSMMU_GSC0_0 COMBINER_IRQ(2, 0)
> diff --git a/arch/arm/mach-exynos/include/mach/map.h
> b/arch/arm/mach-exynos/include/mach/map.h index 8480849..d0602d3 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -53,12 +53,14 @@
> #define EXYNOS4_PA_ONENAND_DMA 0x0C600000
>
> #define EXYNOS_PA_CHIPID 0x10000000
> +#define EXYNOS5440_PA_CHIPID 0x00160000
>
> #define EXYNOS4_PA_SYSCON 0x10010000
> #define EXYNOS5_PA_SYSCON 0x10050100
>
> #define EXYNOS4_PA_PMU 0x10020000
> #define EXYNOS5_PA_PMU 0x10040000
> +#define EXYNOS5440_PA_PMU 0x00160000
>
> #define EXYNOS4_PA_CMU 0x10030000
> #define EXYNOS5_PA_CMU 0x10010000
> @@ -83,6 +85,8 @@
> #define EXYNOS4_PA_GIC_DIST 0x10490000
> #define EXYNOS5_PA_GIC_CPU 0x10482000
> #define EXYNOS5_PA_GIC_DIST 0x10481000
> +#define EXYNOS5440_PA_GIC_CPU 0x002E2000
> +#define EXYNOS5440_PA_GIC_DIST 0x002E1000
>
> #define EXYNOS4_PA_COREPERI 0x10500000
> #define EXYNOS4_PA_TWD 0x10500600
> @@ -281,6 +285,10 @@
> #define EXYNOS5_PA_UART3 0x12C30000
> #define EXYNOS5_SZ_UART SZ_256
>
> +#define EXYNOS5440_PA_UART0 0x000B0000
> +#define EXYNOS5440_PA_UART1 0x000C0000
> +#define EXYNOS5440_SZ_UART SZ_256
> +
> #define S3C_VA_UARTx(x) (S3C_VA_UART + ((x) * S3C_UART_OFFSET))
>
> #endif /* __ASM_ARCH_MAP_H */
> diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> b/arch/arm/mach-exynos/include/mach/regs-pmu.h index d4e392b..c0b74f3
> 100644
> --- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> +++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
> @@ -31,6 +31,7 @@
>
> #define S5P_SWRESET S5P_PMUREG(0x0400)
> #define EXYNOS_SWRESET S5P_PMUREG(0x0400)
> +#define EXYNOS5440_SWRESET S5P_PMUREG(0x00C4)
>
> #define S5P_WAKEUP_STAT S5P_PMUREG(0x0600)
> #define S5P_EINT_WAKEUP_MASK S5P_PMUREG(0x0604)
> diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c
> b/arch/arm/mach-exynos/mach-exynos5-dt.c index db1cd8e..7052f80 100644
> --- a/arch/arm/mach-exynos/mach-exynos5-dt.c
> +++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
> @@ -75,20 +75,24 @@ static const struct of_dev_auxdata
> exynos5250_auxdata_lookup[] __initconst = { {},
> };
>
> -static void __init exynos5250_dt_map_io(void)
> +static void __init exynos5_dt_map_io(void)
> {
> exynos_init_io(NULL, 0);
> - s3c24xx_init_clocks(24000000);
> +
> + if (of_machine_is_compatible("samsung,exynos5250"))
> + s3c24xx_init_clocks(24000000);
Again soc_is_XXX().
> }
>
> -static void __init exynos5250_dt_machine_init(void)
> +static void __init exynos5_dt_machine_init(void)
> {
> - of_platform_populate(NULL, of_default_bus_match_table,
> - exynos5250_auxdata_lookup, NULL);
> + if (of_machine_is_compatible("samsung,exynos5250"))
> + of_platform_populate(NULL, of_default_bus_match_table,
> + exynos5250_auxdata_lookup, NULL);
> }
>
> -static char const *exynos5250_dt_compat[] __initdata = {
> +static char const *exynos5_dt_compat[] __initdata = {
> "samsung,exynos5250",
> + "samsung,exynos5440",
> NULL
> };
Something doesn't seem right here. How do you distinguish between
MACH_EXYNOS5_DT and MACH_EXYNOS5440_DT if both have the same compatible
matches?
Those machines doesn't seem to share much definitions, so maybe a separate
mach-exynos5440-dt.c file would be a better approach?
> @@ -96,11 +100,23 @@ DT_MACHINE_START(EXYNOS5_DT, "SAMSUNG EXYNOS5
> (Flattened Device Tree)") /* Maintainer: Kukjin Kim
> <kgene.kim@xxxxxxxxxxx> */
> .init_irq = exynos5_init_irq,
> .smp = smp_ops(exynos_smp_ops),
> - .map_io = exynos5250_dt_map_io,
> + .map_io = exynos5_dt_map_io,
> .handle_irq = gic_handle_irq,
> - .init_machine = exynos5250_dt_machine_init,
> + .init_machine = exynos5_dt_machine_init,
> .init_late = exynos_init_late,
> .timer = &exynos4_timer,
> - .dt_compat = exynos5250_dt_compat,
> + .dt_compat = exynos5_dt_compat,
> + .restart = exynos5_restart,
> +MACHINE_END
> +
> +DT_MACHINE_START(EXYNOS5440_DT, "SAMSUNG EXYNOS5440 (Flattened Device
> Tree)") + /* Maintainer: Kukjin Kim <kgene.kim@xxxxxxxxxxx> */
> + .init_irq = exynos5_init_irq,
> + .smp = smp_ops(exynos_smp_ops),
> + .map_io = exynos5_dt_map_io,
> + .handle_irq = gic_handle_irq,
> + .init_machine = exynos5_dt_machine_init,
> + .timer = &exynos5_timer,
> + .dt_compat = exynos5_dt_compat,
> .restart = exynos5_restart,
Since restarts for both differ, why not to add separate exynos5440 restart
and use it here?
> MACHINE_END
> diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
> index cc3805a..31f45ec 100644
> --- a/arch/arm/mach-exynos/mct.c
> +++ b/arch/arm/mach-exynos/mct.c
> @@ -19,7 +19,9 @@
> #include <linux/platform_device.h>
> #include <linux/delay.h>
> #include <linux/percpu.h>
> +#include <linux/of.h>
>
> +#include <asm/arch_timer.h>
> #include <asm/hardware/gic.h>
> #include <asm/localtimer.h>
>
> @@ -487,6 +489,9 @@ static void __init exynos4_timer_init(void)
> exynos4x12_clk_init();
> #endif
>
> + if (of_machine_is_compatible("samsung,exynos5440"))
> + arch_timer_of_register();
> +
Why exynos4_timer_init is being touched here, if exynos5_timer_init is
being added?
I would rather keep exynos4_timer (which is used for all Exynos4 SoCs
and for Exynos5250) as is and define new exynos5250_timer if it needs
completely different initialization...
> if ((soc_is_exynos4210()) || (soc_is_exynos5250()))
> mct_int_type = MCT_INT_SPI;
> else
> @@ -500,3 +505,13 @@ static void __init exynos4_timer_init(void)
> struct sys_timer exynos4_timer = {
> .init = exynos4_timer_init,
> };
> +
> +static void __init exynos5_timer_init(void)
> +{
> + if (of_machine_is_compatible("samsung,exynos5440"))
> + arch_timer_of_register();
> +}
Also again this of_machine_is_compatible().
Best regards,
Tomasz Figa
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
