[PATCH 0/6] Exynos 5410 Dual cluster support

[PATCH 0/6] Exynos 5410 Dual cluster support

[Vyacheslav Tyrtov]

The series of patches represent support of Exynos 5410 SoC
    
The Exynos 5410 is the first Samsung SoC based on bigLITTLE architecture.
Patches allow all 8 CPU cores (4 x A7 and 4 x A15) to run at the same time
    
Patches add new platform description, support of clock controller,
dual cluster support, device tree for Exynos 5410 and minor fixes (CCI, MCT)
   
Has been builded on v3.12-rc3.
Has been tested on Exynos 5410 reference board (smdk5410_defconfig).
    
Thanks,
	Vyacheslav.

Tarek Dakhran (6):
  ARM: EXYNOS: Add support for EXYNOS5410 SoC
  clk: exynos5410: register clocks using common clock framework
  ARM: EXYNOS: add Exynos Dual Cluster Support
  ARM: dts: Add initial device tree support for EXYNOS5410
  ARM: EXYNOS: Minor fixes to enable EXYNOS5410 support
  ARM: smdk5410_defconfig: add defconfig for smdk5410

 .../devicetree/bindings/clock/exynos5410-clock.txt |  72 ++++++
 arch/arm/Kconfig                                   |   2 +-
 arch/arm/boot/dts/Makefile                         |   1 +
 arch/arm/boot/dts/exynos5410-smdk5410.dts          |  67 +++++
 arch/arm/boot/dts/exynos5410.dtsi                  | 189 ++++++++++++++
 arch/arm/configs/smdk5410_defconfig                | 134 ++++++++++
 arch/arm/mach-exynos/Kconfig                       |  10 +
 arch/arm/mach-exynos/Makefile                      |   2 +
 arch/arm/mach-exynos/common.c                      |  18 ++
 arch/arm/mach-exynos/edcs.c                        | 217 ++++++++++++++++
 arch/arm/mach-exynos/edcs.h                        |  41 +++
 arch/arm/mach-exynos/edcs_status.c                 | 110 +++++++++
 arch/arm/mach-exynos/include/mach/map.h            |   1 +
 arch/arm/mach-exynos/mach-exynos5-dt.c             |   1 +
 arch/arm/plat-samsung/include/plat/cpu.h           |   8 +
 drivers/bus/arm-cci.c                              |   7 +
 drivers/clk/samsung/Makefile                       |   1 +
 drivers/clk/samsung/clk-exynos5410.c               | 274 +++++++++++++++++++++
 drivers/clocksource/exynos_mct.c                   |   8 +-
 drivers/irqchip/exynos-combiner.c                  |  12 +-
 20 files changed, 1172 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/exynos5410-clock.txt
 create mode 100644 arch/arm/boot/dts/exynos5410-smdk5410.dts
 create mode 100644 arch/arm/boot/dts/exynos5410.dtsi
 create mode 100644 arch/arm/configs/smdk5410_defconfig
 create mode 100644 arch/arm/mach-exynos/edcs.c
 create mode 100644 arch/arm/mach-exynos/edcs.h
 create mode 100644 arch/arm/mach-exynos/edcs_status.c
 create mode 100644 drivers/clk/samsung/clk-exynos5410.c

-- 
1.8.1.5

[PATCH 1/6] ARM: EXYNOS: Add support for EXYNOS5410 SoC

[Vyacheslav Tyrtov]

From: Tarek Dakhran <t.dakhran@samsung.com>

EXYNOS5410 is SoC in Samsung's Exynos5 SoC series.
Add initial support for this SoC.

Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
---
 arch/arm/mach-exynos/Kconfig             | 10 ++++++++++
 arch/arm/mach-exynos/common.c            | 18 ++++++++++++++++++
 arch/arm/mach-exynos/include/mach/map.h  |  1 +
 arch/arm/mach-exynos/mach-exynos5-dt.c   |  1 +
 arch/arm/plat-samsung/include/plat/cpu.h |  8 ++++++++
 5 files changed, 38 insertions(+)

diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index 56fe819..af7e787 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -84,6 +84,16 @@ config SOC_EXYNOS5250
 	help
 	  Enable EXYNOS5250 SoC support
 
+config SOC_EXYNOS5410
+	bool "SAMSUNG EXYNOS5410"
+	default y
+	depends on ARCH_EXYNOS5
+	select PM_GENERIC_DOMAINS if PM
+	select S5P_PM if PM
+	select S5P_SLEEP if PM
+	help
+	  Enable EXYNOS5410 SoC support
+
 config SOC_EXYNOS5420
 	bool "SAMSUNG EXYNOS5420"
 	default y
diff --git a/arch/arm/mach-exynos/common.c b/arch/arm/mach-exynos/common.c
index ba95e5d..187c0a4 100644
--- a/arch/arm/mach-exynos/common.c
+++ b/arch/arm/mach-exynos/common.c
@@ -53,6 +53,7 @@ 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_exynos5410[] = "EXYNOS5410";
 static const char name_exynos5420[] = "EXYNOS5420";
 static const char name_exynos5440[] = "EXYNOS5440";
 
@@ -86,6 +87,12 @@ static struct cpu_table cpu_ids[] __initdata = {
 		.init		= exynos_init,
 		.name		= name_exynos5250,
 	}, {
+		.idcode		= EXYNOS5410_SOC_ID,
+		.idmask		= EXYNOS5_SOC_MASK,
+		.map_io		= exynos5_map_io,
+		.init		= exynos_init,
+		.name		= name_exynos5410,
+	}, {
 		.idcode		= EXYNOS5420_SOC_ID,
 		.idmask		= EXYNOS5_SOC_MASK,
 		.map_io		= exynos5_map_io,
@@ -216,6 +223,15 @@ static struct map_desc exynos4x12_iodesc[] __initdata = {
 	},
 };
 
+static struct map_desc exynos5410_iodesc[] __initdata = {
+	{
+		.virtual	= (unsigned long)S5P_VA_SYSRAM_NS,
+		.pfn		= __phys_to_pfn(EXYNOS5410_PA_SYSRAM_NS),
+		.length		= SZ_4K,
+		.type		= MT_DEVICE,
+	},
+};
+
 static struct map_desc exynos5250_iodesc[] __initdata = {
 	{
 		.virtual	= (unsigned long)S5P_VA_SYSRAM_NS,
@@ -365,6 +381,8 @@ static void __init exynos5_map_io(void)
 
 	if (soc_is_exynos5250())
 		iotable_init(exynos5250_iodesc, ARRAY_SIZE(exynos5250_iodesc));
+	if (soc_is_exynos5410())
+		iotable_init(exynos5410_iodesc, ARRAY_SIZE(exynos5410_iodesc));
 }
 
 void __init exynos_init_time(void)
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 7b046b5..894f431 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -29,6 +29,7 @@
 #define EXYNOS4210_PA_SYSRAM_NS		0x0203F000
 #define EXYNOS4x12_PA_SYSRAM_NS		0x0204F000
 #define EXYNOS5250_PA_SYSRAM_NS		0x0204F000
+#define EXYNOS5410_PA_SYSRAM_NS		0x02073000
 
 #define EXYNOS_PA_CHIPID		0x10000000
 
diff --git a/arch/arm/mach-exynos/mach-exynos5-dt.c b/arch/arm/mach-exynos/mach-exynos5-dt.c
index f874b77..9515186 100644
--- a/arch/arm/mach-exynos/mach-exynos5-dt.c
+++ b/arch/arm/mach-exynos/mach-exynos5-dt.c
@@ -52,6 +52,7 @@ static void __init exynos5_dt_machine_init(void)
 
 static char const *exynos5_dt_compat[] __initdata = {
 	"samsung,exynos5250",
+	"samsung,exynos5410",
 	"samsung,exynos5420",
 	"samsung,exynos5440",
 	NULL
diff --git a/arch/arm/plat-samsung/include/plat/cpu.h b/arch/arm/plat-samsung/include/plat/cpu.h
index 4fb1f03..aad7c40 100644
--- a/arch/arm/plat-samsung/include/plat/cpu.h
+++ b/arch/arm/plat-samsung/include/plat/cpu.h
@@ -46,6 +46,7 @@ extern unsigned long samsung_cpu_id;
 #define EXYNOS4_CPU_MASK	0xFFFE0000
 
 #define EXYNOS5250_SOC_ID	0x43520000
+#define EXYNOS5410_SOC_ID	0xE5410023
 #define EXYNOS5420_SOC_ID	0xE5420000
 #define EXYNOS5440_SOC_ID	0xE5440000
 #define EXYNOS5_SOC_MASK	0xFFFFF000
@@ -68,6 +69,7 @@ IS_SAMSUNG_CPU(exynos4210, EXYNOS4210_CPU_ID, EXYNOS4_CPU_MASK)
 IS_SAMSUNG_CPU(exynos4212, EXYNOS4212_CPU_ID, EXYNOS4_CPU_MASK)
 IS_SAMSUNG_CPU(exynos4412, EXYNOS4412_CPU_ID, EXYNOS4_CPU_MASK)
 IS_SAMSUNG_CPU(exynos5250, EXYNOS5250_SOC_ID, EXYNOS5_SOC_MASK)
+IS_SAMSUNG_CPU(exynos5410, EXYNOS5410_SOC_ID, EXYNOS5_SOC_MASK)
 IS_SAMSUNG_CPU(exynos5420, EXYNOS5420_SOC_ID, EXYNOS5_SOC_MASK)
 IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 
@@ -144,6 +146,12 @@ IS_SAMSUNG_CPU(exynos5440, EXYNOS5440_SOC_ID, EXYNOS5_SOC_MASK)
 # define soc_is_exynos5250()	0
 #endif
 
+#if defined(CONFIG_SOC_EXYNOS5410)
+# define soc_is_exynos5410()	is_samsung_exynos5410()
+#else
+# define soc_is_exynos5410()	0
+#endif
+
 #if defined(CONFIG_SOC_EXYNOS5420)
 # define soc_is_exynos5420()	is_samsung_exynos5420()
 #else
-- 
1.8.1.5

[PATCH 2/6] clk: exynos5410: register clocks using common clock framework

[Vyacheslav Tyrtov]

From: Tarek Dakhran <t.dakhran@samsung.com>

The EXYNOS5410 clocks are statically listed and registered
using the Samsung specific common clock helper functions.

Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
---
 .../devicetree/bindings/clock/exynos5410-clock.txt |  72 ++++++
 drivers/clk/samsung/Makefile                       |   1 +
 drivers/clk/samsung/clk-exynos5410.c               | 274 +++++++++++++++++++++
 3 files changed, 347 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/exynos5410-clock.txt
 create mode 100644 drivers/clk/samsung/clk-exynos5410.c

diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
new file mode 100644
index 0000000..8c3a7c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
@@ -0,0 +1,72 @@
+* Samsung Exynos5410 Clock Controller
+
+The Exynos5410 clock controller generates and supplies clock to various
+controllers within the Exynos5410 SoC.
+
+Required Properties:
+
+- comptible: should be one of the following.
+  - "samsung,exynos5410-clock" - controller compatible with Exynos5410 SoC.
+
+- reg: physical base address of the controller and length of memory mapped
+  region.
+
+- #clock-cells: should be 1.
+
+The following is the list of clocks generated by the controller. Each clock is
+assigned an identifier and client nodes use this identifier to specify the
+clock which they consume.
+
+
+       [Core Clocks]
+
+  Clock			ID
+  ----------------------------
+
+  fin_pll		1
+
+  [Clock Gate for Special Clocks]
+
+  Clock			ID
+  ----------------------------
+  sclk_uart0		128
+  sclk_uart1		129
+  sclk_uart2		130
+  sclk_uart3		131
+  sclk_mmc0		132
+  sclk_mmc1		133
+  sclk_mmc2		134
+
+   [Peripheral Clock Gates]
+
+  Clock			ID
+  ----------------------------
+
+  uart0			257
+  uart1			258
+  uart2			259
+  uart3			260
+  mct			315
+  mmc0			351
+  mmc1			352
+  mmc2			353
+
+Example 1: An example of a clock controller node is listed below.
+
+	clock: clock-controller@0x10010000 {
+		compatible = "samsung,exynos5410-clock";
+		reg = <0x10010000 0x30000>;
+		#clock-cells = <1>;
+	};
+
+Example 2: UART controller node that consumes the clock generated by the clock
+	   controller. Refer to the standard clock bindings for information
+	   about 'clocks' and 'clock-names' property.
+
+	serial@13820000 {
+		compatible = "samsung,exynos4210-uart";
+		reg = <0x13820000 0x100>;
+		interrupts = <0 54 0>;
+		clocks = <&clock 259>, <&clock 130>;
+		clock-names = "uart", "clk_uart_baud0";
+	};
diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
index 3413380..5a446ca 100644
--- a/drivers/clk/samsung/Makefile
+++ b/drivers/clk/samsung/Makefile
@@ -5,6 +5,7 @@
 obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
 obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
 obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
+obj-$(CONFIG_SOC_EXYNOS5410)	+= clk-exynos5410.o
 obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5420.o
 obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
 obj-$(CONFIG_ARCH_EXYNOS)	+= clk-exynos-audss.o
diff --git a/drivers/clk/samsung/clk-exynos5410.c b/drivers/clk/samsung/clk-exynos5410.c
new file mode 100644
index 0000000..a5d6cac
--- /dev/null
+++ b/drivers/clk/samsung/clk-exynos5410.c
@@ -0,0 +1,274 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tarek Dakhran <t.dakhran@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Common Clock Framework support for Exynos5410 SoC.
+*/
+
+#include <linux/clk.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#include "clk.h"
+#include "clk-pll.h"
+
+#define SRC_CPU			0x200
+#define DIV_CPU0		0x500
+#define SRC_CPERI1		0x4204
+#define DIV_TOP0		0x10510
+#define DIV_TOP1		0x10514
+#define DIV_FSYS1		0x1054c
+#define DIV_FSYS2		0x10550
+#define DIV_PERIC0		0x10558
+#define SRC_TOP0		0x10210
+#define SRC_TOP1		0x10214
+#define SRC_TOP2		0x10218
+#define SRC_FSYS		0x10244
+#define SRC_PERIC0		0x10250
+#define SRC_MASK_FSYS		0x10340
+#define SRC_MASK_PERIC0		0x10350
+#define GATE_BUS_FSYS0		0x10740
+#define GATE_IP_FSYS		0x10944
+#define GATE_IP_PERIC		0x10950
+#define GATE_IP_PERIS		0x10960
+#define SRC_CDREX		0x20200
+#define SRC_KFC			0x28200
+#define DIV_KFC0		0x28500
+
+/* list of PLLs */
+enum exynos5420_plls {
+	apll, cpll, mpll,
+	bpll, kpll,
+	nr_plls			/* number of PLLs */
+};
+
+enum exynos5410_clks {
+	none,
+
+	/* core clocks */
+	fin_pll, fout_apll, fout_cpll, fout_mpll, fout_bpll, fout_kpll,
+
+	/* gate for special clocks (sclk) */
+	sclk_uart0 = 128, sclk_uart1, sclk_uart2, sclk_uart3, sclk_mmc0,
+	sclk_mmc1, sclk_mmc2,
+
+	/* gate clocks */
+	aclk66_peric = 256, uart0, uart1, uart2, uart3,
+	mct = 315,
+	sdmmc0 = 351, sdmmc1, sdmmc2,
+
+	nr_clks,
+};
+
+/*
+ * list of controller registers to be saved and restored during a
+ * suspend/resume cycle.
+ */
+
+static unsigned long exynos5410_clk_regs[] __initdata = {
+	SRC_CPU,
+	DIV_CPU0,
+	SRC_CPERI1,
+	DIV_TOP0,
+	DIV_TOP1,
+	DIV_FSYS1,
+	DIV_FSYS2,
+	DIV_PERIC0,
+	SRC_TOP0,
+	SRC_TOP1,
+	SRC_TOP2,
+	SRC_FSYS,
+	SRC_PERIC0,
+	SRC_MASK_FSYS,
+	SRC_MASK_PERIC0,
+	GATE_BUS_FSYS0,
+	GATE_IP_FSYS,
+	GATE_IP_PERIC,
+	GATE_IP_PERIS,
+	SRC_CDREX,
+	SRC_KFC,
+	DIV_KFC0,
+};
+
+/* list of all parent clocks */
+
+PNAME(apll_p)		= { "fin_pll", "fout_apll", };
+PNAME(bpll_p)		= { "fin_pll", "fout_bpll", };
+PNAME(cpll_p)		= { "fin_pll", "fout_cpll" };
+PNAME(mpll_p)		= { "fin_pll", "fout_mpll", };
+PNAME(kpll_p)		= { "fin_pll", "fout_kpll", };
+
+PNAME(mout_cpu_p)	= { "mout_apll", "sclk_mpll", };
+PNAME(mout_kfc_p)	= { "mout_kpll", "sclk_mpll", };
+
+PNAME(mpll_user_p)	= { "fin_pll", "sclk_mpll", };
+PNAME(bpll_user_p)	= { "fin_pll", "sclk_bpll", };
+PNAME(mpll_bpll_p)	= { "sclk_mpll_muxed", "sclk_bpll_muxed", };
+
+
+
+PNAME(group_main)	= {	"fin_pll", "fin_pll",
+				"sclk_hdmi27m", "sclk_dptxphty",
+				"sclk_usbhost20phy", "sclk_hdmiphy",
+				"sclk_mpll_bpll", "sclk_epll",
+				"sclk_vpll", "sclk_cpll" };
+
+/* fixed rate clocks generated outside the soc */
+struct samsung_fixed_rate_clock exynos5410_fixed_rate_ext_clks[] __initdata = {
+	FRATE(fin_pll, "fin_pll", NULL, CLK_IS_ROOT, 0),
+};
+
+struct samsung_mux_clock exynos5410_mux_clks[] __initdata = {
+	MUX_A(none, "mout_apll", apll_p, SRC_CPU, 0, 1, "mout_apll"),
+	MUX_A(none, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1, "mout_cpu"),
+
+	MUX_A(none, "mout_kpll", kpll_p, SRC_KFC, 0, 1, "mout_kpll"),
+	MUX_A(none, "mout_kfc", mout_kfc_p, SRC_KFC, 16, 1, "mout_kfc"),
+
+	MUX(none, "sclk_mpll", mpll_p, SRC_CPERI1, 8, 1),
+	MUX(none, "sclk_mpll_muxed", mpll_user_p, SRC_TOP2, 20, 1),
+
+	MUX(none, "sclk_bpll", bpll_p, SRC_CDREX, 0, 1),
+	MUX(none, "sclk_bpll_muxed", bpll_user_p, SRC_TOP2, 24, 1),
+
+	MUX(none, "sclk_cpll", cpll_p, SRC_TOP2, 8, 1),
+
+	MUX(none, "sclk_mpll_bpll", mpll_bpll_p, SRC_TOP1, 20, 1),
+
+	MUX(none, "mout_mmc0", group_main, SRC_FSYS, 0, 4),
+	MUX(none, "mout_mmc1", group_main, SRC_FSYS, 4, 4),
+	MUX(none, "mout_mmc2", group_main, SRC_FSYS, 8, 4),
+
+	MUX(none, "mout_uart0", group_main, SRC_PERIC0, 0, 4),
+	MUX(none, "mout_uart1", group_main, SRC_PERIC0, 4, 4),
+	MUX(none, "mout_uart2", group_main, SRC_PERIC0, 8, 4),
+
+	MUX(none, "mout_aclk200", mpll_bpll_p, SRC_TOP0, 12, 1),
+	MUX(none, "mout_aclk400", mpll_bpll_p, SRC_TOP0, 20, 1),
+};
+
+struct samsung_div_clock exynos5410_div_clks[] __initdata = {
+
+	DIV(none, "div_arm", "mout_cpu", DIV_CPU0, 0, 3),
+	DIV(none, "div_arm2", "div_arm", DIV_CPU0, 28, 3),
+
+	DIV_A(none, "div_acp", "div_arm2", DIV_CPU0, 8, 3, "cpu_arm_clk"),
+	DIV_A(none, "div_cpud", "div_arm2", DIV_CPU0, 4, 3, "cpu_aclk_cpud"),
+	DIV_A(none, "div_atb", "div_arm2", DIV_CPU0, 16, 3, "cpu_atclk"),
+	DIV_A(none, "pclk_dbg", "div_arm2", DIV_CPU0, 20, 3, "cpu_pclk_dbg"),
+
+
+	DIV_A(none, "div_kfc", "mout_kfc", DIV_KFC0, 0, 3, "kfc_arm_clk"),
+	DIV_A(none, "div_aclk", "div_kfc", DIV_KFC0, 4, 3, "kfc_aclk_cpud"),
+	DIV_A(none, "div_pclk", "div_kfc", DIV_KFC0, 20, 3, "kfc_pclk_dbg"),
+
+
+	DIV(none, "aclk66_pre", "sclk_mpll_muxed", DIV_TOP1, 24, 3),
+	DIV(none, "aclk66", "aclk66_pre", DIV_TOP0, 0, 3),
+
+	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
+	DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
+	DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
+
+	DIV_F(none, "div_mmc_pre0", "div_mmc0",
+			DIV_FSYS1, 8, 8, CLK_SET_RATE_PARENT, 0),
+	DIV_F(none, "div_mmc_pre1", "div_mmc1",
+			DIV_FSYS1, 24, 8, CLK_SET_RATE_PARENT, 0),
+	DIV_F(none, "div_mmc_pre2", "div_mmc2",
+			DIV_FSYS2, 8, 8, CLK_SET_RATE_PARENT, 0),
+
+	DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
+	DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
+	DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
+	DIV(none, "div_uart3", "mout_uart3", DIV_PERIC0, 12, 4),
+
+	DIV(none, "aclk200", "mout_aclk200", DIV_TOP0, 12, 3),
+	DIV(none, "aclk400", "mout_aclk400", DIV_TOP0, 24, 3),
+};
+
+struct samsung_gate_clock exynos5410_gate_clks[] __initdata = {
+
+	GATE(mct, "mct", "aclk66", GATE_IP_PERIS, 18, 0, 0),
+
+	GATE(sclk_mmc0, "sclk_mmc0", "div_mmc_pre0",
+			SRC_MASK_FSYS, 0, CLK_SET_RATE_PARENT, 0),
+	GATE(sclk_mmc1, "sclk_mmc1", "div_mmc_pre1",
+			SRC_MASK_FSYS, 4, CLK_SET_RATE_PARENT, 0),
+	GATE(sclk_mmc2, "sclk_mmc2", "div_mmc_pre2",
+			SRC_MASK_FSYS, 8, CLK_SET_RATE_PARENT, 0),
+
+	GATE(sdmmc0, "sdmmc0", "aclk200", GATE_BUS_FSYS0, 12, 0, 0),
+	GATE(sdmmc1, "sdmmc1", "aclk200", GATE_BUS_FSYS0, 13, 0, 0),
+	GATE(sdmmc2, "sdmmc2", "aclk200", GATE_BUS_FSYS0, 14, 0, 0),
+
+	GATE(uart0, "uart0", "aclk66", GATE_IP_PERIC, 0, 0, 0),
+	GATE(uart1, "uart1", "aclk66", GATE_IP_PERIC, 1, 0, 0),
+	GATE(uart2, "uart2", "aclk66", GATE_IP_PERIC, 2, 0, 0),
+
+	GATE(sclk_uart0, "sclk_uart0", "div_uart0",
+			SRC_MASK_PERIC0, 0, CLK_SET_RATE_PARENT, 0),
+	GATE(sclk_uart1, "sclk_uart1", "div_uart1",
+			SRC_MASK_PERIC0, 4, CLK_SET_RATE_PARENT, 0),
+	GATE(sclk_uart2, "sclk_uart2", "div_uart2",
+			SRC_MASK_PERIC0, 8, CLK_SET_RATE_PARENT, 0),
+
+};
+
+static struct samsung_pll_clock exynos5410_plls[nr_plls] __initdata = {
+	[apll] = PLL(pll_35xx, fout_apll, "fout_apll", "fin_pll", 0x0,
+		0x100, NULL),
+	[cpll] = PLL(pll_35xx, fout_mpll, "fout_cpll", "fin_pll", 0x10020,
+		0x10120, NULL),
+	[mpll] = PLL(pll_35xx, fout_mpll, "fout_mpll", "fin_pll", 0x4000,
+		0x4100, NULL),
+	[bpll] = PLL(pll_35xx, fout_bpll, "fout_bpll", "fin_pll", 0x20010,
+		0x20110, NULL),
+	[kpll] = PLL(pll_35xx, fout_kpll, "fout_kpll", "fin_pll", 0x28000,
+		0x28100, NULL),
+};
+
+static struct of_device_id ext_clk_match[] __initdata = {
+	{ .compatible = "samsung,exynos5410-oscclk", .data = (void *)0, },
+	{ },
+};
+
+DEFINE_SPINLOCK(int_div_lock);
+
+/* register exynos5410 clocks */
+void __init exynos5410_clk_init(struct device_node *np)
+{
+	void __iomem *reg_base;
+
+	if (np) {
+		reg_base = of_iomap(np, 0);
+		if (!reg_base)
+			panic("%s: failed to map registers\n", __func__);
+	} else {
+		panic("%s: unable to determine soc\n", __func__);
+	}
+
+	samsung_clk_init(np, reg_base, nr_clks,
+			exynos5410_clk_regs, ARRAY_SIZE(exynos5410_clk_regs),
+			NULL, 0);
+	samsung_clk_of_register_fixed_ext(exynos5410_fixed_rate_ext_clks,
+			ARRAY_SIZE(exynos5410_fixed_rate_ext_clks),
+			ext_clk_match);
+	samsung_clk_register_pll(exynos5410_plls, ARRAY_SIZE(exynos5410_plls),
+					reg_base);
+
+	samsung_clk_register_mux(exynos5410_mux_clks,
+			ARRAY_SIZE(exynos5410_mux_clks));
+	samsung_clk_register_div(exynos5410_div_clks,
+			ARRAY_SIZE(exynos5410_div_clks));
+	samsung_clk_register_gate(exynos5410_gate_clks,
+			ARRAY_SIZE(exynos5410_gate_clks));
+
+	pr_info("Exynos5410: clock setup completed.\n");
+}
+CLK_OF_DECLARE(exynos5410_clk, "samsung,exynos5410-clock", exynos5410_clk_init);
-- 
1.8.1.5

Re: [PATCH 2/6] clk: exynos5410: register clocks using common clock framework

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday, October 01, 2013 08:17:03 PM Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran@samsung.com>
> 
> The EXYNOS5410 clocks are statically listed and registered
> using the Samsung specific common clock helper functions.
> 
> Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
> ---
>  .../devicetree/bindings/clock/exynos5410-clock.txt |  72 ++++++
>  drivers/clk/samsung/Makefile                       |   1 +
>  drivers/clk/samsung/clk-exynos5410.c               | 274 +++++++++++++++++++++
>  3 files changed, 347 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/exynos5410-clock.txt
>  create mode 100644 drivers/clk/samsung/clk-exynos5410.c
> 
> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> new file mode 100644
> index 0000000..8c3a7c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt
> @@ -0,0 +1,72 @@
> +* Samsung Exynos5410 Clock Controller
> +
> +The Exynos5410 clock controller generates and supplies clock to various
> +controllers within the Exynos5410 SoC.
> +
> +Required Properties:
> +
> +- comptible: should be one of the following.
> +  - "samsung,exynos5410-clock" - controller compatible with Exynos5410 SoC.
> +
> +- reg: physical base address of the controller and length of memory mapped
> +  region.
> +
> +- #clock-cells: should be 1.
> +
> +The following is the list of clocks generated by the controller. Each clock is
> +assigned an identifier and client nodes use this identifier to specify the
> +clock which they consume.
> +
> +
> +       [Core Clocks]
> +
> +  Clock			ID
> +  ----------------------------
> +
> +  fin_pll		1
> +
> +  [Clock Gate for Special Clocks]
> +
> +  Clock			ID
> +  ----------------------------
> +  sclk_uart0		128
> +  sclk_uart1		129
> +  sclk_uart2		130
> +  sclk_uart3		131
> +  sclk_mmc0		132
> +  sclk_mmc1		133
> +  sclk_mmc2		134
> +
> +   [Peripheral Clock Gates]
> +
> +  Clock			ID
> +  ----------------------------
> +
> +  uart0			257
> +  uart1			258
> +  uart2			259
> +  uart3			260
> +  mct			315
> +  mmc0			351
> +  mmc1			352
> +  mmc2			353
> +
> +Example 1: An example of a clock controller node is listed below.
> +
> +	clock: clock-controller@0x10010000 {
> +		compatible = "samsung,exynos5410-clock";
> +		reg = <0x10010000 0x30000>;
> +		#clock-cells = <1>;
> +	};
> +
> +Example 2: UART controller node that consumes the clock generated by the clock
> +	   controller. Refer to the standard clock bindings for information
> +	   about 'clocks' and 'clock-names' property.
> +
> +	serial@13820000 {
> +		compatible = "samsung,exynos4210-uart";
> +		reg = <0x13820000 0x100>;
> +		interrupts = <0 54 0>;
> +		clocks = <&clock 259>, <&clock 130>;
> +		clock-names = "uart", "clk_uart_baud0";
> +	};
> diff --git a/drivers/clk/samsung/Makefile b/drivers/clk/samsung/Makefile
> index 3413380..5a446ca 100644
> --- a/drivers/clk/samsung/Makefile
> +++ b/drivers/clk/samsung/Makefile
> @@ -5,6 +5,7 @@
>  obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-pll.o
>  obj-$(CONFIG_ARCH_EXYNOS4)	+= clk-exynos4.o
>  obj-$(CONFIG_SOC_EXYNOS5250)	+= clk-exynos5250.o
> +obj-$(CONFIG_SOC_EXYNOS5410)	+= clk-exynos5410.o
>  obj-$(CONFIG_SOC_EXYNOS5420)	+= clk-exynos5420.o
>  obj-$(CONFIG_SOC_EXYNOS5440)	+= clk-exynos5440.o
>  obj-$(CONFIG_ARCH_EXYNOS)	+= clk-exynos-audss.o
> diff --git a/drivers/clk/samsung/clk-exynos5410.c b/drivers/clk/samsung/clk-exynos5410.c
> new file mode 100644
> index 0000000..a5d6cac
> --- /dev/null
> +++ b/drivers/clk/samsung/clk-exynos5410.c
> @@ -0,0 +1,274 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran <t.dakhran@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Common Clock Framework support for Exynos5410 SoC.
> +*/
> +
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +
> +#include "clk.h"
> +#include "clk-pll.h"
> +
> +#define SRC_CPU			0x200
> +#define DIV_CPU0		0x500
> +#define SRC_CPERI1		0x4204
> +#define DIV_TOP0		0x10510
> +#define DIV_TOP1		0x10514
> +#define DIV_FSYS1		0x1054c
> +#define DIV_FSYS2		0x10550
> +#define DIV_PERIC0		0x10558
> +#define SRC_TOP0		0x10210
> +#define SRC_TOP1		0x10214
> +#define SRC_TOP2		0x10218
> +#define SRC_FSYS		0x10244
> +#define SRC_PERIC0		0x10250
> +#define SRC_MASK_FSYS		0x10340
> +#define SRC_MASK_PERIC0		0x10350
> +#define GATE_BUS_FSYS0		0x10740
> +#define GATE_IP_FSYS		0x10944
> +#define GATE_IP_PERIC		0x10950
> +#define GATE_IP_PERIS		0x10960
> +#define SRC_CDREX		0x20200
> +#define SRC_KFC			0x28200
> +#define DIV_KFC0		0x28500
> +
> +/* list of PLLs */
> +enum exynos5420_plls {
> +	apll, cpll, mpll,
> +	bpll, kpll,
> +	nr_plls			/* number of PLLs */
> +};
> +
> +enum exynos5410_clks {
> +	none,
> +
> +	/* core clocks */
> +	fin_pll, fout_apll, fout_cpll, fout_mpll, fout_bpll, fout_kpll,
> +
> +	/* gate for special clocks (sclk) */
> +	sclk_uart0 = 128, sclk_uart1, sclk_uart2, sclk_uart3, sclk_mmc0,
> +	sclk_mmc1, sclk_mmc2,
> +
> +	/* gate clocks */
> +	aclk66_peric = 256, uart0, uart1, uart2, uart3,
> +	mct = 315,
> +	sdmmc0 = 351, sdmmc1, sdmmc2,
> +
> +	nr_clks,
> +};

All other EXYNOS clock drivers have been recently converted to use macros
instead of magic numbers/enums to describe clock bindings.

Please take a look at:

http://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg22376.html

and convert your code accordingly.

> +/*
> + * list of controller registers to be saved and restored during a
> + * suspend/resume cycle.
> + */
> +
> +static unsigned long exynos5410_clk_regs[] __initdata = {
> +	SRC_CPU,
> +	DIV_CPU0,
> +	SRC_CPERI1,
> +	DIV_TOP0,
> +	DIV_TOP1,
> +	DIV_FSYS1,
> +	DIV_FSYS2,
> +	DIV_PERIC0,
> +	SRC_TOP0,
> +	SRC_TOP1,
> +	SRC_TOP2,
> +	SRC_FSYS,
> +	SRC_PERIC0,
> +	SRC_MASK_FSYS,
> +	SRC_MASK_PERIC0,
> +	GATE_BUS_FSYS0,
> +	GATE_IP_FSYS,
> +	GATE_IP_PERIC,
> +	GATE_IP_PERIS,
> +	SRC_CDREX,
> +	SRC_KFC,
> +	DIV_KFC0,
> +};
> +
> +/* list of all parent clocks */
> +
> +PNAME(apll_p)		= { "fin_pll", "fout_apll", };
> +PNAME(bpll_p)		= { "fin_pll", "fout_bpll", };
> +PNAME(cpll_p)		= { "fin_pll", "fout_cpll" };
> +PNAME(mpll_p)		= { "fin_pll", "fout_mpll", };
> +PNAME(kpll_p)		= { "fin_pll", "fout_kpll", };
> +
> +PNAME(mout_cpu_p)	= { "mout_apll", "sclk_mpll", };
> +PNAME(mout_kfc_p)	= { "mout_kpll", "sclk_mpll", };
> +
> +PNAME(mpll_user_p)	= { "fin_pll", "sclk_mpll", };
> +PNAME(bpll_user_p)	= { "fin_pll", "sclk_bpll", };
> +PNAME(mpll_bpll_p)	= { "sclk_mpll_muxed", "sclk_bpll_muxed", };
> +
> +
> +
> +PNAME(group_main)	= {	"fin_pll", "fin_pll",
> +				"sclk_hdmi27m", "sclk_dptxphty",
> +				"sclk_usbhost20phy", "sclk_hdmiphy",
> +				"sclk_mpll_bpll", "sclk_epll",
> +				"sclk_vpll", "sclk_cpll" };
> +
> +/* fixed rate clocks generated outside the soc */
> +struct samsung_fixed_rate_clock exynos5410_fixed_rate_ext_clks[] __initdata = {
> +	FRATE(fin_pll, "fin_pll", NULL, CLK_IS_ROOT, 0),
> +};
> +
> +struct samsung_mux_clock exynos5410_mux_clks[] __initdata = {
> +	MUX_A(none, "mout_apll", apll_p, SRC_CPU, 0, 1, "mout_apll"),
> +	MUX_A(none, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1, "mout_cpu"),
> +
> +	MUX_A(none, "mout_kpll", kpll_p, SRC_KFC, 0, 1, "mout_kpll"),
> +	MUX_A(none, "mout_kfc", mout_kfc_p, SRC_KFC, 16, 1, "mout_kfc"),
> +
> +	MUX(none, "sclk_mpll", mpll_p, SRC_CPERI1, 8, 1),
> +	MUX(none, "sclk_mpll_muxed", mpll_user_p, SRC_TOP2, 20, 1),
> +
> +	MUX(none, "sclk_bpll", bpll_p, SRC_CDREX, 0, 1),
> +	MUX(none, "sclk_bpll_muxed", bpll_user_p, SRC_TOP2, 24, 1),
> +
> +	MUX(none, "sclk_cpll", cpll_p, SRC_TOP2, 8, 1),
> +
> +	MUX(none, "sclk_mpll_bpll", mpll_bpll_p, SRC_TOP1, 20, 1),
> +
> +	MUX(none, "mout_mmc0", group_main, SRC_FSYS, 0, 4),
> +	MUX(none, "mout_mmc1", group_main, SRC_FSYS, 4, 4),
> +	MUX(none, "mout_mmc2", group_main, SRC_FSYS, 8, 4),
> +
> +	MUX(none, "mout_uart0", group_main, SRC_PERIC0, 0, 4),
> +	MUX(none, "mout_uart1", group_main, SRC_PERIC0, 4, 4),
> +	MUX(none, "mout_uart2", group_main, SRC_PERIC0, 8, 4),
> +
> +	MUX(none, "mout_aclk200", mpll_bpll_p, SRC_TOP0, 12, 1),
> +	MUX(none, "mout_aclk400", mpll_bpll_p, SRC_TOP0, 20, 1),
> +};
> +
> +struct samsung_div_clock exynos5410_div_clks[] __initdata = {
> +
> +	DIV(none, "div_arm", "mout_cpu", DIV_CPU0, 0, 3),
> +	DIV(none, "div_arm2", "div_arm", DIV_CPU0, 28, 3),
> +
> +	DIV_A(none, "div_acp", "div_arm2", DIV_CPU0, 8, 3, "cpu_arm_clk"),
> +	DIV_A(none, "div_cpud", "div_arm2", DIV_CPU0, 4, 3, "cpu_aclk_cpud"),
> +	DIV_A(none, "div_atb", "div_arm2", DIV_CPU0, 16, 3, "cpu_atclk"),
> +	DIV_A(none, "pclk_dbg", "div_arm2", DIV_CPU0, 20, 3, "cpu_pclk_dbg"),
> +
> +
> +	DIV_A(none, "div_kfc", "mout_kfc", DIV_KFC0, 0, 3, "kfc_arm_clk"),
> +	DIV_A(none, "div_aclk", "div_kfc", DIV_KFC0, 4, 3, "kfc_aclk_cpud"),
> +	DIV_A(none, "div_pclk", "div_kfc", DIV_KFC0, 20, 3, "kfc_pclk_dbg"),
> +
> +
> +	DIV(none, "aclk66_pre", "sclk_mpll_muxed", DIV_TOP1, 24, 3),
> +	DIV(none, "aclk66", "aclk66_pre", DIV_TOP0, 0, 3),
> +
> +	DIV(none, "div_mmc0", "mout_mmc0", DIV_FSYS1, 0, 4),
> +	DIV(none, "div_mmc1", "mout_mmc1", DIV_FSYS1, 16, 4),
> +	DIV(none, "div_mmc2", "mout_mmc2", DIV_FSYS2, 0, 4),
> +
> +	DIV_F(none, "div_mmc_pre0", "div_mmc0",
> +			DIV_FSYS1, 8, 8, CLK_SET_RATE_PARENT, 0),
> +	DIV_F(none, "div_mmc_pre1", "div_mmc1",
> +			DIV_FSYS1, 24, 8, CLK_SET_RATE_PARENT, 0),
> +	DIV_F(none, "div_mmc_pre2", "div_mmc2",
> +			DIV_FSYS2, 8, 8, CLK_SET_RATE_PARENT, 0),
> +
> +	DIV(none, "div_uart0", "mout_uart0", DIV_PERIC0, 0, 4),
> +	DIV(none, "div_uart1", "mout_uart1", DIV_PERIC0, 4, 4),
> +	DIV(none, "div_uart2", "mout_uart2", DIV_PERIC0, 8, 4),
> +	DIV(none, "div_uart3", "mout_uart3", DIV_PERIC0, 12, 4),
> +
> +	DIV(none, "aclk200", "mout_aclk200", DIV_TOP0, 12, 3),
> +	DIV(none, "aclk400", "mout_aclk400", DIV_TOP0, 24, 3),
> +};
> +
> +struct samsung_gate_clock exynos5410_gate_clks[] __initdata = {
> +
> +	GATE(mct, "mct", "aclk66", GATE_IP_PERIS, 18, 0, 0),
> +
> +	GATE(sclk_mmc0, "sclk_mmc0", "div_mmc_pre0",
> +			SRC_MASK_FSYS, 0, CLK_SET_RATE_PARENT, 0),
> +	GATE(sclk_mmc1, "sclk_mmc1", "div_mmc_pre1",
> +			SRC_MASK_FSYS, 4, CLK_SET_RATE_PARENT, 0),
> +	GATE(sclk_mmc2, "sclk_mmc2", "div_mmc_pre2",
> +			SRC_MASK_FSYS, 8, CLK_SET_RATE_PARENT, 0),
> +
> +	GATE(sdmmc0, "sdmmc0", "aclk200", GATE_BUS_FSYS0, 12, 0, 0),
> +	GATE(sdmmc1, "sdmmc1", "aclk200", GATE_BUS_FSYS0, 13, 0, 0),
> +	GATE(sdmmc2, "sdmmc2", "aclk200", GATE_BUS_FSYS0, 14, 0, 0),
> +
> +	GATE(uart0, "uart0", "aclk66", GATE_IP_PERIC, 0, 0, 0),
> +	GATE(uart1, "uart1", "aclk66", GATE_IP_PERIC, 1, 0, 0),
> +	GATE(uart2, "uart2", "aclk66", GATE_IP_PERIC, 2, 0, 0),
> +
> +	GATE(sclk_uart0, "sclk_uart0", "div_uart0",
> +			SRC_MASK_PERIC0, 0, CLK_SET_RATE_PARENT, 0),
> +	GATE(sclk_uart1, "sclk_uart1", "div_uart1",
> +			SRC_MASK_PERIC0, 4, CLK_SET_RATE_PARENT, 0),
> +	GATE(sclk_uart2, "sclk_uart2", "div_uart2",
> +			SRC_MASK_PERIC0, 8, CLK_SET_RATE_PARENT, 0),
> +
> +};
> +
> +static struct samsung_pll_clock exynos5410_plls[nr_plls] __initdata = {
> +	[apll] = PLL(pll_35xx, fout_apll, "fout_apll", "fin_pll", 0x0,
> +		0x100, NULL),
> +	[cpll] = PLL(pll_35xx, fout_mpll, "fout_cpll", "fin_pll", 0x10020,
> +		0x10120, NULL),
> +	[mpll] = PLL(pll_35xx, fout_mpll, "fout_mpll", "fin_pll", 0x4000,
> +		0x4100, NULL),
> +	[bpll] = PLL(pll_35xx, fout_bpll, "fout_bpll", "fin_pll", 0x20010,
> +		0x20110, NULL),
> +	[kpll] = PLL(pll_35xx, fout_kpll, "fout_kpll", "fin_pll", 0x28000,
> +		0x28100, NULL),
> +};
> +
> +static struct of_device_id ext_clk_match[] __initdata = {
> +	{ .compatible = "samsung,exynos5410-oscclk", .data = (void *)0, },
> +	{ },
> +};
> +
> +DEFINE_SPINLOCK(int_div_lock);

this is unused currently

> +/* register exynos5410 clocks */
> +void __init exynos5410_clk_init(struct device_node *np)

static?

> +{
> +	void __iomem *reg_base;
> +
> +	if (np) {
> +		reg_base = of_iomap(np, 0);
> +		if (!reg_base)
> +			panic("%s: failed to map registers\n", __func__);
> +	} else {
> +		panic("%s: unable to determine soc\n", __func__);
> +	}
> +
> +	samsung_clk_init(np, reg_base, nr_clks,
> +			exynos5410_clk_regs, ARRAY_SIZE(exynos5410_clk_regs),
> +			NULL, 0);
> +	samsung_clk_of_register_fixed_ext(exynos5410_fixed_rate_ext_clks,
> +			ARRAY_SIZE(exynos5410_fixed_rate_ext_clks),
> +			ext_clk_match);
> +	samsung_clk_register_pll(exynos5410_plls, ARRAY_SIZE(exynos5410_plls),
> +					reg_base);
> +
> +	samsung_clk_register_mux(exynos5410_mux_clks,
> +			ARRAY_SIZE(exynos5410_mux_clks));
> +	samsung_clk_register_div(exynos5410_div_clks,
> +			ARRAY_SIZE(exynos5410_div_clks));
> +	samsung_clk_register_gate(exynos5410_gate_clks,
> +			ARRAY_SIZE(exynos5410_gate_clks));
> +
> +	pr_info("Exynos5410: clock setup completed.\n");
> +}
> +CLK_OF_DECLARE(exynos5410_clk, "samsung,exynos5410-clock", exynos5410_clk_init);

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 2/6] clk: exynos5410: register clocks using common clock framework

[Stephen Warren]

On 10/01/2013 10:17 AM, Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> The EXYNOS5410 clocks are statically listed and registered
> using the Samsung specific common clock helper functions.

> diff --git a/Documentation/devicetree/bindings/clock/exynos5410-clock.txt b/Documentation/devicetree/bindings/clock/exynos5410-clock.txt

> +       [Core Clocks]
> +  [Clock Gate for Special Clocks]
> +   [Peripheral Clock Gates]

These headers/titles for the sections/lists aren't consistently aligned.

> +  [Clock Gate for Special Clocks]
> +
> +  Clock			ID
> +  ----------------------------
> +  sclk_uart0		128
> +  sclk_uart1		129
> +  sclk_uart2		130
> +  sclk_uart3		131
> +  sclk_mmc0		132
> +  sclk_mmc1		133
> +  sclk_mmc2		134
> +
> +   [Peripheral Clock Gates]
> +
> +  Clock			ID
> +  ----------------------------
> +
> +  uart0			257
> +  uart1			258
> +  uart2			259
> +  uart3			260
> +  mct			315
> +  mmc0			351
> +  mmc1			352
> +  mmc2			353

That's not many clocks. I assume you're planning on adding more IDs
later, in a backwards-compatible fashion? I suppose that's OK since it
won't break any existing usage, as long as there's no need to renumber
any existing values.

On that topic, are any of those clock IDs derived from HW, e.g. register
numbers, or bit numbers in an array of registers? Numbering clocks in a
HW-derived fashion would make it easier or more obvious how to add new
clock IDs later while maintaining some consistency and without
introducing the desire to break any ABI.

Finally, how about creating a header file such as
include/dt-bindings/clock/exynos5410.h to define those clock IDs, so
that both *.dts and the clock driver can share the values without having
to manually write them?

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] clk: exynos5410: register clocks using common clock framework

[Tomasz Figa]

Hi Vyacheslav, Tarek,

On Tuesday 01 of October 2013 20:17:03 Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran@samsung.com>
> 
> The EXYNOS5410 clocks are statically listed and registered
> using the Samsung specific common clock helper functions.
[snip]
> +Required Properties:
> +
> +- comptible: should be one of the following.
> +  - "samsung,exynos5410-clock" - controller compatible with Exynos5410
> SoC. +

nit: There is only one compatible value supported by this driver, so "one 
of the following" sounds a bit strange.

> +- reg: physical base address of the controller and length of memory
> mapped +  region.
> +
> +- #clock-cells: should be 1.
[snip]
> +  mct			315
> +  mmc0			351
> +  mmc1			352
> +  mmc2			353
> +

As Bart already mentioned, it would be better to use preprocessor macros 
to define all the clock IDs, like it is done in s3c64xx clock driver and 
after applying Andrzej Hajda's patchs on all Exynos clock drivers.

> +Example 1: An example of a clock controller node is listed below.
> +
> +	clock: clock-controller@0x10010000 {
> +		compatible = "samsung,exynos5410-clock";
> +		reg = <0x10010000 0x30000>;
> +		#clock-cells = <1>;
> +	};
[snip]
> +PNAME(mpll_user_p)	= { "fin_pll", "sclk_mpll", };
> +PNAME(bpll_user_p)	= { "fin_pll", "sclk_bpll", };
> +PNAME(mpll_bpll_p)	= { "sclk_mpll_muxed", "sclk_bpll_muxed", };
> +
> +
> +
> +PNAME(group_main)	= {	"fin_pll", "fin_pll",
> +				"sclk_hdmi27m", "sclk_dptxphty",
> +				"sclk_usbhost20phy", "sclk_hdmiphy",
> +				"sclk_mpll_bpll", "sclk_epll",
> +				"sclk_vpll", "sclk_cpll" };

All mux inputs must be specified in parent lists, even those unused. This 
is, if a mux has 4-bit selector, then all 2^4 inputs must be specified, 
with unused ones set to "none". Otherwise this would lead to out of bound 
accesses from clock code.

> +
> +/* fixed rate clocks generated outside the soc */
> +struct samsung_fixed_rate_clock exynos5410_fixed_rate_ext_clks[]
> __initdata = {

static

> +	FRATE(fin_pll, "fin_pll", NULL, CLK_IS_ROOT, 0),
> +};
> +
> +struct samsung_mux_clock exynos5410_mux_clks[] __initdata = {

static

> +	MUX_A(none, "mout_apll", apll_p, SRC_CPU, 0, 1, "mout_apll"),
> +	MUX_A(none, "mout_cpu", mout_cpu_p, SRC_CPU, 16, 1, "mout_cpu"),
> +
> +	MUX_A(none, "mout_kpll", kpll_p, SRC_KFC, 0, 1, "mout_kpll"),
> +	MUX_A(none, "mout_kfc", mout_kfc_p, SRC_KFC, 16, 1, "mout_kfc"),

Do you actually need aliases for the four clocks above?

> +	MUX(none, "sclk_mpll", mpll_p, SRC_CPERI1, 8, 1),
> +	MUX(none, "sclk_mpll_muxed", mpll_user_p, SRC_TOP2, 20, 1),
[snip]
> +	DIV_A(none, "div_acp", "div_arm2", DIV_CPU0, 8, 3, "cpu_arm_clk"),
> +	DIV_A(none, "div_cpud", "div_arm2", DIV_CPU0, 4, 3, 
"cpu_aclk_cpud"),
> +	DIV_A(none, "div_atb", "div_arm2", DIV_CPU0, 16, 3, "cpu_atclk"),
> +	DIV_A(none, "pclk_dbg", "div_arm2", DIV_CPU0, 20, 3, 
"cpu_pclk_dbg"),
> +
> +
> +	DIV_A(none, "div_kfc", "mout_kfc", DIV_KFC0, 0, 3, "kfc_arm_clk"),
> +	DIV_A(none, "div_aclk", "div_kfc", DIV_KFC0, 4, 3, 
"kfc_aclk_cpud"),
> +	DIV_A(none, "div_pclk", "div_kfc", DIV_KFC0, 20, 3, 
"kfc_pclk_dbg"),

Same here.

> +
> +
> +	DIV(none, "aclk66_pre", "sclk_mpll_muxed", DIV_TOP1, 24, 3),
> +	DIV(none, "aclk66", "aclk66_pre", DIV_TOP0, 0, 3),
[snip]
> +struct samsung_gate_clock exynos5410_gate_clks[] __initdata = {

static

> +
> +	GATE(mct, "mct", "aclk66", GATE_IP_PERIS, 18, 0, 0),
[snip]
> +static struct samsung_pll_clock exynos5410_plls[nr_plls] __initdata = {
> +	[apll] = PLL(pll_35xx, fout_apll, "fout_apll", "fin_pll", 0x0,
> +		0x100, NULL),
> +	[cpll] = PLL(pll_35xx, fout_mpll, "fout_cpll", "fin_pll", 0x10020,
> +		0x10120, NULL),
> +	[mpll] = PLL(pll_35xx, fout_mpll, "fout_mpll", "fin_pll", 0x4000,
> +		0x4100, NULL),
> +	[bpll] = PLL(pll_35xx, fout_bpll, "fout_bpll", "fin_pll", 0x20010,
> +		0x20110, NULL),
> +	[kpll] = PLL(pll_35xx, fout_kpll, "fout_kpll", "fin_pll", 0x28000,
> +		0x28100, NULL),

nit: It would be better if the magic register offsets above were defined 
as preprocessor macros as other registers used in this driver.

> +};
> +
> +static struct of_device_id ext_clk_match[] __initdata = {
> +	{ .compatible = "samsung,exynos5410-oscclk", .data = (void *)0, },
> +	{ },
> +};

Please consider using generic fixed rate clock bindings.

> +DEFINE_SPINLOCK(int_div_lock);

This does not seem to be used anywhere.

> +
> +/* register exynos5410 clocks */
> +void __init exynos5410_clk_init(struct device_node *np)

static

> +{
> +	void __iomem *reg_base;
> +
> +	if (np) {

This check is redundant, since this function can be only called from 
of_clk_init() with a valid pointer to device tree node.

Best regards,
Tomasz

[PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Vyacheslav Tyrtov]

From: Tarek Dakhran <t.dakhran@samsung.com>

Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.

Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
---
 arch/arm/mach-exynos/Makefile      |   2 +
 arch/arm/mach-exynos/edcs.c        | 217 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-exynos/edcs.h        |  41 +++++++
 arch/arm/mach-exynos/edcs_status.c | 110 +++++++++++++++++++
 4 files changed, 370 insertions(+)
 create mode 100644 arch/arm/mach-exynos/edcs.c
 create mode 100644 arch/arm/mach-exynos/edcs.h
 create mode 100644 arch/arm/mach-exynos/edcs_status.c

diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
index 5369615..18e6162 100644
--- a/arch/arm/mach-exynos/Makefile
+++ b/arch/arm/mach-exynos/Makefile
@@ -34,3 +34,5 @@ AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-a$(plus_sec)
 
 obj-$(CONFIG_MACH_EXYNOS4_DT)		+= mach-exynos4-dt.o
 obj-$(CONFIG_MACH_EXYNOS5_DT)		+= mach-exynos5-dt.o
+
+obj-$(CONFIG_ARM_CCI)		+= edcs.o	edcs_status.o
diff --git a/arch/arm/mach-exynos/edcs.c b/arch/arm/mach-exynos/edcs.c
new file mode 100644
index 0000000..34b4e4b
--- /dev/null
+++ b/arch/arm/mach-exynos/edcs.c
@@ -0,0 +1,217 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tarek Dakhran <t.dakhran@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
+ */
+
+#include <linux/kernel.h>
+#include <linux/spinlock.h>
+#include <linux/errno.h>
+#include <linux/vexpress.h>
+#include <asm/mcpm.h>
+#include <asm/proc-fns.h>
+#include <asm/cacheflush.h>
+#include <asm/cputype.h>
+#include <asm/cp15.h>
+
+#include "edcs.h"
+
+static arch_spinlock_t exynos_lock = __ARCH_SPIN_LOCK_UNLOCKED;
+static int kfs_use_count[MAX_CPUS_PER_CLUSTER][MAX_NR_CLUSTERS];
+static int core_count[MAX_NR_CLUSTERS];
+
+static int exynos_core_power_control(unsigned int cpu, unsigned int cluster,
+				int enable)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(10);
+	unsigned int offset = cluster * MAX_CPUS_PER_CLUSTER + cpu;
+	int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
+
+	if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3) == value)
+		return 0;
+
+	__raw_writel(value, EXYNOS5410_CORE_CONFIGURATION(offset));
+	do {
+		if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3)
+				== value)
+			return 0;
+	} while (time_before(jiffies, timeout));
+
+	return -EDEADLK;
+}
+static int exynos_cluster_power_control(unsigned int cluster, int enable)
+{
+	unsigned long timeout = jiffies + msecs_to_jiffies(10);
+	int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
+
+	if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) & 0x3)
+			== value)
+		return 0;
+
+	__raw_writel(value, EXYNOS5410_COMMON_CONFIGURATION(cluster));
+	do {
+		if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) &
+			__raw_readl(EXYNOS5410_L2_STATUS(cluster)) & 0x3)
+				== value)
+			return 0;
+	} while (time_before(jiffies, timeout));
+
+	return -EDEADLK;
+}
+
+static int exynos_core_power_up(unsigned int cpu, unsigned int cluster)
+{
+	return exynos_core_power_control(cpu, cluster, 1);
+}
+
+static int exynos_core_power_down(unsigned int cpu, unsigned int cluster)
+{
+	return exynos_core_power_control(cpu, cluster, 0);
+}
+
+static int exynos_cluster_power_up(unsigned int cluster)
+{
+	return exynos_cluster_power_control(cluster, 1);
+}
+
+static int exynos_power_up(unsigned int cpu, unsigned int cluster)
+{
+	int ret;
+	local_irq_disable();
+	arch_spin_lock(&exynos_lock);
+
+	pr_info("A%d CORE%d\n", 15 - cluster * 8, cpu);
+	kfs_use_count[cpu][cluster]++;
+	if (kfs_use_count[cpu][cluster] == 1) {
+		++core_count[cluster];
+		if (core_count[cluster] == 1) {
+			ret = exynos_cluster_power_up(cluster);
+			if (ret) {
+				pr_err("%s: cluster %u power up error\n",
+					__func__, cluster);
+				return ret;
+			}
+			__cci_control_port_by_index(MAX_NR_CLUSTERS
+					 - cluster, true);
+		}
+		ret = exynos_core_power_up(cpu, cluster);
+		if (ret) {
+			pr_err("%s: cpu %u cluster %u power up error\n",
+				__func__, cpu, cluster);
+			return ret;
+		}
+	} else if (kfs_use_count[cpu][cluster] != 2) {
+		/*
+		 * The only possible values are:
+		 * 0 = CPU down
+		 * 1 = CPU (still) up
+		 * 2 = CPU requested to be up before it had a chance
+		 *     to actually make itself down.
+		 * Any other value is a bug.
+		 */
+		BUG();
+	}
+	arch_spin_unlock(&exynos_lock);
+	local_irq_enable();
+	return 0;
+}
+static void exynos_power_down(void)
+{
+	unsigned int mpidr, cpu, cluster;
+	bool last_man = false, skip_wfi = false;
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+
+	pr_info("A%d CORE%d\n", 15 - cluster * 8, cpu);
+	BUG_ON(cpu >= MAX_CPUS_PER_CLUSTER  || cluster >= MAX_NR_CLUSTERS);
+	__mcpm_cpu_going_down(cpu, cluster);
+	arch_spin_lock(&exynos_lock);
+	BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
+	kfs_use_count[cpu][cluster]--;
+
+	if (kfs_use_count[cpu][cluster] == 0) {
+		exynos_core_power_down(cpu, cluster);
+		--core_count[cluster];
+		if (core_count[cluster] == 0)
+			last_man = true;
+
+	} else if (kfs_use_count[cpu][cluster] == 1) {
+		skip_wfi = true;
+	} else
+		BUG();
+	if (last_man && __mcpm_outbound_enter_critical(cpu, cluster)) {
+		arch_spin_unlock(&exynos_lock);
+		flush_cache_all();
+		set_cr(get_cr() & ~CR_C);
+		flush_cache_all();
+		outer_flush_all();
+		set_auxcr(get_auxcr() & ~(1 << 6));
+		cci_disable_port_by_cpu(mpidr);
+		__mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
+	} else {
+		arch_spin_unlock(&exynos_lock);
+		flush_cache_louis();
+		set_cr(get_cr() & ~CR_C);
+		flush_cache_louis();
+		set_auxcr(get_auxcr() & ~(1 << 6));
+	}
+	__mcpm_cpu_down(cpu, cluster);
+
+	dsb();
+	if (!skip_wfi)
+		wfi();
+}
+static const struct mcpm_platform_ops exynos_power_ops = {
+	.power_up	= exynos_power_up,
+	.power_down	= exynos_power_down,
+};
+
+static void __init edcs_data_init(void)
+{
+	unsigned int mpidr, cpu, cluster;
+
+	mpidr = read_cpuid_mpidr();
+	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
+	pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
+	BUG_ON(cpu >= 4 || cluster >= 2);
+	kfs_use_count[cpu][cluster] = 1;
+	++core_count[cluster];
+	__cci_control_port_by_index(MAX_NR_CLUSTERS - cluster, true);
+}
+
+
+static int __init edcs_init(void)
+{
+	struct device_node *node;
+
+	if (!cci_probed())
+		return -ENODEV;
+
+	node = of_find_compatible_node(NULL, NULL, "samsung,edcs");
+	if (!node)
+		return -ENODEV;
+	edcs_data_init();
+	mcpm_smp_set_ops();
+	mcpm_platform_register(&exynos_power_ops);
+
+	/*
+	 * Future entries into the kernel can now go
+	 * through the cluster entry vectors.
+	 */
+
+	__raw_writel(virt_to_phys(mcpm_entry_point),
+			S5P_VA_SYSRAM_NS + 0x1c);
+
+	pr_info("EDCS: EXYNOS5410 DUAL CLUSTER SUPPORT installed\n");
+
+	return 0;
+}
+
+early_initcall(edcs_init);
diff --git a/arch/arm/mach-exynos/edcs.h b/arch/arm/mach-exynos/edcs.h
new file mode 100644
index 0000000..dd3e204
--- /dev/null
+++ b/arch/arm/mach-exynos/edcs.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tarek Dakhran <t.dakhran@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
+ */
+
+#include <linux/arm-cci.h>
+#include <linux/of_address.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <mach/regs-pmu.h>
+
+#define EXYNOS5410_ARM_COMMON_CONFIGURATION     S5P_PMUREG(0x2500)
+#define EXYNOS5410_COMMON_CONFIGURATION(_nr)        \
+	(EXYNOS5410_ARM_COMMON_CONFIGURATION + ((_nr) * 0x80))
+#define EXYNOS5410_COMMON_STATUS(_nr)               \
+	(EXYNOS5410_COMMON_CONFIGURATION(_nr) + 0x4)
+#define EXYNOS5410_COMMON_OPTION(_nr)               \
+	(EXYNOS5410_COMMON_CONFIGURATION(_nr) + 0x8)
+
+#define EXYNOS5410_ARM_L2_CONFIGURATION         S5P_PMUREG(0x2600)
+#define EXYNOS5410_L2_CONFIGURATION(_nr)    \
+	(EXYNOS5410_ARM_L2_CONFIGURATION + ((_nr) * 0x80))
+#define EXYNOS5410_L2_STATUS(_nr)           \
+	(EXYNOS5410_L2_CONFIGURATION(_nr) + 0x4)
+#define EXYNOS5410_L2_OPTION(_nr)           \
+	(EXYNOS5410_L2_CONFIGURATION(_nr) + 0x8)
+
+#define EXYNOS5410_CORE_CONFIGURATION(_nr)        \
+	(S5P_ARM_CORE0_CONFIGURATION + ((_nr) * 0x80))
+#define EXYNOS5410_CORE_STATUS(_nr)               \
+	(EXYNOS5410_CORE_CONFIGURATION(_nr) + 0x4)
+#define EXYNOS5410_CORE_OPTION(_nr)               \
+	(EXYNOS5410_CORE_CONFIGURATION(_nr) + 0x8)
+
+#define EXYNOS5_PA_CCI                  0x10D20000
diff --git a/arch/arm/mach-exynos/edcs_status.c b/arch/arm/mach-exynos/edcs_status.c
new file mode 100644
index 0000000..bff1e7f
--- /dev/null
+++ b/arch/arm/mach-exynos/edcs_status.c
@@ -0,0 +1,110 @@
+/*
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tarek Dakhran <t.dakhran@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
+ */
+
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <asm/uaccess.h>
+
+#include "edcs.h"
+
+int edcs_check_status(char *info)
+{
+	size_t it = 30;
+	int i;
+
+	void __iomem *cci_base;
+
+	cci_base = ioremap(EXYNOS5_PA_CCI, SZ_64K);
+	if (!cci_base)
+		return -ENOMEM;
+
+	if ((__raw_readl(EXYNOS5410_COMMON_STATUS(0)) & 0x3) == 3)
+		info[it] = '1';
+	it += 3;
+
+	for (i = 0; i < 4; i++) {
+		if ((__raw_readl(EXYNOS5410_CORE_STATUS(i)) & 0x3) == 3)
+			info[it] = '1';
+		it += 3;
+	}
+
+	if ((__raw_readl(EXYNOS5410_L2_STATUS(0)) & 0x3)	== 3)
+		info[it] = '1';
+	it += 3;
+
+	if ((readl(cci_base + 0x4000 + 1 * 0x1000) & 0x3) == 3)
+		info[it] = '1';
+
+	it += 10;
+
+	if ((__raw_readl(EXYNOS5410_COMMON_STATUS(1)) & 0x3) == 3)
+		info[it] = '1';
+	it += 3;
+
+	for (i = 4; i < 8; i++) {
+		if ((__raw_readl(EXYNOS5410_CORE_STATUS(i)) & 0x3) == 3)
+			info[it] = '1';
+		it += 3;
+	}
+
+	if ((__raw_readl(EXYNOS5410_L2_STATUS(1)) & 0x3) == 3)
+		info[it] = '1';
+	it += 3;
+
+	if ((readl(cci_base + 0x4000 + 0 * 0x1000) & 0x3) == 3)
+		info[it] = '1';
+
+	iounmap(cci_base);
+	return 0;
+}
+
+static ssize_t edcs_status_read(struct file *file, char __user *buf,
+				size_t len, loff_t *pos)
+{
+
+	char info[] = "\tC  0  1  2  3 L2 CCI\n"
+		"[A15]   0  0  0  0  0  0  0\n"
+		" [A7]   0  0  0  0  0  0  0\n";
+	size_t count = sizeof(info);
+	edcs_check_status(info);
+	return simple_read_from_buffer(buf, len, pos, info, count);
+}
+
+static const struct file_operations edcs_status_fops = {
+	.read		= edcs_status_read,
+	.owner		= THIS_MODULE,
+};
+static struct miscdevice edcs_status_device = {
+	.name = "edcs_status",
+	.minor = 255,
+	.fops = &edcs_status_fops,
+};
+
+static int __init edcs_dev_init(void)
+{
+	struct device_node *node;
+	int ret;
+	if (!cci_probed())
+		return -ENODEV;
+
+	node = of_find_compatible_node(NULL, NULL, "samsung,edcs");
+	if (!node)
+		return -ENODEV;
+
+	ret = misc_register(&edcs_status_device);
+	if (ret) {
+		pr_err("EDCS: edcs_status device is not registered\n");
+		return ret;
+	}
+	pr_info("EDCS: edcs_status device registered\n");
+	return 0;
+}
+device_initcall(edcs_dev_init);
-- 
1.8.1.5

Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Nicolas Pitre]

On Tue, 1 Oct 2013, Vyacheslav Tyrtov wrote:

> From: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
> This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.
> 
> Signed-off-by: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm/mach-exynos/Makefile      |   2 +
>  arch/arm/mach-exynos/edcs.c        | 217 +++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-exynos/edcs.h        |  41 +++++++
>  arch/arm/mach-exynos/edcs_status.c | 110 +++++++++++++++++++
>  4 files changed, 370 insertions(+)
>  create mode 100644 arch/arm/mach-exynos/edcs.c
>  create mode 100644 arch/arm/mach-exynos/edcs.h
>  create mode 100644 arch/arm/mach-exynos/edcs_status.c
> 
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 5369615..18e6162 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -34,3 +34,5 @@ AFLAGS_exynos-smc.o		:=-Wa,-march=armv7-a$(plus_sec)
>  
>  obj-$(CONFIG_MACH_EXYNOS4_DT)		+= mach-exynos4-dt.o
>  obj-$(CONFIG_MACH_EXYNOS5_DT)		+= mach-exynos5-dt.o
> +
> +obj-$(CONFIG_ARM_CCI)		+= edcs.o	edcs_status.o
> diff --git a/arch/arm/mach-exynos/edcs.c b/arch/arm/mach-exynos/edcs.c
> new file mode 100644
> index 0000000..34b4e4b
> --- /dev/null
> +++ b/arch/arm/mach-exynos/edcs.c
> @@ -0,0 +1,217 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/spinlock.h>
> +#include <linux/errno.h>
> +#include <linux/vexpress.h>

Why do you need a vexpress include?

> +#include <asm/mcpm.h>
> +#include <asm/proc-fns.h>
> +#include <asm/cacheflush.h>
> +#include <asm/cputype.h>
> +#include <asm/cp15.h>
> +
> +#include "edcs.h"
> +
> +static arch_spinlock_t exynos_lock = __ARCH_SPIN_LOCK_UNLOCKED;
> +static int kfs_use_count[MAX_CPUS_PER_CLUSTER][MAX_NR_CLUSTERS];

The "kfs_" prefix looks like a remnant of ancient code which stood for 
"Kingfisher" before it was renamed to "DCSCB".  :-)

You might consider using "edcs_" instead.  Same thing for exynos_lock 
above which is rather generic a name.  Maybe edcs_lock would be better.

Also please define your own constant such as EDCS_CPUS_PER_CLUSTER 
instead of using MAX_CPUS_PER_CLUSTER as the later is related to the 
code limit and not the actual hardware topology, and it could change in 
the future.

> +static int core_count[MAX_NR_CLUSTERS]; > +
> +static int exynos_core_power_control(unsigned int cpu, unsigned int cluster,
> +				int enable)
> +{
> +	unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +	unsigned int offset = cluster * MAX_CPUS_PER_CLUSTER + cpu;

Here's an example of where your code would break if MAX_CPUS_PER_CLUSTER 
changes.

> +	int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
> +
> +	if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3) == value)
> +		return 0;
> +
> +	__raw_writel(value, EXYNOS5410_CORE_CONFIGURATION(offset));
> +	do {
> +		if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3)
> +				== value)
> +			return 0;
> +	} while (time_before(jiffies, timeout));
> +
> +	return -EDEADLK;
> +}
> +static int exynos_cluster_power_control(unsigned int cluster, int enable)
> +{
> +	unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +	int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
> +
> +	if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) & 0x3)
> +			== value)

You could put the above if () all on the same line.

> +		return 0;
> +
> +	__raw_writel(value, EXYNOS5410_COMMON_CONFIGURATION(cluster));
> +	do {
> +		if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) &
> +			__raw_readl(EXYNOS5410_L2_STATUS(cluster)) & 0x3)
> +				== value)
> +			return 0;
> +	} while (time_before(jiffies, timeout));
> +
> +	return -EDEADLK;
> +}
> +
> +static int exynos_core_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +	return exynos_core_power_control(cpu, cluster, 1);
> +}
> +
> +static int exynos_core_power_down(unsigned int cpu, unsigned int cluster)
> +{
> +	return exynos_core_power_control(cpu, cluster, 0);
> +}
> +
> +static int exynos_cluster_power_up(unsigned int cluster)
> +{
> +	return exynos_cluster_power_control(cluster, 1);
> +}
> +
> +static int exynos_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +	int ret;
> +	local_irq_disable();
> +	arch_spin_lock(&exynos_lock);
> +
> +	pr_info("A%d CORE%d\n", 15 - cluster * 8, cpu);

You should consider tuning this down to pr_debug.

> +	kfs_use_count[cpu][cluster]++;
> +	if (kfs_use_count[cpu][cluster] == 1) {
> +		++core_count[cluster];
> +		if (core_count[cluster] == 1) {
> +			ret = exynos_cluster_power_up(cluster);
> +			if (ret) {
> +				pr_err("%s: cluster %u power up error\n",
> +					__func__, cluster);
> +				return ret;
> +			}
> +			__cci_control_port_by_index(MAX_NR_CLUSTERS
> +					 - cluster, true);

This is wrong and very racy.  The state machine implemented in 
mcpm-head.S is there already to handle proper synchronization for you.  

You need to provide a tiny bit of code to mcpm_sync_init() in order to 
set things up properly.

Please have a look at arch/arm/mach-vexpress/tc2_pm.c in latest mainline 
kernel tree, in particular tc2_pm_power_up_setup().

> +		}
> +		ret = exynos_core_power_up(cpu, cluster);
> +		if (ret) {
> +			pr_err("%s: cpu %u cluster %u power up error\n",
> +				__func__, cpu, cluster);
> +			return ret;
> +		}
> +	} else if (kfs_use_count[cpu][cluster] != 2) {
> +		/*
> +		 * The only possible values are:
> +		 * 0 = CPU down
> +		 * 1 = CPU (still) up
> +		 * 2 = CPU requested to be up before it had a chance
> +		 *     to actually make itself down.
> +		 * Any other value is a bug.
> +		 */
> +		BUG();
> +	}
> +	arch_spin_unlock(&exynos_lock);
> +	local_irq_enable();
> +	return 0;
> +}
> +static void exynos_power_down(void)
> +{
> +	unsigned int mpidr, cpu, cluster;
> +	bool last_man = false, skip_wfi = false;
> +	mpidr = read_cpuid_mpidr();
> +	cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +	cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +	pr_info("A%d CORE%d\n", 15 - cluster * 8, cpu);
> +	BUG_ON(cpu >= MAX_CPUS_PER_CLUSTER  || cluster >= MAX_NR_CLUSTERS);
> +	__mcpm_cpu_going_down(cpu, cluster);
> +	arch_spin_lock(&exynos_lock);
> +	BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
> +	kfs_use_count[cpu][cluster]--;
> +
> +	if (kfs_use_count[cpu][cluster] == 0) {
> +		exynos_core_power_down(cpu, cluster);
> +		--core_count[cluster];
> +		if (core_count[cluster] == 0)
> +			last_man = true;
> +
> +	} else if (kfs_use_count[cpu][cluster] == 1) {
> +		skip_wfi = true;
> +	} else
> +		BUG();
> +	if (last_man && __mcpm_outbound_enter_critical(cpu, cluster)) {
> +		arch_spin_unlock(&exynos_lock);
> +		flush_cache_all();
> +		set_cr(get_cr() & ~CR_C);
> +		flush_cache_all();
> +		outer_flush_all();
> +		set_auxcr(get_auxcr() & ~(1 << 6));
> +		cci_disable_port_by_cpu(mpidr);
> +		__mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);
> +	} else {
> +		arch_spin_unlock(&exynos_lock);
> +		flush_cache_louis();
> +		set_cr(get_cr() & ~CR_C);
> +		flush_cache_louis();
> +		set_auxcr(get_auxcr() & ~(1 << 6));

This cache handling sequence is also buggy.  Please have a look at 
arch/arm/mach-vexpress/tc2_pm.c for an example of how it should be done.

This code sequence should also be abstracted into a common 
implementation eventually, but in the mean time the TC2 implementation 
is a good model to follow.

> + } + __mcpm_cpu_down(cpu, cluster); + + dsb(); + if (!skip_wfi) + 
> wfi(); +} +static const struct mcpm_platform_ops exynos_power_ops = { 
> + .power_up = exynos_power_up, + .power_down = exynos_power_down, +}; 
> + +static void __init edcs_data_init(void) +{ + unsigned int mpidr, 
> cpu, cluster; + + mpidr = read_cpuid_mpidr(); + cpu = 
> MPIDR_AFFINITY_LEVEL(mpidr, 0); + cluster = 
> MPIDR_AFFINITY_LEVEL(mpidr, 1); + pr_debug("%s: cpu %u cluster %u\n", 
> __func__, cpu, cluster); + BUG_ON(cpu >= 4 || cluster >= 2); + 
> kfs_use_count[cpu][cluster] = 1; + ++core_count[cluster]; + 
> __cci_control_port_by_index(MAX_NR_CLUSTERS - cluster, true); +} + + 
> +static int __init edcs_init(void) +{ + struct device_node *node; + + 
> if (!cci_probed()) + return -ENODEV; + + node = 
> of_find_compatible_node(NULL, NULL, "samsung,edcs"); + if (!node) + 
> return -ENODEV; + edcs_data_init(); + mcpm_smp_set_ops(); + 
> mcpm_platform_register(&exynos_power_ops);

You are lacking a call to mcpm_sync_init().

> +
> +	/*
> +	 * Future entries into the kernel can now go
> +	 * through the cluster entry vectors.
> +	 */
> +
> +	__raw_writel(virt_to_phys(mcpm_entry_point),
> +			S5P_VA_SYSRAM_NS + 0x1c);
> +
> +	pr_info("EDCS: EXYNOS5410 DUAL CLUSTER SUPPORT installed\n");
> +
> +	return 0;
> +}
> +
> +early_initcall(edcs_init);
> diff --git a/arch/arm/mach-exynos/edcs.h b/arch/arm/mach-exynos/edcs.h
> new file mode 100644
> index 0000000..dd3e204
> --- /dev/null
> +++ b/arch/arm/mach-exynos/edcs.h
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
> + */
> +
> +#include <linux/arm-cci.h>
> +#include <linux/of_address.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <mach/regs-pmu.h>
> +
> +#define EXYNOS5410_ARM_COMMON_CONFIGURATION     S5P_PMUREG(0x2500)
> +#define EXYNOS5410_COMMON_CONFIGURATION(_nr)        \
> +	(EXYNOS5410_ARM_COMMON_CONFIGURATION + ((_nr) * 0x80))
> +#define EXYNOS5410_COMMON_STATUS(_nr)               \
> +	(EXYNOS5410_COMMON_CONFIGURATION(_nr) + 0x4)
> +#define EXYNOS5410_COMMON_OPTION(_nr)               \
> +	(EXYNOS5410_COMMON_CONFIGURATION(_nr) + 0x8)
> +
> +#define EXYNOS5410_ARM_L2_CONFIGURATION         S5P_PMUREG(0x2600)
> +#define EXYNOS5410_L2_CONFIGURATION(_nr)    \
> +	(EXYNOS5410_ARM_L2_CONFIGURATION + ((_nr) * 0x80))
> +#define EXYNOS5410_L2_STATUS(_nr)           \
> +	(EXYNOS5410_L2_CONFIGURATION(_nr) + 0x4)
> +#define EXYNOS5410_L2_OPTION(_nr)           \
> +	(EXYNOS5410_L2_CONFIGURATION(_nr) + 0x8)
> +
> +#define EXYNOS5410_CORE_CONFIGURATION(_nr)        \
> +	(S5P_ARM_CORE0_CONFIGURATION + ((_nr) * 0x80))
> +#define EXYNOS5410_CORE_STATUS(_nr)               \
> +	(EXYNOS5410_CORE_CONFIGURATION(_nr) + 0x4)
> +#define EXYNOS5410_CORE_OPTION(_nr)               \
> +	(EXYNOS5410_CORE_CONFIGURATION(_nr) + 0x8)
> +
> +#define EXYNOS5_PA_CCI                  0x10D20000
> diff --git a/arch/arm/mach-exynos/edcs_status.c b/arch/arm/mach-exynos/edcs_status.c
> new file mode 100644
> index 0000000..bff1e7f
> --- /dev/null
> +++ b/arch/arm/mach-exynos/edcs_status.c
> @@ -0,0 +1,110 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
> + */
> +
> +#include <linux/fs.h>
> +#include <linux/miscdevice.h>
> +#include <asm/uaccess.h>
> +
> +#include "edcs.h"
> +
> +int edcs_check_status(char *info)
[...]

What is the purpose of this driver?  What information does it provide?

In any case, creating a misc device for this is most likely not the best 
way to export that kind of information.


> +{
> +	size_t it = 30;
> +	int i;
> +
> +	void __iomem *cci_base;
> +
> +	cci_base = ioremap(EXYNOS5_PA_CCI, SZ_64K);
> +	if (!cci_base)
> +		return -ENOMEM;
> +
> +	if ((__raw_readl(EXYNOS5410_COMMON_STATUS(0)) & 0x3) == 3)
> +		info[it] = '1';
> +	it += 3;
> +
> +	for (i = 0; i < 4; i++) {
> +		if ((__raw_readl(EXYNOS5410_CORE_STATUS(i)) & 0x3) == 3)
> +			info[it] = '1';
> +		it += 3;
> +	}
> +
> +	if ((__raw_readl(EXYNOS5410_L2_STATUS(0)) & 0x3)	== 3)
> +		info[it] = '1';
> +	it += 3;
> +
> +	if ((readl(cci_base + 0x4000 + 1 * 0x1000) & 0x3) == 3)
> +		info[it] = '1';
> +
> +	it += 10;
> +
> +	if ((__raw_readl(EXYNOS5410_COMMON_STATUS(1)) & 0x3) == 3)
> +		info[it] = '1';
> +	it += 3;
> +
> +	for (i = 4; i < 8; i++) {
> +		if ((__raw_readl(EXYNOS5410_CORE_STATUS(i)) & 0x3) == 3)
> +			info[it] = '1';
> +		it += 3;
> +	}
> +
> +	if ((__raw_readl(EXYNOS5410_L2_STATUS(1)) & 0x3) == 3)
> +		info[it] = '1';
> +	it += 3;
> +
> +	if ((readl(cci_base + 0x4000 + 0 * 0x1000) & 0x3) == 3)
> +		info[it] = '1';
> +
> +	iounmap(cci_base);
> +	return 0;
> +}
> +
> +static ssize_t edcs_status_read(struct file *file, char __user *buf,
> +				size_t len, loff_t *pos)
> +{
> +
> +	char info[] = "\tC  0  1  2  3 L2 CCI\n"
> +		"[A15]   0  0  0  0  0  0  0\n"
> +		" [A7]   0  0  0  0  0  0  0\n";
> +	size_t count = sizeof(info);
> +	edcs_check_status(info);
> +	return simple_read_from_buffer(buf, len, pos, info, count);
> +}
> +
> +static const struct file_operations edcs_status_fops = {
> +	.read		= edcs_status_read,
> +	.owner		= THIS_MODULE,
> +};
> +static struct miscdevice edcs_status_device = {
> +	.name = "edcs_status",
> +	.minor = 255,
> +	.fops = &edcs_status_fops,
> +};
> +
> +static int __init edcs_dev_init(void)
> +{
> +	struct device_node *node;
> +	int ret;
> +	if (!cci_probed())
> +		return -ENODEV;
> +
> +	node = of_find_compatible_node(NULL, NULL, "samsung,edcs");
> +	if (!node)
> +		return -ENODEV;
> +
> +	ret = misc_register(&edcs_status_device);
> +	if (ret) {
> +		pr_err("EDCS: edcs_status device is not registered\n");
> +		return ret;
> +	}
> +	pr_info("EDCS: edcs_status device registered\n");
> +	return 0;
> +}
> +device_initcall(edcs_dev_init);
> -- 
> 1.8.1.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Dave Martin]

On Tue, Oct 01, 2013 at 03:55:24PM -0400, Nicolas Pitre wrote:
> On Tue, 1 Oct 2013, Vyacheslav Tyrtov wrote:
> 
> > From: Tarek Dakhran <t.dakhran@samsung.com>

[...]

> > +	kfs_use_count[cpu][cluster]++;
> > +	if (kfs_use_count[cpu][cluster] == 1) {
> > +		++core_count[cluster];
> > +		if (core_count[cluster] == 1) {
> > +			ret = exynos_cluster_power_up(cluster);
> > +			if (ret) {
> > +				pr_err("%s: cluster %u power up error\n",
> > +					__func__, cluster);
> > +				return ret;
> > +			}
> > +			__cci_control_port_by_index(MAX_NR_CLUSTERS
> > +					 - cluster, true);
> 
> This is wrong and very racy.  The state machine implemented in 
> mcpm-head.S is there already to handle proper synchronization for you.  

Maybe this issue didn't make itself obvious yet due to the lack of
suspend support.

Moving the CCI maintenance to power_up_setup() is essential for suspend/
resume to work, because then CPUs can power up randomly in response to
interrupts -- exynos_lock is not sufficient protection in that case.

The TC2 code should provide a good example of what to do.

[...]

Cheers
---Dave

Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Dave Martin]

On Tue, Oct 01, 2013 at 08:17:04PM +0400, Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran@samsung.com>
>
> Add EDCS(Exynos Dual Cluster Support) for Samsung Exynos5410 SoC.
> This enables all 8 cores, 4 x A7 and 4 x A15 run at the same time.
>
> Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
> ---
>  arch/arm/mach-exynos/Makefile      |   2 +
>  arch/arm/mach-exynos/edcs.c        | 217 +++++++++++++++++++++++++++++++++++++
>  arch/arm/mach-exynos/edcs.h        |  41 +++++++
>  arch/arm/mach-exynos/edcs_status.c | 110 +++++++++++++++++++
>  4 files changed, 370 insertions(+)
>  create mode 100644 arch/arm/mach-exynos/edcs.c
>  create mode 100644 arch/arm/mach-exynos/edcs.h
>  create mode 100644 arch/arm/mach-exynos/edcs_status.c
>
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 5369615..18e6162 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -34,3 +34,5 @@ AFLAGS_exynos-smc.o         :=-Wa,-march=armv7-a$(plus_sec)
>
>  obj-$(CONFIG_MACH_EXYNOS4_DT)                += mach-exynos4-dt.o
>  obj-$(CONFIG_MACH_EXYNOS5_DT)                += mach-exynos5-dt.o
> +
> +obj-$(CONFIG_ARM_CCI)                += edcs.o       edcs_status.o
> diff --git a/arch/arm/mach-exynos/edcs.c b/arch/arm/mach-exynos/edcs.c
> new file mode 100644
> index 0000000..34b4e4b
> --- /dev/null
> +++ b/arch/arm/mach-exynos/edcs.c
> @@ -0,0 +1,217 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran <t.dakhran@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/spinlock.h>
> +#include <linux/errno.h>
> +#include <linux/vexpress.h>
> +#include <asm/mcpm.h>
> +#include <asm/proc-fns.h>
> +#include <asm/cacheflush.h>
> +#include <asm/cputype.h>
> +#include <asm/cp15.h>
> +
> +#include "edcs.h"
> +
> +static arch_spinlock_t exynos_lock = __ARCH_SPIN_LOCK_UNLOCKED;
> +static int kfs_use_count[MAX_CPUS_PER_CLUSTER][MAX_NR_CLUSTERS];
> +static int core_count[MAX_NR_CLUSTERS];
> +
> +static int exynos_core_power_control(unsigned int cpu, unsigned int cluster,
> +                             int enable)
> +{
> +     unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +     unsigned int offset = cluster * MAX_CPUS_PER_CLUSTER + cpu;
> +     int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
> +
> +     if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3) == value)
> +             return 0;
> +
> +     __raw_writel(value, EXYNOS5410_CORE_CONFIGURATION(offset));
> +     do {
> +             if ((__raw_readl(EXYNOS5410_CORE_STATUS(offset)) & 0x3)
> +                             == value)
> +                     return 0;
> +     } while (time_before(jiffies, timeout));
> +
> +     return -EDEADLK;

Are you sure you need to wait here?  In MCPM, the power_up() and
power_down() methods are expected to trigger the requested operation,
but they aren't required to wait for it to complete.

Waiting may just burn energy and add pointless latency -- particularly
bad for power_up() with IKS, because the calling CPU could go off and do
something useful in the meantime.


For CPU hotplug and kexec, it is neecssary to wait for a CPU really to
be halted or powered down, so a new power_down_wait() method will need
to be implemented.

See this series (which includes an implementation example for TC2):

http://lists.infradead.org/pipermail/linux-arm-kernel/2013-October/201618.html

If Russell accepts the patches, they will likely be in 3.13.


Your wait loop may be relevant to this, but for power_down_finish() the
loop should be more relaxed to avoid thrashing the bus and power
controller unnecessarily.

Without a power_down_wait() method, the kernel will build OK, but will
BUG() if hotplug or kexec is attempted.


> +}
> +static int exynos_cluster_power_control(unsigned int cluster, int enable)
> +{
> +     unsigned long timeout = jiffies + msecs_to_jiffies(10);
> +     int value = enable ? S5P_CORE_LOCAL_PWR_EN : 0;
> +
> +     if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) & 0x3)
> +                     == value)
> +             return 0;
> +
> +     __raw_writel(value, EXYNOS5410_COMMON_CONFIGURATION(cluster));
> +     do {
> +             if ((__raw_readl(EXYNOS5410_COMMON_STATUS(cluster)) &
> +                     __raw_readl(EXYNOS5410_L2_STATUS(cluster)) & 0x3)
> +                             == value)
> +                     return 0;
> +     } while (time_before(jiffies, timeout));
> +
> +     return -EDEADLK;
> +}

The same comment about waiting applies here, unless we must really wait
for this to finish before exynos_core_power_up() will work.  This might
be the case if the EXYNOS5410_CORE_CONFIGURATION() registers are implemented
in the cluster-local power domains, but it doesn't look that way.

For example, can you set the target core's PWR_EN bit first and then set
the PWR_UP bit for the cluster, so that the target CPU is brought up as
soon as the cluster comes online?  The correct way to do this, and whether
it's possible at all, depend on the hardware...

> +
> +static int exynos_core_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +     return exynos_core_power_control(cpu, cluster, 1);
> +}
> +
> +static int exynos_core_power_down(unsigned int cpu, unsigned int cluster)
> +{
> +     return exynos_core_power_control(cpu, cluster, 0);
> +}
> +
> +static int exynos_cluster_power_up(unsigned int cluster)
> +{
> +     return exynos_cluster_power_control(cluster, 1);
> +}

No exynos_cluster_power_down()?

> +
> +static int exynos_power_up(unsigned int cpu, unsigned int cluster)
> +{
> +     int ret;
> +     local_irq_disable();

Should there be a local_fiq_disable() here also?

We actually don't have this for TC2 either.  Linux on TC2 doesn't make
use of FIQs, but it would be better to have it for completeness, unless
FIQ is already masked for some reason.

Nico might have a view on this -- if local_fiq_disable() is needed here,
we should add it in the TC2 code too.

> +     arch_spin_lock(&exynos_lock);
> +
> +     pr_info("A%d CORE%d\n", 15 - cluster * 8, cpu);

Minor nits: in addition to Nico's comment, please add some context (like
"%s:", ... __func__).

The rest of the MCPM stack uses the "cpu, cluster" convention in most
places for its messages, so it's best to follow that convention for
consistency.

Some boards allow the cluster numbers to be swapped via DIP switches or
other config.  I don't know whether this applies to EXYNOS5410, but
unnecessary assumptions should be avoided.  Nothing else in this file
cares which cluster is A15 and which is A7, if I understand correctly.

> +     kfs_use_count[cpu][cluster]++;
> +     if (kfs_use_count[cpu][cluster] == 1) {
> +             ++core_count[cluster];
> +             if (core_count[cluster] == 1) {
> +                     ret = exynos_cluster_power_up(cluster);
> +                     if (ret) {
> +                             pr_err("%s: cluster %u power up error\n",
> +                                     __func__, cluster);
> +                             return ret;
> +                     }
> +                     __cci_control_port_by_index(MAX_NR_CLUSTERS
> +                                      - cluster, true);
> +             }
> +             ret = exynos_core_power_up(cpu, cluster);
> +             if (ret) {
> +                     pr_err("%s: cpu %u cluster %u power up error\n",
> +                             __func__, cpu, cluster);
> +                     return ret;

All these "return ret" in the middle of the function will leave local
IRQs masked, and exynos_lock will remain locked, leading to future
deadlocks.

> +             }
> +     } else if (kfs_use_count[cpu][cluster] != 2) {
> +             /*
> +              * The only possible values are:
> +              * 0 = CPU down
> +              * 1 = CPU (still) up
> +              * 2 = CPU requested to be up before it had a chance
> +              *     to actually make itself down.
> +              * Any other value is a bug.
> +              */
> +             BUG();
> +     }

If this function might return prematurely on error, then I suggest you
put a label here, and goto it on errors, with "return ret" in place of
"return 0".  ret should be initialised to zero if you do that.

> +     arch_spin_unlock(&exynos_lock);
> +     local_irq_enable();
> +     return 0;
> +}
> +static void exynos_power_down(void)
> +{
> +     unsigned int mpidr, cpu, cluster;
> +     bool last_man = false, skip_wfi = false;
> +     mpidr = read_cpuid_mpidr();
> +     cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +     cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +
> +     pr_info("A%d CORE%d\n", 15 - cluster * 8, cpu);
> +     BUG_ON(cpu >= MAX_CPUS_PER_CLUSTER  || cluster >= MAX_NR_CLUSTERS);
> +     __mcpm_cpu_going_down(cpu, cluster);
> +     arch_spin_lock(&exynos_lock);
> +     BUG_ON(__mcpm_cluster_state(cluster) != CLUSTER_UP);
> +     kfs_use_count[cpu][cluster]--;
> +
> +     if (kfs_use_count[cpu][cluster] == 0) {
> +             exynos_core_power_down(cpu, cluster);
> +             --core_count[cluster];
> +             if (core_count[cluster] == 0)
> +                     last_man = true;
> +
> +     } else if (kfs_use_count[cpu][cluster] == 1) {
> +             skip_wfi = true;
> +     } else
> +             BUG();
> +     if (last_man && __mcpm_outbound_enter_critical(cpu, cluster)) {
> +             arch_spin_unlock(&exynos_lock);
> +             flush_cache_all();
> +             set_cr(get_cr() & ~CR_C);
> +             flush_cache_all();
> +             outer_flush_all();
> +             set_auxcr(get_auxcr() & ~(1 << 6));
> +             cci_disable_port_by_cpu(mpidr);
> +             __mcpm_outbound_leave_critical(cluster, CLUSTER_DOWN);

How does the cluster get powered down?  Does the power controller always
do that automatically when there are no powered-up CPUs left in a
cluster?

> +     } else {
> +             arch_spin_unlock(&exynos_lock);
> +             flush_cache_louis();
> +             set_cr(get_cr() & ~CR_C);
> +             flush_cache_louis();
> +             set_auxcr(get_auxcr() & ~(1 << 6));
> +     }
> +     __mcpm_cpu_down(cpu, cluster);
> +
> +     dsb();

You don't need this dsb, because __mcpm_cpu_down() contains one already.
This is not just coincidence, because __mcpm_cpu_down() is a signalling
operation, which kicks CPUs waiting in __mcpm_outbound_enter_critical()
or in mcpm_head.S.

> +     if (!skip_wfi)
> +             wfi();
> +}
> +static const struct mcpm_platform_ops exynos_power_ops = {
> +     .power_up       = exynos_power_up,
> +     .power_down     = exynos_power_down,

What about .suspend ?

> +};
> +
> +static void __init edcs_data_init(void)
> +{
> +     unsigned int mpidr, cpu, cluster;
> +
> +     mpidr = read_cpuid_mpidr();
> +     cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +     cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1);
> +     pr_debug("%s: cpu %u cluster %u\n", __func__, cpu, cluster);
> +     BUG_ON(cpu >= 4 || cluster >= 2);
> +     kfs_use_count[cpu][cluster] = 1;
> +     ++core_count[cluster];
> +     __cci_control_port_by_index(MAX_NR_CLUSTERS - cluster, true);
> +}
> +
> +
> +static int __init edcs_init(void)
> +{
> +     struct device_node *node;
> +
> +     if (!cci_probed())
> +             return -ENODEV;
> +
> +     node = of_find_compatible_node(NULL, NULL, "samsung,edcs");
> +     if (!node)
> +             return -ENODEV;
> +     edcs_data_init();
> +     mcpm_smp_set_ops();
> +     mcpm_platform_register(&exynos_power_ops);
> +
> +     /*
> +      * Future entries into the kernel can now go
> +      * through the cluster entry vectors.
> +      */
> +
> +     __raw_writel(virt_to_phys(mcpm_entry_point),
> +                     S5P_VA_SYSRAM_NS + 0x1c);
> +
> +     pr_info("EDCS: EXYNOS5410 DUAL CLUSTER SUPPORT installed\n");
> +
> +     return 0;
> +}
> +
> +early_initcall(edcs_init);
> diff --git a/arch/arm/mach-exynos/edcs.h b/arch/arm/mach-exynos/edcs.h
> new file mode 100644
> index 0000000..dd3e204
> --- /dev/null
> +++ b/arch/arm/mach-exynos/edcs.h
> @@ -0,0 +1,41 @@
> +/*
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + * Author: Tarek Dakhran <t.dakhran@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * EDCS(EXYNOS dual cluster support) for Exynos5410 SoC.
> + */
> +
> +#include <linux/arm-cci.h>
> +#include <linux/of_address.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <mach/regs-pmu.h>
> +
> +#define EXYNOS5410_ARM_COMMON_CONFIGURATION     S5P_PMUREG(0x2500)
> +#define EXYNOS5410_COMMON_CONFIGURATION(_nr)        \
> +     (EXYNOS5410_ARM_COMMON_CONFIGURATION + ((_nr) * 0x80))
> +#define EXYNOS5410_COMMON_STATUS(_nr)               \
> +     (EXYNOS5410_COMMON_CONFIGURATION(_nr) + 0x4)
> +#define EXYNOS5410_COMMON_OPTION(_nr)               \
> +     (EXYNOS5410_COMMON_CONFIGURATION(_nr) + 0x8)
> +
> +#define EXYNOS5410_ARM_L2_CONFIGURATION         S5P_PMUREG(0x2600)
> +#define EXYNOS5410_L2_CONFIGURATION(_nr)    \
> +     (EXYNOS5410_ARM_L2_CONFIGURATION + ((_nr) * 0x80))
> +#define EXYNOS5410_L2_STATUS(_nr)           \
> +     (EXYNOS5410_L2_CONFIGURATION(_nr) + 0x4)
> +#define EXYNOS5410_L2_OPTION(_nr)           \
> +     (EXYNOS5410_L2_CONFIGURATION(_nr) + 0x8)
> +
> +#define EXYNOS5410_CORE_CONFIGURATION(_nr)        \
> +     (S5P_ARM_CORE0_CONFIGURATION + ((_nr) * 0x80))
> +#define EXYNOS5410_CORE_STATUS(_nr)               \
> +     (EXYNOS5410_CORE_CONFIGURATION(_nr) + 0x4)
> +#define EXYNOS5410_CORE_OPTION(_nr)               \
> +     (EXYNOS5410_CORE_CONFIGURATION(_nr) + 0x8)
> +
> +#define EXYNOS5_PA_CCI                  0x10D20000

This physical address should be discovered from the DT by the arm-cci
driver.  If the arm-cci driver and arm,cci-400 binding are insufficient
for your needs, that needs discussion.

If querying the CCI status is necessary, interfaces should be added to
the arm-cci driver to allow that instead of poking it directly.


[...]

Cheers
---Dave

Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Nicolas Pitre]

On Wed, 2 Oct 2013, Dave Martin wrote:

> On Tue, Oct 01, 2013 at 08:17:04PM +0400, Vyacheslav Tyrtov wrote:
> > +static int exynos_power_up(unsigned int cpu, unsigned int cluster)
> > +{
> > +     int ret;
> > +     local_irq_disable();
> 
> Should there be a local_fiq_disable() here also?

No.  In fact this is paired with

> > +     arch_spin_lock(&exynos_lock);

to create the equivalent of a arch_spin_lock_irq().  And the reason is:

/*
 * We can't use regular spinlocks. In the switcher case, it is possible
 * for an outbound CPU to call power_down() after its inbound counterpart
 * is already live using the same logical CPU number which trips lockdep
 * debugging.
 */

Otherwise we simply would have used spin_lock_irq().

No FIQs are supposed to ever race with this code.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] ARM: dts: Add initial device tree support for EXYNOS5410

[Vyacheslav Tyrtov]

From: Tarek Dakhran <t.dakhran@samsung.com>

Add initial device tree nodes for EXYNOS5410 SoC and SMDK5410 board.

Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
---
 arch/arm/boot/dts/Makefile                |   1 +
 arch/arm/boot/dts/exynos5410-smdk5410.dts |  67 +++++++++++
 arch/arm/boot/dts/exynos5410.dtsi         | 189 ++++++++++++++++++++++++++++++
 3 files changed, 257 insertions(+)
 create mode 100644 arch/arm/boot/dts/exynos5410-smdk5410.dts
 create mode 100644 arch/arm/boot/dts/exynos5410.dtsi

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e95af3f..374d446 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -61,6 +61,7 @@ dtb-$(CONFIG_ARCH_EXYNOS) += exynos4210-origen.dtb \
 	exynos5250-arndale.dtb \
 	exynos5250-smdk5250.dtb \
 	exynos5250-snow.dtb \
+	exynos5410-smdk5410.dtb \
 	exynos5420-smdk5420.dtb \
 	exynos5440-sd5v1.dtb \
 	exynos5440-ssdk5440.dtb
diff --git a/arch/arm/boot/dts/exynos5410-smdk5410.dts b/arch/arm/boot/dts/exynos5410-smdk5410.dts
new file mode 100644
index 0000000..7abb2aa
--- /dev/null
+++ b/arch/arm/boot/dts/exynos5410-smdk5410.dts
@@ -0,0 +1,67 @@
+/*
+ * SAMSUNG SMDK5410 board device tree source
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+/dts-v1/;
+#include "exynos5410.dtsi"
+/ {
+	model = "Samsung SMDK5410 board based on EXYNOS5410";
+	compatible = "samsung,smdk5410", "samsung,exynos5410";
+
+	memory {
+		reg = <0x40000000 0x80000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttySAC2,115200";
+	};
+
+	fixed-rate-clocks {
+		oscclk {
+			compatible = "samsung,exynos5410-oscclk";
+			clock-frequency = <24000000>;
+		};
+	};
+
+	dwmmc0@12200000 {
+		num-slots = <1>;
+		supports-highspeed;
+		broken-cd;
+		fifo-depth = <0x80>;
+		card-detect-delay = <200>;
+		samsung,dw-mshc-ciu-div = <3>;
+		samsung,dw-mshc-sdr-timing = <2 3>;
+		samsung,dw-mshc-ddr-timing = <1 2>;
+		slot@0 {
+			reg = <0>;
+			bus-width = <8>;
+		};
+	};
+
+	dwmmc1@12210000 {
+		status = "disabled";
+	};
+
+	dwmmc2@12220000 {
+		num-slots = <1>;
+		supports-highspeed;
+		fifo-depth = <0x80>;
+		card-detect-delay = <200>;
+		samsung,dw-mshc-ciu-div = <3>;
+		samsung,dw-mshc-sdr-timing = <2 3>;
+		samsung,dw-mshc-ddr-timing = <1 2>;
+		slot@0 {
+			reg = <0>;
+			bus-width = <4>;
+			disable-wp;
+		};
+	};
+
+};
diff --git a/arch/arm/boot/dts/exynos5410.dtsi b/arch/arm/boot/dts/exynos5410.dtsi
new file mode 100644
index 0000000..c0ea166
--- /dev/null
+++ b/arch/arm/boot/dts/exynos5410.dtsi
@@ -0,0 +1,189 @@
+/*
+ * SAMSUNG EXYNOS5410 SoC device tree source
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com
+ *
+ * SAMSUNG EXYNOS5410 SoC device nodes are listed in this file.
+ * EXYNOS5410 based board files can include this file and provide
+ * values for board specfic bindings.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "exynos5.dtsi"
+/ {
+	compatible = "samsung,exynos5410";
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		CPU0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0>;
+			cci-control-port = <&cci_control2>;
+			clock-frequency = <1600000000>;
+		};
+		CPU1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <1>;
+			cci-control-port = <&cci_control2>;
+			clock-frequency = <1600000000>;
+		};
+		CPU2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <2>;
+			cci-control-port = <&cci_control2>;
+			clock-frequency = <1600000000>;
+		};
+		CPU3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <3>;
+			cci-control-port = <&cci_control2>;
+			clock-frequency = <1600000000>;
+		};
+		CPU4: cpu@4 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a7";
+			reg = <0x100>;
+			cci-control-port = <&cci_control1>;
+			clock-frequency = <1200000000>;
+		};
+		CPU5: cpu@5 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a7";
+			reg = <0x101>;
+			cci-control-port = <&cci_control1>;
+			clock-frequency = <1200000000>;
+		};
+		CPU6: cpu@6 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a7";
+			reg = <0x102>;
+			cci-control-port = <&cci_control1>;
+			clock-frequency = <1200000000>;
+		};
+		CPU7: cpu@7 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a7";
+			reg = <0x103>;
+			cci-control-port = <&cci_control1>;
+			clock-frequency = <1200000000>;
+		};
+
+	};
+
+	edcs{
+		compatible = "samsung,edcs";
+	};
+
+	cci@10d20000 {
+		compatible = "arm,cci-400";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		reg = <0x10d20000 0x1000>;
+		ranges = <0 0x10d20000 0x6000>;
+
+		cci_control0: slave-if@1000 {
+			compatible = "arm,cci-400-ctrl-if";
+			interface-type = "ace-lite";
+			reg = <0x1000 0x1000>;
+		};
+
+		cci_control1: slave-if@4000 {
+			compatible = "arm,cci-400-ctrl-if";
+			interface-type = "ace";
+			reg = <0x4000 0x1000>;
+		};
+
+		cci_control2: slave-if@5000 {
+			compatible = "arm,cci-400-ctrl-if";
+			interface-type = "ace";
+			reg = <0x5000 0x1000>;
+		};
+	};
+
+	clock: clock-controller@10010000 {
+		compatible = "samsung,exynos5410-clock";
+		reg = <0x10010000 0x30000>;
+		#clock-cells = <1>;
+	};
+
+
+	mct@101C0000 {
+		compatible = "samsung,exynos4210-mct";
+		reg = <0x101C0000 0xb00>;
+		interrupt-controller;
+		#interrups-cells = <1>;
+		interrupt-parent = <&mct_map>;
+		interrupts = <0>, <1>, <2>, <3>,
+			<4>, <5>, <6>, <7>,
+			<8>, <9>, <10>, <11>;
+		clocks = <&clock 1>, <&clock 315>;
+		clock-names = "fin_pll", "mct";
+
+		mct_map: mct-map {
+			#interrupt-cells = <1>;
+			#address-cells = <0>;
+			#size-cells = <0>;
+			interrupt-map = <0 &combiner 23 3>,
+					<1 &combiner 23 4>,
+					<2 &combiner 25 2>,
+					<3 &combiner 25 3>,
+					<4 &gic 0 120 0>,
+					<5 &gic 0 121 0>,
+					<6 &gic 0 122 0>,
+					<7 &gic 0 123 0>,
+					<8 &gic 0 128 0>,
+					<9 &gic 0 129 0>,
+					<10 &gic 0 130 0>,
+					<11 &gic 0 131 0>;
+		};
+	};
+
+	dwmmc_0: dwmmc0@12200000 {
+		reg = <0x12200000 0x1000>;
+		clocks = <&clock 351>, <&clock 132>;
+		clock-names = "biu", "ciu";
+	};
+
+	dwmmc_1: dwmmc1@12210000 {
+		reg = <0x12210000 0x1000>;
+		clocks = <&clock 352>, <&clock 133>;
+		clock-names = "biu", "ciu";
+	};
+
+	dwmmc_2: dwmmc2@12220000 {
+		reg = <0x12220000 0x1000>;
+		clocks = <&clock 353>, <&clock 134>;
+		clock-names = "biu", "ciu";
+	};
+
+	serial@12C00000 {
+		clocks = <&clock 257>, <&clock 128>;
+		clock-names = "uart", "clk_uart_baud0";
+	};
+
+	serial@12C10000 {
+		clocks = <&clock 258>, <&clock 129>;
+		clock-names = "uart", "clk_uart_baud0";
+	};
+
+	serial@12C20000 {
+		clocks = <&clock 259>, <&clock 130>;
+		clock-names = "uart", "clk_uart_baud0";
+	};
+
+	serial@12C30000 {
+		clocks = <&clock 260>, <&clock 131>;
+		clock-names = "uart", "clk_uart_baud0";
+	};
+
+};
-- 
1.8.1.5

Re: [PATCH 4/6] ARM: dts: Add initial device tree support for EXYNOS5410

[Tomasz Figa]

Hi Vyacheslav, Tarek,

On Tuesday 01 of October 2013 20:17:05 Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Add initial device tree nodes for EXYNOS5410 SoC and SMDK5410 board.
[snip]
> +/dts-v1/;
> +#include "exynos5410.dtsi"
> +/ {
> +	model = "Samsung SMDK5410 board based on EXYNOS5410";
> +	compatible = "samsung,smdk5410", "samsung,exynos5410";
> +
> +	memory {
> +		reg = <0x40000000 0x80000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttySAC2,115200";
> +	};
> +
> +	fixed-rate-clocks {
> +		oscclk {
> +			compatible = "samsung,exynos5410-oscclk";
> +			clock-frequency = <24000000>;
> +		};
> +	};

As I mentioned in my reply to the patch adding the clock driver, please 
consider using generic fixed rate clock bindings instead of introducing 
new platform specific ones. I know that previous Exynos SoCs do it this 
way, but this is no good.

> +
> +	dwmmc0@12200000 {

I believe there has been a patch series renaming these dwmmcX nodes to 
follow correct DT conventions and doing some more clean-up. IMHO you 
should consider basing your patch on top of that series.

> +		num-slots = <1>;
> +		supports-highspeed;
> +		broken-cd;
> +		fifo-depth = <0x80>;
> +		card-detect-delay = <200>;
> +		samsung,dw-mshc-ciu-div = <3>;
> +		samsung,dw-mshc-sdr-timing = <2 3>;
> +		samsung,dw-mshc-ddr-timing = <1 2>;

nit: Please separate subnodes from properties with a blank line.

> +		slot@0 {
> +			reg = <0>;
> +			bus-width = <8>;
> +		};
> +	};
> +
> +	dwmmc1@12210000 {
> +		status = "disabled";

Oh, the series I mentioned above also corrected the way the status 
property is used, so board level dtses need to enable nodes which they use 
instead of disabling nodes they can't use, which makes much more sense, 
since you don't need to specify nodes in board level dts just to say it's 
disabled.

> +	};
> +
> +	dwmmc2@12220000 {
> +		num-slots = <1>;
> +		supports-highspeed;
> +		fifo-depth = <0x80>;
> +		card-detect-delay = <200>;
> +		samsung,dw-mshc-ciu-div = <3>;
> +		samsung,dw-mshc-sdr-timing = <2 3>;
> +		samsung,dw-mshc-ddr-timing = <1 2>;

nit: A blank line would be nice here.

> +		slot@0 {
> +			reg = <0>;
> +			bus-width = <4>;
> +			disable-wp;
> +		};
> +	};
> +
> +};
> diff --git a/arch/arm/boot/dts/exynos5410.dtsi
> b/arch/arm/boot/dts/exynos5410.dtsi new file mode 100644
> index 0000000..c0ea166
> --- /dev/null
> +++ b/arch/arm/boot/dts/exynos5410.dtsi
> @@ -0,0 +1,189 @@
> +/*
> + * SAMSUNG EXYNOS5410 SoC device tree source
> + *
> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
> + *		http://www.samsung.com
> + *
> + * SAMSUNG EXYNOS5410 SoC device nodes are listed in this file.
> + * EXYNOS5410 based board files can include this file and provide
> + * values for board specfic bindings.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as +
> * published by the Free Software Foundation.
> + */
> +
> +#include "exynos5.dtsi"
> +/ {
> +	compatible = "samsung,exynos5410";
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		CPU0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a15";
> +			reg = <0>;
> +			cci-control-port = <&cci_control2>;
> +			clock-frequency = <1600000000>;
> +		};

nit: Please keep spacing between nodes.

> +		CPU1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a15";
> +			reg = <1>;
> +			cci-control-port = <&cci_control2>;
> +			clock-frequency = <1600000000>;
> +		};
> +		CPU2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a15";
> +			reg = <2>;
> +			cci-control-port = <&cci_control2>;
> +			clock-frequency = <1600000000>;
> +		};
> +		CPU3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a15";
> +			reg = <3>;
> +			cci-control-port = <&cci_control2>;
> +			clock-frequency = <1600000000>;
> +		};
> +		CPU4: cpu@4 {

The @unit-address suffix of node name must be equal to first address entry 
of reg property.

> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0x100>;
> +			cci-control-port = <&cci_control1>;
> +			clock-frequency = <1200000000>;
> +		};
> +		CPU5: cpu@5 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0x101>;
> +			cci-control-port = <&cci_control1>;
> +			clock-frequency = <1200000000>;
> +		};
> +		CPU6: cpu@6 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0x102>;
> +			cci-control-port = <&cci_control1>;
> +			clock-frequency = <1200000000>;
> +		};
> +		CPU7: cpu@7 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a7";
> +			reg = <0x103>;
> +			cci-control-port = <&cci_control1>;
> +			clock-frequency = <1200000000>;
> +		};
> +
> +	};
> +
> +	edcs{

nit: Missing space before the brace.

> +		compatible = "samsung,edcs";
> +	};

Is this a real device? Are bindings for it documented somewhere?

> +
> +	cci@10d20000 {

nit: Please be consistent with case of hexadecimal characters.

> +		compatible = "arm,cci-400";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		reg = <0x10d20000 0x1000>;

nit: Ditto.

> +		ranges = <0 0x10d20000 0x6000>;

nit: Ditto.

> +
> +		cci_control0: slave-if@1000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace-lite";
> +			reg = <0x1000 0x1000>;
> +		};
> +
> +		cci_control1: slave-if@4000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace";
> +			reg = <0x4000 0x1000>;
> +		};
> +
> +		cci_control2: slave-if@5000 {
> +			compatible = "arm,cci-400-ctrl-if";
> +			interface-type = "ace";
> +			reg = <0x5000 0x1000>;
> +		};
> +	};
> +
> +	clock: clock-controller@10010000 {
> +		compatible = "samsung,exynos5410-clock";
> +		reg = <0x10010000 0x30000>;
> +		#clock-cells = <1>;
> +	};
> +
> +

nit: Unnecessary extra blank line.

> +	mct@101C0000 {
> +		compatible = "samsung,exynos4210-mct";
> +		reg = <0x101C0000 0xb00>;

nit: Inconsistent case of hexadecimal characters.

> +		interrupt-controller;
> +		#interrups-cells = <1>;

I don't think MCT has ever been an interrupt controller.

Best regards,
Tomasz

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] ARM: EXYNOS: Minor fixes to enable EXYNOS5410 support

[Vyacheslav Tyrtov]

From: Tarek Dakhran <t.dakhran@samsung.com>

Configure ARM_NR_BANKS as 16 for EXYNOS SoC.
Enable cci_control_port_by_index for ACE_PORT.
Add additional irqs for Exynos MCT.
Set irq base as 256 for EXYNOS5410 SoC.

Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
---
 arch/arm/Kconfig                  |  2 +-
 drivers/bus/arm-cci.c             |  7 +++++++
 drivers/clocksource/exynos_mct.c  |  8 +++++++-
 drivers/irqchip/exynos-combiner.c | 12 +++++++++++-
 4 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 3f7714d..7f88896 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1080,7 +1080,7 @@ source arch/arm/mm/Kconfig
 
 config ARM_NR_BANKS
 	int
-	default 16 if ARCH_EP93XX
+	default 16 if ARCH_EP93XX || ARCH_EXYNOS
 	default 8
 
 config IWMMXT
diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 2009266..f2f5df1 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -363,8 +363,15 @@ int notrace __cci_control_port_by_index(u32 port, bool enable)
 	 * interface (ie cci_disable_port_by_cpu(); control by general purpose
 	 * indexing is therefore disabled for ACE ports.
 	 */
+
+	/*
+	 * Using this way to enable cci_port on EXYNOS5410 SoC
+	 */
+
+#ifndef CONFIG_SOC_EXYNOS5410
 	if (ports[port].type == ACE_PORT)
 		return -EPERM;
+#endif
 
 	cci_port_control(port, enable);
 	return 0;
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 5b34768..33884d7 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -71,6 +71,12 @@ enum {
 	MCT_L1_IRQ,
 	MCT_L2_IRQ,
 	MCT_L3_IRQ,
+#ifdef CONFIG_ARM_CCI
+	MCT_L4_IRQ,
+	MCT_L5_IRQ,
+	MCT_L6_IRQ,
+	MCT_L7_IRQ,
+#endif
 	MCT_NR_IRQS,
 };
 
@@ -406,7 +412,7 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt)
 	mevt = container_of(evt, struct mct_clock_event_device, evt);
 
 	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
-	sprintf(mevt->name, "mct_tick%d", cpu);
+	snprintf(mevt->name, 10, "mct_tick%d", cpu);
 
 	evt->name = mevt->name;
 	evt->cpumask = cpumask_of(cpu);
diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
index 868ed40..2e056fc 100644
--- a/drivers/irqchip/exynos-combiner.c
+++ b/drivers/irqchip/exynos-combiner.c
@@ -18,6 +18,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <asm/mach/irq.h>
+#include <plat/cpu.h>
 
 #include "irqchip.h"
 
@@ -66,6 +67,11 @@ static void combiner_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
 	struct irq_chip *chip = irq_get_chip(irq);
 	unsigned int cascade_irq, combiner_irq;
 	unsigned long status;
+	if (unlikely(!chip || !chip_data)) {
+		printk_once(KERN_ALERT "%s: Chip not found for IRQ %d\n"
+				, __func__, irq);
+		return;
+	}
 
 	chained_irq_enter(chip, desc);
 
@@ -226,7 +232,11 @@ static int __init combiner_of_init(struct device_node *np,
 	 * get their IRQ from DT, remove this in order to get dynamic
 	 * allocation.
 	 */
-	irq_base = 160;
+
+	if (soc_is_exynos5410())
+		irq_base = 256;
+	else
+		irq_base = 160;
 
 	combiner_init(combiner_base, np, max_nr, irq_base);
 
-- 
1.8.1.5

Re: [PATCH 5/6] ARM: EXYNOS: Minor fixes to enable EXYNOS5410 support

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday, October 01, 2013 08:17:06 PM Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Configure ARM_NR_BANKS as 16 for EXYNOS SoC.
> Enable cci_control_port_by_index for ACE_PORT.
> Add additional irqs for Exynos MCT.
> Set irq base as 256 for EXYNOS5410 SoC.

It would be better if these changes were separate patches as they touch
different code areas.

> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  arch/arm/Kconfig                  |  2 +-
>  drivers/bus/arm-cci.c             |  7 +++++++
>  drivers/clocksource/exynos_mct.c  |  8 +++++++-
>  drivers/irqchip/exynos-combiner.c | 12 +++++++++++-
>  4 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3f7714d..7f88896 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1080,7 +1080,7 @@ source arch/arm/mm/Kconfig
>  
>  config ARM_NR_BANKS
>  	int
> -	default 16 if ARCH_EP93XX
> +	default 16 if ARCH_EP93XX || ARCH_EXYNOS
>  	default 8
>  
>  config IWMMXT
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 2009266..f2f5df1 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -363,8 +363,15 @@ int notrace __cci_control_port_by_index(u32 port, bool enable)
>  	 * interface (ie cci_disable_port_by_cpu(); control by general purpose
>  	 * indexing is therefore disabled for ACE ports.
>  	 */
> +
> +	/*
> +	 * Using this way to enable cci_port on EXYNOS5410 SoC
> +	 */
> +
> +#ifndef CONFIG_SOC_EXYNOS5410
>  	if (ports[port].type == ACE_PORT)
>  		return -EPERM;
> +#endif

This won't work for multiplatform kernels, please detect EXYNOS5410
at runtime using the device tree info.

>  	cci_port_control(port, enable);
>  	return 0;
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 5b34768..33884d7 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -71,6 +71,12 @@ enum {
>  	MCT_L1_IRQ,
>  	MCT_L2_IRQ,
>  	MCT_L3_IRQ,
> +#ifdef CONFIG_ARM_CCI
> +	MCT_L4_IRQ,
> +	MCT_L5_IRQ,
> +	MCT_L6_IRQ,
> +	MCT_L7_IRQ,
> +#endif
>  	MCT_NR_IRQS,
>  };
>  
> @@ -406,7 +412,7 @@ static int exynos4_local_timer_setup(struct clock_event_device *evt)
>  	mevt = container_of(evt, struct mct_clock_event_device, evt);
>  
>  	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
> -	sprintf(mevt->name, "mct_tick%d", cpu);
> +	snprintf(mevt->name, 10, "mct_tick%d", cpu);

What is the rationale behind this change?

Also there is nothing about it in the patch description.

>  	evt->name = mevt->name;
>  	evt->cpumask = cpumask_of(cpu);
> diff --git a/drivers/irqchip/exynos-combiner.c b/drivers/irqchip/exynos-combiner.c
> index 868ed40..2e056fc 100644
> --- a/drivers/irqchip/exynos-combiner.c
> +++ b/drivers/irqchip/exynos-combiner.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <asm/mach/irq.h>
> +#include <plat/cpu.h>
>  
>  #include "irqchip.h"
>  
> @@ -66,6 +67,11 @@ static void combiner_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
>  	struct irq_chip *chip = irq_get_chip(irq);
>  	unsigned int cascade_irq, combiner_irq;
>  	unsigned long status;

Please add a newline here.

> +	if (unlikely(!chip || !chip_data)) {
> +		printk_once(KERN_ALERT "%s: Chip not found for IRQ %d\n"
> +				, __func__, irq);
> +		return;
> +	}

There is nothing about this change in the patch description.

>  	chained_irq_enter(chip, desc);
>  
> @@ -226,7 +232,11 @@ static int __init combiner_of_init(struct device_node *np,
>  	 * get their IRQ from DT, remove this in order to get dynamic
>  	 * allocation.
>  	 */
> -	irq_base = 160;
> +
> +	if (soc_is_exynos5410())
> +		irq_base = 256;
> +	else
> +		irq_base = 160;
>  
>  	combiner_init(combiner_base, np, max_nr, irq_base);

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] ARM: EXYNOS: Minor fixes to enable EXYNOS5410 support

[Tomasz Figa]

Hi Vyacheslav, Tarek,

On Tuesday 01 of October 2013 20:17:06 Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran@samsung.com>
> 
> Configure ARM_NR_BANKS as 16 for EXYNOS SoC.
> Enable cci_control_port_by_index for ACE_PORT.
> Add additional irqs for Exynos MCT.
> Set irq base as 256 for EXYNOS5410 SoC.
> 
> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
> ---
>  arch/arm/Kconfig                  |  2 +-
>  drivers/bus/arm-cci.c             |  7 +++++++
>  drivers/clocksource/exynos_mct.c  |  8 +++++++-
>  drivers/irqchip/exynos-combiner.c | 12 +++++++++++-
>  4 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 3f7714d..7f88896 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1080,7 +1080,7 @@ source arch/arm/mm/Kconfig
> 
>  config ARM_NR_BANKS
>  	int
> -	default 16 if ARCH_EP93XX
> +	default 16 if ARCH_EP93XX || ARCH_EXYNOS

Could you explain why this is needed, please?

>  	default 8
> 
>  config IWMMXT
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 2009266..f2f5df1 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -363,8 +363,15 @@ int notrace __cci_control_port_by_index(u32 port,
> bool enable) * interface (ie cci_disable_port_by_cpu(); control by
> general purpose * indexing is therefore disabled for ACE ports.
>  	 */
> +
> +	/*
> +	 * Using this way to enable cci_port on EXYNOS5410 SoC
> +	 */
> +
> +#ifndef CONFIG_SOC_EXYNOS5410
>  	if (ports[port].type == ACE_PORT)
>  		return -EPERM;
> +#endif

Huh? Could you explain a) why this is needed b) why this can't be detected 
at runtime? Any new code being added must be ready for multiplatform 
builds and this clearly isn't.

I'd recommend extending the CCI binding with a boolean property that makes 
the driver bypass this check, but I'd like to see an answer to question a) 
first.

> 
>  	cci_port_control(port, enable);
>  	return 0;
> diff --git a/drivers/clocksource/exynos_mct.c
> b/drivers/clocksource/exynos_mct.c index 5b34768..33884d7 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -71,6 +71,12 @@ enum {
>  	MCT_L1_IRQ,
>  	MCT_L2_IRQ,
>  	MCT_L3_IRQ,
> +#ifdef CONFIG_ARM_CCI
> +	MCT_L4_IRQ,
> +	MCT_L5_IRQ,
> +	MCT_L6_IRQ,
> +	MCT_L7_IRQ,
> +#endif

This #ifdef is useless.

Basically this whole enum is, as it is a remnant of legacy non-DT support, 
but it is a material for separate patch.

>  	MCT_NR_IRQS,
>  };
> 
> @@ -406,7 +412,7 @@ static int exynos4_local_timer_setup(struct
> clock_event_device *evt) mevt = container_of(evt, struct
> mct_clock_event_device, evt);
> 
>  	mevt->base = EXYNOS4_MCT_L_BASE(cpu);
> -	sprintf(mevt->name, "mct_tick%d", cpu);
> +	snprintf(mevt->name, 10, "mct_tick%d", cpu);

Is this really necessary to enable EXYNOS5410 support?

> 
>  	evt->name = mevt->name;
>  	evt->cpumask = cpumask_of(cpu);
> diff --git a/drivers/irqchip/exynos-combiner.c
> b/drivers/irqchip/exynos-combiner.c index 868ed40..2e056fc 100644
> --- a/drivers/irqchip/exynos-combiner.c
> +++ b/drivers/irqchip/exynos-combiner.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_address.h>
>  #include <linux/of_irq.h>
>  #include <asm/mach/irq.h>
> +#include <plat/cpu.h>
> 
>  #include "irqchip.h"
> 
> @@ -66,6 +67,11 @@ static void combiner_handle_cascade_irq(unsigned int
> irq, struct irq_desc *desc) struct irq_chip *chip = irq_get_chip(irq);
>  	unsigned int cascade_irq, combiner_irq;
>  	unsigned long status;
> +	if (unlikely(!chip || !chip_data)) {
> +		printk_once(KERN_ALERT "%s: Chip not found for IRQ %d\n"
> +				, __func__, irq);
> +		return;
> +	}

What is the reason for this change?

> 
>  	chained_irq_enter(chip, desc);
> 
> @@ -226,7 +232,11 @@ static int __init combiner_of_init(struct
> device_node *np, * get their IRQ from DT, remove this in order to get
> dynamic * allocation.
>  	 */
> -	irq_base = 160;
> +
> +	if (soc_is_exynos5410())
> +		irq_base = 256;
> +	else
> +		irq_base = 160;
> 
>  	combiner_init(combiner_base, np, max_nr, irq_base);

There was a patch floating on the ML, possibly already merged, removing 
static IRQ base assignment for combiner (which is a remnant of legacy non-
DT support) and moving the driver to normal linear IRQ domain. That patch 
is what you need instead of this change.

Best regards,
Tomasz

Re: [PATCH 5/6] ARM: EXYNOS: Minor fixes to enable EXYNOS5410 support

[Chander Kashyap]

On 3 October 2013 02:37, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Vyacheslav, Tarek,
>
> On Tuesday 01 of October 2013 20:17:06 Vyacheslav Tyrtov wrote:
>> From: Tarek Dakhran <t.dakhran@samsung.com>
>>
>> Configure ARM_NR_BANKS as 16 for EXYNOS SoC.
>> Enable cci_control_port_by_index for ACE_PORT.
>> Add additional irqs for Exynos MCT.
>> Set irq base as 256 for EXYNOS5410 SoC.
>>
>> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
>> ---
>>  arch/arm/Kconfig                  |  2 +-
>>  drivers/bus/arm-cci.c             |  7 +++++++
>>  drivers/clocksource/exynos_mct.c  |  8 +++++++-
>>  drivers/irqchip/exynos-combiner.c | 12 +++++++++++-
>>  4 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 3f7714d..7f88896 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -1080,7 +1080,7 @@ source arch/arm/mm/Kconfig
>>
>>  config ARM_NR_BANKS
>>       int
>> -     default 16 if ARCH_EP93XX
>> +     default 16 if ARCH_EP93XX || ARCH_EXYNOS
>
> Could you explain why this is needed, please?
>
>>       default 8
>>
>>  config IWMMXT
>> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
>> index 2009266..f2f5df1 100644
>> --- a/drivers/bus/arm-cci.c
>> +++ b/drivers/bus/arm-cci.c
>> @@ -363,8 +363,15 @@ int notrace __cci_control_port_by_index(u32 port,
>> bool enable) * interface (ie cci_disable_port_by_cpu(); control by
>> general purpose * indexing is therefore disabled for ACE ports.
>>        */
>> +
>> +     /*
>> +      * Using this way to enable cci_port on EXYNOS5410 SoC
>> +      */
>> +
>> +#ifndef CONFIG_SOC_EXYNOS5410
>>       if (ports[port].type == ACE_PORT)
>>               return -EPERM;
>> +#endif
>
> Huh? Could you explain a) why this is needed b) why this can't be detected
> at runtime? Any new code being added must be ready for multiplatform
> builds and this clearly isn't.
>
> I'd recommend extending the CCI binding with a boolean property that makes
> the driver bypass this check, but I'd like to see an answer to question a)
> first.
>
>>
>>       cci_port_control(port, enable);
>>       return 0;
>> diff --git a/drivers/clocksource/exynos_mct.c
>> b/drivers/clocksource/exynos_mct.c index 5b34768..33884d7 100644
>> --- a/drivers/clocksource/exynos_mct.c
>> +++ b/drivers/clocksource/exynos_mct.c
>> @@ -71,6 +71,12 @@ enum {
>>       MCT_L1_IRQ,
>>       MCT_L2_IRQ,
>>       MCT_L3_IRQ,

>> +#ifdef CONFIG_ARM_CCI
>> +     MCT_L4_IRQ,
>> +     MCT_L5_IRQ,
>> +     MCT_L6_IRQ,
>> +     MCT_L7_IRQ,
>> +#endif

The above change is not required as patches are already submitted to
boot all eight cores, which include this change also.

>
> This #ifdef is useless.
>
> Basically this whole enum is, as it is a remnant of legacy non-DT support,
> but it is a material for separate patch.
>
>>       MCT_NR_IRQS,
>>  };
>>
>> @@ -406,7 +412,7 @@ static int exynos4_local_timer_setup(struct
>> clock_event_device *evt) mevt = container_of(evt, struct
>> mct_clock_event_device, evt);
>>
>>       mevt->base = EXYNOS4_MCT_L_BASE(cpu);
>> -     sprintf(mevt->name, "mct_tick%d", cpu);
>> +     snprintf(mevt->name, 10, "mct_tick%d", cpu);
>
> Is this really necessary to enable EXYNOS5410 support?
>
>>
>>       evt->name = mevt->name;
>>       evt->cpumask = cpumask_of(cpu);
>> diff --git a/drivers/irqchip/exynos-combiner.c
>> b/drivers/irqchip/exynos-combiner.c index 868ed40..2e056fc 100644
>> --- a/drivers/irqchip/exynos-combiner.c
>> +++ b/drivers/irqchip/exynos-combiner.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/of_address.h>
>>  #include <linux/of_irq.h>
>>  #include <asm/mach/irq.h>
>> +#include <plat/cpu.h>
>>
>>  #include "irqchip.h"
>>
>> @@ -66,6 +67,11 @@ static void combiner_handle_cascade_irq(unsigned int
>> irq, struct irq_desc *desc) struct irq_chip *chip = irq_get_chip(irq);
>>       unsigned int cascade_irq, combiner_irq;
>>       unsigned long status;
>> +     if (unlikely(!chip || !chip_data)) {
>> +             printk_once(KERN_ALERT "%s: Chip not found for IRQ %d\n"
>> +                             , __func__, irq);
>> +             return;
>> +     }
>
> What is the reason for this change?
>
>>
>>       chained_irq_enter(chip, desc);
>>
>> @@ -226,7 +232,11 @@ static int __init combiner_of_init(struct
>> device_node *np, * get their IRQ from DT, remove this in order to get
>> dynamic * allocation.
>>        */
>> -     irq_base = 160;
>> +
>> +     if (soc_is_exynos5410())
>> +             irq_base = 256;
>> +     else
>> +             irq_base = 160;
>>
>>       combiner_init(combiner_base, np, max_nr, irq_base);
>
> There was a patch floating on the ML, possibly already merged, removing
> static IRQ base assignment for combiner (which is a remnant of legacy non-
> DT support) and moving the driver to normal linear IRQ domain. That patch
> is what you need instead of this change.

That's right Tomasz. Patch is already merged.
>
> Best regards,
> Tomasz
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
with warm regards,
Chander Kashyap

[PATCH 6/6] ARM: smdk5410_defconfig: add defconfig for smdk5410

[Vyacheslav Tyrtov]

From: Tarek Dakhran <t.dakhran@samsung.com>

Since current exynos_defconfig cannot support for EXYNOS5410 because
of big.LITTLE architecture, MCPM and ARM_CCI bus there added
smdk5410_defconfig which is tested on SMDK5410 board
for all 8 cpu running at the same time.
This defconfig enables ARM_CCI, MCPM, EDCS.

Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
---
 arch/arm/configs/smdk5410_defconfig | 134 ++++++++++++++++++++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 arch/arm/configs/smdk5410_defconfig

diff --git a/arch/arm/configs/smdk5410_defconfig b/arch/arm/configs/smdk5410_defconfig
new file mode 100644
index 0000000..1c19594
--- /dev/null
+++ b/arch/arm/configs/smdk5410_defconfig
@@ -0,0 +1,134 @@
+CONFIG_SYSVIPC=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_BLK_DEV_INITRD=y
+CONFIG_KALLSYMS_ALL=y
+CONFIG_MODULES=y
+CONFIG_MODULE_UNLOAD=y
+CONFIG_PARTITION_ADVANCED=y
+CONFIG_ARCH_EXYNOS=y
+CONFIG_S3C_LOWLEVEL_UART_PORT=3
+CONFIG_S3C24XX_PWM=y
+CONFIG_ARCH_EXYNOS5=y
+CONFIG_SMP=y
+CONFIG_NR_CPUS=8
+CONFIG_PREEMPT=y
+CONFIG_AEABI=y
+CONFIG_HIGHMEM=y
+CONFIG_ZBOOT_ROM_TEXT=0x0
+CONFIG_ZBOOT_ROM_BSS=0x0
+CONFIG_ARM_APPENDED_DTB=y
+CONFIG_ARM_ATAG_DTB_COMPAT=y
+CONFIG_CMDLINE="console=ttySAC2,115200"
+CONFIG_VFP=y
+CONFIG_NEON=y
+CONFIG_NET=y
+CONFIG_PACKET=y
+CONFIG_UNIX=y
+CONFIG_NET_KEY=y
+CONFIG_INET=y
+CONFIG_RFKILL_REGULATOR=y
+CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
+CONFIG_DEVTMPFS=y
+CONFIG_DEVTMPFS_MOUNT=y
+CONFIG_PROC_DEVICETREE=y
+CONFIG_BLK_DEV_LOOP=y
+CONFIG_BLK_DEV_CRYPTOLOOP=y
+CONFIG_BLK_DEV_RAM=y
+CONFIG_BLK_DEV_RAM_SIZE=8192
+CONFIG_SCSI=y
+CONFIG_BLK_DEV_SD=y
+CONFIG_CHR_DEV_SG=y
+CONFIG_MD=y
+CONFIG_BLK_DEV_DM=y
+CONFIG_DM_CRYPT=m
+CONFIG_NETDEVICES=y
+CONFIG_SMSC911X=y
+CONFIG_USB_USBNET=y
+CONFIG_USB_NET_SMSC75XX=y
+CONFIG_USB_NET_SMSC95XX=y
+CONFIG_INPUT_EVDEV=y
+CONFIG_KEYBOARD_GPIO=y
+CONFIG_KEYBOARD_CROS_EC=y
+# CONFIG_MOUSE_PS2 is not set
+CONFIG_MOUSE_CYAPA=y
+CONFIG_INPUT_TOUCHSCREEN=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_SAMSUNG=y
+CONFIG_SERIAL_SAMSUNG_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_HW_RANDOM=y
+CONFIG_TCG_TPM=y
+CONFIG_TCG_TIS_I2C_INFINEON=y
+CONFIG_I2C=y
+CONFIG_I2C_MUX=y
+CONFIG_I2C_ARB_GPIO_CHALLENGE=y
+CONFIG_I2C_S3C2410=y
+CONFIG_DEBUG_GPIO=y
+# CONFIG_HWMON is not set
+CONFIG_MFD_CROS_EC=y
+CONFIG_MFD_CROS_EC_I2C=y
+CONFIG_MFD_MAX77686=y
+CONFIG_MFD_MAX8997=y
+CONFIG_MFD_SEC_CORE=y
+CONFIG_MFD_TPS65090=y
+CONFIG_REGULATOR=y
+CONFIG_REGULATOR_FIXED_VOLTAGE=y
+CONFIG_REGULATOR_GPIO=y
+CONFIG_REGULATOR_MAX8997=y
+CONFIG_REGULATOR_MAX77686=y
+CONFIG_REGULATOR_S5M8767=y
+CONFIG_REGULATOR_TPS65090=y
+CONFIG_FB=y
+CONFIG_FB_MODE_HELPERS=y
+CONFIG_FB_SIMPLE=y
+CONFIG_EXYNOS_VIDEO=y
+CONFIG_EXYNOS_MIPI_DSI=y
+CONFIG_EXYNOS_DP=y
+CONFIG_FRAMEBUFFER_CONSOLE=y
+CONFIG_FONTS=y
+CONFIG_FONT_7x14=y
+CONFIG_LOGO=y
+CONFIG_USB=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_S5P=y
+CONFIG_USB_STORAGE=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_PHY=y
+CONFIG_SAMSUNG_USB2PHY=y
+CONFIG_SAMSUNG_USB3PHY=y
+CONFIG_MMC=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_S3C=y
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_IDMAC=y
+CONFIG_MMC_DW_EXYNOS=y
+CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_S3C=y
+CONFIG_COMMON_CLK_MAX77686=y
+CONFIG_EXT2_FS=y
+CONFIG_EXT3_FS=y
+CONFIG_EXT4_FS=y
+CONFIG_MSDOS_FS=y
+CONFIG_VFAT_FS=y
+CONFIG_TMPFS=y
+CONFIG_TMPFS_POSIX_ACL=y
+CONFIG_CRAMFS=y
+CONFIG_ROMFS_FS=y
+CONFIG_NLS_CODEPAGE_437=y
+CONFIG_NLS_ASCII=y
+CONFIG_NLS_ISO8859_1=y
+CONFIG_PRINTK_TIME=y
+CONFIG_MAGIC_SYSRQ=y
+CONFIG_DEBUG_KERNEL=y
+CONFIG_DETECT_HUNG_TASK=y
+CONFIG_DEBUG_RT_MUTEXES=y
+CONFIG_DEBUG_SPINLOCK=y
+CONFIG_DEBUG_MUTEXES=y
+CONFIG_DEBUG_INFO=y
+CONFIG_DEBUG_USER=y
+CONFIG_CRYPTO_SHA256=y
+CONFIG_CRC_CCITT=y
+CONFIG_ARM_CCI=y
+CONFIG_MCPM=y
+CONFIG_CMDLINE_FORCE=y
-- 
1.8.1.5

Re: [PATCH 6/6] ARM: smdk5410_defconfig: add defconfig for smdk5410

[Bartlomiej Zolnierkiewicz]

Hi,

On Tuesday, October 01, 2013 08:17:07 PM Vyacheslav Tyrtov wrote:
> From: Tarek Dakhran <t.dakhran@samsung.com>
> 
> Since current exynos_defconfig cannot support for EXYNOS5410 because
> of big.LITTLE architecture, MCPM and ARM_CCI bus there added

CONFIG_MCPM and CONFIG_ARM_CCI are optional features that won't be used
unless explicitly enabled in the device tree. CONFIG_BIG_LITTLE is not
enabled in the new defconfig at all (I also believe that it is optional
or at least can be made optional).

Therefore please modify existing exynos_defconfig to support EXYNOS5410
instead of adding the new defconfig. Having the common defconfig is really
important from the long-term maintainance POV.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> smdk5410_defconfig which is tested on SMDK5410 board
> for all 8 cpu running at the same time.
> This defconfig enables ARM_CCI, MCPM, EDCS.
> 
> Signed-off-by: Tarek Dakhran <t.dakhran@samsung.com>
> Signed-off-by: Vyacheslav Tyrtov <v.tyrtov@samsung.com>
> ---
>  arch/arm/configs/smdk5410_defconfig | 134 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 134 insertions(+)
>  create mode 100644 arch/arm/configs/smdk5410_defconfig
> 
> diff --git a/arch/arm/configs/smdk5410_defconfig b/arch/arm/configs/smdk5410_defconfig
> new file mode 100644
> index 0000000..1c19594
> --- /dev/null
> +++ b/arch/arm/configs/smdk5410_defconfig
> @@ -0,0 +1,134 @@
> +CONFIG_SYSVIPC=y
> +CONFIG_NO_HZ=y
> +CONFIG_HIGH_RES_TIMERS=y
> +CONFIG_BLK_DEV_INITRD=y
> +CONFIG_KALLSYMS_ALL=y
> +CONFIG_MODULES=y
> +CONFIG_MODULE_UNLOAD=y
> +CONFIG_PARTITION_ADVANCED=y
> +CONFIG_ARCH_EXYNOS=y
> +CONFIG_S3C_LOWLEVEL_UART_PORT=3
> +CONFIG_S3C24XX_PWM=y
> +CONFIG_ARCH_EXYNOS5=y
> +CONFIG_SMP=y
> +CONFIG_NR_CPUS=8
> +CONFIG_PREEMPT=y
> +CONFIG_AEABI=y
> +CONFIG_HIGHMEM=y
> +CONFIG_ZBOOT_ROM_TEXT=0x0
> +CONFIG_ZBOOT_ROM_BSS=0x0
> +CONFIG_ARM_APPENDED_DTB=y
> +CONFIG_ARM_ATAG_DTB_COMPAT=y
> +CONFIG_CMDLINE="console=ttySAC2,115200"
> +CONFIG_VFP=y
> +CONFIG_NEON=y
> +CONFIG_NET=y
> +CONFIG_PACKET=y
> +CONFIG_UNIX=y
> +CONFIG_NET_KEY=y
> +CONFIG_INET=y
> +CONFIG_RFKILL_REGULATOR=y
> +CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
> +CONFIG_DEVTMPFS=y
> +CONFIG_DEVTMPFS_MOUNT=y
> +CONFIG_PROC_DEVICETREE=y
> +CONFIG_BLK_DEV_LOOP=y
> +CONFIG_BLK_DEV_CRYPTOLOOP=y
> +CONFIG_BLK_DEV_RAM=y
> +CONFIG_BLK_DEV_RAM_SIZE=8192
> +CONFIG_SCSI=y
> +CONFIG_BLK_DEV_SD=y
> +CONFIG_CHR_DEV_SG=y
> +CONFIG_MD=y
> +CONFIG_BLK_DEV_DM=y
> +CONFIG_DM_CRYPT=m
> +CONFIG_NETDEVICES=y
> +CONFIG_SMSC911X=y
> +CONFIG_USB_USBNET=y
> +CONFIG_USB_NET_SMSC75XX=y
> +CONFIG_USB_NET_SMSC95XX=y
> +CONFIG_INPUT_EVDEV=y
> +CONFIG_KEYBOARD_GPIO=y
> +CONFIG_KEYBOARD_CROS_EC=y
> +# CONFIG_MOUSE_PS2 is not set
> +CONFIG_MOUSE_CYAPA=y
> +CONFIG_INPUT_TOUCHSCREEN=y
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_SAMSUNG=y
> +CONFIG_SERIAL_SAMSUNG_CONSOLE=y
> +CONFIG_SERIAL_OF_PLATFORM=y
> +CONFIG_HW_RANDOM=y
> +CONFIG_TCG_TPM=y
> +CONFIG_TCG_TIS_I2C_INFINEON=y
> +CONFIG_I2C=y
> +CONFIG_I2C_MUX=y
> +CONFIG_I2C_ARB_GPIO_CHALLENGE=y
> +CONFIG_I2C_S3C2410=y
> +CONFIG_DEBUG_GPIO=y
> +# CONFIG_HWMON is not set
> +CONFIG_MFD_CROS_EC=y
> +CONFIG_MFD_CROS_EC_I2C=y
> +CONFIG_MFD_MAX77686=y
> +CONFIG_MFD_MAX8997=y
> +CONFIG_MFD_SEC_CORE=y
> +CONFIG_MFD_TPS65090=y
> +CONFIG_REGULATOR=y
> +CONFIG_REGULATOR_FIXED_VOLTAGE=y
> +CONFIG_REGULATOR_GPIO=y
> +CONFIG_REGULATOR_MAX8997=y
> +CONFIG_REGULATOR_MAX77686=y
> +CONFIG_REGULATOR_S5M8767=y
> +CONFIG_REGULATOR_TPS65090=y
> +CONFIG_FB=y
> +CONFIG_FB_MODE_HELPERS=y
> +CONFIG_FB_SIMPLE=y
> +CONFIG_EXYNOS_VIDEO=y
> +CONFIG_EXYNOS_MIPI_DSI=y
> +CONFIG_EXYNOS_DP=y
> +CONFIG_FRAMEBUFFER_CONSOLE=y
> +CONFIG_FONTS=y
> +CONFIG_FONT_7x14=y
> +CONFIG_LOGO=y
> +CONFIG_USB=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_EHCI_S5P=y
> +CONFIG_USB_STORAGE=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_PHY=y
> +CONFIG_SAMSUNG_USB2PHY=y
> +CONFIG_SAMSUNG_USB3PHY=y
> +CONFIG_MMC=y
> +CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_S3C=y
> +CONFIG_MMC_DW=y
> +CONFIG_MMC_DW_IDMAC=y
> +CONFIG_MMC_DW_EXYNOS=y
> +CONFIG_RTC_CLASS=y
> +CONFIG_RTC_DRV_S3C=y
> +CONFIG_COMMON_CLK_MAX77686=y
> +CONFIG_EXT2_FS=y
> +CONFIG_EXT3_FS=y
> +CONFIG_EXT4_FS=y
> +CONFIG_MSDOS_FS=y
> +CONFIG_VFAT_FS=y
> +CONFIG_TMPFS=y
> +CONFIG_TMPFS_POSIX_ACL=y
> +CONFIG_CRAMFS=y
> +CONFIG_ROMFS_FS=y
> +CONFIG_NLS_CODEPAGE_437=y
> +CONFIG_NLS_ASCII=y
> +CONFIG_NLS_ISO8859_1=y
> +CONFIG_PRINTK_TIME=y
> +CONFIG_MAGIC_SYSRQ=y
> +CONFIG_DEBUG_KERNEL=y
> +CONFIG_DETECT_HUNG_TASK=y
> +CONFIG_DEBUG_RT_MUTEXES=y
> +CONFIG_DEBUG_SPINLOCK=y
> +CONFIG_DEBUG_MUTEXES=y
> +CONFIG_DEBUG_INFO=y
> +CONFIG_DEBUG_USER=y
> +CONFIG_CRYPTO_SHA256=y
> +CONFIG_CRC_CCITT=y
> +CONFIG_ARM_CCI=y
> +CONFIG_MCPM=y
> +CONFIG_CMDLINE_FORCE=y

Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Dave Martin]

On Fri, Oct 04, 2013 at 03:51:31PM -0400, Nicolas Pitre wrote:
> On Wed, 2 Oct 2013, Dave Martin wrote:
> 
> > On Tue, Oct 01, 2013 at 08:17:04PM +0400, Vyacheslav Tyrtov wrote:
> > > +static int exynos_power_up(unsigned int cpu, unsigned int cluster)
> > > +{
> > > +     int ret;
> > > +     local_irq_disable();
> > 
> > Should there be a local_fiq_disable() here also?
> 
> No.  In fact this is paired with
> 
> > > +     arch_spin_lock(&exynos_lock);
> 
> to create the equivalent of a arch_spin_lock_irq().  And the reason is:
> 
> /*
>  * We can't use regular spinlocks. In the switcher case, it is possible
>  * for an outbound CPU to call power_down() after its inbound counterpart
>  * is already live using the same logical CPU number which trips lockdep
>  * debugging.
>  */
> 
> Otherwise we simply would have used spin_lock_irq().

Duh, of course.  Looks like I suffered temporary brain failure there.

> No FIQs are supposed to ever race with this code.

There is an anomaly though: FIQ and external abort don't seem to get
explicitly masked anywhere, either on the suspend or powerdown paths.
Sometimes either or both remains unmasked (I tried some trace in the
TC2 MCPM backend to confirm this.)

Looks like a possible omission in the arch/arm/ suspend and shutdown
code, rather than a problem specific to MCPM.

Shouldn't be an issue for this series, though.

Cheers
---Dave

Re: [PATCH 3/6] ARM: EXYNOS: add Exynos Dual Cluster Support

[Nicolas Pitre]

On Mon, 7 Oct 2013, Dave Martin wrote:

> On Fri, Oct 04, 2013 at 03:51:31PM -0400, Nicolas Pitre wrote:
> > No FIQs are supposed to ever race with this code.
> 
> There is an anomaly though: FIQ and external abort don't seem to get
> explicitly masked anywhere, either on the suspend or powerdown paths.
> Sometimes either or both remains unmasked (I tried some trace in the
> TC2 MCPM backend to confirm this.)
> 
> Looks like a possible omission in the arch/arm/ suspend and shutdown
> code, rather than a problem specific to MCPM.

Possibly, yes.

Feel free to post a patch.


Nicolas