[v2 0/9] ARM: Initial support for Tegra 114 SoC

[v2 0/9] ARM: Initial support for Tegra 114 SoC

[Hiroshi Doyu]

Hi,

This patchset adds initial support for NVIDIA's new Tegra 114 SoC
(T114) based on the ARM Cortex-A15 MP. It has the minimal support to
allow the kernel to boot up into shell console. This can be used as a
basis for adding other device drivers for this SoC. Currently there
are 2 evaluation boards available, "Dalmore" and "Pluto".

This patchset is against Stephen Warren's linux-next_common branch:

  git://nv-tegra.nvidia.com/user/swarren/linux-2.6 linux-next_common

For those who want to try:

  $ make ARCH=arm tegra_defconfig
  $ scripts/config -e ARCH_TEGRA_114_SOC -d DRM -d SUSPEND \
    	-d PM_RUNTIME -d CPU_FREQ -d CPU_IDLE
  $ make ARCH=arm menuconfig # if needed to configure more
  $ make ARCH=arm all -j9

You may also want to enable CONFIG_ARM_APPENDED_DTB and
CONFIG_ARM_ATAG_DTB_COMPAT if the bootloader doesn't support DT yet.

Verified that this single image booted up with "Dalmore(T114)",
"Pluto(T114)" and "Cardhu(T30)". For "Cardhu(T30)" with this single
image, SPI driver doesn't seem to afford the above configuration , it
hangs at boot-up. With SPI disabled, it's ok.

v2:
Rebased against the latest Stephen Warren's linux-next_common
Add /cpus entry in DT
Add comment to initialize TSC only in secure mode.

Hiroshi Doyu (9):
  ARM: tegra: fuse: Add chipid TEGRA114 0x35
  HACK: ARM: tegra: Use CLK_IGNORE_UNUSED for Tegra 114 SoC
  ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU
  clocksource: tegra: Reorganize funcs by clock functionarities
  clocksource: tegra: Enable ARM arch_timer with TSC
  ARM: dt: tegra114: Add new SoC base, Tegra 114 SoC
  ARM: dt: tegra114: Add new board, Dalmore
  ARM: dt: tegra114: Add new board, Pluto
  ARM: tegra: Add initial support for Tegra 114 SoC.

 .../bindings/arm/tegra/nvidia,tegra114-tsc.txt     |   11 +
 arch/arm/boot/dts/Makefile                         |    4 +-
 arch/arm/boot/dts/tegra114-dalmore.dts             |   21 ++
 arch/arm/boot/dts/tegra114-pluto.dts               |   21 ++
 arch/arm/boot/dts/tegra114.dtsi                    |  118 +++++++++++
 arch/arm/mach-tegra/Kconfig                        |   10 +
 arch/arm/mach-tegra/Makefile                       |    1 +
 arch/arm/mach-tegra/board-dt-tegra114.c            |   48 +++++
 arch/arm/mach-tegra/common.c                       |    1 +
 arch/arm/mach-tegra/fuse.h                         |    1 +
 arch/arm/mach-tegra/platsmp.c                      |   31 ++-
 arch/arm/mach-tegra/tegra30_clocks_data.c          |    2 +
 drivers/clocksource/tegra20_timer.c                |  223 +++++++++++++-------
 13 files changed, 414 insertions(+), 78 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
 create mode 100644 arch/arm/boot/dts/tegra114-dalmore.dts
 create mode 100644 arch/arm/boot/dts/tegra114-pluto.dts
 create mode 100644 arch/arm/boot/dts/tegra114.dtsi
 create mode 100644 arch/arm/mach-tegra/board-dt-tegra114.c

-- 
1.7.9.5

[v2 1/9] ARM: tegra: fuse: Add chipid TEGRA114 0x35

[Hiroshi Doyu]

Add tegra_chip_id TEGRA114 0x35

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/fuse.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-tegra/fuse.h b/arch/arm/mach-tegra/fuse.h
index ff1383d..da78434 100644
--- a/arch/arm/mach-tegra/fuse.h
+++ b/arch/arm/mach-tegra/fuse.h
@@ -37,6 +37,7 @@ enum tegra_revision {
 
 #define TEGRA20		0x20
 #define TEGRA30		0x30
+#define TEGRA114	0x35
 
 extern int tegra_sku_id;
 extern int tegra_cpu_process_id;
-- 
1.7.9.5

[v2 2/9] HACK: ARM: tegra: Use CLK_IGNORE_UNUSED for Tegra 114 SoC

[Hiroshi Doyu]

Use CLK_IGNORE_UNUSED for the Tegra 114 SoC to ensure
clk_disable_unused() is not called. Otherwise the system will die,
because the usecount of the clocks is incorrect. This patch will be
reverted once the Tegra 114 clocks are implemented.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/tegra30_clocks_data.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-tegra/tegra30_clocks_data.c b/arch/arm/mach-tegra/tegra30_clocks_data.c
index 6942c7a..4865ba5 100644
--- a/arch/arm/mach-tegra/tegra30_clocks_data.c
+++ b/arch/arm/mach-tegra/tegra30_clocks_data.c
@@ -1384,6 +1384,8 @@ static void tegra30_init_one_clock(struct clk *c)
 	if (!clk->lookup.dev_id && !clk->lookup.con_id)
 		clk->lookup.con_id = c->name;
 	clk->lookup.clk = c;
+	if (tegra_chip_id == TEGRA114) /* FIXME: Implement T114 clocks */
+		c->flags |= CLK_IGNORE_UNUSED;
 	clkdev_add(&clk->lookup);
 	tegra_clk_add(c);
 }
-- 
1.7.9.5

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

The method to detect the number of CPU cores on Cortex-A9 MPCore and
Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
have to read it from the system coprocessor(CP15), because the SCU on
Cortex-A15 MPCore does not have software readable registers. This
patch selects the correct method at runtime based on the CPU ID.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
index 1b926df..68e76ef 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/arch/arm/mach-tegra/platsmp.c
@@ -23,6 +23,7 @@
 #include <asm/hardware/gic.h>
 #include <asm/mach-types.h>
 #include <asm/smp_scu.h>
+#include <asm/cputype.h>
 
 #include <mach/powergate.h>
 
@@ -34,9 +35,13 @@
 #include "common.h"
 #include "iomap.h"
 
+#define CPU_MASK		0xff0ffff0
+#define CPU_CORTEX_A9		0x410fc090
+#define CPU_CORTEX_A15		0x410fc0f0
+
 extern void tegra_secondary_startup(void);
 
-static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
+static void __iomem *scu_base;
 
 #define EVP_CPU_RESET_VECTOR \
 	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
@@ -149,7 +154,26 @@ done:
  */
 static void __init tegra_smp_init_cpus(void)
 {
-	unsigned int i, ncores = scu_get_core_count(scu_base);
+	unsigned int i, cpu_id, ncores;
+	u32 l2ctlr;
+	phys_addr_t pa;
+
+	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
+	switch (cpu_id) {
+	case CPU_CORTEX_A15:
+		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
+		ncores = ((l2ctlr >> 24) & 3) + 1;
+		break;
+	case CPU_CORTEX_A9:
+		/* Get SCU physical base */
+		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
+		scu_base = IO_ADDRESS(pa);
+		ncores = scu_get_core_count(scu_base);
+		break;
+	default:
+		BUG();
+		break;
+	}
 
 	if (ncores > nr_cpu_ids) {
 		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
@@ -166,7 +190,8 @@ static void __init tegra_smp_init_cpus(void)
 static void __init tegra_smp_prepare_cpus(unsigned int max_cpus)
 {
 	tegra_cpu_reset_handler_init();
-	scu_enable(scu_base);
+	if (scu_base)
+		scu_enable(scu_base);
 }
 
 struct smp_operations tegra_smp_ops __initdata = {
-- 
1.7.9.5

[v2 4/9] clocksource: tegra: Reorganize funcs by clock functionarities

[Hiroshi Doyu]

Relocate functions by clock functionarities{RTC, TMR}. Also created
some new functions as helper.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 drivers/clocksource/tegra20_timer.c |  160 +++++++++++++++++++----------------
 1 file changed, 86 insertions(+), 74 deletions(-)

diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
index 5bc1429..1d25de8 100644
--- a/drivers/clocksource/tegra20_timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -35,6 +35,83 @@
 #define RTC_SHADOW_SECONDS     0x0c
 #define RTC_MILLISECONDS       0x10
 
+static void __iomem *rtc_base;
+static struct timespec persistent_ts;
+static u64 persistent_ms, last_persistent_ms;
+
+/*
+ * tegra_rtc_read - Reads the Tegra RTC registers
+ * Care must be taken that this funciton is not called while the
+ * tegra_rtc driver could be executing to avoid race conditions
+ * on the RTC shadow register
+ */
+static u64 tegra_rtc_read_ms(void)
+{
+	u32 ms = readl(rtc_base + RTC_MILLISECONDS);
+	u32 s = readl(rtc_base + RTC_SHADOW_SECONDS);
+	return (u64)s * MSEC_PER_SEC + ms;
+}
+
+/*
+ * tegra_read_persistent_clock -  Return time from a persistent clock.
+ *
+ * Reads the time from a source which isn't disabled during PM, the
+ * 32k sync timer.  Convert the cycles elapsed since last read into
+ * nsecs and adds to a monotonically increasing timespec.
+ * Care must be taken that this funciton is not called while the
+ * tegra_rtc driver could be executing to avoid race conditions
+ * on the RTC shadow register
+ */
+static void tegra_read_persistent_clock(struct timespec *ts)
+{
+	u64 delta;
+	struct timespec *tsp = &persistent_ts;
+
+	last_persistent_ms = persistent_ms;
+	persistent_ms = tegra_rtc_read_ms();
+	delta = persistent_ms - last_persistent_ms;
+
+	timespec_add_ns(tsp, delta * NSEC_PER_MSEC);
+	*ts = *tsp;
+}
+
+static const struct of_device_id rtc_match[] __initconst = {
+	{ .compatible = "nvidia,tegra20-rtc" },
+	{}
+};
+
+static void __init tegra20_init_rtc(void)
+{
+	struct device_node *np;
+	struct clk *clk;
+
+	np = of_find_matching_node(NULL, rtc_match);
+	if (!np) {
+		pr_err("Failed to find RTC DT node\n");
+		BUG();
+	}
+
+	rtc_base = of_iomap(np, 0);
+	if (!rtc_base) {
+		pr_err("Can't map RTC registers");
+		BUG();
+	}
+
+	/*
+	 * rtc registers are used by read_persistent_clock, keep the rtc clock
+	 * enabled
+	 */
+	clk = clk_get_sys("rtc-tegra", NULL);
+	if (IS_ERR(clk))
+		pr_warn("Unable to get rtc-tegra clock\n");
+	else
+		clk_prepare_enable(clk);
+
+	of_node_put(np);
+
+	register_persistent_clock(NULL, tegra_read_persistent_clock);
+}
+
 #define TIMERUS_CNTR_1US 0x10
 #define TIMERUS_USEC_CFG 0x14
 #define TIMERUS_CNTR_FREEZE 0x4c
@@ -48,10 +125,6 @@
 #define TIMER_PCR 0x4
 
 static void __iomem *timer_reg_base;
-static void __iomem *rtc_base;
-
-static struct timespec persistent_ts;
-static u64 persistent_ms, last_persistent_ms;
 
 #define timer_writel(value, reg) \
 	__raw_writel(value, timer_reg_base + (reg))
@@ -103,46 +176,10 @@ static u32 notrace tegra_read_sched_clock(void)
 	return timer_readl(TIMERUS_CNTR_1US);
 }
 
-/*
- * tegra_rtc_read - Reads the Tegra RTC registers
- * Care must be taken that this funciton is not called while the
- * tegra_rtc driver could be executing to avoid race conditions
- * on the RTC shadow register
- */
-static u64 tegra_rtc_read_ms(void)
-{
-	u32 ms = readl(rtc_base + RTC_MILLISECONDS);
-	u32 s = readl(rtc_base + RTC_SHADOW_SECONDS);
-	return (u64)s * MSEC_PER_SEC + ms;
-}
-
-/*
- * tegra_read_persistent_clock -  Return time from a persistent clock.
- *
- * Reads the time from a source which isn't disabled during PM, the
- * 32k sync timer.  Convert the cycles elapsed since last read into
- * nsecs and adds to a monotonically increasing timespec.
- * Care must be taken that this funciton is not called while the
- * tegra_rtc driver could be executing to avoid race conditions
- * on the RTC shadow register
- */
-static void tegra_read_persistent_clock(struct timespec *ts)
-{
-	u64 delta;
-	struct timespec *tsp = &persistent_ts;
-
-	last_persistent_ms = persistent_ms;
-	persistent_ms = tegra_rtc_read_ms();
-	delta = persistent_ms - last_persistent_ms;
-
-	timespec_add_ns(tsp, delta * NSEC_PER_MSEC);
-	*ts = *tsp;
-}
-
 static irqreturn_t tegra_timer_interrupt(int irq, void *dev_id)
 {
 	struct clock_event_device *evt = (struct clock_event_device *)dev_id;
-	timer_writel(1<<30, TIMER3_BASE + TIMER_PCR);
+	timer_writel(1 << 30, TIMER3_BASE + TIMER_PCR);
 	evt->event_handler(evt);
 	return IRQ_HANDLED;
 }
@@ -159,12 +196,7 @@ static const struct of_device_id timer_match[] __initconst = {
 	{}
 };
 
-static const struct of_device_id rtc_match[] __initconst = {
-	{ .compatible = "nvidia,tegra20-rtc" },
-	{}
-};
-
-static void __init tegra20_init_timer(void)
+static void __init tegra20_init_tmr(void)
 {
 	struct device_node *np;
 	struct clk *clk;
@@ -200,30 +232,6 @@ static void __init tegra20_init_timer(void)
 
 	of_node_put(np);
 
-	np = of_find_matching_node(NULL, rtc_match);
-	if (!np) {
-		pr_err("Failed to find RTC DT node\n");
-		BUG();
-	}
-
-	rtc_base = of_iomap(np, 0);
-	if (!rtc_base) {
-		pr_err("Can't map RTC registers");
-		BUG();
-	}
-
-	/*
-	 * rtc registers are used by read_persistent_clock, keep the rtc clock
-	 * enabled
-	 */
-	clk = clk_get_sys("rtc-tegra", NULL);
-	if (IS_ERR(clk))
-		pr_warn("Unable to get rtc-tegra clock\n");
-	else
-		clk_prepare_enable(clk);
-
-	of_node_put(np);
-
 	switch (rate) {
 	case 12000000:
 		timer_writel(0x000b, TIMERUS_USEC_CFG);
@@ -241,8 +249,6 @@ static void __init tegra20_init_timer(void)
 		WARN(1, "Unknown clock rate");
 	}
 
-	setup_sched_clock(tegra_read_sched_clock, 32, 1000000);
-
 	if (clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
 		"timer_us", 1000000, 300, 32, clocksource_mmio_readl_up)) {
 		pr_err("Failed to register clocksource\n");
@@ -263,10 +269,16 @@ static void __init tegra20_init_timer(void)
 	tegra_clockevent.cpumask = cpu_all_mask;
 	tegra_clockevent.irq = tegra_timer_irq.irq;
 	clockevents_register_device(&tegra_clockevent);
+}
+
+static void __init tegra20_init_timer(void)
+{
+	tegra20_init_tmr();
+	setup_sched_clock(tegra_read_sched_clock, 32, 1000000);
+	tegra20_init_rtc();
 #ifdef CONFIG_HAVE_ARM_TWD
 	twd_local_timer_of_register();
 #endif
-	register_persistent_clock(NULL, tegra_read_persistent_clock);
 }
 CLOCKSOURCE_OF_DECLARE(tegra20, "nvidia,tegra20-timer", tegra20_init_timer);
 
-- 
1.7.9.5

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Hiroshi Doyu]

Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
timer than TMR0. If it's available, it will be used for clock source
and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
necessary for clock event.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 .../bindings/arm/tegra/nvidia,tegra114-tsc.txt     |   11 ++++
 drivers/clocksource/tegra20_timer.c                |   65 +++++++++++++++++++-
 2 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
new file mode 100644
index 0000000..9de936a
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
@@ -0,0 +1,11 @@
+NVIDIA Tegra Timer Stamp Counter(TSC)
+
+Required properties:
+- compatible : "nvidia,tegra114-tsc
+- reg : Should contain 1 register ranges(address and length)
+
+Example:
+	tsc {
+		compatible = "nvidia,tegra114-tsc";
+		reg = <0x700f0000 0x20000>;
+	};
diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
index 1d25de8..564266d 100644
--- a/drivers/clocksource/tegra20_timer.c
+++ b/drivers/clocksource/tegra20_timer.c
@@ -30,6 +30,7 @@
 #include <asm/mach/time.h>
 #include <asm/smp_twd.h>
 #include <asm/sched_clock.h>
+#include <asm/arch_timer.h>
 
 #define RTC_SECONDS            0x08
 #define RTC_SHADOW_SECONDS     0x0c
@@ -271,10 +272,72 @@ static void __init tegra20_init_tmr(void)
 	clockevents_register_device(&tegra_clockevent);
 }
 
+#define TSC_CNTCR		0		/* TSC control registers */
+#define TSC_CNTCR_ENABLE	(1 << 0)	/* Enable */
+#define TSC_CNTCR_HDBG		(1 << 1)	/* Halt on debug */
+
+#define TSC_CNTCV0		0x8		/* TSC counter (LSW) */
+#define TSC_CNTCV1		0xc		/* TSC counter (MSW) */
+#define TSC_CNTFID0		0x20		/* TSC freq id 0 */
+
+static const struct of_device_id tegra_tsc_match[] __initconst = {
+	{ .compatible = "nvidia,tegra114-tsc" },
+	{}
+};
+
+/* FIXME: only secure mode is supported. */
+static int tegra_arch_timer_init(void)
+{
+	int err;
+	struct device_node *np;
+	struct clk *clk;
+	void __iomem *tsc_base;
+	u32 freq, val;
+
+	np = of_find_matching_node(NULL, tegra_tsc_match);
+	if (!np)
+		return -ENODEV;
+
+	tsc_base = of_iomap(np, 0);
+	if (!tsc_base)
+		return -ENODEV;
+
+	clk = clk_get_sys("clk_m", NULL);
+	if (IS_ERR(clk)) {
+		freq = 12000000;
+		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
+	} else {
+		freq = clk_get_rate(clk);
+		clk_put(clk);
+	}
+	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
+
+	/* CNTFRQ */
+	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
+	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
+	BUG_ON(val != freq);
+
+	val = readl_relaxed(tsc_base + TSC_CNTCR);
+	val |= TSC_CNTCR_ENABLE | TSC_CNTCR_HDBG;
+	writel_relaxed(val, tsc_base + TSC_CNTCR);
+
+	err = arch_timer_of_register();
+	if (!err)
+		err = arch_timer_sched_clock_init();
+	if (err)
+		pr_err("Failed to register ARM arch_timer(TSC)\n");
+	return err;
+}
+
 static void __init tegra20_init_timer(void)
 {
+	int err = -ENODEV;
+
 	tegra20_init_tmr();
-	setup_sched_clock(tegra_read_sched_clock, 32, 1000000);
+	if (IS_ENABLED(CONFIG_ARM_ARCH_TIMER))
+		err = tegra_arch_timer_init();
+	if (err)
+		setup_sched_clock(tegra_read_sched_clock, 32, 1000000);
 	tegra20_init_rtc();
 #ifdef CONFIG_HAVE_ARM_TWD
 	twd_local_timer_of_register();
-- 
1.7.9.5

[v2 6/9] ARM: dt: tegra114: Add new SoC base, Tegra 114 SoC

[Hiroshi Doyu]

Initial support for Tegra 114 SoC. This is expected to be included in
the board DTS files, Tegra 114 SoC based evaluation board family.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/tegra114.dtsi |  118 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)
 create mode 100644 arch/arm/boot/dts/tegra114.dtsi

diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
new file mode 100644
index 0000000..b7ba7c3
--- /dev/null
+++ b/arch/arm/boot/dts/tegra114.dtsi
@@ -0,0 +1,118 @@
+/include/ "skeleton.dtsi"
+
+/ {
+	compatible = "nvidia,tegra114";
+	interrupt-parent = <&gic>;
+
+	gic: interrupt-controller {
+		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
+		reg = <0x50041000 0x1000
+		       0x50042000 0x1000>;
+		interrupt-controller;
+		#interrupt-cells = <3>;
+	};
+
+	timer at 60005000 {
+		compatible = "nvidia,tegra114-timer", "nvidia,tegra20-timer";
+		reg = <0x60005000 0x400>;
+		interrupts = <0 0 0x04
+			      0 1 0x04
+			      0 41 0x04
+			      0 42 0x04
+			      0 121 0x04
+			      0 122 0x04>;
+	};
+
+	serial at 70006000 {
+		compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
+		reg = <0x70006000 0x40>;
+		reg-shift = <2>;
+		interrupts = <0 36 0x04>;
+		status = "disabled";
+	};
+
+	serial at 70006040 {
+		compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
+		reg = <0x70006040 0x40>;
+		reg-shift = <2>;
+		interrupts = <0 37 0x04>;
+		status = "disabled";
+	};
+
+	serial at 70006200 {
+		compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
+		reg = <0x70006200 0x100>;
+		reg-shift = <2>;
+		interrupts = <0 46 0x04>;
+		status = "disabled";
+	};
+
+	serial at 70006300 {
+		compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
+		reg = <0x70006300 0x100>;
+		reg-shift = <2>;
+		interrupts = <0 90 0x04>;
+		status = "disabled";
+	};
+
+	serial at 70006400 {
+		compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
+		reg = <0x70006400 0x100>;
+		reg-shift = <2>;
+		interrupts = <0 91 0x04>;
+		status = "disabled";
+	};
+
+	rtc {
+		compatible = "nvidia,tegra114-rtc", "nvidia,tegra20-rtc";
+		reg = <0x7000e000 0x100>;
+		interrupts = <0 2 0x04>;
+	};
+
+	pmc {
+		compatible = "nvidia,tegra114-pmc", "nvidia,tegra20-pmc";
+		reg = <0x7000e400 0x400>;
+	};
+
+	tsc {
+		compatible = "nvidia,tegra114-tsc";
+		reg = <0x700f0000 0x20000>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu at 0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <0>;
+		};
+
+		cpu at 1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <1>;
+		};
+
+		cpu at 2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <2>;
+		};
+
+		cpu at 3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a15";
+			reg = <3>;
+		};
+	};
+
+	timer {
+		compatible = "arm,armv7-timer";
+		interrupts = <1 13 0xf08>,
+			     <1 14 0xf08>,
+			     <1 11 0xf08>,
+			     <1 10 0xf08>;
+	};
+};
-- 
1.7.9.5

[v2 7/9] ARM: dt: tegra114: Add new board, Dalmore

[Hiroshi Doyu]

Add a new evaluation board, Dalmore for Tegra 114 family.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/Makefile             |    3 ++-
 arch/arm/boot/dts/tegra114-dalmore.dts |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/tegra114-dalmore.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index e44da40..88ea9fc 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -143,7 +143,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
 	tegra20-ventana.dtb \
 	tegra20-whistler.dtb \
 	tegra30-cardhu-a02.dtb \
-	tegra30-cardhu-a04.dtb
+	tegra30-cardhu-a04.dtb \
+	tegra114-dalmore.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2p-ca5s.dtb \
 	vexpress-v2p-ca9.dtb \
 	vexpress-v2p-ca15-tc1.dtb \
diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts
new file mode 100644
index 0000000..a30aca6
--- /dev/null
+++ b/arch/arm/boot/dts/tegra114-dalmore.dts
@@ -0,0 +1,21 @@
+/dts-v1/;
+
+/include/ "tegra114.dtsi"
+
+/ {
+	model = "NVIDIA Tegra114 Dalmore evaluation board";
+	compatible = "nvidia,dalmore", "nvidia,tegra114";
+
+	memory {
+		reg = <0x80000000 0x40000000>;
+	};
+
+	serial at 70006300 {
+		status = "okay";
+		clock-frequency = <408000000>;
+	};
+
+	pmc {
+		nvidia,invert-interrupt;
+	};
+};
-- 
1.7.9.5

[v2 8/9] ARM: dt: tegra114: Add new board, Pluto

[Hiroshi Doyu]

Add a new evaluation board, Pluto for Tegra 114 family.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/boot/dts/Makefile           |    3 ++-
 arch/arm/boot/dts/tegra114-pluto.dts |   21 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/tegra114-pluto.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 88ea9fc..b53f18f 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -144,7 +144,8 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
 	tegra20-whistler.dtb \
 	tegra30-cardhu-a02.dtb \
 	tegra30-cardhu-a04.dtb \
-	tegra114-dalmore.dtb
+	tegra114-dalmore.dtb \
+	tegra114-pluto.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2p-ca5s.dtb \
 	vexpress-v2p-ca9.dtb \
 	vexpress-v2p-ca15-tc1.dtb \
diff --git a/arch/arm/boot/dts/tegra114-pluto.dts b/arch/arm/boot/dts/tegra114-pluto.dts
new file mode 100644
index 0000000..9bea8f5
--- /dev/null
+++ b/arch/arm/boot/dts/tegra114-pluto.dts
@@ -0,0 +1,21 @@
+/dts-v1/;
+
+/include/ "tegra114.dtsi"
+
+/ {
+	model = "NVIDIA Tegra114 Pluto evaluation board";
+	compatible = "nvidia,pluto", "nvidia,tegra114";
+
+	memory {
+		reg = <0x80000000 0x40000000>;
+	};
+
+	serial at 70006300 {
+		status = "okay";
+		clock-frequency = <408000000>;
+	};
+
+	pmc {
+		nvidia,invert-interrupt;
+	};
+};
-- 
1.7.9.5

[v2 9/9] ARM: tegra: Add initial support for Tegra 114 SoC.

[Hiroshi Doyu]

Add new Tegra 114 SoC support.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/Kconfig             |   10 +++++++
 arch/arm/mach-tegra/Makefile            |    1 +
 arch/arm/mach-tegra/board-dt-tegra114.c |   48 +++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/common.c            |    1 +
 4 files changed, 60 insertions(+)
 create mode 100644 arch/arm/mach-tegra/board-dt-tegra114.c

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index dd8ff7b..f2f9be1 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -61,6 +61,16 @@ config ARCH_TEGRA_3x_SOC
 	  Support for NVIDIA Tegra T30 processor family, based on the
 	  ARM CortexA9MP CPU and the ARM PL310 L2 cache controller
 
+config ARCH_TEGRA_114_SOC
+	bool "Enable support for Tegra114 family"
+	select ARM_GIC
+	select CPU_V7
+	select ARM_L1_CACHE_SHIFT_6
+	select ARM_ARCH_TIMER
+	help
+	  Support for NVIDIA Tegra T114 processor family, based on the
+	  ARM CortexA15MP CPU
+
 config TEGRA_PCI
 	bool "PCI Express support"
 	depends on ARCH_TEGRA_2x_SOC
diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index 8a108ef..3d6f645 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_TEGRA_PCI)			+= pcie.o
 
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-dt-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= board-dt-tegra30.o
+obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= board-dt-tegra114.o
 
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= board-harmony-pcie.o
 
diff --git a/arch/arm/mach-tegra/board-dt-tegra114.c b/arch/arm/mach-tegra/board-dt-tegra114.c
new file mode 100644
index 0000000..4c36dd0
--- /dev/null
+++ b/arch/arm/mach-tegra/board-dt-tegra114.c
@@ -0,0 +1,48 @@
+/*
+ * NVIDIA Tegra114 device tree board support
+ *
+ * Copyright (C) 2012 NVIDIA Corporation
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/clocksource.h>
+
+#include <asm/mach/arch.h>
+#include <asm/hardware/gic.h>
+
+#include "board.h"
+#include "common.h"
+
+static void __init tegra114_dt_init(void)
+{
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+}
+
+static const char * const tegra114_dt_board_compat[] = {
+	"nvidia,tegra114",
+	NULL,
+};
+
+DT_MACHINE_START(TEGRA114_DT, "NVIDIA Tegra114 (Flattened Device Tree)")
+	.smp		= smp_ops(tegra_smp_ops),
+	.map_io		= tegra_map_common_io,
+	.init_early	= tegra30_init_early,
+	.init_irq	= tegra_dt_init_irq,
+	.handle_irq	= gic_handle_irq,
+	.init_time	= clocksource_of_init,
+	.init_machine	= tegra114_dt_init,
+	.init_late	= tegra_init_late,
+	.restart	= tegra_assert_system_reset,
+	.dt_compat	= tegra114_dt_board_compat,
+MACHINE_END
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index d54cfc5..debcaf4 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -59,6 +59,7 @@ u32 tegra_uart_config[4] = {
 #ifdef CONFIG_OF
 static const struct of_device_id tegra_dt_irq_match[] __initconst = {
 	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init },
+	{ .compatible = "arm,cortex-a15-gic", .data = gic_of_init },
 	{ }
 };
 
-- 
1.7.9.5

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Russell King - ARM Linux]

On Tue, Jan 08, 2013 at 02:47:37PM +0200, Hiroshi Doyu wrote:
> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> have to read it from the system coprocessor(CP15), because the SCU on
> Cortex-A15 MPCore does not have software readable registers. This
> patch selects the correct method at runtime based on the CPU ID.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> index 1b926df..68e76ef 100644
> --- a/arch/arm/mach-tegra/platsmp.c
> +++ b/arch/arm/mach-tegra/platsmp.c
> @@ -23,6 +23,7 @@
>  #include <asm/hardware/gic.h>
>  #include <asm/mach-types.h>
>  #include <asm/smp_scu.h>
> +#include <asm/cputype.h>
>  
>  #include <mach/powergate.h>
>  
> @@ -34,9 +35,13 @@
>  #include "common.h"
>  #include "iomap.h"
>  
> +#define CPU_MASK		0xff0ffff0
> +#define CPU_CORTEX_A9		0x410fc090
> +#define CPU_CORTEX_A15		0x410fc0f0

NAK. There's some patches around to make this stuff generic, we don't
need more ifdefs springing up.  We need to get those generic patches
in.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Mark Rutland]

Hello,

On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> have to read it from the system coprocessor(CP15), because the SCU on
> Cortex-A15 MPCore does not have software readable registers. This
> patch selects the correct method at runtime based on the CPU ID.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> index 1b926df..68e76ef 100644
> --- a/arch/arm/mach-tegra/platsmp.c
> +++ b/arch/arm/mach-tegra/platsmp.c
> @@ -23,6 +23,7 @@
>  #include <asm/hardware/gic.h>
>  #include <asm/mach-types.h>
>  #include <asm/smp_scu.h>
> +#include <asm/cputype.h>
>  
>  #include <mach/powergate.h>
>  
> @@ -34,9 +35,13 @@
>  #include "common.h"
>  #include "iomap.h"
>  
> +#define CPU_MASK		0xff0ffff0
> +#define CPU_CORTEX_A9		0x410fc090
> +#define CPU_CORTEX_A15		0x410fc0f0
> +
>  extern void tegra_secondary_startup(void);
>  
> -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> +static void __iomem *scu_base;
>  
>  #define EVP_CPU_RESET_VECTOR \
>  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> @@ -149,7 +154,26 @@ done:
>   */
>  static void __init tegra_smp_init_cpus(void)
>  {
> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> +	unsigned int i, cpu_id, ncores;
> +	u32 l2ctlr;
> +	phys_addr_t pa;
> +
> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> +	switch (cpu_id) {
> +	case CPU_CORTEX_A15:
> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> +		break;

[...]

As mentioned last time [1], you should get this information from the dt
instead.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/138319.html

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Hi Mark,

Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:

> Hello,
> 
> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > have to read it from the system coprocessor(CP15), because the SCU on
> > Cortex-A15 MPCore does not have software readable registers. This
> > patch selects the correct method at runtime based on the CPU ID.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
> >  1 file changed, 28 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > index 1b926df..68e76ef 100644
> > --- a/arch/arm/mach-tegra/platsmp.c
> > +++ b/arch/arm/mach-tegra/platsmp.c
> > @@ -23,6 +23,7 @@
> >  #include <asm/hardware/gic.h>
> >  #include <asm/mach-types.h>
> >  #include <asm/smp_scu.h>
> > +#include <asm/cputype.h>
> >  
> >  #include <mach/powergate.h>
> >  
> > @@ -34,9 +35,13 @@
> >  #include "common.h"
> >  #include "iomap.h"
> >  
> > +#define CPU_MASK		0xff0ffff0
> > +#define CPU_CORTEX_A9		0x410fc090
> > +#define CPU_CORTEX_A15		0x410fc0f0
> > +
> >  extern void tegra_secondary_startup(void);
> >  
> > -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> > +static void __iomem *scu_base;
> >  
> >  #define EVP_CPU_RESET_VECTOR \
> >  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> > @@ -149,7 +154,26 @@ done:
> >   */
> >  static void __init tegra_smp_init_cpus(void)
> >  {
> > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > +	unsigned int i, cpu_id, ncores;
> > +	u32 l2ctlr;
> > +	phys_addr_t pa;
> > +
> > +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > +	switch (cpu_id) {
> > +	case CPU_CORTEX_A15:
> > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > +		break;
> 
> [...]
> 
> As mentioned last time [1], you should get this information from the dt
> instead.

Most of platsmp.c:.smp_init_cpus() implementations seem just to
overwrite # of cores by SCU/MRC detection. Is there any implementation
to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Marc Zyngier]

On 08/01/13 12:47, Hiroshi Doyu wrote:
> Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
> timer than TMR0. If it's available, it will be used for clock source
> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
> necessary for clock event.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>  .../bindings/arm/tegra/nvidia,tegra114-tsc.txt     |   11 ++++
>  drivers/clocksource/tegra20_timer.c                |   65 +++++++++++++++++++-
>  2 files changed, 75 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
> new file mode 100644
> index 0000000..9de936a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
> @@ -0,0 +1,11 @@
> +NVIDIA Tegra Timer Stamp Counter(TSC)
> +
> +Required properties:
> +- compatible : "nvidia,tegra114-tsc
> +- reg : Should contain 1 register ranges(address and length)
> +
> +Example:
> +	tsc {
> +		compatible = "nvidia,tegra114-tsc";
> +		reg = <0x700f0000 0x20000>;
> +	};
> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
> index 1d25de8..564266d 100644
> --- a/drivers/clocksource/tegra20_timer.c
> +++ b/drivers/clocksource/tegra20_timer.c
> @@ -30,6 +30,7 @@
>  #include <asm/mach/time.h>
>  #include <asm/smp_twd.h>
>  #include <asm/sched_clock.h>
> +#include <asm/arch_timer.h>
>  
>  #define RTC_SECONDS            0x08
>  #define RTC_SHADOW_SECONDS     0x0c
> @@ -271,10 +272,72 @@ static void __init tegra20_init_tmr(void)
>  	clockevents_register_device(&tegra_clockevent);
>  }
>  
> +#define TSC_CNTCR		0		/* TSC control registers */
> +#define TSC_CNTCR_ENABLE	(1 << 0)	/* Enable */
> +#define TSC_CNTCR_HDBG		(1 << 1)	/* Halt on debug */
> +
> +#define TSC_CNTCV0		0x8		/* TSC counter (LSW) */
> +#define TSC_CNTCV1		0xc		/* TSC counter (MSW) */
> +#define TSC_CNTFID0		0x20		/* TSC freq id 0 */
> +
> +static const struct of_device_id tegra_tsc_match[] __initconst = {
> +	{ .compatible = "nvidia,tegra114-tsc" },
> +	{}
> +};
> +
> +/* FIXME: only secure mode is supported. */

And this is a bug, as far as I'm concerned.

> +static int tegra_arch_timer_init(void)
> +{
> +	int err;
> +	struct device_node *np;
> +	struct clk *clk;
> +	void __iomem *tsc_base;
> +	u32 freq, val;
> +
> +	np = of_find_matching_node(NULL, tegra_tsc_match);
> +	if (!np)
> +		return -ENODEV;
> +
> +	tsc_base = of_iomap(np, 0);
> +	if (!tsc_base)
> +		return -ENODEV;
> +
> +	clk = clk_get_sys("clk_m", NULL);
> +	if (IS_ERR(clk)) {
> +		freq = 12000000;
> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
> +	} else {
> +		freq = clk_get_rate(clk);
> +		clk_put(clk);
> +	}
> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
> +
> +	/* CNTFRQ */
> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
> +	BUG_ON(val != freq);

No, not again! Like I said last year, this won't fly. So instead of
trying to work around a broken firmware, let's do the right thing.

> +
> +	val = readl_relaxed(tsc_base + TSC_CNTCR);
> +	val |= TSC_CNTCR_ENABLE | TSC_CNTCR_HDBG;
> +	writel_relaxed(val, tsc_base + TSC_CNTCR);
> +
> +	err = arch_timer_of_register();

What about adding an optional property to the binding, pointing to the
required clock? That would solve the above problem in a sensible way,
and your kernel wouldn't go bust.

	M.
-- 
Jazz is not dead. It just smells funny...

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Mark Rutland]

On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> Hi Mark,
> 
> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
> 
> > Hello,
> > 
> > On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> > > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > > have to read it from the system coprocessor(CP15), because the SCU on
> > > Cortex-A15 MPCore does not have software readable registers. This
> > > patch selects the correct method at runtime based on the CPU ID.
> > > 
> > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > ---
> > >  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
> > >  1 file changed, 28 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > > index 1b926df..68e76ef 100644
> > > --- a/arch/arm/mach-tegra/platsmp.c
> > > +++ b/arch/arm/mach-tegra/platsmp.c
> > > @@ -23,6 +23,7 @@
> > >  #include <asm/hardware/gic.h>
> > >  #include <asm/mach-types.h>
> > >  #include <asm/smp_scu.h>
> > > +#include <asm/cputype.h>
> > >  
> > >  #include <mach/powergate.h>
> > >  
> > > @@ -34,9 +35,13 @@
> > >  #include "common.h"
> > >  #include "iomap.h"
> > >  
> > > +#define CPU_MASK		0xff0ffff0
> > > +#define CPU_CORTEX_A9		0x410fc090
> > > +#define CPU_CORTEX_A15		0x410fc0f0
> > > +
> > >  extern void tegra_secondary_startup(void);
> > >  
> > > -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> > > +static void __iomem *scu_base;
> > >  
> > >  #define EVP_CPU_RESET_VECTOR \
> > >  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> > > @@ -149,7 +154,26 @@ done:
> > >   */
> > >  static void __init tegra_smp_init_cpus(void)
> > >  {
> > > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > > +	unsigned int i, cpu_id, ncores;
> > > +	u32 l2ctlr;
> > > +	phys_addr_t pa;
> > > +
> > > +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > > +	switch (cpu_id) {
> > > +	case CPU_CORTEX_A15:
> > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > +		break;
> > 
> > [...]
> > 
> > As mentioned last time [1], you should get this information from the dt
> > instead.
> 
> Most of platsmp.c:.smp_init_cpus() implementations seem just to
> overwrite # of cores by SCU/MRC detection. Is there any implementation
> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?

As far as I can see, there's no other platform which just relies on
arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
be handled by arm_dt_init_cpus.

I think the best option would be to have a separate smp_ops for your dt
platforms where we know cpu nodes are populated (e.g. Tegra 114), where
smp_init_cpus is different to that for non-dt platforms. That way non dt
platforms can keep the SCU hack for now, and won't be broken, and the dt
platforms are far removed from the SCU hack and just use common infrastructure.

Maybe someone else has a better idea?

Thanks,
Mark.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Tue, Jan 08, 2013 at 04:21:38PM +0000, Mark Rutland wrote:
> On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:

[...]

> > > >  static void __init tegra_smp_init_cpus(void)
> > > >  {
> > > > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > > > +	unsigned int i, cpu_id, ncores;
> > > > +	u32 l2ctlr;
> > > > +	phys_addr_t pa;
> > > > +
> > > > +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > > > +	switch (cpu_id) {
> > > > +	case CPU_CORTEX_A15:
> > > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > > +		break;
> > > 
> > > [...]
> > > 
> > > As mentioned last time [1], you should get this information from the dt
> > > instead.
> > 
> > Most of platsmp.c:.smp_init_cpus() implementations seem just to
> > overwrite # of cores by SCU/MRC detection. Is there any implementation
> > to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> 
> As far as I can see, there's no other platform which just relies on
> arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> be handled by arm_dt_init_cpus.
> 
> I think the best option would be to have a separate smp_ops for your dt
> platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> smp_init_cpus is different to that for non-dt platforms. That way non dt
> platforms can keep the SCU hack for now, and won't be broken, and the dt
> platforms are far removed from the SCU hack and just use common infrastructure.
> 
> Maybe someone else has a better idea?

Have a look at mach-vexpress/platsmp.c (keeping in mind that the
GENERIC_SCU case will be refactored/removed since arm_dt_init_cpu_maps() is
doing most of the required steps), please let me know what you think.

Thanks,
Lorenzo

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Stephen Warren]

On 01/08/2013 09:21 AM, Mark Rutland wrote:
> On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
>> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
>>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
>>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
>>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
>>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
>>>> have to read it from the system coprocessor(CP15), because the SCU on
>>>> Cortex-A15 MPCore does not have software readable registers. This
>>>> patch selects the correct method at runtime based on the CPU ID.
...
>>>>  static void __init tegra_smp_init_cpus(void)
>>>>  {
>>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
>>>> +	unsigned int i, cpu_id, ncores;
>>>> +	u32 l2ctlr;
>>>> +	phys_addr_t pa;
>>>> +
>>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
>>>> +	switch (cpu_id) {
>>>> +	case CPU_CORTEX_A15:
>>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
>>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
>>>> +		break;
>>>
>>> [...]
>>>
>>> As mentioned last time [1], you should get this information from the dt
>>> instead.
>>
>> Most of platsmp.c:.smp_init_cpus() implementations seem just to
>> overwrite # of cores by SCU/MRC detection. Is there any implementation
>> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> 
> As far as I can see, there's no other platform which just relies on
> arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> be handled by arm_dt_init_cpus.
> 
> I think the best option would be to have a separate smp_ops for your dt
> platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> smp_init_cpus is different to that for non-dt platforms. That way non dt
> platforms can keep the SCU hack for now, and won't be broken, and the dt
> platforms are far removed from the SCU hack and just use common infrastructure.

Tegra doesn't have any non-DT support now.

If we're going to make Tegra114 read the CPU information from DT, then
I'd rather just switch over all Tegra platforms to requiring CPU nodes
in the DT. We have few enough SoC variants that it should be easy to
switch everything to DT before adding Tegra114 support without causing
any problems.

But that all said, I'm not convinced it's a good idea to force the
information to be present in DT when it's just duplicating stuff that
can be runtime-probed from the HW...

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Stephen Warren]

On 01/08/2013 09:07 AM, Marc Zyngier wrote:
> On 08/01/13 12:47, Hiroshi Doyu wrote:
>> Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
>> timer than TMR0. If it's available, it will be used for clock source
>> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
>> necessary for clock event.

>> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c

>> +/* FIXME: only secure mode is supported. */
> 
> And this is a bug, as far as I'm concerned.
> 
>> +static int tegra_arch_timer_init(void)

>> +	clk = clk_get_sys("clk_m", NULL);
>> +	if (IS_ERR(clk)) {
>> +		freq = 12000000;
>> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
>> +	} else {
>> +		freq = clk_get_rate(clk);
>> +		clk_put(clk);
>> +	}
>> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
>> +
>> +	/* CNTFRQ */
>> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
>> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
>> +	BUG_ON(val != freq);
> 
> No, not again! Like I said last year, this won't fly. So instead of
> trying to work around a broken firmware, let's do the right thing.

OK, so I understand you want to firmware/bootloader/... to write the
value into that register instead, so this all works in non-secure mode
(which sounds like a fine objection), but ...

>> +
>> +	val = readl_relaxed(tsc_base + TSC_CNTCR);
>> +	val |= TSC_CNTCR_ENABLE | TSC_CNTCR_HDBG;
>> +	writel_relaxed(val, tsc_base + TSC_CNTCR);
>> +
>> +	err = arch_timer_of_register();
> 
> What about adding an optional property to the binding, pointing to the
> required clock? That would solve the above problem in a sensible way,
> and your kernel wouldn't go bust.

... I'm not sure how the comment about adding a clock to the DT binding
is related to that. Sorry for not following the plot!

[v2 6/9] ARM: dt: tegra114: Add new SoC base, Tegra 114 SoC

[Stephen Warren]

On 01/08/2013 05:47 AM, Hiroshi Doyu wrote:
> Initial support for Tegra 114 SoC. This is expected to be included in
> the board DTS files, Tegra 114 SoC based evaluation board family.

> diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi

> +	gic: interrupt-controller {
> +		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
> +		reg = <0x50041000 0x1000
> +		       0x50042000 0x1000>;
> +		interrupt-controller;
> +		#interrupt-cells = <3>;
> +	};

Last time around, Marc Zyngier asked:

> If this is indeed an A15 GIC, how about adding the GICH and GICV
> regions, as well as the VGIC maintenance interrupt?

[v2 9/9] ARM: tegra: Add initial support for Tegra 114 SoC.

[Stephen Warren]

On 01/08/2013 05:47 AM, Hiroshi Doyu wrote:
> Add new Tegra 114 SoC support.

> diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c

>  static const struct of_device_id tegra_dt_irq_match[] __initconst = {
>  	{ .compatible = "arm,cortex-a9-gic", .data = gic_of_init },
> +	{ .compatible = "arm,cortex-a15-gic", .data = gic_of_init },
>  	{ }
>  };

It'd probably be a good idea to add newer entries at the start of this
table. Not so much an issue here perhaps, but at least when
instantiating drivers, the earlier entries match first irrespective of
the order of compatible values in the DT node, due to a misfeature in
the kernel code, which while it's in the process of being fixed, we
probably still want to work around.

[v2 2/9] HACK: ARM: tegra: Use CLK_IGNORE_UNUSED for Tegra 114 SoC

[Stephen Warren]

On 01/08/2013 05:47 AM, Hiroshi Doyu wrote:
> Use CLK_IGNORE_UNUSED for the Tegra 114 SoC to ensure
> clk_disable_unused() is not called. Otherwise the system will die,
> because the usecount of the clocks is incorrect. This patch will be
> reverted once the Tegra 114 clocks are implemented.

> diff --git a/arch/arm/mach-tegra/tegra30_clocks_data.c b/arch/arm/mach-tegra/tegra30_clocks_data.c

Just as an FYI, I'll almost certainly apply Prashant's common clock
conversion before this series, so this patch will need some rework to be
applied on top of that.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Russell King - ARM Linux <linux@arm.linux.org.uk> wrote @ Tue, 8 Jan 2013 15:26:51 +0100:

> On Tue, Jan 08, 2013 at 02:47:37PM +0200, Hiroshi Doyu wrote:
> > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > have to read it from the system coprocessor(CP15), because the SCU on
> > Cortex-A15 MPCore does not have software readable registers. This
> > patch selects the correct method at runtime based on the CPU ID.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
> >  1 file changed, 28 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > index 1b926df..68e76ef 100644
> > --- a/arch/arm/mach-tegra/platsmp.c
> > +++ b/arch/arm/mach-tegra/platsmp.c
> > @@ -23,6 +23,7 @@
> >  #include <asm/hardware/gic.h>
> >  #include <asm/mach-types.h>
> >  #include <asm/smp_scu.h>
> > +#include <asm/cputype.h>
> >  
> >  #include <mach/powergate.h>
> >  
> > @@ -34,9 +35,13 @@
> >  #include "common.h"
> >  #include "iomap.h"
> >  
> > +#define CPU_MASK		0xff0ffff0
> > +#define CPU_CORTEX_A9		0x410fc090
> > +#define CPU_CORTEX_A15		0x410fc0f0
> 
> NAK. There's some patches around to make this stuff generic, we don't
> need more ifdefs springing up.  We need to get those generic patches
> in.

Does anyone give some pointers to the above patches?

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Hi,

Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 20:32:32 +0100:

> On 01/08/2013 09:21 AM, Mark Rutland wrote:
> > On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> >> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
> >>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> >>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> >>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> >>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> >>>> have to read it from the system coprocessor(CP15), because the SCU on
> >>>> Cortex-A15 MPCore does not have software readable registers. This
> >>>> patch selects the correct method at runtime based on the CPU ID.
> ...
> >>>>  static void __init tegra_smp_init_cpus(void)
> >>>>  {
> >>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> >>>> +	unsigned int i, cpu_id, ncores;
> >>>> +	u32 l2ctlr;
> >>>> +	phys_addr_t pa;
> >>>> +
> >>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> >>>> +	switch (cpu_id) {
> >>>> +	case CPU_CORTEX_A15:
> >>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> >>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> >>>> +		break;
> >>>
> >>> [...]
> >>>
> >>> As mentioned last time [1], you should get this information from the dt
> >>> instead.
> >>
> >> Most of platsmp.c:.smp_init_cpus() implementations seem just to
> >> overwrite # of cores by SCU/MRC detection. Is there any implementation
> >> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> > 
> > As far as I can see, there's no other platform which just relies on
> > arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> > sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> > to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> > be handled by arm_dt_init_cpus.

True.

> > I think the best option would be to have a separate smp_ops for your dt
> > platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> > smp_init_cpus is different to that for non-dt platforms. That way non dt
> > platforms can keep the SCU hack for now, and won't be broken, and the dt
> > platforms are far removed from the SCU hack and just use common
> > infrastructure.
>
> Tegra doesn't have any non-DT support now.

What about falling down to SCU/MRC hack only when DT detection fails
or no /cpus entry in DT?

Can arm_dt_init_cpu_maps() return if it suceeds or not?

> 
> If we're going to make Tegra114 read the CPU information from DT, then
> I'd rather just switch over all Tegra platforms to requiring CPU nodes
> in the DT. We have few enough SoC variants that it should be easy to
> switch everything to DT before adding Tegra114 support without causing
> any problems.

Possible.

> But that all said, I'm not convinced it's a good idea to force the
> information to be present in DT when it's just duplicating stuff that
> can be runtime-probed from the HW...

That's also my concern somewhat.

The main purpose to DT CPU core detection is for multiple CPU
clusters. SCU/MRC cannot know anything outside of their own
clusters. In tegra, we have 4 Cortex-A9/A15 core plus 1 shadow
core. Switching those clusters is done transparently. The last A9/A15
core is transparently switched back and forth to the shadow one. So in
this case, SCU/MRC detection seems to work fine.

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Hiroshi Doyu]

Hi Marc,

Marc Zyngier <marc.zyngier@arm.com> wrote @ Tue, 8 Jan 2013 17:07:39 +0100:

> On 08/01/13 12:47, Hiroshi Doyu wrote:
> > Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
> > timer than TMR0. If it's available, it will be used for clock source
> > and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
> > necessary for clock event.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  .../bindings/arm/tegra/nvidia,tegra114-tsc.txt     |   11 ++++
> >  drivers/clocksource/tegra20_timer.c                |   65 +++++++++++++++++++-
> >  2 files changed, 75 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
> > new file mode 100644
> > index 0000000..9de936a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
> > @@ -0,0 +1,11 @@
> > +NVIDIA Tegra Timer Stamp Counter(TSC)
> > +
> > +Required properties:
> > +- compatible : "nvidia,tegra114-tsc
> > +- reg : Should contain 1 register ranges(address and length)
> > +
> > +Example:
> > +	tsc {
> > +		compatible = "nvidia,tegra114-tsc";
> > +		reg = <0x700f0000 0x20000>;
> > +	};
> > diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
> > index 1d25de8..564266d 100644
> > --- a/drivers/clocksource/tegra20_timer.c
> > +++ b/drivers/clocksource/tegra20_timer.c
> > @@ -30,6 +30,7 @@
> >  #include <asm/mach/time.h>
> >  #include <asm/smp_twd.h>
> >  #include <asm/sched_clock.h>
> > +#include <asm/arch_timer.h>
> >  
> >  #define RTC_SECONDS            0x08
> >  #define RTC_SHADOW_SECONDS     0x0c
> > @@ -271,10 +272,72 @@ static void __init tegra20_init_tmr(void)
> >  	clockevents_register_device(&tegra_clockevent);
> >  }
> >  
> > +#define TSC_CNTCR		0		/* TSC control registers */
> > +#define TSC_CNTCR_ENABLE	(1 << 0)	/* Enable */
> > +#define TSC_CNTCR_HDBG		(1 << 1)	/* Halt on debug */
> > +
> > +#define TSC_CNTCV0		0x8		/* TSC counter (LSW) */
> > +#define TSC_CNTCV1		0xc		/* TSC counter (MSW) */
> > +#define TSC_CNTFID0		0x20		/* TSC freq id 0 */
> > +
> > +static const struct of_device_id tegra_tsc_match[] __initconst = {
> > +	{ .compatible = "nvidia,tegra114-tsc" },
> > +	{}
> > +};
> > +
> > +/* FIXME: only secure mode is supported. */
> 
> And this is a bug, as far as I'm concerned.
> 
> > +static int tegra_arch_timer_init(void)
> > +{
> > +	int err;
> > +	struct device_node *np;
> > +	struct clk *clk;
> > +	void __iomem *tsc_base;
> > +	u32 freq, val;
> > +
> > +	np = of_find_matching_node(NULL, tegra_tsc_match);
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	tsc_base = of_iomap(np, 0);
> > +	if (!tsc_base)
> > +		return -ENODEV;
> > +
> > +	clk = clk_get_sys("clk_m", NULL);
> > +	if (IS_ERR(clk)) {
> > +		freq = 12000000;
> > +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
> > +	} else {
> > +		freq = clk_get_rate(clk);
> > +		clk_put(clk);
> > +	}
> > +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
> > +
> > +	/* CNTFRQ */
> > +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
> > +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
> > +	BUG_ON(val != freq);
> 
> No, not again! Like I said last year, this won't fly. So instead of
> trying to work around a broken firmware, let's do the right thing.
> 
> > +
> > +	val = readl_relaxed(tsc_base + TSC_CNTCR);
> > +	val |= TSC_CNTCR_ENABLE | TSC_CNTCR_HDBG;
> > +	writel_relaxed(val, tsc_base + TSC_CNTCR);
> > +
> > +	err = arch_timer_of_register();
> 
> What about adding an optional property to the binding, pointing to the
> required clock? That would solve the above problem in a sensible way,
> and your kernel wouldn't go bust.

Could you explain a bit more? We have the follwing DT entries related to this:

	tsc {
		compatible = "nvidia,tegra114-tsc";
		reg = <0x700f0000 0x20000>;
	};

	...

	timer {
		compatible = "arm,armv7-timer";
		interrupts = <1 13 0xf08>,
			     <1 14 0xf08>,
			     <1 11 0xf08>,
			     <1 10 0xf08>;
	};

The above "tsc" is needed for platform specific enabler for ARM arch timer.

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 23:41:42 +0100:

> On 01/08/2013 09:07 AM, Marc Zyngier wrote:
> > On 08/01/13 12:47, Hiroshi Doyu wrote:
> >> Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
> >> timer than TMR0. If it's available, it will be used for clock source
> >> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
> >> necessary for clock event.
> 
> >> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
> 
> >> +/* FIXME: only secure mode is supported. */
> > 
> > And this is a bug, as far as I'm concerned.
> > 
> >> +static int tegra_arch_timer_init(void)
> 
> >> +	clk = clk_get_sys("clk_m", NULL);
> >> +	if (IS_ERR(clk)) {
> >> +		freq = 12000000;
> >> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
> >> +	} else {
> >> +		freq = clk_get_rate(clk);
> >> +		clk_put(clk);
> >> +	}
> >> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
> >> +
> >> +	/* CNTFRQ */
> >> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
> >> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
> >> +	BUG_ON(val != freq);
> > 
> > No, not again! Like I said last year, this won't fly. So instead of
> > trying to work around a broken firmware, let's do the right thing.
> 
> OK, so I understand you want to firmware/bootloader/... to write the
> value into that register instead, so this all works in non-secure mode
> (which sounds like a fine objection), but ...

If possible, I want to run kernel without bootloader initializing TSC
. IOW, I want to allow legacy bootloaders to boot kernel with ARM arch
timer.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Joseph Lo]

On Wed, 2013-01-09 at 13:46 +0800, Hiroshi Doyu wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote @ Tue, 8 Jan 2013 15:26:51 +0100:
> 
> > On Tue, Jan 08, 2013 at 02:47:37PM +0200, Hiroshi Doyu wrote:
> > > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > > have to read it from the system coprocessor(CP15), because the SCU on
> > > Cortex-A15 MPCore does not have software readable registers. This
> > > patch selects the correct method at runtime based on the CPU ID.
> > > 
> > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > ---
> > >  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
> > >  1 file changed, 28 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > > index 1b926df..68e76ef 100644
> > > --- a/arch/arm/mach-tegra/platsmp.c
> > > +++ b/arch/arm/mach-tegra/platsmp.c
> > > @@ -23,6 +23,7 @@
> > >  #include <asm/hardware/gic.h>
> > >  #include <asm/mach-types.h>
> > >  #include <asm/smp_scu.h>
> > > +#include <asm/cputype.h>
> > >  
> > >  #include <mach/powergate.h>
> > >  
> > > @@ -34,9 +35,13 @@
> > >  #include "common.h"
> > >  #include "iomap.h"
> > >  
> > > +#define CPU_MASK		0xff0ffff0
> > > +#define CPU_CORTEX_A9		0x410fc090
> > > +#define CPU_CORTEX_A15		0x410fc0f0
> > 
> > NAK. There's some patches around to make this stuff generic, we don't
> > need more ifdefs springing up.  We need to get those generic patches
> > in.
> 
> Does anyone give some pointers to the above patches?

Hi Hiroshi,

Please check "arch/arm/include/asm/cputype.h" in next-20130109. It just
be merged.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Hi Joseph,

Joseph Lo <josephl@nvidia.com> wrote @ Wed, 9 Jan 2013 07:07:41 +0100:

> On Wed, 2013-01-09 at 13:46 +0800, Hiroshi Doyu wrote:
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote @ Tue, 8 Jan 2013 15:26:51 +0100:
> > 
> > > On Tue, Jan 08, 2013 at 02:47:37PM +0200, Hiroshi Doyu wrote:
> > > > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > > > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > > > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > > > have to read it from the system coprocessor(CP15), because the SCU on
> > > > Cortex-A15 MPCore does not have software readable registers. This
> > > > patch selects the correct method at runtime based on the CPU ID.
> > > > 
> > > > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > > ---
> > > >  arch/arm/mach-tegra/platsmp.c |   31 ++++++++++++++++++++++++++++---
> > > >  1 file changed, 28 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > > > index 1b926df..68e76ef 100644
> > > > --- a/arch/arm/mach-tegra/platsmp.c
> > > > +++ b/arch/arm/mach-tegra/platsmp.c
> > > > @@ -23,6 +23,7 @@
> > > >  #include <asm/hardware/gic.h>
> > > >  #include <asm/mach-types.h>
> > > >  #include <asm/smp_scu.h>
> > > > +#include <asm/cputype.h>
> > > >  
> > > >  #include <mach/powergate.h>
> > > >  
> > > > @@ -34,9 +35,13 @@
> > > >  #include "common.h"
> > > >  #include "iomap.h"
> > > >  
> > > > +#define CPU_MASK		0xff0ffff0
> > > > +#define CPU_CORTEX_A9		0x410fc090
> > > > +#define CPU_CORTEX_A15		0x410fc0f0
> > > 
> > > NAK. There's some patches around to make this stuff generic, we don't
> > > need more ifdefs springing up.  We need to get those generic patches
> > > in.
> > 
> > Does anyone give some pointers to the above patches?
> 
> Hi Hiroshi,
> 
> Please check "arch/arm/include/asm/cputype.h" in next-20130109. It just
> be merged.

I found the following commit. Thanks.

commit cd8dde6fc347b56b6cb9afa7cc0f9073da163176
Author: Christoffer Dall <c.dall@virtualopensystems.com>
Date:   Tue Dec 18 04:06:37 2012 +0000

    ARM: Define CPU part numbers and implementors
    
    Define implementor IDs, part numbers and Xscale architecture versions in
    cputype.h.  Also create accessor functions for reading the implementor,
    part number, and Xscale architecture versions from the CPUID regiser.
    
    Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com>
    Signed-off-by: Will Deacon <will.deacon@arm.com>

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Stephen Warren]

On 01/08/2013 11:00 PM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 23:41:42 +0100:
>> On 01/08/2013 09:07 AM, Marc Zyngier wrote:
>>> On 08/01/13 12:47, Hiroshi Doyu wrote:
>>>> Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
>>>> timer than TMR0. If it's available, it will be used for clock source
>>>> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
>>>> necessary for clock event.
>>
>>>> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
>>
>>>> +/* FIXME: only secure mode is supported. */
>>>
>>> And this is a bug, as far as I'm concerned.
>>>
>>>> +static int tegra_arch_timer_init(void)
>>
>>>> +	clk = clk_get_sys("clk_m", NULL);
>>>> +	if (IS_ERR(clk)) {
>>>> +		freq = 12000000;
>>>> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
>>>> +	} else {
>>>> +		freq = clk_get_rate(clk);
>>>> +		clk_put(clk);
>>>> +	}
>>>> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
>>>> +
>>>> +	/* CNTFRQ */
>>>> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
>>>> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
>>>> +	BUG_ON(val != freq);
>>>
>>> No, not again! Like I said last year, this won't fly. So instead of
>>> trying to work around a broken firmware, let's do the right thing.
>>
>> OK, so I understand you want to firmware/bootloader/... to write the
>> value into that register instead, so this all works in non-secure mode
>> (which sounds like a fine objection), but ...
> 
> If possible, I want to run kernel without bootloader initializing TSC
> . IOW, I want to allow legacy bootloaders to boot kernel with ARM arch
> timer.

Is it really hard to just fix the bootloader? Tegra114 is new enough,
and upstream support new enough, it should be fine to require a fixed
bootloader in order to run the upstream kernel even if we don't yet do
this right in the downstream kernels.

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Wed, 9 Jan 2013 07:40:49 +0100:

> On 01/08/2013 11:00 PM, Hiroshi Doyu wrote:
> > Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 23:41:42 +0100:
> >> On 01/08/2013 09:07 AM, Marc Zyngier wrote:
> >>> On 08/01/13 12:47, Hiroshi Doyu wrote:
> >>>> Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
> >>>> timer than TMR0. If it's available, it will be used for clock source
> >>>> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
> >>>> necessary for clock event.
> >>
> >>>> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
> >>
> >>>> +/* FIXME: only secure mode is supported. */
> >>>
> >>> And this is a bug, as far as I'm concerned.
> >>>
> >>>> +static int tegra_arch_timer_init(void)
> >>
> >>>> +	clk = clk_get_sys("clk_m", NULL);
> >>>> +	if (IS_ERR(clk)) {
> >>>> +		freq = 12000000;
> >>>> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
> >>>> +	} else {
> >>>> +		freq = clk_get_rate(clk);
> >>>> +		clk_put(clk);
> >>>> +	}
> >>>> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
> >>>> +
> >>>> +	/* CNTFRQ */
> >>>> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
> >>>> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
> >>>> +	BUG_ON(val != freq);
> >>>
> >>> No, not again! Like I said last year, this won't fly. So instead of
> >>> trying to work around a broken firmware, let's do the right thing.
> >>
> >> OK, so I understand you want to firmware/bootloader/... to write the
> >> value into that register instead, so this all works in non-secure mode
> >> (which sounds like a fine objection), but ...
> > 
> > If possible, I want to run kernel without bootloader initializing TSC
> > . IOW, I want to allow legacy bootloaders to boot kernel with ARM arch
> > timer.
> 
> Is it really hard to just fix the bootloader? Tegra114 is new enough,
> and upstream support new enough, it should be fine to require a fixed
> bootloader in order to run the upstream kernel even if we don't yet do
> this right in the downstream kernels.

Ok, I'll drop this part. Still kernel can boot up.

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Santosh Shilimkar]

On Wednesday 09 January 2013 11:27 AM, Hiroshi Doyu wrote:
> Hi Marc,
>
> Marc Zyngier <marc.zyngier@arm.com> wrote @ Tue, 8 Jan 2013 17:07:39 +0100:
>
>> On 08/01/13 12:47, Hiroshi Doyu wrote:
>>> Add platform enabler for ARM arch_timer(TSC). TSC is more fine grained
>>> timer than TMR0. If it's available, it will be used for clock source
>>> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
>>> necessary for clock event.
>>>
>>> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
>>> ---
>>>   .../bindings/arm/tegra/nvidia,tegra114-tsc.txt     |   11 ++++
>>>   drivers/clocksource/tegra20_timer.c                |   65 +++++++++++++++++++-
>>>   2 files changed, 75 insertions(+), 1 deletion(-)
>>>   create mode 100644 Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>> new file mode 100644
>>> index 0000000..9de936a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>> @@ -0,0 +1,11 @@
>>> +NVIDIA Tegra Timer Stamp Counter(TSC)
>>> +
>>> +Required properties:
>>> +- compatible : "nvidia,tegra114-tsc
>>> +- reg : Should contain 1 register ranges(address and length)
>>> +
>>> +Example:
>>> +	tsc {
>>> +		compatible = "nvidia,tegra114-tsc";
>>> +		reg = <0x700f0000 0x20000>;
>>> +	};
>>> diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c
>>> index 1d25de8..564266d 100644
>>> --- a/drivers/clocksource/tegra20_timer.c
>>> +++ b/drivers/clocksource/tegra20_timer.c
>>> @@ -30,6 +30,7 @@
>>>   #include <asm/mach/time.h>
>>>   #include <asm/smp_twd.h>
>>>   #include <asm/sched_clock.h>
>>> +#include <asm/arch_timer.h>
>>>
>>>   #define RTC_SECONDS            0x08
>>>   #define RTC_SHADOW_SECONDS     0x0c
>>> @@ -271,10 +272,72 @@ static void __init tegra20_init_tmr(void)
>>>   	clockevents_register_device(&tegra_clockevent);
>>>   }
>>>
>>> +#define TSC_CNTCR		0		/* TSC control registers */
>>> +#define TSC_CNTCR_ENABLE	(1 << 0)	/* Enable */
>>> +#define TSC_CNTCR_HDBG		(1 << 1)	/* Halt on debug */
>>> +
>>> +#define TSC_CNTCV0		0x8		/* TSC counter (LSW) */
>>> +#define TSC_CNTCV1		0xc		/* TSC counter (MSW) */
>>> +#define TSC_CNTFID0		0x20		/* TSC freq id 0 */
>>> +
>>> +static const struct of_device_id tegra_tsc_match[] __initconst = {
>>> +	{ .compatible = "nvidia,tegra114-tsc" },
>>> +	{}
>>> +};
>>> +
>>> +/* FIXME: only secure mode is supported. */
>>
>> And this is a bug, as far as I'm concerned.
>>
>>> +static int tegra_arch_timer_init(void)
>>> +{
>>> +	int err;
>>> +	struct device_node *np;
>>> +	struct clk *clk;
>>> +	void __iomem *tsc_base;
>>> +	u32 freq, val;
>>> +
>>> +	np = of_find_matching_node(NULL, tegra_tsc_match);
>>> +	if (!np)
>>> +		return -ENODEV;
>>> +
>>> +	tsc_base = of_iomap(np, 0);
>>> +	if (!tsc_base)
>>> +		return -ENODEV;
>>> +
>>> +	clk = clk_get_sys("clk_m", NULL);
>>> +	if (IS_ERR(clk)) {
>>> +		freq = 12000000;
>>> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input clock.\n");
>>> +	} else {
>>> +		freq = clk_get_rate(clk);
>>> +		clk_put(clk);
>>> +	}
>>> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
>>> +
>>> +	/* CNTFRQ */
>>> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
>>> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
>>> +	BUG_ON(val != freq);
>>
>> No, not again! Like I said last year, this won't fly. So instead of
>> trying to work around a broken firmware, let's do the right thing.
>>
>>> +
>>> +	val = readl_relaxed(tsc_base + TSC_CNTCR);
>>> +	val |= TSC_CNTCR_ENABLE | TSC_CNTCR_HDBG;
>>> +	writel_relaxed(val, tsc_base + TSC_CNTCR);
>>> +
>>> +	err = arch_timer_of_register();
>>
>> What about adding an optional property to the binding, pointing to the
>> required clock? That would solve the above problem in a sensible way,
>> and your kernel wouldn't go bust.
>
> Could you explain a bit more? We have the follwing DT entries related to this:
>
> 	tsc {
> 		compatible = "nvidia,tegra114-tsc";
> 		reg = <0x700f0000 0x20000>;
> 	};
>
> 	...
>
> 	timer {
> 		compatible = "arm,armv7-timer";
> 		interrupts = <1 13 0xf08>,
> 			     <1 14 0xf08>,
> 			     <1 11 0xf08>,
> 			     <1 10 0xf08>;
> 	};
>
Arch timer frequency can be specified in DT since the code
already has provision to extract it. Below is extract
from OMAP5 DT code.

timer {
                                 compatible = "arm,armv7-timer";

                                 interrupts = <1 14 0x308>;
                                 clock-frequency = <6144000>;
                         };

I think thats what Marc is pointing out.

Regards
Santosh

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Marc Zyngier]

On Wed, 9 Jan 2013 13:13:11 +0530, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On Wednesday 09 January 2013 11:27 AM, Hiroshi Doyu wrote:
>> Hi Marc,
>>
>> Marc Zyngier <marc.zyngier@arm.com> wrote @ Tue, 8 Jan 2013 17:07:39
>> +0100:
>>
>>> On 08/01/13 12:47, Hiroshi Doyu wrote:
>>>> Add platform enabler for ARM arch_timer(TSC). TSC is more fine
grained
>>>> timer than TMR0. If it's available, it will be used for clock source
>>>> and sched_clock. Otherwise, TMR0 is used. In any case TMR0 is
>>>> necessary for clock event.
>>>>
>>>> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
>>>> ---
>>>>   .../bindings/arm/tegra/nvidia,tegra114-tsc.txt     |   11 ++++
>>>>   drivers/clocksource/tegra20_timer.c                |   65
>>>>   +++++++++++++++++++-
>>>>   2 files changed, 75 insertions(+), 1 deletion(-)
>>>>   create mode 100644
>>>>   Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>>> new file mode 100644
>>>> index 0000000..9de936a
>>>> --- /dev/null
>>>> +++
>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra114-tsc.txt
>>>> @@ -0,0 +1,11 @@
>>>> +NVIDIA Tegra Timer Stamp Counter(TSC)
>>>> +
>>>> +Required properties:
>>>> +- compatible : "nvidia,tegra114-tsc
>>>> +- reg : Should contain 1 register ranges(address and length)
>>>> +
>>>> +Example:
>>>> +	tsc {
>>>> +		compatible = "nvidia,tegra114-tsc";
>>>> +		reg = <0x700f0000 0x20000>;
>>>> +	};
>>>> diff --git a/drivers/clocksource/tegra20_timer.c
>>>> b/drivers/clocksource/tegra20_timer.c
>>>> index 1d25de8..564266d 100644
>>>> --- a/drivers/clocksource/tegra20_timer.c
>>>> +++ b/drivers/clocksource/tegra20_timer.c
>>>> @@ -30,6 +30,7 @@
>>>>   #include <asm/mach/time.h>
>>>>   #include <asm/smp_twd.h>
>>>>   #include <asm/sched_clock.h>
>>>> +#include <asm/arch_timer.h>
>>>>
>>>>   #define RTC_SECONDS            0x08
>>>>   #define RTC_SHADOW_SECONDS     0x0c
>>>> @@ -271,10 +272,72 @@ static void __init tegra20_init_tmr(void)
>>>>   	clockevents_register_device(&tegra_clockevent);
>>>>   }
>>>>
>>>> +#define TSC_CNTCR		0		/* TSC control registers */
>>>> +#define TSC_CNTCR_ENABLE	(1 << 0)	/* Enable */
>>>> +#define TSC_CNTCR_HDBG		(1 << 1)	/* Halt on debug */
>>>> +
>>>> +#define TSC_CNTCV0		0x8		/* TSC counter (LSW) */
>>>> +#define TSC_CNTCV1		0xc		/* TSC counter (MSW) */
>>>> +#define TSC_CNTFID0		0x20		/* TSC freq id 0 */
>>>> +
>>>> +static const struct of_device_id tegra_tsc_match[] __initconst = {
>>>> +	{ .compatible = "nvidia,tegra114-tsc" },
>>>> +	{}
>>>> +};
>>>> +
>>>> +/* FIXME: only secure mode is supported. */
>>>
>>> And this is a bug, as far as I'm concerned.
>>>
>>>> +static int tegra_arch_timer_init(void)
>>>> +{
>>>> +	int err;
>>>> +	struct device_node *np;
>>>> +	struct clk *clk;
>>>> +	void __iomem *tsc_base;
>>>> +	u32 freq, val;
>>>> +
>>>> +	np = of_find_matching_node(NULL, tegra_tsc_match);
>>>> +	if (!np)
>>>> +		return -ENODEV;
>>>> +
>>>> +	tsc_base = of_iomap(np, 0);
>>>> +	if (!tsc_base)
>>>> +		return -ENODEV;
>>>> +
>>>> +	clk = clk_get_sys("clk_m", NULL);
>>>> +	if (IS_ERR(clk)) {
>>>> +		freq = 12000000;
>>>> +		pr_warn("Unable to get timer clock. Assuming 12Mhz input
clock.\n");
>>>> +	} else {
>>>> +		freq = clk_get_rate(clk);
>>>> +		clk_put(clk);
>>>> +	}
>>>> +	writel_relaxed(freq, tsc_base + TSC_CNTFID0);
>>>> +
>>>> +	/* CNTFRQ */
>>>> +	asm("mcr p15, 0, %0, c14, c0, 0\n" : : "r" (freq));
>>>> +	asm("mrc p15, 0, %0, c14, c0, 0\n" : "=r" (val));
>>>> +	BUG_ON(val != freq);
>>>
>>> No, not again! Like I said last year, this won't fly. So instead of
>>> trying to work around a broken firmware, let's do the right thing.
>>>
>>>> +
>>>> +	val = readl_relaxed(tsc_base + TSC_CNTCR);
>>>> +	val |= TSC_CNTCR_ENABLE | TSC_CNTCR_HDBG;
>>>> +	writel_relaxed(val, tsc_base + TSC_CNTCR);
>>>> +
>>>> +	err = arch_timer_of_register();
>>>
>>> What about adding an optional property to the binding, pointing to the
>>> required clock? That would solve the above problem in a sensible way,
>>> and your kernel wouldn't go bust.
>>
>> Could you explain a bit more? We have the follwing DT entries related
to
>> this:
>>
>> 	tsc {
>> 		compatible = "nvidia,tegra114-tsc";
>> 		reg = <0x700f0000 0x20000>;
>> 	};
>>
>> 	...
>>
>> 	timer {
>> 		compatible = "arm,armv7-timer";
>> 		interrupts = <1 13 0xf08>,
>> 			     <1 14 0xf08>,
>> 			     <1 11 0xf08>,
>> 			     <1 10 0xf08>;
>> 	};
>>
> Arch timer frequency can be specified in DT since the code
> already has provision to extract it. Below is extract
> from OMAP5 DT code.
> 
> timer {
>                                  compatible = "arm,armv7-timer";
> 
>                                  interrupts = <1 14 0x308>;
>                                  clock-frequency = <6144000>;
>                          };
> 
> I think thats what Marc is pointing out.

Almost. I already proposed this in the past, but because the source clock
is variable in the Tegra case, this is not flexible enough.

What I was suggesting was to do the following:

timer {
        compatible = "arm,armv7-timer";
        [...]
        clocks = <&tsc>;
}

tsc: tsc {
        compatible = "nvidia,tegra114-tsc";
        reg = <0x700f0000 0x20000>;
        freq-range = <... ...>;
        clock-output-names = "tsc";
}

In the arch_timer code, start searching for the "clocks" property, and use
that if there is one. Otherwise, fall back to "clock-frequency", and
ultimately to reading CNTFRQ.

This requires some changes (converting the tsc code to be a clock), but
this is at least a proper description of the hardware, and should give you
the required flexibility.

Thanks,

        M.
-- 
Fast, cheap, reliable. Pick two.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Wed, Jan 09, 2013 at 05:49:46AM +0000, Hiroshi Doyu wrote:
> Hi,
> 
> Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 20:32:32 +0100:
> 
> > On 01/08/2013 09:21 AM, Mark Rutland wrote:
> > > On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> > >> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
> > >>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> > >>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > >>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > >>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > >>>> have to read it from the system coprocessor(CP15), because the SCU on
> > >>>> Cortex-A15 MPCore does not have software readable registers. This
> > >>>> patch selects the correct method at runtime based on the CPU ID.
> > ...
> > >>>>  static void __init tegra_smp_init_cpus(void)
> > >>>>  {
> > >>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > >>>> +	unsigned int i, cpu_id, ncores;
> > >>>> +	u32 l2ctlr;
> > >>>> +	phys_addr_t pa;
> > >>>> +
> > >>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > >>>> +	switch (cpu_id) {
> > >>>> +	case CPU_CORTEX_A15:
> > >>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > >>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > >>>> +		break;
> > >>>
> > >>> [...]
> > >>>
> > >>> As mentioned last time [1], you should get this information from the dt
> > >>> instead.
> > >>
> > >> Most of platsmp.c:.smp_init_cpus() implementations seem just to
> > >> overwrite # of cores by SCU/MRC detection. Is there any implementation
> > >> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> > > 
> > > As far as I can see, there's no other platform which just relies on
> > > arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> > > sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> > > to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> > > be handled by arm_dt_init_cpus.
> 
> True.
> 
> > > I think the best option would be to have a separate smp_ops for your dt
> > > platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> > > smp_init_cpus is different to that for non-dt platforms. That way non dt
> > > platforms can keep the SCU hack for now, and won't be broken, and the dt
> > > platforms are far removed from the SCU hack and just use common
> > > infrastructure.
> >
> > Tegra doesn't have any non-DT support now.
> 
> What about falling down to SCU/MRC hack only when DT detection fails
> or no /cpus entry in DT?
> 
> Can arm_dt_init_cpu_maps() return if it suceeds or not?

I think the best solution to this problem consists in /me adding a
function, say, arm_dt_nb_cores(), that returns an error if DT probing
failed and the number of cores if it succeeded.

If it fails, you can fall back to SCU detection of cores for legacy
reasons (A9), but from A15 onwards I have quite a strong feeling against
using L2 as number of cores detection mechanism since it does not work
for multi-cluster systems. Fair enough, on Tegra this is not a problem
(now) but I do not want this method to propagate to other platforms where
multi-cluster number of cores detection will fail if we rely on the L2
mechanism.

This means that if the DT /cpu nodes are updated for all Tegra
platforms, DT becomes the mechanism to detect the number of cores
for all of them.

If you allow the A9 SCU mechanism to override the DT information (even
if it is correct) we might end up in the rather unwieldy situation where
there is a disconnect between the cpu_logical_map initialized entries and
the HW probed number of cores (if the DT bindings are correct the
cpu_logical_map entries are initialized by DT).

The gist is:

- if DT /cpu nodes are there and defined properly, we build the
  cpu_logical_map and count the number of cores from DT
- if DT /cpu nodes are missing or bogus we rely on the cpu_logical_map
  in smp_setup_processor_id() and fall back to legacy mechanism to detect
  the number of cores. This just for legacy boards/SoC.

Let me know if this is reasonable please.

> 
> > 
> > If we're going to make Tegra114 read the CPU information from DT, then
> > I'd rather just switch over all Tegra platforms to requiring CPU nodes
> > in the DT. We have few enough SoC variants that it should be easy to
> > switch everything to DT before adding Tegra114 support without causing
> > any problems.
> 
> Possible.
> 
> > But that all said, I'm not convinced it's a good idea to force the
> > information to be present in DT when it's just duplicating stuff that
> > can be runtime-probed from the HW...
> 
> That's also my concern somewhat.
> 
> The main purpose to DT CPU core detection is for multiple CPU
> clusters. SCU/MRC cannot know anything outside of their own
> clusters. In tegra, we have 4 Cortex-A9/A15 core plus 1 shadow
> core. Switching those clusters is done transparently. The last A9/A15
> core is transparently switched back and forth to the shadow one. So in
> this case, SCU/MRC detection seems to work fine.

You can probe the number of cores through SCU but remember
that MPIDRs of existing CPUs are generated, not probed if the DT is not
present (smp_setup_processor_id()). cpu_logical_map content, before the
DT bindings were introduced, or even now if DT /cpu nodes are bogus, is
simply generated as a sequential number but is not *probeable* per-se.
To tell the truth, this works well for all existing ARM platforms in the
mainline but moving forward we'd better stick at bits of information provided
by the DT, at least that's my opinion.

Thoughts ?

Lorenzo

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Hi Lorenzo,

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Tue, 8 Jan 2013 18:11:03 +0100:

> On Tue, Jan 08, 2013 at 04:21:38PM +0000, Mark Rutland wrote:
> > On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> 
> [...]
> 
> > > > >  static void __init tegra_smp_init_cpus(void)
> > > > >  {
> > > > > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > > > > +	unsigned int i, cpu_id, ncores;
> > > > > +	u32 l2ctlr;
> > > > > +	phys_addr_t pa;
> > > > > +
> > > > > +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > > > > +	switch (cpu_id) {
> > > > > +	case CPU_CORTEX_A15:
> > > > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > > > +		break;
> > > > 
> > > > [...]
> > > > 
> > > > As mentioned last time [1], you should get this information from the dt
> > > > instead.
> > > 
> > > Most of platsmp.c:.smp_init_cpus() implementations seem just to
> > > overwrite # of cores by SCU/MRC detection. Is there any implementation
> > > to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> > 
> > As far as I can see, there's no other platform which just relies on
> > arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> > sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> > to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> > be handled by arm_dt_init_cpus.
> > 
> > I think the best option would be to have a separate smp_ops for your dt
> > platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> > smp_init_cpus is different to that for non-dt platforms. That way non dt
> > platforms can keep the SCU hack for now, and won't be broken, and the dt
> > platforms are far removed from the SCU hack and just use common infrastructure.
> > 
> > Maybe someone else has a better idea?
> 
> Have a look at mach-vexpress/platsmp.c (keeping in mind that the
> GENERIC_SCU case will be refactored/removed since arm_dt_init_cpu_maps() is
> doing most of the required steps), please let me know what you think.

In tegra, we don't support non-DT. What about falling back SCU/MRC
based detection only when arm_dt_init_cpu_maps() fails?

diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
index 68e76ef..51d24ae 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/arch/arm/mach-tegra/platsmp.c
@@ -152,7 +152,7 @@ done:
  * Initialise the CPU possible map early - this describes the CPUs
  * which may be present or become present in the system.
  */
-static void __init tegra_smp_init_cpus(void)
+static void __init tegra_smp_detect_cores(void)
 {
 	unsigned int i, cpu_id, ncores;
 	u32 l2ctlr;
@@ -183,6 +183,12 @@ static void __init tegra_smp_init_cpus(void)
 
 	for (i = 0; i < ncores; i++)
 		set_cpu_possible(i, true);
+}
+
+static void __init tegra_smp_init_cpus(void)
+{
+	if (!num_possible_cpus())
+		tegra_smp_detect_cores();
 
 	set_smp_cross_call(gic_raise_softirq);
 }

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Wed, Jan 09, 2013 at 11:46:41AM +0000, Hiroshi Doyu wrote:
> Hi Lorenzo,
> 
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Tue, 8 Jan 2013 18:11:03 +0100:
> 
> > On Tue, Jan 08, 2013 at 04:21:38PM +0000, Mark Rutland wrote:
> > > On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> > 
> > [...]
> > 
> > > > > >  static void __init tegra_smp_init_cpus(void)
> > > > > >  {
> > > > > > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > > > > > +	unsigned int i, cpu_id, ncores;
> > > > > > +	u32 l2ctlr;
> > > > > > +	phys_addr_t pa;
> > > > > > +
> > > > > > +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > > > > > +	switch (cpu_id) {
> > > > > > +	case CPU_CORTEX_A15:
> > > > > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > > > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > > > > +		break;
> > > > > 
> > > > > [...]
> > > > > 
> > > > > As mentioned last time [1], you should get this information from the dt
> > > > > instead.
> > > > 
> > > > Most of platsmp.c:.smp_init_cpus() implementations seem just to
> > > > overwrite # of cores by SCU/MRC detection. Is there any implementation
> > > > to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> > > 
> > > As far as I can see, there's no other platform which just relies on
> > > arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> > > sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> > > to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> > > be handled by arm_dt_init_cpus.
> > > 
> > > I think the best option would be to have a separate smp_ops for your dt
> > > platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> > > smp_init_cpus is different to that for non-dt platforms. That way non dt
> > > platforms can keep the SCU hack for now, and won't be broken, and the dt
> > > platforms are far removed from the SCU hack and just use common infrastructure.
> > > 
> > > Maybe someone else has a better idea?
> > 
> > Have a look at mach-vexpress/platsmp.c (keeping in mind that the
> > GENERIC_SCU case will be refactored/removed since arm_dt_init_cpu_maps() is
> > doing most of the required steps), please let me know what you think.
> 
> In tegra, we don't support non-DT. What about falling back SCU/MRC
> based detection only when arm_dt_init_cpu_maps() fails?
> 
> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> index 68e76ef..51d24ae 100644
> --- a/arch/arm/mach-tegra/platsmp.c
> +++ b/arch/arm/mach-tegra/platsmp.c
> @@ -152,7 +152,7 @@ done:
>   * Initialise the CPU possible map early - this describes the CPUs
>   * which may be present or become present in the system.
>   */
> -static void __init tegra_smp_init_cpus(void)
> +static void __init tegra_smp_detect_cores(void)
>  {
>  	unsigned int i, cpu_id, ncores;
>  	u32 l2ctlr;
> @@ -183,6 +183,12 @@ static void __init tegra_smp_init_cpus(void)
>  
>  	for (i = 0; i < ncores; i++)
>  		set_cpu_possible(i, true);
> +}
> +
> +static void __init tegra_smp_init_cpus(void)
> +{
> +	if (!num_possible_cpus())
> +		tegra_smp_detect_cores();

Thought about this, I would prefer writing an inline function that
provides this information (DT missing /cpu information or parsing failure)
instead of relying on num_possible_cpus to be == 0 as a way to check that.

Lorenzo

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Stephen Warren]

On 01/09/2013 04:34 AM, Lorenzo Pieralisi wrote:
> On Wed, Jan 09, 2013 at 05:49:46AM +0000, Hiroshi Doyu wrote:
>> Hi,
>>
>> Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 20:32:32 +0100:
>>
>>> On 01/08/2013 09:21 AM, Mark Rutland wrote:
>>>> On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
>>>>> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
>>>>>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
>>>>>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
>>>>>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
>>>>>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
>>>>>>> have to read it from the system coprocessor(CP15), because the SCU on
>>>>>>> Cortex-A15 MPCore does not have software readable registers. This
>>>>>>> patch selects the correct method at runtime based on the CPU ID.
>>> ...
>>>>>>>  static void __init tegra_smp_init_cpus(void)
>>>>>>>  {
>>>>>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
>>>>>>> +	unsigned int i, cpu_id, ncores;
>>>>>>> +	u32 l2ctlr;
>>>>>>> +	phys_addr_t pa;
>>>>>>> +
>>>>>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
>>>>>>> +	switch (cpu_id) {
>>>>>>> +	case CPU_CORTEX_A15:
>>>>>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
>>>>>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
>>>>>>> +		break;
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>> As mentioned last time [1], you should get this information from the dt
>>>>>> instead.
>>>>>
>>>>> Most of platsmp.c:.smp_init_cpus() implementations seem just to
>>>>> overwrite # of cores by SCU/MRC detection. Is there any implementation
>>>>> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
>>>>
>>>> As far as I can see, there's no other platform which just relies on
>>>> arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
>>>> sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
>>>> to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
>>>> be handled by arm_dt_init_cpus.
>>
>> True.
>>
>>>> I think the best option would be to have a separate smp_ops for your dt
>>>> platforms where we know cpu nodes are populated (e.g. Tegra 114), where
>>>> smp_init_cpus is different to that for non-dt platforms. That way non dt
>>>> platforms can keep the SCU hack for now, and won't be broken, and the dt
>>>> platforms are far removed from the SCU hack and just use common
>>>> infrastructure.
>>>
>>> Tegra doesn't have any non-DT support now.
>>
>> What about falling down to SCU/MRC hack only when DT detection fails
>> or no /cpus entry in DT?
>>
>> Can arm_dt_init_cpu_maps() return if it suceeds or not?
> 
> I think the best solution to this problem consists in /me adding a
> function, say, arm_dt_nb_cores(), that returns an error if DT probing
> failed and the number of cores if it succeeded.

Well, if the primary mechanism needs to be DT-based going forward, I'd
say just require the DT nodes to be present to use the new function, and
outright fail if they aren't. That way, everything always works one way.
It shouldn't be hard to add the CPU nodes for Tegra, right?

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Wed, Jan 09, 2013 at 04:17:15PM +0000, Stephen Warren wrote:
> On 01/09/2013 04:34 AM, Lorenzo Pieralisi wrote:
> > On Wed, Jan 09, 2013 at 05:49:46AM +0000, Hiroshi Doyu wrote:
> >> Hi,
> >>
> >> Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 20:32:32 +0100:
> >>
> >>> On 01/08/2013 09:21 AM, Mark Rutland wrote:
> >>>> On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> >>>>> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
> >>>>>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> >>>>>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> >>>>>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> >>>>>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> >>>>>>> have to read it from the system coprocessor(CP15), because the SCU on
> >>>>>>> Cortex-A15 MPCore does not have software readable registers. This
> >>>>>>> patch selects the correct method at runtime based on the CPU ID.
> >>> ...
> >>>>>>>  static void __init tegra_smp_init_cpus(void)
> >>>>>>>  {
> >>>>>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> >>>>>>> +	unsigned int i, cpu_id, ncores;
> >>>>>>> +	u32 l2ctlr;
> >>>>>>> +	phys_addr_t pa;
> >>>>>>> +
> >>>>>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> >>>>>>> +	switch (cpu_id) {
> >>>>>>> +	case CPU_CORTEX_A15:
> >>>>>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> >>>>>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> >>>>>>> +		break;
> >>>>>>
> >>>>>> [...]
> >>>>>>
> >>>>>> As mentioned last time [1], you should get this information from the dt
> >>>>>> instead.
> >>>>>
> >>>>> Most of platsmp.c:.smp_init_cpus() implementations seem just to
> >>>>> overwrite # of cores by SCU/MRC detection. Is there any implementation
> >>>>> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> >>>>
> >>>> As far as I can see, there's no other platform which just relies on
> >>>> arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> >>>> sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> >>>> to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> >>>> be handled by arm_dt_init_cpus.
> >>
> >> True.
> >>
> >>>> I think the best option would be to have a separate smp_ops for your dt
> >>>> platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> >>>> smp_init_cpus is different to that for non-dt platforms. That way non dt
> >>>> platforms can keep the SCU hack for now, and won't be broken, and the dt
> >>>> platforms are far removed from the SCU hack and just use common
> >>>> infrastructure.
> >>>
> >>> Tegra doesn't have any non-DT support now.
> >>
> >> What about falling down to SCU/MRC hack only when DT detection fails
> >> or no /cpus entry in DT?
> >>
> >> Can arm_dt_init_cpu_maps() return if it suceeds or not?
> > 
> > I think the best solution to this problem consists in /me adding a
> > function, say, arm_dt_nb_cores(), that returns an error if DT probing
> > failed and the number of cores if it succeeded.
> 
> Well, if the primary mechanism needs to be DT-based going forward, I'd
> say just require the DT nodes to be present to use the new function, and
> outright fail if they aren't. That way, everything always works one way.
> It shouldn't be hard to add the CPU nodes for Tegra, right?

Adding /cpu nodes to DT is trivial, but I would like to keep a fall back
mechanism in place for existing platforms to carry out the transition
as smoothly as possible (eg people using new kernels with old DTS).

/cpu nodes will have priority over HW based probing for cores counting.

As discussed with Hiroshi I will post a patch to provide platforms with
a DT cpu map validity check so that the fall back mechanism can be
triggered properly.

Thank you very much,
Lorenzo

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 12:34:32 +0100:

> On Wed, Jan 09, 2013 at 05:49:46AM +0000, Hiroshi Doyu wrote:
> > Hi,
> > 
> > Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 20:32:32 +0100:
> > 
> > > On 01/08/2013 09:21 AM, Mark Rutland wrote:
> > > > On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> > > >> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
> > > >>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> > > >>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > > >>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > > >>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > > >>>> have to read it from the system coprocessor(CP15), because the SCU on
> > > >>>> Cortex-A15 MPCore does not have software readable registers. This
> > > >>>> patch selects the correct method at runtime based on the CPU ID.
> > > ...
> > > >>>>  static void __init tegra_smp_init_cpus(void)
> > > >>>>  {
> > > >>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > > >>>> +	unsigned int i, cpu_id, ncores;
> > > >>>> +	u32 l2ctlr;
> > > >>>> +	phys_addr_t pa;
> > > >>>> +
> > > >>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > > >>>> +	switch (cpu_id) {
> > > >>>> +	case CPU_CORTEX_A15:
> > > >>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > >>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > >>>> +		break;
> > > >>>
> > > >>> [...]
> > > >>>
> > > >>> As mentioned last time [1], you should get this information from the dt
> > > >>> instead.
> > > >>
> > > >> Most of platsmp.c:.smp_init_cpus() implementations seem just to
> > > >> overwrite # of cores by SCU/MRC detection. Is there any implementation
> > > >> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> > > > 
> > > > As far as I can see, there's no other platform which just relies on
> > > > arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> > > > sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> > > > to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> > > > be handled by arm_dt_init_cpus.
> > 
> > True.
> > 
> > > > I think the best option would be to have a separate smp_ops for your dt
> > > > platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> > > > smp_init_cpus is different to that for non-dt platforms. That way non dt
> > > > platforms can keep the SCU hack for now, and won't be broken, and the dt
> > > > platforms are far removed from the SCU hack and just use common
> > > > infrastructure.
> > >
> > > Tegra doesn't have any non-DT support now.
> > 
> > What about falling down to SCU/MRC hack only when DT detection fails
> > or no /cpus entry in DT?
> > 
> > Can arm_dt_init_cpu_maps() return if it suceeds or not?
> 
> I think the best solution to this problem consists in /me adding a
> function, say, arm_dt_nb_cores(), that returns an error if DT probing
> failed and the number of cores if it succeeded.
> 
> If it fails, you can fall back to SCU detection of cores for legacy
> reasons (A9), but from A15 onwards I have quite a strong feeling against
> using L2 as number of cores detection mechanism since it does not work
> for multi-cluster systems. Fair enough, on Tegra this is not a problem
> (now) but I do not want this method to propagate to other platforms where
> multi-cluster number of cores detection will fail if we rely on the L2
> mechanism.
> 
> This means that if the DT /cpu nodes are updated for all Tegra
> platforms, DT becomes the mechanism to detect the number of cores
> for all of them.
>
> If you allow the A9 SCU mechanism to override the DT information (even
> if it is correct) we might end up in the rather unwieldy situation where
> there is a disconnect between the cpu_logical_map initialized entries and
> the HW probed number of cores (if the DT bindings are correct the
> cpu_logical_map entries are initialized by DT).
> 
> The gist is:
> 
> - if DT /cpu nodes are there and defined properly, we build the
>   cpu_logical_map and count the number of cores from DT
> - if DT /cpu nodes are missing or bogus we rely on the cpu_logical_map
>   in smp_setup_processor_id() and fall back to legacy mechanism to detect
>   the number of cores. This just for legacy boards/SoC.
> 
> Let me know if this is reasonable please.

Sounds quite reasonable to me.

Please review another mail which falls back to SCU/MRC detection when
arm_dt_init_cpu_maps() fails.

> > > If we're going to make Tegra114 read the CPU information from DT, then
> > > I'd rather just switch over all Tegra platforms to requiring CPU nodes
> > > in the DT. We have few enough SoC variants that it should be easy to
> > > switch everything to DT before adding Tegra114 support without causing
> > > any problems.
> > 
> > Possible.
> > 
> > > But that all said, I'm not convinced it's a good idea to force the
> > > information to be present in DT when it's just duplicating stuff that
> > > can be runtime-probed from the HW...
> > 
> > That's also my concern somewhat.
> > 
> > The main purpose to DT CPU core detection is for multiple CPU
> > clusters. SCU/MRC cannot know anything outside of their own
> > clusters. In tegra, we have 4 Cortex-A9/A15 core plus 1 shadow
> > core. Switching those clusters is done transparently. The last A9/A15
> > core is transparently switched back and forth to the shadow one. So in
> > this case, SCU/MRC detection seems to work fine.
> 
> You can probe the number of cores through SCU but remember
> that MPIDRs of existing CPUs are generated, not probed if the DT is not
> present (smp_setup_processor_id()). cpu_logical_map content, before the
> DT bindings were introduced, or even now if DT /cpu nodes are bogus, is
> simply generated as a sequential number but is not *probeable* per-se.

Honestly I don't understand the above so well. Could you please
explain a little bit more?

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 19:07:13 +0100:

> On Wed, Jan 09, 2013 at 04:17:15PM +0000, Stephen Warren wrote:
> > On 01/09/2013 04:34 AM, Lorenzo Pieralisi wrote:
> > > On Wed, Jan 09, 2013 at 05:49:46AM +0000, Hiroshi Doyu wrote:
> > >> Hi,
> > >>
> > >> Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 20:32:32 +0100:
> > >>
> > >>> On 01/08/2013 09:21 AM, Mark Rutland wrote:
> > >>>> On Tue, Jan 08, 2013 at 02:53:42PM +0000, Hiroshi Doyu wrote:
> > >>>>> Mark Rutland <mark.rutland@arm.com> wrote @ Tue, 8 Jan 2013 15:28:28 +0100:
> > >>>>>> On Tue, Jan 08, 2013 at 12:47:37PM +0000, Hiroshi Doyu wrote:
> > >>>>>>> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > >>>>>>> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > >>>>>>> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > >>>>>>> have to read it from the system coprocessor(CP15), because the SCU on
> > >>>>>>> Cortex-A15 MPCore does not have software readable registers. This
> > >>>>>>> patch selects the correct method at runtime based on the CPU ID.
> > >>> ...
> > >>>>>>>  static void __init tegra_smp_init_cpus(void)
> > >>>>>>>  {
> > >>>>>>> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > >>>>>>> +	unsigned int i, cpu_id, ncores;
> > >>>>>>> +	u32 l2ctlr;
> > >>>>>>> +	phys_addr_t pa;
> > >>>>>>> +
> > >>>>>>> +	cpu_id = read_cpuid(CPUID_ID) & CPU_MASK;
> > >>>>>>> +	switch (cpu_id) {
> > >>>>>>> +	case CPU_CORTEX_A15:
> > >>>>>>> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > >>>>>>> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > >>>>>>> +		break;
> > >>>>>>
> > >>>>>> [...]
> > >>>>>>
> > >>>>>> As mentioned last time [1], you should get this information from the dt
> > >>>>>> instead.
> > >>>>>
> > >>>>> Most of platsmp.c:.smp_init_cpus() implementations seem just to
> > >>>>> overwrite # of cores by SCU/MRC detection. Is there any implementation
> > >>>>> to use the DT's # and skip SCU/MRC detection in .smp_init_cpus()?
> > >>>>
> > >>>> As far as I can see, there's no other platform which just relies on
> > >>>> arm_dt_init_cpu_maps. Until recently, it didn't exist, so that makes some
> > >>>> sense. As far as I can see, for the Tegra 114 you only need your smp_init_cpus
> > >>>> to call set_smp_cross_call(gic_raise_softirq). Everything else you do seems to
> > >>>> be handled by arm_dt_init_cpus.
> > >>
> > >> True.
> > >>
> > >>>> I think the best option would be to have a separate smp_ops for your dt
> > >>>> platforms where we know cpu nodes are populated (e.g. Tegra 114), where
> > >>>> smp_init_cpus is different to that for non-dt platforms. That way non dt
> > >>>> platforms can keep the SCU hack for now, and won't be broken, and the dt
> > >>>> platforms are far removed from the SCU hack and just use common
> > >>>> infrastructure.
> > >>>
> > >>> Tegra doesn't have any non-DT support now.
> > >>
> > >> What about falling down to SCU/MRC hack only when DT detection fails
> > >> or no /cpus entry in DT?
> > >>
> > >> Can arm_dt_init_cpu_maps() return if it suceeds or not?
> > > 
> > > I think the best solution to this problem consists in /me adding a
> > > function, say, arm_dt_nb_cores(), that returns an error if DT probing
> > > failed and the number of cores if it succeeded.
> > 
> > Well, if the primary mechanism needs to be DT-based going forward, I'd
> > say just require the DT nodes to be present to use the new function, and
> > outright fail if they aren't. That way, everything always works one way.
> > It shouldn't be hard to add the CPU nodes for Tegra, right?
> 
> Adding /cpu nodes to DT is trivial, but I would like to keep a fall back
> mechanism in place for existing platforms to carry out the transition
> as smoothly as possible (eg people using new kernels with old DTS).
> 
> /cpu nodes will have priority over HW based probing for cores counting.
> 
> As discussed with Hiroshi I will post a patch to provide platforms with
> a DT cpu map validity check so that the fall back mechanism can be
> triggered properly.

I'll add /cpu nodes for all tegra, anyway.

Until your patch comes, I'll make a code just consider a single core
if DT fails at first. So T114 initial support patch could be merged as
well.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 06:31:43AM +0000, Hiroshi Doyu wrote:
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 12:34:32 +0100:

[...]

> > > > But that all said, I'm not convinced it's a good idea to force the
> > > > information to be present in DT when it's just duplicating stuff that
> > > > can be runtime-probed from the HW...
> > > 
> > > That's also my concern somewhat.
> > > 
> > > The main purpose to DT CPU core detection is for multiple CPU
> > > clusters. SCU/MRC cannot know anything outside of their own
> > > clusters. In tegra, we have 4 Cortex-A9/A15 core plus 1 shadow
> > > core. Switching those clusters is done transparently. The last A9/A15
> > > core is transparently switched back and forth to the shadow one. So in
> > > this case, SCU/MRC detection seems to work fine.
> > 
> > You can probe the number of cores through SCU but remember
> > that MPIDRs of existing CPUs are generated, not probed if the DT is not
> > present (smp_setup_processor_id()). cpu_logical_map content, before the
> > DT bindings were introduced, or even now if DT /cpu nodes are bogus, is
> > simply generated as a sequential number but is not *probeable* per-se.
> 
> Honestly I don't understand the above so well. Could you please
> explain a little bit more?

The DT /cpu bindings provide the kernel with a list of MPIDR ("reg"
property) of all CPUs in the system. What I wanted to say is that, you
can probe in HW the number of cores in some situations, but the DT
provides more information than that, so I was debating the "duplicating
stuff that can be runtime probed" point.

In some platforms you can probe the number of cores in HW, but the MPIDR
of existing CPUs is "generated" in smp_setup_processor_id() (you cannot
probe the MPIDR of CPUs that are off or held in a pen), as a sequential number
and stored in the cpu_logical_map. This works well for single cluster systems,
since the MPIDR[23:0] of processors in a single cluster system is a sequential
number starting from 0 (well, at least MPIDR[7:0], but let's not get bogged
down into details).

For multi-cluster systems this fails, and DT comes to the rescue along
with its companion parsing function, arm_dt_init_cpu_maps().

The long and short of it is: DT /cpu nodes are not there just for number
of cores counting, but there is more. Probably on Tegra this is not
needed at the moment, but this provides the groundwork for a generic and
solid solution for all upcoming ARM platforms.

I hope this helps, let me know if it is not clear.

Lorenzo

[v2 6/9] ARM: dt: tegra114: Add new SoC base, Tegra 114 SoC

[Hiroshi Doyu]

Stephen Warren <swarren@wwwdotorg.org> wrote @ Tue, 8 Jan 2013 23:49:27 +0100:

> On 01/08/2013 05:47 AM, Hiroshi Doyu wrote:
> > Initial support for Tegra 114 SoC. This is expected to be included in
> > the board DTS files, Tegra 114 SoC based evaluation board family.
> 
> > diff --git a/arch/arm/boot/dts/tegra114.dtsi b/arch/arm/boot/dts/tegra114.dtsi
> 
> > +	gic: interrupt-controller {
> > +		compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
> > +		reg = <0x50041000 0x1000
> > +		       0x50042000 0x1000>;
> > +		interrupt-controller;
> > +		#interrupt-cells = <3>;
> > +	};
> 
> Last time around, Marc Zyngier asked:
> 
> > If this is indeed an A15 GIC, how about adding the GICH and GICV
> > regions, as well as the VGIC maintenance interrupt?

I missed to update. I'll update v3 as bellow.

	gic: interrupt-controller {
		compatible = "arm,cortex-a15-gic";
		#interrupt-cells = <3>;
		interrupt-controller;
		reg = <0x50041000 0x1000>,
		      <0x50042000 0x1000>,
		      <0x50044000 0x2000>,
		      <0x50046000 0x2000>;
		interrupts = <1 9 0xf04>;
	};

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 16:17:00 +0100:

....

> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > index 68e76ef..51d24ae 100644
> > --- a/arch/arm/mach-tegra/platsmp.c
> > +++ b/arch/arm/mach-tegra/platsmp.c
> > @@ -152,7 +152,7 @@ done:
> >   * Initialise the CPU possible map early - this describes the CPUs
> >   * which may be present or become present in the system.
> >   */
> > -static void __init tegra_smp_init_cpus(void)
> > +static void __init tegra_smp_detect_cores(void)
> >  {
> >  	unsigned int i, cpu_id, ncores;
> >  	u32 l2ctlr;
> > @@ -183,6 +183,12 @@ static void __init tegra_smp_init_cpus(void)
> >  
> >  	for (i = 0; i < ncores; i++)
> >  		set_cpu_possible(i, true);
> > +}
> > +
> > +static void __init tegra_smp_init_cpus(void)
> > +{
> > +	if (!num_possible_cpus())
> > +		tegra_smp_detect_cores();
> 
> Thought about this, I would prefer writing an inline function that
> provides this information (DT missing /cpu information or parsing failure)
> instead of relying on num_possible_cpus to be == 0 as a way to check that.

With your "[PATCH] ARM: kernel: DT cpu map validity check helper
function", my original patch gets as below. I think that the following
"smp_detect_ncores()" function may be so generic that it could be
moved in some common place than platform?

>From f5f2a43952ce75fe061b3808b994c3ceb07f1af1 Mon Sep 17 00:00:00 2001
From: Hiroshi Doyu <hdoyu@nvidia.com>
Date: Mon, 26 Nov 2012 12:25:14 +0200
Subject: [PATCH 1/1] ARM: tegra: # of CPU cores detection w/ & w/o
 HAVE_ARM_SCU

The method to detect the number of CPU cores on Cortex-A9 MPCore and
Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
have to read it from the system coprocessor(CP15), because the SCU on
Cortex-A15 MPCore does not have software readable registers. This
patch selects the correct method at runtime based on the CPU ID.

Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
 arch/arm/mach-tegra/platsmp.c |   54 ++++++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
index 6867030..9a1324a 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/arch/arm/mach-tegra/platsmp.c
@@ -24,6 +24,8 @@
 #include <asm/mach-types.h>
 #include <asm/smp_scu.h>
 #include <asm/smp_plat.h>
+#include <asm/cputype.h>
+#include <asm/prom.h>
 
 #include <mach/powergate.h>
 
@@ -38,7 +40,7 @@
 extern void tegra_secondary_startup(void);
 
 static cpumask_t tegra_cpu_init_mask;
-static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
+static void __iomem *scu_base;
 
 #define EVP_CPU_RESET_VECTOR \
 	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
@@ -177,23 +179,52 @@ done:
 	return status;
 }
 
+static int __init smp_detect_ncores(void)
+{
+	unsigned int ncores, mpidr;
+	u32 l2ctlr;
+	phys_addr_t pa;
+
+	mpidr = read_cpuid_mpidr();
+	switch (mpidr) {
+	case ARM_CPU_PART_CORTEX_A15:
+		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
+		ncores = ((l2ctlr >> 24) & 3) + 1;
+		break;
+	case ARM_CPU_PART_CORTEX_A9:
+		/* Get SCU physical base */
+		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
+		scu_base = IO_ADDRESS(pa);
+		ncores = scu_get_core_count(scu_base);
+		break;
+	default:
+		pr_warn("Unsupported mpidr\n");
+		ncores = 1;
+		break;
+	}
+
+	return ncores;
+}
+
 /*
  * Initialise the CPU possible map early - this describes the CPUs
  * which may be present or become present in the system.
  */
 static void __init tegra_smp_init_cpus(void)
 {
-	unsigned int i, ncores = scu_get_core_count(scu_base);
-
-	if (ncores > nr_cpu_ids) {
-		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
-			ncores, nr_cpu_ids);
-		ncores = nr_cpu_ids;
+	if (!arm_dt_cpu_map_valid()) {
+		unsigned int i, ncores;
+
+		ncores = smp_detect_ncores();
+		if (ncores > nr_cpu_ids) {
+			pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
+				ncores, nr_cpu_ids);
+			ncores = nr_cpu_ids;
+		}
+		for (i = 0; i < ncores; i++)
+			set_cpu_possible(i, true);
 	}
 
-	for (i = 0; i < ncores; i++)
-		set_cpu_possible(i, true);
-
 	set_smp_cross_call(gic_raise_softirq);
 }
 
@@ -202,7 +233,8 @@ static void __init tegra_smp_prepare_cpus(unsigned int max_cpus)
 	/* Always mark the boot CPU (CPU0) as initialized. */
 	cpumask_set_cpu(0, &tegra_cpu_init_mask);
 
-	scu_enable(scu_base);
+	if (scu_base)
+		scu_enable(scu_base);
 }
 
 struct smp_operations tegra_smp_ops __initdata = {
-- 
1.7.9.5

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 12:58:13PM +0000, Hiroshi Doyu wrote:
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 16:17:00 +0100:
> 
> ....
> 
> > > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > > index 68e76ef..51d24ae 100644
> > > --- a/arch/arm/mach-tegra/platsmp.c
> > > +++ b/arch/arm/mach-tegra/platsmp.c
> > > @@ -152,7 +152,7 @@ done:
> > >   * Initialise the CPU possible map early - this describes the CPUs
> > >   * which may be present or become present in the system.
> > >   */
> > > -static void __init tegra_smp_init_cpus(void)
> > > +static void __init tegra_smp_detect_cores(void)
> > >  {
> > >  	unsigned int i, cpu_id, ncores;
> > >  	u32 l2ctlr;
> > > @@ -183,6 +183,12 @@ static void __init tegra_smp_init_cpus(void)
> > >  
> > >  	for (i = 0; i < ncores; i++)
> > >  		set_cpu_possible(i, true);
> > > +}
> > > +
> > > +static void __init tegra_smp_init_cpus(void)
> > > +{
> > > +	if (!num_possible_cpus())
> > > +		tegra_smp_detect_cores();
> > 
> > Thought about this, I would prefer writing an inline function that
> > provides this information (DT missing /cpu information or parsing failure)
> > instead of relying on num_possible_cpus to be == 0 as a way to check that.
> 
> With your "[PATCH] ARM: kernel: DT cpu map validity check helper
> function", my original patch gets as below. I think that the following
> "smp_detect_ncores()" function may be so generic that it could be
> moved in some common place than platform?

Possibly, but I have still some comments to add.

> 
> From f5f2a43952ce75fe061b3808b994c3ceb07f1af1 Mon Sep 17 00:00:00 2001
> From: Hiroshi Doyu <hdoyu@nvidia.com>
> Date: Mon, 26 Nov 2012 12:25:14 +0200
> Subject: [PATCH 1/1] ARM: tegra: # of CPU cores detection w/ & w/o
>  HAVE_ARM_SCU
> 
> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> have to read it from the system coprocessor(CP15), because the SCU on
> Cortex-A15 MPCore does not have software readable registers. This
> patch selects the correct method at runtime based on the CPU ID.
> 
> Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> ---
>  arch/arm/mach-tegra/platsmp.c |   54 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 43 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> index 6867030..9a1324a 100644
> --- a/arch/arm/mach-tegra/platsmp.c
> +++ b/arch/arm/mach-tegra/platsmp.c
> @@ -24,6 +24,8 @@
>  #include <asm/mach-types.h>
>  #include <asm/smp_scu.h>
>  #include <asm/smp_plat.h>
> +#include <asm/cputype.h>
> +#include <asm/prom.h>
>  
>  #include <mach/powergate.h>
>  
> @@ -38,7 +40,7 @@
>  extern void tegra_secondary_startup(void);
>  
>  static cpumask_t tegra_cpu_init_mask;
> -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> +static void __iomem *scu_base;
>  
>  #define EVP_CPU_RESET_VECTOR \
>  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> @@ -177,23 +179,52 @@ done:
>  	return status;
>  }
>  
> +static int __init smp_detect_ncores(void)
> +{
> +	unsigned int ncores, mpidr;
> +	u32 l2ctlr;
> +	phys_addr_t pa;
> +
> +	mpidr = read_cpuid_mpidr();
> +	switch (mpidr) {
> +	case ARM_CPU_PART_CORTEX_A15:
> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> +		break;

I do not want to see the case above merged. We keep the existing legacy
methods there for legacy reasons and as a fall-back mechanism but I am not
keen on adding new HW probing code to detect the number of cores.

>From A15/A7 onwards DT-only cpu map initialization is the way to go.

Lorenzo

> +	case ARM_CPU_PART_CORTEX_A9:
> +		/* Get SCU physical base */
> +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> +		scu_base = IO_ADDRESS(pa);
> +		ncores = scu_get_core_count(scu_base);
> +		break;
> +	default:
> +		pr_warn("Unsupported mpidr\n");
> +		ncores = 1;
> +		break;
> +	}
> +
> +	return ncores;
> +}
> +
>  /*
>   * Initialise the CPU possible map early - this describes the CPUs
>   * which may be present or become present in the system.
>   */
>  static void __init tegra_smp_init_cpus(void)
>  {
> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> -
> -	if (ncores > nr_cpu_ids) {
> -		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
> -			ncores, nr_cpu_ids);
> -		ncores = nr_cpu_ids;
> +	if (!arm_dt_cpu_map_valid()) {
> +		unsigned int i, ncores;
> +
> +		ncores = smp_detect_ncores();
> +		if (ncores > nr_cpu_ids) {
> +			pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
> +				ncores, nr_cpu_ids);
> +			ncores = nr_cpu_ids;
> +		}
> +		for (i = 0; i < ncores; i++)
> +			set_cpu_possible(i, true);
>  	}
>  
> -	for (i = 0; i < ncores; i++)
> -		set_cpu_possible(i, true);
> -
>  	set_smp_cross_call(gic_raise_softirq);
>  }
>  
> @@ -202,7 +233,8 @@ static void __init tegra_smp_prepare_cpus(unsigned int max_cpus)
>  	/* Always mark the boot CPU (CPU0) as initialized. */
>  	cpumask_set_cpu(0, &tegra_cpu_init_mask);
>  
> -	scu_enable(scu_base);
> +	if (scu_base)
> +		scu_enable(scu_base);
>  }
>  
>  struct smp_operations tegra_smp_ops __initdata = {
> -- 
> 1.7.9.5
> 
> 
> 
> 

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Thu, 10 Jan 2013 14:47:23 +0100:

> On Thu, Jan 10, 2013 at 12:58:13PM +0000, Hiroshi Doyu wrote:
> > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 16:17:00 +0100:
> > 
> > ....
> > 
> > > > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > > > index 68e76ef..51d24ae 100644
> > > > --- a/arch/arm/mach-tegra/platsmp.c
> > > > +++ b/arch/arm/mach-tegra/platsmp.c
> > > > @@ -152,7 +152,7 @@ done:
> > > >   * Initialise the CPU possible map early - this describes the CPUs
> > > >   * which may be present or become present in the system.
> > > >   */
> > > > -static void __init tegra_smp_init_cpus(void)
> > > > +static void __init tegra_smp_detect_cores(void)
> > > >  {
> > > >  	unsigned int i, cpu_id, ncores;
> > > >  	u32 l2ctlr;
> > > > @@ -183,6 +183,12 @@ static void __init tegra_smp_init_cpus(void)
> > > >  
> > > >  	for (i = 0; i < ncores; i++)
> > > >  		set_cpu_possible(i, true);
> > > > +}
> > > > +
> > > > +static void __init tegra_smp_init_cpus(void)
> > > > +{
> > > > +	if (!num_possible_cpus())
> > > > +		tegra_smp_detect_cores();
> > > 
> > > Thought about this, I would prefer writing an inline function that
> > > provides this information (DT missing /cpu information or parsing failure)
> > > instead of relying on num_possible_cpus to be == 0 as a way to check that.
> > 
> > With your "[PATCH] ARM: kernel: DT cpu map validity check helper
> > function", my original patch gets as below. I think that the following
> > "smp_detect_ncores()" function may be so generic that it could be
> > moved in some common place than platform?
> 
> Possibly, but I have still some comments to add.
> 
> > 
> > From f5f2a43952ce75fe061b3808b994c3ceb07f1af1 Mon Sep 17 00:00:00 2001
> > From: Hiroshi Doyu <hdoyu@nvidia.com>
> > Date: Mon, 26 Nov 2012 12:25:14 +0200
> > Subject: [PATCH 1/1] ARM: tegra: # of CPU cores detection w/ & w/o
> >  HAVE_ARM_SCU
> > 
> > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > have to read it from the system coprocessor(CP15), because the SCU on
> > Cortex-A15 MPCore does not have software readable registers. This
> > patch selects the correct method at runtime based on the CPU ID.
> > 
> > Signed-off-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > ---
> >  arch/arm/mach-tegra/platsmp.c |   54 ++++++++++++++++++++++++++++++++---------
> >  1 file changed, 43 insertions(+), 11 deletions(-)
> > 
> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > index 6867030..9a1324a 100644
> > --- a/arch/arm/mach-tegra/platsmp.c
> > +++ b/arch/arm/mach-tegra/platsmp.c
> > @@ -24,6 +24,8 @@
> >  #include <asm/mach-types.h>
> >  #include <asm/smp_scu.h>
> >  #include <asm/smp_plat.h>
> > +#include <asm/cputype.h>
> > +#include <asm/prom.h>
> >  
> >  #include <mach/powergate.h>
> >  
> > @@ -38,7 +40,7 @@
> >  extern void tegra_secondary_startup(void);
> >  
> >  static cpumask_t tegra_cpu_init_mask;
> > -static void __iomem *scu_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE);
> > +static void __iomem *scu_base;
> >  
> >  #define EVP_CPU_RESET_VECTOR \
> >  	(IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE) + 0x100)
> > @@ -177,23 +179,52 @@ done:
> >  	return status;
> >  }
> >  
> > +static int __init smp_detect_ncores(void)
> > +{
> > +	unsigned int ncores, mpidr;
> > +	u32 l2ctlr;
> > +	phys_addr_t pa;
> > +
> > +	mpidr = read_cpuid_mpidr();
> > +	switch (mpidr) {
> > +	case ARM_CPU_PART_CORTEX_A15:
> > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > +		break;
> 
> I do not want to see the case above merged. We keep the existing legacy
> methods there for legacy reasons and as a fall-back mechanism but I am not
> keen on adding new HW probing code to detect the number of cores.
> 
> From A15/A7 onwards DT-only cpu map initialization is the way to go.

Ok, then, smp_detect_ncores(void) could be...

static int __init smp_detect_ncores(void)
{
	unsigned int ncores, mpidr;
	phys_addr_t pa;

	mpidr = read_cpuid_mpidr();
	switch (mpidr) {
	case ARM_CPU_PART_CORTEX_A9:
		/* Get SCU physical base */
		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
		scu_base = IO_ADDRESS(pa);
		ncores = scu_get_core_count(scu_base);
		break;
	case ARM_CPU_PART_CORTEX_A15:
	default:
		pr_warn("Unsupported MPIDR %x\n", mpidr);
		ncores = 1;
		break;
	}
	return ncores;
}

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 02:03:50PM +0000, Hiroshi Doyu wrote:
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Thu, 10 Jan 2013 14:47:23 +0100:

[...]

> > > +static int __init smp_detect_ncores(void)
> > > +{
> > > +	unsigned int ncores, mpidr;
> > > +	u32 l2ctlr;
> > > +	phys_addr_t pa;
> > > +
> > > +	mpidr = read_cpuid_mpidr();
> > > +	switch (mpidr) {
> > > +	case ARM_CPU_PART_CORTEX_A15:
> > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > +		break;
> > 
> > I do not want to see the case above merged. We keep the existing legacy
> > methods there for legacy reasons and as a fall-back mechanism but I am not
> > keen on adding new HW probing code to detect the number of cores.
> > 
> > From A15/A7 onwards DT-only cpu map initialization is the way to go.
> 
> Ok, then, smp_detect_ncores(void) could be...
> 
> static int __init smp_detect_ncores(void)
> {
> 	unsigned int ncores, mpidr;
> 	phys_addr_t pa;
> 
> 	mpidr = read_cpuid_mpidr();

Careful, MPIDR is not at all what you want. MIDR is.

> 	switch (mpidr) {
> 	case ARM_CPU_PART_CORTEX_A9:
> 		/* Get SCU physical base */
> 		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> 		scu_base = IO_ADDRESS(pa);
> 		ncores = scu_get_core_count(scu_base);
> 		break;
> 	case ARM_CPU_PART_CORTEX_A15:

Remove the ARM_CPU_PART_CORTEX_A15 case, I do not see how it helps.

And be careful with the case matching, if I am not mistaken read_cpuid_id()
(which is the function you have to use) will return the full MIDR, you need
to mask the return value.

> 	default:
> 		pr_warn("Unsupported MPIDR %x\n", mpidr);
> 		ncores = 1;
> 		break;

This is reasonable.

Lorenzo

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Thu, 10 Jan 2013 15:33:34 +0100:

> On Thu, Jan 10, 2013 at 02:03:50PM +0000, Hiroshi Doyu wrote:
> > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Thu, 10 Jan 2013 14:47:23 +0100:
> 
> [...]
> 
> > > > +static int __init smp_detect_ncores(void)
> > > > +{
> > > > +	unsigned int ncores, mpidr;
> > > > +	u32 l2ctlr;
> > > > +	phys_addr_t pa;
> > > > +
> > > > +	mpidr = read_cpuid_mpidr();
> > > > +	switch (mpidr) {
> > > > +	case ARM_CPU_PART_CORTEX_A15:
> > > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > > +		break;
> > > 
> > > I do not want to see the case above merged. We keep the existing legacy
> > > methods there for legacy reasons and as a fall-back mechanism but I am not
> > > keen on adding new HW probing code to detect the number of cores.
> > > 
> > > From A15/A7 onwards DT-only cpu map initialization is the way to go.
> > 
> > Ok, then, smp_detect_ncores(void) could be...
> > 
> > static int __init smp_detect_ncores(void)
> > {
> > 	unsigned int ncores, mpidr;
> > 	phys_addr_t pa;
> > 
> > 	mpidr = read_cpuid_mpidr();
> 
> Careful, MPIDR is not at all what you want. MIDR is.
> 
> > 	switch (mpidr) {
> > 	case ARM_CPU_PART_CORTEX_A9:
> > 		/* Get SCU physical base */
> > 		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> > 		scu_base = IO_ADDRESS(pa);
> > 		ncores = scu_get_core_count(scu_base);
> > 		break;
> > 	case ARM_CPU_PART_CORTEX_A15:
> 
> Remove the ARM_CPU_PART_CORTEX_A15 case, I do not see how it helps.
> 
> And be careful with the case matching, if I am not mistaken read_cpuid_id()
> (which is the function you have to use) will return the full MIDR, you need
> to mask the return value.

Oops, I'll fix.

> > 	default:
> > 		pr_warn("Unsupported MPIDR %x\n", mpidr);
> > 		ncores = 1;
> > 		break;
> 
> This is reasonable.

Ok, thank you for your reivew, I'll keep this func,
"smp_detect_ncores()" in this file since it's mostly for the backward
compatibility. Not necessary to be relocated in common place newly..

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Hiroshi Doyu]

Hi Mark,

Marc Zyngier <marc.zyngier@arm.com> wrote @ Wed, 9 Jan 2013 10:01:05 +0100:

> Almost. I already proposed this in the past, but because the source clock
> is variable in the Tegra case, this is not flexible enough.
> 
> What I was suggesting was to do the following:
> 
> timer {
>         compatible = "arm,armv7-timer";
>         [...]
>         clocks = <&tsc>;
> }
> 
> tsc: tsc {
>         compatible = "nvidia,tegra114-tsc";
>         reg = <0x700f0000 0x20000>;
>         freq-range = <... ...>;
>         clock-output-names = "tsc";
> }
> 
> In the arch_timer code, start searching for the "clocks" property, and use
> that if there is one. Otherwise, fall back to "clock-frequency", and
> ultimately to reading CNTFRQ.
> 
> This requires some changes (converting the tsc code to be a clock), but
> this is at least a proper description of the hardware, and should give you
> the required flexibility.

The above seems ok to me. I'll drop this original patch from this
series for now until T114 clock comes to implement correctly.

[v2 5/9] clocksource: tegra: Enable ARM arch_timer with TSC

[Marc Zyngier]

On 10/01/13 15:03, Hiroshi Doyu wrote:
> Hi Mark,
> 
> Marc Zyngier <marc.zyngier@arm.com> wrote @ Wed, 9 Jan 2013 10:01:05 +0100:
> 
>> Almost. I already proposed this in the past, but because the source clock
>> is variable in the Tegra case, this is not flexible enough.
>>
>> What I was suggesting was to do the following:
>>
>> timer {
>>         compatible = "arm,armv7-timer";
>>         [...]
>>         clocks = <&tsc>;
>> }
>>
>> tsc: tsc {
>>         compatible = "nvidia,tegra114-tsc";
>>         reg = <0x700f0000 0x20000>;
>>         freq-range = <... ...>;
>>         clock-output-names = "tsc";
>> }
>>
>> In the arch_timer code, start searching for the "clocks" property, and use
>> that if there is one. Otherwise, fall back to "clock-frequency", and
>> ultimately to reading CNTFRQ.
>>
>> This requires some changes (converting the tsc code to be a clock), but
>> this is at least a proper description of the hardware, and should give you
>> the required flexibility.
> 
> The above seems ok to me. I'll drop this original patch from this
> series for now until T114 clock comes to implement correctly.

Thanks Hiroshi. Please Cc me when you have a patch implementing this
functionality.

	M.
-- 
Jazz is not dead. It just smells funny...

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Stephen Warren]

On 01/10/2013 05:58 AM, Hiroshi Doyu wrote:
> Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 16:17:00 +0100:
...
> With your "[PATCH] ARM: kernel: DT cpu map validity check helper
> function", my original patch gets as below. I think that the following
> "smp_detect_ncores()" function may be so generic that it could be
> moved in some common place than platform?
> 
> From f5f2a43952ce75fe061b3808b994c3ceb07f1af1 Mon Sep 17 00:00:00 2001
> From: Hiroshi Doyu <hdoyu@nvidia.com>
> Date: Mon, 26 Nov 2012 12:25:14 +0200
> Subject: [PATCH 1/1] ARM: tegra: # of CPU cores detection w/ & w/o
>  HAVE_ARM_SCU
> 
> The method to detect the number of CPU cores on Cortex-A9 MPCore and
> Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> have to read it from the system coprocessor(CP15), because the SCU on
> Cortex-A15 MPCore does not have software readable registers. This
> patch selects the correct method at runtime based on the CPU ID.

> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c

> +static int __init smp_detect_ncores(void)
> +{
> +	unsigned int ncores, mpidr;
> +	u32 l2ctlr;
> +	phys_addr_t pa;
> +
> +	mpidr = read_cpuid_mpidr();
> +	switch (mpidr) {
> +	case ARM_CPU_PART_CORTEX_A15:
> +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> +		ncores = ((l2ctlr >> 24) & 3) + 1;
> +		break;
> +	case ARM_CPU_PART_CORTEX_A9:
> +		/* Get SCU physical base */
> +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> +		scu_base = IO_ADDRESS(pa);
> +		ncores = scu_get_core_count(scu_base);
> +		break;
> +	default:
> +		pr_warn("Unsupported mpidr\n");
> +		ncores = 1;
> +		break;
> +	}
> +
> +	return ncores;
> +}

Why not just remove that function...

>  /*
>   * Initialise the CPU possible map early - this describes the CPUs
>   * which may be present or become present in the system.
>   */
>  static void __init tegra_smp_init_cpus(void)
>  {
> -	unsigned int i, ncores = scu_get_core_count(scu_base);
> -
> -	if (ncores > nr_cpu_ids) {
> -		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
> -			ncores, nr_cpu_ids);
> -		ncores = nr_cpu_ids;
> +	if (!arm_dt_cpu_map_valid()) {
> +		unsigned int i, ncores;
> +
> +		ncores = smp_detect_ncores();

And just say "ncores = 1" here?

For Tegra, it's trivial to simply put the patch adding the required DT
nodes before this patch, and then there's no need for any
backwards-compatibility, since the nodes are guaranteed to be there.

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Lorenzo Pieralisi]

On Thu, Jan 10, 2013 at 04:54:13PM +0000, Stephen Warren wrote:
> On 01/10/2013 05:58 AM, Hiroshi Doyu wrote:
> > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 16:17:00 +0100:
> ...
> > With your "[PATCH] ARM: kernel: DT cpu map validity check helper
> > function", my original patch gets as below. I think that the following
> > "smp_detect_ncores()" function may be so generic that it could be
> > moved in some common place than platform?
> > 
> > From f5f2a43952ce75fe061b3808b994c3ceb07f1af1 Mon Sep 17 00:00:00 2001
> > From: Hiroshi Doyu <hdoyu@nvidia.com>
> > Date: Mon, 26 Nov 2012 12:25:14 +0200
> > Subject: [PATCH 1/1] ARM: tegra: # of CPU cores detection w/ & w/o
> >  HAVE_ARM_SCU
> > 
> > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > have to read it from the system coprocessor(CP15), because the SCU on
> > Cortex-A15 MPCore does not have software readable registers. This
> > patch selects the correct method at runtime based on the CPU ID.
> 
> > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> 
> > +static int __init smp_detect_ncores(void)
> > +{
> > +	unsigned int ncores, mpidr;
> > +	u32 l2ctlr;
> > +	phys_addr_t pa;
> > +
> > +	mpidr = read_cpuid_mpidr();
> > +	switch (mpidr) {
> > +	case ARM_CPU_PART_CORTEX_A15:
> > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > +		break;
> > +	case ARM_CPU_PART_CORTEX_A9:
> > +		/* Get SCU physical base */
> > +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> > +		scu_base = IO_ADDRESS(pa);
> > +		ncores = scu_get_core_count(scu_base);
> > +		break;
> > +	default:
> > +		pr_warn("Unsupported mpidr\n");
> > +		ncores = 1;
> > +		break;
> > +	}
> > +
> > +	return ncores;
> > +}
> 
> Why not just remove that function...
> 
> >  /*
> >   * Initialise the CPU possible map early - this describes the CPUs
> >   * which may be present or become present in the system.
> >   */
> >  static void __init tegra_smp_init_cpus(void)
> >  {
> > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > -
> > -	if (ncores > nr_cpu_ids) {
> > -		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
> > -			ncores, nr_cpu_ids);
> > -		ncores = nr_cpu_ids;
> > +	if (!arm_dt_cpu_map_valid()) {
> > +		unsigned int i, ncores;
> > +
> > +		ncores = smp_detect_ncores();
> 
> And just say "ncores = 1" here?
> 
> For Tegra, it's trivial to simply put the patch adding the required DT
> nodes before this patch, and then there's no need for any
> backwards-compatibility, since the nodes are guaranteed to be there.

If you are willing to accept that your code proposal is perfectly fine by me.

Thanks,
Lorenzo

[v2 3/9] ARM: tegra: # of CPU cores detection w/ & w/o HAVE_ARM_SCU

[Hiroshi Doyu]

Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Fri, 11 Jan 2013 11:11:34 +0100:

> On Thu, Jan 10, 2013 at 04:54:13PM +0000, Stephen Warren wrote:
> > On 01/10/2013 05:58 AM, Hiroshi Doyu wrote:
> > > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote @ Wed, 9 Jan 2013 16:17:00 +0100:
> > ...
> > > With your "[PATCH] ARM: kernel: DT cpu map validity check helper
> > > function", my original patch gets as below. I think that the following
> > > "smp_detect_ncores()" function may be so generic that it could be
> > > moved in some common place than platform?
> > > 
> > > From f5f2a43952ce75fe061b3808b994c3ceb07f1af1 Mon Sep 17 00:00:00 2001
> > > From: Hiroshi Doyu <hdoyu@nvidia.com>
> > > Date: Mon, 26 Nov 2012 12:25:14 +0200
> > > Subject: [PATCH 1/1] ARM: tegra: # of CPU cores detection w/ & w/o
> > >  HAVE_ARM_SCU
> > > 
> > > The method to detect the number of CPU cores on Cortex-A9 MPCore and
> > > Cortex-A15 MPCore is different. On Cortex-A9 MPCore we can get this
> > > information from the Snoop Control Unit(SCU). On Cortex-A15 MPCore we
> > > have to read it from the system coprocessor(CP15), because the SCU on
> > > Cortex-A15 MPCore does not have software readable registers. This
> > > patch selects the correct method at runtime based on the CPU ID.
> > 
> > > diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
> > 
> > > +static int __init smp_detect_ncores(void)
> > > +{
> > > +	unsigned int ncores, mpidr;
> > > +	u32 l2ctlr;
> > > +	phys_addr_t pa;
> > > +
> > > +	mpidr = read_cpuid_mpidr();
> > > +	switch (mpidr) {
> > > +	case ARM_CPU_PART_CORTEX_A15:
> > > +		asm("mrc p15, 1, %0, c9, c0, 2\n" : "=r" (l2ctlr));
> > > +		ncores = ((l2ctlr >> 24) & 3) + 1;
> > > +		break;
> > > +	case ARM_CPU_PART_CORTEX_A9:
> > > +		/* Get SCU physical base */
> > > +		asm("mrc p15, 4, %0, c15, c0, 0" : "=r" (pa));
> > > +		scu_base = IO_ADDRESS(pa);
> > > +		ncores = scu_get_core_count(scu_base);
> > > +		break;
> > > +	default:
> > > +		pr_warn("Unsupported mpidr\n");
> > > +		ncores = 1;
> > > +		break;
> > > +	}
> > > +
> > > +	return ncores;
> > > +}
> > 
> > Why not just remove that function...
> > 
> > >  /*
> > >   * Initialise the CPU possible map early - this describes the CPUs
> > >   * which may be present or become present in the system.
> > >   */
> > >  static void __init tegra_smp_init_cpus(void)
> > >  {
> > > -	unsigned int i, ncores = scu_get_core_count(scu_base);
> > > -
> > > -	if (ncores > nr_cpu_ids) {
> > > -		pr_warn("SMP: %u cores greater than maximum (%u), clipping\n",
> > > -			ncores, nr_cpu_ids);
> > > -		ncores = nr_cpu_ids;
> > > +	if (!arm_dt_cpu_map_valid()) {
> > > +		unsigned int i, ncores;
> > > +
> > > +		ncores = smp_detect_ncores();
> > 
> > And just say "ncores = 1" here?
> > 
> > For Tegra, it's trivial to simply put the patch adding the required DT
> > nodes before this patch, and then there's no need for any
> > backwards-compatibility, since the nodes are guaranteed to be there.
> 
> If you are willing to accept that your code proposal is perfectly fine by me.

That's simple. I'll send ones to add cpu nodes, and then this without detection.