2021-03-04 15:24:33

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 0/2] Fixes for for dra7 timer wrap errata i940

Hi all,

Here are fixes for dra7 ARM architected timer wrap errata i940 where it
fails to wrap after 388 days. The workaround is to use two dmtimers as
the local timers instead.

Note that these patches depend on timer posted mode fixes series
"[PATCH 0/3] Fixes for timer-ti-dm systimer posted mode" for the write
status register check fix. Also the spurious timer interrupt fix is
good to have from that series.

Regards,

Tony


Tony Lindgren (2):
clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap
issue
clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940

arch/arm/boot/dts/dra7-l4.dtsi | 4 +-
arch/arm/boot/dts/dra7.dtsi | 20 +++
drivers/clocksource/timer-ti-dm-systimer.c | 148 +++++++++++++++++----
include/linux/cpuhotplug.h | 1 +
4 files changed, 148 insertions(+), 25 deletions(-)

--
2.30.1


2021-03-05 00:05:04

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 1/2] clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue

There is a timer wrap issue on dra7 for the ARM architected timer.
In a typical clock configuration the timer fails to wrap after 388 days.

To work around the issue, we need to use timer-ti-dm timers instead.

Let's prepare for adding support for percpu timers by adding a common
dmtimer_clkevt_init_common() and call it from dmtimer_clockevent_init().
This patch makes no intentional functional changes.

Signed-off-by: Tony Lindgren <[email protected]>
---
drivers/clocksource/timer-ti-dm-systimer.c | 72 +++++++++++++++-------
1 file changed, 49 insertions(+), 23 deletions(-)

diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/clocksource/timer-ti-dm-systimer.c
--- a/drivers/clocksource/timer-ti-dm-systimer.c
+++ b/drivers/clocksource/timer-ti-dm-systimer.c
@@ -528,17 +528,18 @@ static void omap_clockevent_unidle(struct clock_event_device *evt)
writel_relaxed(OMAP_TIMER_INT_OVERFLOW, t->base + t->wakeup);
}

-static int __init dmtimer_clockevent_init(struct device_node *np)
+static int __init dmtimer_clkevt_init_common(struct dmtimer_clockevent *clkevt,
+ struct device_node *np,
+ unsigned int features,
+ const struct cpumask *cpumask,
+ const char *name,
+ int rating)
{
- struct dmtimer_clockevent *clkevt;
struct clock_event_device *dev;
struct dmtimer_systimer *t;
+ unsigned long irqflags;
int error;

- clkevt = kzalloc(sizeof(*clkevt), GFP_KERNEL);
- if (!clkevt)
- return -ENOMEM;
-
t = &clkevt->t;
dev = &clkevt->dev;

@@ -546,25 +547,23 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
* We mostly use cpuidle_coupled with ARM local timers for runtime,
* so there's probably no use for CLOCK_EVT_FEAT_DYNIRQ here.
*/
- dev->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
- dev->rating = 300;
+ dev->features = features;
+ dev->rating = rating;
dev->set_next_event = dmtimer_set_next_event;
dev->set_state_shutdown = dmtimer_clockevent_shutdown;
dev->set_state_periodic = dmtimer_set_periodic;
dev->set_state_oneshot = dmtimer_clockevent_shutdown;
dev->set_state_oneshot_stopped = dmtimer_clockevent_shutdown;
dev->tick_resume = dmtimer_clockevent_shutdown;
- dev->cpumask = cpu_possible_mask;
+ dev->cpumask = cpumask;

dev->irq = irq_of_parse_and_map(np, 0);
- if (!dev->irq) {
- error = -ENXIO;
- goto err_out_free;
- }
+ if (!dev->irq)
+ return -ENXIO;

error = dmtimer_systimer_setup(np, &clkevt->t);
if (error)
- goto err_out_free;
+ return error;

clkevt->period = 0xffffffff - DIV_ROUND_CLOSEST(t->rate, HZ);

@@ -575,33 +574,60 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
*/
writel_relaxed(OMAP_TIMER_CTRL_POSTED, t->base + t->ifctrl);

+ if (dev->cpumask == cpu_possible_mask)
+ irqflags = IRQF_TIMER;
+ else
+ irqflags = IRQF_TIMER | IRQF_NOBALANCING;
+
error = request_irq(dev->irq, dmtimer_clockevent_interrupt,
- IRQF_TIMER, "clockevent", clkevt);
+ irqflags, name, clkevt);
if (error)
goto err_out_unmap;

writel_relaxed(OMAP_TIMER_INT_OVERFLOW, t->base + t->irq_ena);
writel_relaxed(OMAP_TIMER_INT_OVERFLOW, t->base + t->wakeup);

- pr_info("TI gptimer clockevent: %s%lu Hz at %pOF\n",
- of_find_property(np, "ti,timer-alwon", NULL) ?
+ pr_info("TI gptimer %s: %s%lu Hz at %pOF\n",
+ name, of_find_property(np, "ti,timer-alwon", NULL) ?
"always-on " : "", t->rate, np->parent);

- clockevents_config_and_register(dev, t->rate,
+ return 0;
+
+err_out_unmap:
+ iounmap(t->base);
+
+ return error;
+}
+
+static int __init dmtimer_clockevent_init(struct device_node *np)
+{
+ struct dmtimer_clockevent *clkevt;
+ int error;
+
+ clkevt = kzalloc(sizeof(*clkevt), GFP_KERNEL);
+ if (!clkevt)
+ return -ENOMEM;
+
+ error = dmtimer_clkevt_init_common(clkevt, np,
+ CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_ONESHOT,
+ cpu_possible_mask, "clockevent",
+ 300);
+ if (error)
+ goto err_out_free;
+
+ clockevents_config_and_register(&clkevt->dev, clkevt->t.rate,
3, /* Timer internal resynch latency */
0xffffffff);

if (of_machine_is_compatible("ti,am33xx") ||
of_machine_is_compatible("ti,am43")) {
- dev->suspend = omap_clockevent_idle;
- dev->resume = omap_clockevent_unidle;
+ clkevt->dev.suspend = omap_clockevent_idle;
+ clkevt->dev.resume = omap_clockevent_unidle;
}

return 0;

-err_out_unmap:
- iounmap(t->base);
-
err_out_free:
kfree(clkevt);

--
2.30.1

2021-03-05 00:05:05

by Tony Lindgren

[permalink] [raw]
Subject: [PATCH 2/2] clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940

There is a timer wrap issue on dra7 for the ARM architected timer.
In a typical clock configuration the timer fails to wrap after 388 days.

To work around the issue, we need to use timer-ti-dm percpu timers instead.

Let's configure dmtimer3 and 4 as percpu timers by default, and warn about
the issue if the dtb is not configured properly.

Let's do this as a single patch so it can be backported to v5.8 and later
kernels easily. Note that this patch depends on earlier timer-ti-dm
systimer posted mode fixes, and a preparatory clockevent patch
"clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue".

For more information, please see the errata for "AM572x Sitara Processors
Silicon Revisions 1.1, 2.0":

https://www.ti.com/lit/er/sprz429m/sprz429m.pdf

The concept is based on earlier reference patches done by Tero Kristo and
Keerthy.

Cc: Keerthy <[email protected]>
Cc: Tero Kristo <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>
---
arch/arm/boot/dts/dra7-l4.dtsi | 4 +-
arch/arm/boot/dts/dra7.dtsi | 20 ++++++
drivers/clocksource/timer-ti-dm-systimer.c | 76 ++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
--- a/arch/arm/boot/dts/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/dra7-l4.dtsi
@@ -1168,7 +1168,7 @@ timer2: timer@0 {
};
};

- target-module@34000 { /* 0x48034000, ap 7 46.0 */
+ timer3_target: target-module@34000 { /* 0x48034000, ap 7 46.0 */
compatible = "ti,sysc-omap4-timer", "ti,sysc";
reg = <0x34000 0x4>,
<0x34010 0x4>;
@@ -1195,7 +1195,7 @@ timer3: timer@0 {
};
};

- target-module@36000 { /* 0x48036000, ap 9 4e.0 */
+ timer4_target: target-module@36000 { /* 0x48036000, ap 9 4e.0 */
compatible = "ti,sysc-omap4-timer", "ti,sysc";
reg = <0x36000 0x4>,
<0x36010 0x4>;
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -46,6 +46,7 @@ aliases {

timer {
compatible = "arm,armv7-timer";
+ status = "disabled"; /* See ARM architected timer wrap erratum i940 */
interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
@@ -1241,3 +1242,22 @@ timer@0 {
assigned-clock-parents = <&sys_32k_ck>;
};
};
+
+/* Local timers, see ARM architected timer wrap erratum i940 */
+&timer3_target {
+ ti,no-reset-on-init;
+ ti,no-idle;
+ timer@0 {
+ assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER3_CLKCTRL 24>;
+ assigned-clock-parents = <&timer_sys_clk_div>;
+ };
+};
+
+&timer4_target {
+ ti,no-reset-on-init;
+ ti,no-idle;
+ timer@0 {
+ assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER4_CLKCTRL 24>;
+ assigned-clock-parents = <&timer_sys_clk_div>;
+ };
+};
diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/clocksource/timer-ti-dm-systimer.c
--- a/drivers/clocksource/timer-ti-dm-systimer.c
+++ b/drivers/clocksource/timer-ti-dm-systimer.c
@@ -2,6 +2,7 @@
#include <linux/clk.h>
#include <linux/clocksource.h>
#include <linux/clockchips.h>
+#include <linux/cpuhotplug.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
@@ -634,6 +635,78 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
return error;
}

+/* Dmtimer as percpu timer. See dra7 ARM architected timer wrap erratum i940 */
+static DEFINE_PER_CPU(struct dmtimer_clockevent, dmtimer_percpu_timer);
+
+static int __init dmtimer_percpu_timer_init(struct device_node *np, int cpu)
+{
+ struct dmtimer_clockevent *clkevt;
+ int error;
+
+ if (!cpu_possible(cpu))
+ return -EINVAL;
+
+ if (!of_property_read_bool(np->parent, "ti,no-reset-on-init") ||
+ !of_property_read_bool(np->parent, "ti,no-idle"))
+ pr_warn("Incomplete dtb for percpu dmtimer %pOF\n", np->parent);
+
+ clkevt = per_cpu_ptr(&dmtimer_percpu_timer, cpu);
+
+ error = dmtimer_clkevt_init_common(clkevt, np, CLOCK_EVT_FEAT_ONESHOT,
+ cpumask_of(cpu), "percpu-dmtimer",
+ 500);
+ if (error)
+ return error;
+
+ return 0;
+}
+
+/* See TRM for timer internal resynch latency */
+static int omap_dmtimer_starting_cpu(unsigned int cpu)
+{
+ struct dmtimer_clockevent *clkevt = per_cpu_ptr(&dmtimer_percpu_timer, cpu);
+ struct clock_event_device *dev = &clkevt->dev;
+ struct dmtimer_systimer *t = &clkevt->t;
+
+ clockevents_config_and_register(dev, t->rate, 3, ULONG_MAX);
+ irq_force_affinity(dev->irq, cpumask_of(cpu));
+
+ return 0;
+}
+
+static int __init dmtimer_percpu_timer_startup(void)
+{
+ struct dmtimer_clockevent *clkevt = per_cpu_ptr(&dmtimer_percpu_timer, 0);
+ struct dmtimer_systimer *t = &clkevt->t;
+
+ if (t->sysc) {
+ cpuhp_setup_state(CPUHP_AP_TI_GP_TIMER_STARTING,
+ "clockevents/omap/gptimer:starting",
+ omap_dmtimer_starting_cpu, NULL);
+ }
+
+ return 0;
+}
+subsys_initcall(dmtimer_percpu_timer_startup);
+
+static int __init dmtimer_percpu_quirk_init(struct device_node *np, u32 pa)
+{
+ struct device_node *arm_timer;
+
+ arm_timer = of_find_compatible_node(NULL, NULL, "arm,armv7-timer");
+ if (of_device_is_available(arm_timer)) {
+ pr_warn_once("ARM architected timer wrap issue i940 detected\n");
+ return 0;
+ }
+
+ if (pa == 0x48034000) /* dra7 dmtimer3 */
+ return dmtimer_percpu_timer_init(np, 0);
+ else if (pa == 0x48036000) /* dra7 dmtimer4 */
+ return dmtimer_percpu_timer_init(np, 1);
+
+ return 0;
+}
+
/* Clocksource */
static struct dmtimer_clocksource *
to_dmtimer_clocksource(struct clocksource *cs)
@@ -767,6 +840,9 @@ static int __init dmtimer_systimer_init(struct device_node *np)
if (clockevent == pa)
return dmtimer_clockevent_init(np);

+ if (of_machine_is_compatible("ti,dra7"))
+ return dmtimer_percpu_quirk_init(np, pa);
+
return 0;
}

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -135,6 +135,7 @@ enum cpuhp_state {
CPUHP_AP_RISCV_TIMER_STARTING,
CPUHP_AP_CLINT_TIMER_STARTING,
CPUHP_AP_CSKY_TIMER_STARTING,
+ CPUHP_AP_TI_GP_TIMER_STARTING,
CPUHP_AP_HYPERV_TIMER_STARTING,
CPUHP_AP_KVM_STARTING,
CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
--
2.30.1

2021-03-22 10:17:03

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 2/2] clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940

On 04/03/2021 08:37, Tony Lindgren wrote:
> There is a timer wrap issue on dra7 for the ARM architected timer.
> In a typical clock configuration the timer fails to wrap after 388 days.
>
> To work around the issue, we need to use timer-ti-dm percpu timers instead.
>
> Let's configure dmtimer3 and 4 as percpu timers by default, and warn about
> the issue if the dtb is not configured properly.
>
> Let's do this as a single patch so it can be backported to v5.8 and later
> kernels easily.

Cc: <[email protected]> # v5.8+

??

> Note that this patch depends on earlier timer-ti-dm
> systimer posted mode fixes, and a preparatory clockevent patch
> "clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue".
>
> For more information, please see the errata for "AM572x Sitara Processors
> Silicon Revisions 1.1, 2.0":
>
> https://www.ti.com/lit/er/sprz429m/sprz429m.pdf
>
> The concept is based on earlier reference patches done by Tero Kristo and
> Keerthy.
>
> Cc: Keerthy <[email protected]>
> Cc: Tero Kristo <[email protected]>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> arch/arm/boot/dts/dra7-l4.dtsi | 4 +-
> arch/arm/boot/dts/dra7.dtsi | 20 ++++++
> drivers/clocksource/timer-ti-dm-systimer.c | 76 ++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
> --- a/arch/arm/boot/dts/dra7-l4.dtsi
> +++ b/arch/arm/boot/dts/dra7-l4.dtsi
> @@ -1168,7 +1168,7 @@ timer2: timer@0 {
> };
> };
>
> - target-module@34000 { /* 0x48034000, ap 7 46.0 */
> + timer3_target: target-module@34000 { /* 0x48034000, ap 7 46.0 */
> compatible = "ti,sysc-omap4-timer", "ti,sysc";
> reg = <0x34000 0x4>,
> <0x34010 0x4>;
> @@ -1195,7 +1195,7 @@ timer3: timer@0 {
> };
> };
>
> - target-module@36000 { /* 0x48036000, ap 9 4e.0 */
> + timer4_target: target-module@36000 { /* 0x48036000, ap 9 4e.0 */
> compatible = "ti,sysc-omap4-timer", "ti,sysc";
> reg = <0x36000 0x4>,
> <0x36010 0x4>;
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -46,6 +46,7 @@ aliases {
>
> timer {
> compatible = "arm,armv7-timer";
> + status = "disabled"; /* See ARM architected timer wrap erratum i940 */
> interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,
> @@ -1241,3 +1242,22 @@ timer@0 {
> assigned-clock-parents = <&sys_32k_ck>;
> };
> };
> +
> +/* Local timers, see ARM architected timer wrap erratum i940 */
> +&timer3_target {
> + ti,no-reset-on-init;
> + ti,no-idle;
> + timer@0 {
> + assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER3_CLKCTRL 24>;
> + assigned-clock-parents = <&timer_sys_clk_div>;
> + };
> +};
> +
> +&timer4_target {
> + ti,no-reset-on-init;
> + ti,no-idle;
> + timer@0 {
> + assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER4_CLKCTRL 24>;
> + assigned-clock-parents = <&timer_sys_clk_div>;
> + };
> +};
> diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/clocksource/timer-ti-dm-systimer.c
> --- a/drivers/clocksource/timer-ti-dm-systimer.c
> +++ b/drivers/clocksource/timer-ti-dm-systimer.c
> @@ -2,6 +2,7 @@
> #include <linux/clk.h>
> #include <linux/clocksource.h>
> #include <linux/clockchips.h>
> +#include <linux/cpuhotplug.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> @@ -634,6 +635,78 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
> return error;
> }
>
> +/* Dmtimer as percpu timer. See dra7 ARM architected timer wrap erratum i940 */
> +static DEFINE_PER_CPU(struct dmtimer_clockevent, dmtimer_percpu_timer);
> +
> +static int __init dmtimer_percpu_timer_init(struct device_node *np, int cpu)
> +{
> + struct dmtimer_clockevent *clkevt;
> + int error;
> +
> + if (!cpu_possible(cpu))
> + return -EINVAL;
> +
> + if (!of_property_read_bool(np->parent, "ti,no-reset-on-init") ||
> + !of_property_read_bool(np->parent, "ti,no-idle"))
> + pr_warn("Incomplete dtb for percpu dmtimer %pOF\n", np->parent);
> +
> + clkevt = per_cpu_ptr(&dmtimer_percpu_timer, cpu);
> +
> + error = dmtimer_clkevt_init_common(clkevt, np, CLOCK_EVT_FEAT_ONESHOT,
> + cpumask_of(cpu), "percpu-dmtimer",
> + 500);
> + if (error)
> + return error;
> +
> + return 0;
> +}
> +
> +/* See TRM for timer internal resynch latency */
> +static int omap_dmtimer_starting_cpu(unsigned int cpu)
> +{
> + struct dmtimer_clockevent *clkevt = per_cpu_ptr(&dmtimer_percpu_timer, cpu);
> + struct clock_event_device *dev = &clkevt->dev;
> + struct dmtimer_systimer *t = &clkevt->t;
> +
> + clockevents_config_and_register(dev, t->rate, 3, ULONG_MAX);
> + irq_force_affinity(dev->irq, cpumask_of(cpu));
> +
> + return 0;
> +}
> +
> +static int __init dmtimer_percpu_timer_startup(void)
> +{
> + struct dmtimer_clockevent *clkevt = per_cpu_ptr(&dmtimer_percpu_timer, 0);
> + struct dmtimer_systimer *t = &clkevt->t;
> +
> + if (t->sysc) {
> + cpuhp_setup_state(CPUHP_AP_TI_GP_TIMER_STARTING,
> + "clockevents/omap/gptimer:starting",
> + omap_dmtimer_starting_cpu, NULL);
> + }
> +
> + return 0;
> +}
> +subsys_initcall(dmtimer_percpu_timer_startup);
> +
> +static int __init dmtimer_percpu_quirk_init(struct device_node *np, u32 pa)
> +{
> + struct device_node *arm_timer;
> +
> + arm_timer = of_find_compatible_node(NULL, NULL, "arm,armv7-timer");
> + if (of_device_is_available(arm_timer)) {
> + pr_warn_once("ARM architected timer wrap issue i940 detected\n");
> + return 0;
> + }
> +
> + if (pa == 0x48034000) /* dra7 dmtimer3 */
> + return dmtimer_percpu_timer_init(np, 0);
> + else if (pa == 0x48036000) /* dra7 dmtimer4 */
> + return dmtimer_percpu_timer_init(np, 1);
> +
> + return 0;
> +}
> +
> /* Clocksource */
> static struct dmtimer_clocksource *
> to_dmtimer_clocksource(struct clocksource *cs)
> @@ -767,6 +840,9 @@ static int __init dmtimer_systimer_init(struct device_node *np)
> if (clockevent == pa)
> return dmtimer_clockevent_init(np);
>
> + if (of_machine_is_compatible("ti,dra7"))
> + return dmtimer_percpu_quirk_init(np, pa);
> +
> return 0;
> }
>
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -135,6 +135,7 @@ enum cpuhp_state {
> CPUHP_AP_RISCV_TIMER_STARTING,
> CPUHP_AP_CLINT_TIMER_STARTING,
> CPUHP_AP_CSKY_TIMER_STARTING,
> + CPUHP_AP_TI_GP_TIMER_STARTING,
> CPUHP_AP_HYPERV_TIMER_STARTING,
> CPUHP_AP_KVM_STARTING,
> CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
>


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

2021-03-22 10:19:16

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 2/2] clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940

Hi,

* Daniel Lezcano <[email protected]> [210322 10:16]:
> On 04/03/2021 08:37, Tony Lindgren wrote:
> > There is a timer wrap issue on dra7 for the ARM architected timer.
> > In a typical clock configuration the timer fails to wrap after 388 days.
> >
> > To work around the issue, we need to use timer-ti-dm percpu timers instead.
> >
> > Let's configure dmtimer3 and 4 as percpu timers by default, and warn about
> > the issue if the dtb is not configured properly.
> >
> > Let's do this as a single patch so it can be backported to v5.8 and later
> > kernels easily.
>
> Cc: <[email protected]> # v5.8+
>
> ??

Yes please, that would be great.

Thanks,

Tony

2021-03-22 15:59:27

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/2] clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue

On 04/03/2021 08:37, Tony Lindgren wrote:
> There is a timer wrap issue on dra7 for the ARM architected timer.
> In a typical clock configuration the timer fails to wrap after 388 days.
>
> To work around the issue, we need to use timer-ti-dm timers instead.
>
> Let's prepare for adding support for percpu timers by adding a common
> dmtimer_clkevt_init_common() and call it from dmtimer_clockevent_init().
> This patch makes no intentional functional changes.
>
> Signed-off-by: Tony Lindgren <[email protected]>
> ---

[ ... ]

> @@ -575,33 +574,60 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
> */
> writel_relaxed(OMAP_TIMER_CTRL_POSTED, t->base + t->ifctrl);
>
> + if (dev->cpumask == cpu_possible_mask)
> + irqflags = IRQF_TIMER;
> + else
> + irqflags = IRQF_TIMER | IRQF_NOBALANCING;

Can you explain the reasoning behind the test above ?

[ ... ]


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

2021-03-22 16:36:38

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/2] clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue

Hi,

* Daniel Lezcano <[email protected]> [210322 15:56]:
> On 04/03/2021 08:37, Tony Lindgren wrote:
> > There is a timer wrap issue on dra7 for the ARM architected timer.
> > In a typical clock configuration the timer fails to wrap after 388 days.
> >
> > To work around the issue, we need to use timer-ti-dm timers instead.
> >
> > Let's prepare for adding support for percpu timers by adding a common
> > dmtimer_clkevt_init_common() and call it from dmtimer_clockevent_init().
> > This patch makes no intentional functional changes.
> >
> > Signed-off-by: Tony Lindgren <[email protected]>
> > ---
>
> [ ... ]
>
> > @@ -575,33 +574,60 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
> > */
> > writel_relaxed(OMAP_TIMER_CTRL_POSTED, t->base + t->ifctrl);
> >
> > + if (dev->cpumask == cpu_possible_mask)
> > + irqflags = IRQF_TIMER;
> > + else
> > + irqflags = IRQF_TIMER | IRQF_NOBALANCING;
>
> Can you explain the reasoning behind the test above ?

In the per cpu case we assign one dmtimer per cpu, and we want the
interrupt handling on the assigned CPU. In the per cpu case we have
the cpu specified with dev->cpumask unlike for the normal clockevent
case.

In the per cpu dmtimer case the interrupt line is not wired per cpu
though, so I don't think we want to add IRQF_PERCPU here.

Or do you have some better suggestion for the flags to use here? :)

Regards,

Tony

2021-03-22 18:27:38

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/2] clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue

On 22/03/2021 17:33, Tony Lindgren wrote:
> Hi,
>
> * Daniel Lezcano <[email protected]> [210322 15:56]:
>> On 04/03/2021 08:37, Tony Lindgren wrote:
>>> There is a timer wrap issue on dra7 for the ARM architected timer.
>>> In a typical clock configuration the timer fails to wrap after 388 days.
>>>
>>> To work around the issue, we need to use timer-ti-dm timers instead.
>>>
>>> Let's prepare for adding support for percpu timers by adding a common
>>> dmtimer_clkevt_init_common() and call it from dmtimer_clockevent_init().
>>> This patch makes no intentional functional changes.
>>>
>>> Signed-off-by: Tony Lindgren <[email protected]>
>>> ---
>>
>> [ ... ]
>>
>>> @@ -575,33 +574,60 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
>>> */
>>> writel_relaxed(OMAP_TIMER_CTRL_POSTED, t->base + t->ifctrl);
>>>
>>> + if (dev->cpumask == cpu_possible_mask)
>>> + irqflags = IRQF_TIMER;
>>> + else
>>> + irqflags = IRQF_TIMER | IRQF_NOBALANCING;
>>
>> Can you explain the reasoning behind the test above ?
>
> In the per cpu case we assign one dmtimer per cpu, and we want the
> interrupt handling on the assigned CPU. In the per cpu case we have
> the cpu specified with dev->cpumask unlike for the normal clockevent
> case.
>
> In the per cpu dmtimer case the interrupt line is not wired per cpu
> though, so I don't think we want to add IRQF_PERCPU here.

If it is per cpu, then the parameter will be cpumask_of(cpu). If there
is one cpu, no balancing can happen and then the IRQF_NOBALANCING is not
needed, neither this test and the irqflags, right?



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

2021-03-23 06:34:50

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH 1/2] clocksource/drivers/timer-ti-dm: Prepare to handle dra7 timer wrap issue

* Daniel Lezcano <[email protected]> [210322 18:24]:
> On 22/03/2021 17:33, Tony Lindgren wrote:
> > Hi,
> >
> > * Daniel Lezcano <[email protected]> [210322 15:56]:
> >> On 04/03/2021 08:37, Tony Lindgren wrote:
> >>> There is a timer wrap issue on dra7 for the ARM architected timer.
> >>> In a typical clock configuration the timer fails to wrap after 388 days.
> >>>
> >>> To work around the issue, we need to use timer-ti-dm timers instead.
> >>>
> >>> Let's prepare for adding support for percpu timers by adding a common
> >>> dmtimer_clkevt_init_common() and call it from dmtimer_clockevent_init().
> >>> This patch makes no intentional functional changes.
> >>>
> >>> Signed-off-by: Tony Lindgren <[email protected]>
> >>> ---
> >>
> >> [ ... ]
> >>
> >>> @@ -575,33 +574,60 @@ static int __init dmtimer_clockevent_init(struct device_node *np)
> >>> */
> >>> writel_relaxed(OMAP_TIMER_CTRL_POSTED, t->base + t->ifctrl);
> >>>
> >>> + if (dev->cpumask == cpu_possible_mask)
> >>> + irqflags = IRQF_TIMER;
> >>> + else
> >>> + irqflags = IRQF_TIMER | IRQF_NOBALANCING;
> >>
> >> Can you explain the reasoning behind the test above ?
> >
> > In the per cpu case we assign one dmtimer per cpu, and we want the
> > interrupt handling on the assigned CPU. In the per cpu case we have
> > the cpu specified with dev->cpumask unlike for the normal clockevent
> > case.
> >
> > In the per cpu dmtimer case the interrupt line is not wired per cpu
> > though, so I don't think we want to add IRQF_PERCPU here.
>
> If it is per cpu, then the parameter will be cpumask_of(cpu). If there
> is one cpu, no balancing can happen and then the IRQF_NOBALANCING is not
> needed, neither this test and the irqflags, right?

Oh yeah you're right, none of that is needed. For the percpu case we
already have irq_force_affinity() in omap_dmtimer_starting_cpu(). I'll
update and send out v2 of these two patches.

Thanks,

Tony