[PATCH 0/7] Proper arch timer support for Exynos5433-based TM2(e) boards

[PATCH 0/7] Proper arch timer support for Exynos5433-based TM2(e) boards

[Marek Szyprowski]

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

[PATCH 1/7] clocksource: exynos_mct: Remove dead code

[Marek Szyprowski]

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

Re: [PATCH 1/7] clocksource: exynos_mct: Remove dead code

[Krzysztof Kozlowski]

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

[PATCH 2/7] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64

[Marek Szyprowski]

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

Re: [PATCH 2/7] clocksource: exynos_mct: Add arch_timer cooperation mode for ARM64

[Krzysztof Kozlowski]

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
>

[PATCH 3/7] clocksource: Change CPU hotplug priority of exynos_mct driver

[Marek Szyprowski]

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

Re: [PATCH 3/7] clocksource: Change CPU hotplug priority of exynos_mct driver

[Krzysztof Kozlowski]

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

[PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64

[Marek Szyprowski]

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

Re: [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64

[Marc Zyngier]

+ 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...

Re: [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64

[Marek Szyprowski]

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

Re: [PATCH 4/7] clocksource: arch_timer: Add support for not-fw-configured timer on ARM64

[Daniel Lezcano]

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

[PATCH 5/7] arm64: dts: exynos: Move arch-timer node to right place

[Marek Szyprowski]

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

[PATCH 6/7] arm64: dts: exynos: Mark arch_timer device as not-fw-configured on TM2(e)

[Marek Szyprowski]

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

[PATCH 7/7] arm64: platform: Add enable Exynos Multi-Core Timer driver

[Marek Szyprowski]

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