linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Proper arch timer support for Exynos5433-based TM2(e) boards
       [not found] <CGME20181008125045eucas1p12e1d77f5f605556d4cf6c3b87832fae3@eucas1p1.samsung.com>
@ 2018-10-08 12:50 ` Marek Szyprowski
       [not found]   ` <CGME20181008125046eucas1p1e0304c8c22a4ac128a8e507c86442a37@eucas1p1.samsung.com>
                     ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Dear All,

This patchset is an attempt to submit the last piece of missing code
to have proper support for Exynos5433 SoCs based TM2(e) boards. It
performs a cleanup of timer configuration, which so far needed various
out-of-tree workarounds. The fixes provided by this patchset are also
needed for add proper support for system suspend/resume.

Best regards
Marek Szyprowski
Samsung R&D Institute Poland


Patch summary:

Marek Szyprowski (7):
  clocksource: exynos_mct: Remove dead code
  clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
  clocksource: Change CPU hotplug priority of exynos_mct driver
  clocksource: arch_timer: Add support for not-fw-configured timer on
    ARM64
  arm64: dts: exynos: Move arch-timer node to right place
  arm64: dts: exynos: Mark arch_timer device as not-fw-configured on
    TM2(e)
  arm64: platform: Add enable Exynos Multi-Core Timer driver

 arch/arm64/Kconfig.platforms                  |  1 +
 .../dts/exynos/exynos5433-tm2-common.dtsi     |  1 +
 arch/arm64/boot/dts/exynos/exynos5433.dtsi    | 23 ++++++-------
 drivers/clocksource/Kconfig                   | 11 +++++++
 drivers/clocksource/arm_arch_timer.c          | 15 +++++++--
 drivers/clocksource/exynos_mct.c              | 32 +++++++++++++------
 include/linux/cpuhotplug.h                    |  2 +-
 7 files changed, 60 insertions(+), 25 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/7] clocksource: exynos_mct: Remove dead code
       [not found]   ` <CGME20181008125046eucas1p1e0304c8c22a4ac128a8e507c86442a37@eucas1p1.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  2018-10-09  8:11       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Exynos Multi-Core Timer driver is used only on device-tree based
systems, so remove non-dt related code.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 7a244b681876..43b335ff4a96 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -581,11 +581,7 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
 	 * timer irqs are specified after the four global timer
 	 * irqs are specified.
 	 */
-#ifdef CONFIG_OF
 	nr_irqs = of_irq_count(np);
-#else
-	nr_irqs = 0;
-#endif
 	for (i = MCT_L0_IRQ; i < nr_irqs; i++)
 		mct_irqs[i] = irq_of_parse_and_map(np, i);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/7] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
       [not found]   ` <CGME20181008125047eucas1p23e520d778ad5f18aba57b21641d4cb4c@eucas1p2.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  2018-10-09  8:19       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

To get ARM Architected Timers working on Samsung Exynos SoCs, one has to
first configure and enable Exynos Multi-Core Timer, because they both
share some common hardware blocks. This patch adds a mode of cooperation
with arch_timer driver, so kernel can use CP15 based timer interface via
arch_timer driver, which is mandatory on ARM64. In such mode driver only
configures MCT registers and starts the timer but don't register any
clocksource or events in the system. Those are left to be handled by
arch_timer driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clocksource/exynos_mct.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
index 43b335ff4a96..05b201ed8ef5 100644
--- a/drivers/clocksource/exynos_mct.c
+++ b/drivers/clocksource/exynos_mct.c
@@ -57,6 +57,7 @@
 #define TICK_BASE_CNT	1
 
 enum {
+	MCT_INT_NONE = 0,
 	MCT_INT_SPI,
 	MCT_INT_PPI
 };
@@ -238,6 +239,9 @@ static int __init exynos4_clocksource_init(void)
 {
 	exynos4_mct_frc_start();
 
+	if (!mct_int_type)
+		return 0;
+
 #if defined(CONFIG_ARM)
 	exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
 	exynos4_delay_timer.freq = clk_rate;
@@ -343,6 +347,9 @@ static struct irqaction mct_comp_event_irq = {
 
 static int exynos4_clockevent_init(void)
 {
+	if (!mct_int_type)
+		return 0;
+
 	mct_comp_device.cpumask = cpumask_of(0);
 	clockevents_config_and_register(&mct_comp_device, clk_rate,
 					0xf, 0xffffffff);
@@ -476,12 +483,12 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
 
 		irq_force_affinity(evt->irq, cpumask_of(cpu));
 		enable_irq(evt->irq);
-	} else {
+	} else if (mct_int_type == MCT_INT_PPI) {
 		enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);
 	}
-	clockevents_config_and_register(evt, clk_rate / (TICK_BASE_CNT + 1),
-					0xf, 0x7fffffff);
-
+	if (mct_int_type)
+		clockevents_config_and_register(evt,
+			       clk_rate / (TICK_BASE_CNT + 1), 0xf, 0x7fffffff);
 	return 0;
 }
 
@@ -496,7 +503,7 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
 		if (evt->irq != -1)
 			disable_irq_nosync(evt->irq);
 		exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
-	} else {
+	} else if (mct_int_type == MCT_INT_PPI) {
 		disable_percpu_irq(mct_irqs[MCT_L0_IRQ]);
 	}
 	return 0;
@@ -529,7 +536,7 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
 					 &percpu_mct_tick);
 		WARN(err, "MCT: can't request IRQ %d (%d)\n",
 		     mct_irqs[MCT_L0_IRQ], err);
-	} else {
+	} else if (mct_int_type == MCT_INT_SPI) {
 		for_each_possible_cpu(cpu) {
 			int mct_irq = mct_irqs[MCT_L0_IRQ + cpu];
 			struct mct_clock_event_device *pcpu_mevt =
@@ -573,6 +580,15 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
 
 	mct_int_type = int_type;
 
+	if (IS_ENABLED(CONFIG_ARM64) && IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {
+		struct device_node *np = of_find_compatible_node(NULL, NULL,
+							     "arm,armv8-timer");
+		if (np) {
+			mct_int_type = MCT_INT_NONE;
+			of_node_put(np);
+		}
+	}
+
 	/* This driver uses only one global timer interrupt */
 	mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/7] clocksource: Change CPU hotplug priority of exynos_mct driver
       [not found]   ` <CGME20181008125047eucas1p1bbd0c4759612b6df8fbe25f759e03130@eucas1p1.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  2018-10-09  8:19       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Exynos Multi-Core Timer driver (exynos_mct) must be started before
ARM Architected Timers (arch_timer), because both timers share common
hardware block and turning on MCT is needed to get ARM Architected
Timer working properly.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 include/linux/cpuhotplug.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index caf40ad0bbc6..5d9e4a6ea299 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -115,10 +115,10 @@ enum cpuhp_state {
 	CPUHP_AP_PERF_ARM_ACPI_STARTING,
 	CPUHP_AP_PERF_ARM_STARTING,
 	CPUHP_AP_ARM_L2X0_STARTING,
+	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
 	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
 	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
 	CPUHP_AP_JCORE_TIMER_STARTING,
-	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
 	CPUHP_AP_ARM_TWD_STARTING,
 	CPUHP_AP_QCOM_TIMER_STARTING,
 	CPUHP_AP_ARMADA_TIMER_STARTING,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64
       [not found]   ` <CGME20181008125048eucas1p229a94c202596ddd0a65009c15d9e3001@eucas1p2.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  2018-10-08 13:17       ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Use common infrastructure for ARM Architected Timers erratum to enable
support for systems with broken CPU firmware (timer registers not
properly configured). This mode has been already availabled on ARM
(32bits) architecture. This enables to run Linux kernel on ARM64 boards
using physical architected timers instead of the virtual ones. Examples
of such system with broken firmware are Samsung Exynos5433 SoC based
TM2(e) boards, which is already deployed for years and updating firmware
is not possible.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/clocksource/Kconfig          | 11 +++++++++++
 drivers/clocksource/arm_arch_timer.c | 15 ++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a11f4ba98b05..a30752579b03 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -364,6 +364,17 @@ config ARM64_ERRATUM_858921
 	  The workaround will be dynamically enabled when an affected
 	  core is detected.
 
+config ARCH_TIMER_REGISTERS_NOT_FW_CONFIGURED
+	bool "Workaround for arch timer registers not configured by firmware"
+	default y
+	select ARM_ARCH_TIMER_OOL_WORKAROUND
+	depends on ARM_ARCH_TIMER && ARM64
+	help
+	  This option enables a workaround for boards, on which arch timer
+	  registers are not properly configured by the board firmware.
+	  The workaround will be dynamically enabled when an affected
+	  board is detected.
+
 config ARM_GLOBAL_TIMER
 	bool "Support for the ARM global timer" if COMPILE_TEST
 	select TIMER_OF if OF
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 9a7d4dc00b6e..b6f109654daf 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -423,6 +423,14 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.read_cntvct_el0 = arm64_1188873_read_cntvct_el0,
 	},
 #endif
+#ifdef CONFIG_ARCH_TIMER_REGISTERS_NOT_FW_CONFIGURED
+	{
+		.match_type = ate_match_dt,
+		.id = "arm,cpu-registers-not-fw-configured",
+		.desc = "broken CPU firmware (timer registers not configured)",
+		.read_cntvct_el0 = arch_counter_get_cntpct,
+	},
+#endif
 };
 
 typedef bool (*ate_match_fn_t)(const struct arch_timer_erratum_workaround *,
@@ -1234,9 +1242,10 @@ static int __init arch_timer_of_init(struct device_node *np)
 	 * If we cannot rely on firmware initializing the timer registers then
 	 * we should use the physical timers instead.
 	 */
-	if (IS_ENABLED(CONFIG_ARM) &&
-	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
-		arch_timer_uses_ppi = ARCH_TIMER_PHYS_SECURE_PPI;
+	if (of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
+		arch_timer_uses_ppi = IS_ENABLED(CONFIG_ARM64) ?
+					ARCH_TIMER_PHYS_NONSECURE_PPI :
+					ARCH_TIMER_PHYS_SECURE_PPI;
 	else
 		arch_timer_uses_ppi = arch_timer_select_ppi();
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/7] arm64: dts: exynos: Move arch-timer node to right place
       [not found]   ` <CGME20181008125049eucas1p21a186cd652f968a3609987d4dee2427f@eucas1p2.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  0 siblings, 0 replies; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Move ARM architected timer device-tree node to the beginning of 'soc'
node, to group it together with other ARM CPU core devices (like PMU).

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 23 +++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 77297d66642c..eee3d1d1b52f 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -290,6 +290,18 @@
 			interrupt-affinity = <&cpu4>, <&cpu5>, <&cpu6>, <&cpu7>;
 		};
 
+		timer: timer {
+			compatible = "arm,armv8-timer";
+			interrupts = <GIC_PPI 13
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
+				<GIC_PPI 14
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
+				<GIC_PPI 11
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
+				<GIC_PPI 10
+					(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
+		};
+
 		sysram@2020000 {
 			compatible = "mmio-sram";
 			reg = <0x02020000 0x5c000>;
@@ -1811,17 +1823,6 @@
 		};
 	};
 
-	timer: timer {
-		compatible = "arm,armv8-timer";
-		interrupts = <GIC_PPI 13
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
-			<GIC_PPI 14
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
-			<GIC_PPI 11
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>,
-			<GIC_PPI 10
-				(GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
-	};
 };
 
 #include "exynos5433-bus.dtsi"
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/7] arm64: dts: exynos: Mark arch_timer device as not-fw-configured on TM2(e)
       [not found]   ` <CGME20181008125050eucas1p1caae30a4e791190e6c731816e36832ab@eucas1p1.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  0 siblings, 0 replies; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

TM2(e) boards use old firmware, which some PCSI requirements and doesn't
properly configure arch virtual timers, so add property indicating this
state. This enables respective workarounds in the timer driver.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
index 2606c2ab6456..ded0fb6c5936 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2-common.dtsi
@@ -1265,6 +1265,7 @@
 };
 
 &timer {
+	arm,cpu-registers-not-fw-configured;
 	clock-frequency = <24000000>;
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/7] arm64: platform: Add enable Exynos Multi-Core Timer driver
       [not found]   ` <CGME20181008125050eucas1p2da1c1ace904a331c149f5add71c72af2@eucas1p2.samsung.com>
@ 2018-10-08 12:50     ` Marek Szyprowski
  0 siblings, 0 replies; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-08 12:50 UTC (permalink / raw)
  To: linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Marek Szyprowski, Will Deacon, Catalin Marinas, Marc Zyngier,
	Thomas Gleixner, Daniel Lezcano, Krzysztof Kozlowski,
	Chanwoo Choi, Bartlomiej Zolnierkiewicz, Inki Dae

Exynos MCT driver is required even if ARM Architected Timer driver
is used to properly configure common timer hardware.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 51bc479334a4..7cc687580fad 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -62,6 +62,7 @@ config ARCH_BRCMSTB
 config ARCH_EXYNOS
 	bool "ARMv8 based Samsung Exynos SoC family"
 	select COMMON_CLK_SAMSUNG
+	select CLKSRC_EXYNOS_MCT
 	select EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS
 	select EXYNOS_PMU
 	select HAVE_S3C2410_WATCHDOG if WATCHDOG
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64
  2018-10-08 12:50     ` [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64 Marek Szyprowski
@ 2018-10-08 13:17       ` Marc Zyngier
  2018-10-15 12:12         ` Marek Szyprowski
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2018-10-08 13:17 UTC (permalink / raw)
  To: Marek Szyprowski, linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Thomas Gleixner, Daniel Lezcano,
	Krzysztof Kozlowski, Chanwoo Choi, Bartlomiej Zolnierkiewicz,
	Inki Dae, Mark Rutland

+ Mark Rutland

Hi Marek,

On 08/10/18 13:50, Marek Szyprowski wrote:
> Use common infrastructure for ARM Architected Timers erratum to enable
> support for systems with broken CPU firmware (timer registers not
> properly configured). This mode has been already availabled on ARM
> (32bits) architecture. This enables to run Linux kernel on ARM64 boards
> using physical architected timers instead of the virtual ones. Examples
> of such system with broken firmware are Samsung Exynos5433 SoC based
> TM2(e) boards, which is already deployed for years and updating firmware
> is not possible.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>   drivers/clocksource/Kconfig          | 11 +++++++++++
>   drivers/clocksource/arm_arch_timer.c | 15 ++++++++++++---
>   2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a11f4ba98b05..a30752579b03 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -364,6 +364,17 @@ config ARM64_ERRATUM_858921
>   	  The workaround will be dynamically enabled when an affected
>   	  core is detected.
>   
> +config ARCH_TIMER_REGISTERS_NOT_FW_CONFIGURED
> +	bool "Workaround for arch timer registers not configured by firmware"
> +	default y
> +	select ARM_ARCH_TIMER_OOL_WORKAROUND
> +	depends on ARM_ARCH_TIMER && ARM64
> +	help
> +	  This option enables a workaround for boards, on which arch timer
> +	  registers are not properly configured by the board firmware.
> +	  The workaround will be dynamically enabled when an affected
> +	  board is detected.
> +

I'm sorry, but I'm strongly pushing back on this.

This horrible hack was accepted with the express condition that it would 
be limited to ARMv7 platforms (on the ground that we never really 
documented the arch timer boot requirements on that version of the 
architecture), and would never proliferate on arm64. From day 1, we 
established what the boot protocol was, and we mandated that either:

- kernel is entered at EL2 on all CPUs
- cntvoff_el2 is zeroed on all CPUs

and we've got most people to fix their firmware, or live with the 
consequences. If these machines cannot receive a non-broken firmware, 
what are the odds that they will receive a mainline kernel?

Thanks,

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/7] clocksource: exynos_mct: Remove dead code
  2018-10-08 12:50     ` [PATCH 1/7] clocksource: exynos_mct: Remove dead code Marek Szyprowski
@ 2018-10-09  8:11       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2018-10-09  8:11 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-samsung-soc, linux-arm-kernel, linux-kernel, will.deacon,
	catalin.marinas, marc.zyngier, tglx, daniel.lezcano,
	Chanwoo Choi, Bartłomiej Żołnierkiewicz, Inki Dae

On Mon, 8 Oct 2018 at 14:50, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Exynos Multi-Core Timer driver is used only on device-tree based
> systems, so remove non-dt related code.

You can also mention that in case of !CONFIG_OF the code is anyway
equal because of_irq_count() has a stub returning 0.

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/7] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64
  2018-10-08 12:50     ` [PATCH 2/7] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64 Marek Szyprowski
@ 2018-10-09  8:19       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2018-10-09  8:19 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-samsung-soc, linux-arm-kernel, linux-kernel, will.deacon,
	catalin.marinas, marc.zyngier, tglx, daniel.lezcano,
	Chanwoo Choi, Bartłomiej Żołnierkiewicz, Inki Dae

On Mon, 8 Oct 2018 at 14:50, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> To get ARM Architected Timers working on Samsung Exynos SoCs, one has to
> first configure and enable Exynos Multi-Core Timer, because they both
> share some common hardware blocks. This patch adds a mode of cooperation
> with arch_timer driver, so kernel can use CP15 based timer interface via
> arch_timer driver, which is mandatory on ARM64. In such mode driver only
> configures MCT registers and starts the timer but don't register any
> clocksource or events in the system. Those are left to be handled by
> arch_timer driver.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/clocksource/exynos_mct.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index 43b335ff4a96..05b201ed8ef5 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -57,6 +57,7 @@
>  #define TICK_BASE_CNT  1
>
>  enum {
> +       MCT_INT_NONE = 0,
>         MCT_INT_SPI,
>         MCT_INT_PPI
>  };
> @@ -238,6 +239,9 @@ static int __init exynos4_clocksource_init(void)
>  {
>         exynos4_mct_frc_start();
>
> +       if (!mct_int_type)
> +               return 0;
> +
>  #if defined(CONFIG_ARM)
>         exynos4_delay_timer.read_current_timer = &exynos4_read_current_timer;
>         exynos4_delay_timer.freq = clk_rate;
> @@ -343,6 +347,9 @@ static struct irqaction mct_comp_event_irq = {
>
>  static int exynos4_clockevent_init(void)
>  {
> +       if (!mct_int_type)
> +               return 0;
> +
>         mct_comp_device.cpumask = cpumask_of(0);
>         clockevents_config_and_register(&mct_comp_device, clk_rate,
>                                         0xf, 0xffffffff);
> @@ -476,12 +483,12 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>
>                 irq_force_affinity(evt->irq, cpumask_of(cpu));
>                 enable_irq(evt->irq);
> -       } else {
> +       } else if (mct_int_type == MCT_INT_PPI) {
>                 enable_percpu_irq(mct_irqs[MCT_L0_IRQ], 0);
>         }
> -       clockevents_config_and_register(evt, clk_rate / (TICK_BASE_CNT + 1),
> -                                       0xf, 0x7fffffff);
> -
> +       if (mct_int_type)
> +               clockevents_config_and_register(evt,
> +                              clk_rate / (TICK_BASE_CNT + 1), 0xf, 0x7fffffff);
>         return 0;
>  }
>
> @@ -496,7 +503,7 @@ static int exynos4_mct_dying_cpu(unsigned int cpu)
>                 if (evt->irq != -1)
>                         disable_irq_nosync(evt->irq);
>                 exynos4_mct_write(0x1, mevt->base + MCT_L_INT_CSTAT_OFFSET);
> -       } else {
> +       } else if (mct_int_type == MCT_INT_PPI) {
>                 disable_percpu_irq(mct_irqs[MCT_L0_IRQ]);
>         }
>         return 0;
> @@ -529,7 +536,7 @@ static int __init exynos4_timer_resources(struct device_node *np, void __iomem *
>                                          &percpu_mct_tick);
>                 WARN(err, "MCT: can't request IRQ %d (%d)\n",
>                      mct_irqs[MCT_L0_IRQ], err);
> -       } else {
> +       } else if (mct_int_type == MCT_INT_SPI) {
>                 for_each_possible_cpu(cpu) {
>                         int mct_irq = mct_irqs[MCT_L0_IRQ + cpu];
>                         struct mct_clock_event_device *pcpu_mevt =

The error path of cpuhp_setup_state() does not look correct because
request_percpu_irq() was not called. The error path looks suspicious
also for MCT_INT_SPI path - the IRQs were allocated with regular
request_irq but are freed with percpu version.

> @@ -573,6 +580,15 @@ static int __init mct_init_dt(struct device_node *np, unsigned int int_type)
>
>         mct_int_type = int_type;
>
> +       if (IS_ENABLED(CONFIG_ARM64) && IS_ENABLED(CONFIG_ARM_ARCH_TIMER)) {
> +               struct device_node *np = of_find_compatible_node(NULL, NULL,
> +                                                            "arm,armv8-timer");
> +               if (np) {
> +                       mct_int_type = MCT_INT_NONE;
> +                       of_node_put(np);

In this case, later the mct_irqs are being assigned but totally not
used. Are they needed?

Best regards,
Krzysztof

> +               }
> +       }
> +
>         /* This driver uses only one global timer interrupt */
>         mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/7] clocksource: Change CPU hotplug priority of exynos_mct driver
  2018-10-08 12:50     ` [PATCH 3/7] clocksource: Change CPU hotplug priority of exynos_mct driver Marek Szyprowski
@ 2018-10-09  8:19       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2018-10-09  8:19 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: linux-samsung-soc, linux-arm-kernel, linux-kernel, will.deacon,
	catalin.marinas, marc.zyngier, tglx, daniel.lezcano,
	Chanwoo Choi, Bartłomiej Żołnierkiewicz, Inki Dae

On Mon, 8 Oct 2018 at 14:50, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Exynos Multi-Core Timer driver (exynos_mct) must be started before
> ARM Architected Timers (arch_timer), because both timers share common
> hardware block and turning on MCT is needed to get ARM Architected
> Timer working properly.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  include/linux/cpuhotplug.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64
  2018-10-08 13:17       ` Marc Zyngier
@ 2018-10-15 12:12         ` Marek Szyprowski
  2018-10-15 12:27           ` Daniel Lezcano
  0 siblings, 1 reply; 14+ messages in thread
From: Marek Szyprowski @ 2018-10-15 12:12 UTC (permalink / raw)
  To: Marc Zyngier, linux-samsung-soc, linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Thomas Gleixner, Daniel Lezcano,
	Krzysztof Kozlowski, Chanwoo Choi, Bartlomiej Zolnierkiewicz,
	Inki Dae, Mark Rutland

Hi All,

On 2018-10-08 15:17, Marc Zyngier wrote:
> + Mark Rutland
>
> Hi Marek,
>
> On 08/10/18 13:50, Marek Szyprowski wrote:
>> Use common infrastructure for ARM Architected Timers erratum to enable
>> support for systems with broken CPU firmware (timer registers not
>> properly configured). This mode has been already availabled on ARM
>> (32bits) architecture. This enables to run Linux kernel on ARM64 boards
>> using physical architected timers instead of the virtual ones. Examples
>> of such system with broken firmware are Samsung Exynos5433 SoC based
>> TM2(e) boards, which is already deployed for years and updating firmware
>> is not possible.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/clocksource/Kconfig          | 11 +++++++++++
>>   drivers/clocksource/arm_arch_timer.c | 15 ++++++++++++---
>>   2 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a11f4ba98b05..a30752579b03 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -364,6 +364,17 @@ config ARM64_ERRATUM_858921
>>         The workaround will be dynamically enabled when an affected
>>         core is detected.
>>   +config ARCH_TIMER_REGISTERS_NOT_FW_CONFIGURED
>> +    bool "Workaround for arch timer registers not configured by
>> firmware"
>> +    default y
>> +    select ARM_ARCH_TIMER_OOL_WORKAROUND
>> +    depends on ARM_ARCH_TIMER && ARM64
>> +    help
>> +      This option enables a workaround for boards, on which arch timer
>> +      registers are not properly configured by the board firmware.
>> +      The workaround will be dynamically enabled when an affected
>> +      board is detected.
>> +
>
> I'm sorry, but I'm strongly pushing back on this.
>
> This horrible hack was accepted with the express condition that it
> would be limited to ARMv7 platforms (on the ground that we never
> really documented the arch timer boot requirements on that version of
> the architecture), and would never proliferate on arm64. From day 1,
> we established what the boot protocol was, and we mandated that either:
>
> - kernel is entered at EL2 on all CPUs
> - cntvoff_el2 is zeroed on all CPUs
>
> and we've got most people to fix their firmware, or live with the
> consequences. If these machines cannot receive a non-broken firmware,
> what are the odds that they will receive a mainline kernel?

Well, I know that the firmware is broken, but I cannot do anything about it.
On the other hand updating kernel is still possible and mainline runs fine
on TM2(e) boards. I will send v2 without this patch then.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64
  2018-10-15 12:12         ` Marek Szyprowski
@ 2018-10-15 12:27           ` Daniel Lezcano
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Lezcano @ 2018-10-15 12:27 UTC (permalink / raw)
  To: Marek Szyprowski, Marc Zyngier, linux-samsung-soc,
	linux-arm-kernel, linux-kernel
  Cc: Will Deacon, Catalin Marinas, Thomas Gleixner,
	Krzysztof Kozlowski, Chanwoo Choi, Bartlomiej Zolnierkiewicz,
	Inki Dae, Mark Rutland

On 15/10/2018 14:12, Marek Szyprowski wrote:
> Hi All,
> 
> On 2018-10-08 15:17, Marc Zyngier wrote:
>> + Mark Rutland
>>
>> Hi Marek,
>>
>> On 08/10/18 13:50, Marek Szyprowski wrote:
>>> Use common infrastructure for ARM Architected Timers erratum to enable
>>> support for systems with broken CPU firmware (timer registers not
>>> properly configured). This mode has been already availabled on ARM
>>> (32bits) architecture. This enables to run Linux kernel on ARM64 boards
>>> using physical architected timers instead of the virtual ones. Examples
>>> of such system with broken firmware are Samsung Exynos5433 SoC based
>>> TM2(e) boards, which is already deployed for years and updating firmware
>>> is not possible.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   drivers/clocksource/Kconfig          | 11 +++++++++++
>>>   drivers/clocksource/arm_arch_timer.c | 15 ++++++++++++---
>>>   2 files changed, 23 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index a11f4ba98b05..a30752579b03 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -364,6 +364,17 @@ config ARM64_ERRATUM_858921
>>>         The workaround will be dynamically enabled when an affected
>>>         core is detected.
>>>   +config ARCH_TIMER_REGISTERS_NOT_FW_CONFIGURED
>>> +    bool "Workaround for arch timer registers not configured by
>>> firmware"
>>> +    default y
>>> +    select ARM_ARCH_TIMER_OOL_WORKAROUND
>>> +    depends on ARM_ARCH_TIMER && ARM64
>>> +    help
>>> +      This option enables a workaround for boards, on which arch timer
>>> +      registers are not properly configured by the board firmware.
>>> +      The workaround will be dynamically enabled when an affected
>>> +      board is detected.
>>> +
>>
>> I'm sorry, but I'm strongly pushing back on this.
>>
>> This horrible hack was accepted with the express condition that it
>> would be limited to ARMv7 platforms (on the ground that we never
>> really documented the arch timer boot requirements on that version of
>> the architecture), and would never proliferate on arm64. From day 1,
>> we established what the boot protocol was, and we mandated that either:
>>
>> - kernel is entered at EL2 on all CPUs
>> - cntvoff_el2 is zeroed on all CPUs
>>
>> and we've got most people to fix their firmware, or live with the
>> consequences. If these machines cannot receive a non-broken firmware,
>> what are the odds that they will receive a mainline kernel?
> 
> Well, I know that the firmware is broken, but I cannot do anything about it.
> On the other hand updating kernel is still possible and mainline runs fine
> on TM2(e) boards. I will send v2 without this patch then.

I second Marc's opinion.



-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-10-15 12:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20181008125045eucas1p12e1d77f5f605556d4cf6c3b87832fae3@eucas1p1.samsung.com>
2018-10-08 12:50 ` [PATCH 0/7] Proper arch timer support for Exynos5433-based TM2(e) boards Marek Szyprowski
     [not found]   ` <CGME20181008125046eucas1p1e0304c8c22a4ac128a8e507c86442a37@eucas1p1.samsung.com>
2018-10-08 12:50     ` [PATCH 1/7] clocksource: exynos_mct: Remove dead code Marek Szyprowski
2018-10-09  8:11       ` Krzysztof Kozlowski
     [not found]   ` <CGME20181008125047eucas1p23e520d778ad5f18aba57b21641d4cb4c@eucas1p2.samsung.com>
2018-10-08 12:50     ` [PATCH 2/7] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64 Marek Szyprowski
2018-10-09  8:19       ` Krzysztof Kozlowski
     [not found]   ` <CGME20181008125047eucas1p1bbd0c4759612b6df8fbe25f759e03130@eucas1p1.samsung.com>
2018-10-08 12:50     ` [PATCH 3/7] clocksource: Change CPU hotplug priority of exynos_mct driver Marek Szyprowski
2018-10-09  8:19       ` Krzysztof Kozlowski
     [not found]   ` <CGME20181008125048eucas1p229a94c202596ddd0a65009c15d9e3001@eucas1p2.samsung.com>
2018-10-08 12:50     ` [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64 Marek Szyprowski
2018-10-08 13:17       ` Marc Zyngier
2018-10-15 12:12         ` Marek Szyprowski
2018-10-15 12:27           ` Daniel Lezcano
     [not found]   ` <CGME20181008125049eucas1p21a186cd652f968a3609987d4dee2427f@eucas1p2.samsung.com>
2018-10-08 12:50     ` [PATCH 5/7] arm64: dts: exynos: Move arch-timer node to right place Marek Szyprowski
     [not found]   ` <CGME20181008125050eucas1p1caae30a4e791190e6c731816e36832ab@eucas1p1.samsung.com>
2018-10-08 12:50     ` [PATCH 6/7] arm64: dts: exynos: Mark arch_timer device as not-fw-configured on TM2(e) Marek Szyprowski
     [not found]   ` <CGME20181008125050eucas1p2da1c1ace904a331c149f5add71c72af2@eucas1p2.samsung.com>
2018-10-08 12:50     ` [PATCH 7/7] arm64: platform: Add enable Exynos Multi-Core Timer driver Marek Szyprowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).