2019-05-24 15:36:58

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

Hello,

This series primarily unifies the driver code across all Tegra SoC
generations. In a result the clocksources are allocated per-CPU on
older Tegra's and have a higher rating than the arch-timer, the newer
Tegra210 is getting support for microsecond clocksource and the driver's
code is getting much cleaner. Note that arch-timer usage is discouraged on
all Tegra's due to the time jitter caused by the CPU frequency scaling.

The series was extensively tested on Tegra20 and Tegra30.

Changelog:

v3: Fixed compilation on ARM64. Turned out that it doesn't have the
delay-timer, thanks to Nicolas Chauvet for the report.

Added new "Support COMPILE_TEST universally" patch for better
compile-test coverage.

v2: Rebased on recent linux-next. Now all of #ifdef's are removed from the
code due to the recent patch that generalized persistent clocksource.

Couple other minor cosmetic changes.

Dmitry Osipenko (8):
clocksource/drivers/tegra: Support per-CPU timers on all Tegra's
clocksource/drivers/tegra: Unify timer code
clocksource/drivers/tegra: Reset hardware state on init
clocksource/drivers/tegra: Replace readl/writel with relaxed versions
clocksource/drivers/tegra: Release all IRQ's on request_irq() error
clocksource/drivers/tegra: Minor code clean up
clocksource/drivers/tegra: Use SPDX identifier
clocksource/drivers/tegra: Support COMPILE_TEST universally

drivers/clocksource/Kconfig | 4 +-
drivers/clocksource/timer-tegra20.c | 276 +++++++++++++---------------
2 files changed, 127 insertions(+), 153 deletions(-)

--
2.21.0


2019-05-24 15:37:16

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 6/8] clocksource/drivers/tegra: Minor code clean up

Correct typo and use proper upper casing for acronyms in the comments,
use common style for error messages, prepend error messages with
"tegra-timer:", add error message for cpuhp_setup_state() failure and
clean up whitespaces in the code to fix checkpatch warnings.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 43 ++++++++++++++++-------------
1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 18b81d814b3b..12784a82fd57 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -15,6 +15,8 @@
*
*/

+#define pr_fmt(fmt) "tegra-timer: " fmt
+
#include <linux/clk.h>
#include <linux/clockchips.h>
#include <linux/cpu.h>
@@ -30,13 +32,13 @@

#include "timer-of.h"

-#define RTC_SECONDS 0x08
-#define RTC_SHADOW_SECONDS 0x0c
-#define RTC_MILLISECONDS 0x10
+#define RTC_SECONDS 0x08
+#define RTC_SHADOW_SECONDS 0x0c
+#define RTC_MILLISECONDS 0x10

-#define TIMERUS_CNTR_1US 0x10
-#define TIMERUS_USEC_CFG 0x14
-#define TIMERUS_CNTR_FREEZE 0x4c
+#define TIMERUS_CNTR_1US 0x10
+#define TIMERUS_USEC_CFG 0x14
+#define TIMERUS_CNTR_FREEZE 0x4c

#define TIMER_PTV 0x0
#define TIMER_PTV_EN BIT(31)
@@ -57,7 +59,7 @@ static u32 usec_config;
static void __iomem *timer_reg_base;

static int tegra_timer_set_next_event(unsigned long cycles,
- struct clock_event_device *evt)
+ struct clock_event_device *evt)
{
void __iomem *reg_base = timer_of_base(to_timer_of(evt));

@@ -178,15 +180,17 @@ static struct timer_of suspend_rtc_to = {

/*
* tegra_rtc_read - Reads the Tegra RTC registers
- * Care must be taken that this funciton is not called while the
+ * Care must be taken that this function 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(struct clocksource *cs)
{
void __iomem *reg_base = timer_of_base(&suspend_rtc_to);
+
u32 ms = readl_relaxed(reg_base + RTC_MILLISECONDS);
u32 s = readl_relaxed(reg_base + RTC_SHADOW_SECONDS);
+
return (u64)s * MSEC_PER_SEC + ms;
}

@@ -231,7 +235,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)

to = this_cpu_ptr(&tegra_to);
ret = timer_of_init(np, to);
- if (ret < 0)
+ if (ret)
goto out;

timer_reg_base = timer_of_base(to);
@@ -290,8 +294,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
cpu_to->clkevt.cpumask = cpumask_of(cpu);
cpu_to->clkevt.irq = irq_of_parse_and_map(np, idx);
if (!cpu_to->clkevt.irq) {
- pr_err("%s: can't map IRQ for CPU%d\n",
- __func__, cpu);
+ pr_err("failed to map irq for cpu%d\n", cpu);
ret = -EINVAL;
goto out_irq;
}
@@ -301,8 +304,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
IRQF_TIMER | IRQF_NOBALANCING,
cpu_to->clkevt.name, &cpu_to->clkevt);
if (ret) {
- pr_err("%s: cannot setup irq %d for CPU%d\n",
- __func__, cpu_to->clkevt.irq, cpu);
+ pr_err("failed to set up irq for cpu%d: %d\n",
+ cpu, ret);
irq_dispose_mapping(cpu_to->clkevt.irq);
cpu_to->clkevt.irq = 0;
goto out_irq;
@@ -321,11 +324,14 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
register_current_timer_delay(&tegra_delay_timer);
#endif

- cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
- "AP_TEGRA_TIMER_STARTING", tegra_timer_setup,
- tegra_timer_stop);
+ ret = cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
+ "AP_TEGRA_TIMER_STARTING", tegra_timer_setup,
+ tegra_timer_stop);
+ if (ret)
+ pr_err("failed to set up cpu hp state: %d\n", ret);

return ret;
+
out_irq:
for_each_possible_cpu(cpu) {
struct timer_of *cpu_to;
@@ -338,6 +344,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
}
out:
timer_of_cleanup(to);
+
return ret;
}

@@ -361,8 +368,6 @@ static int __init tegra20_init_rtc(struct device_node *np)
if (ret)
return ret;

- clocksource_register_hz(&suspend_rtc_clocksource, 1000);
-
- return 0;
+ return clocksource_register_hz(&suspend_rtc_clocksource, 1000);
}
TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc);
--
2.21.0

2019-05-24 15:37:35

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 4/8] clocksource/drivers/tegra: Replace readl/writel with relaxed versions

The readl/writel functions are inserting memory barrier to ensure that
outstanding memory writes are completed, this results in L2 cache syncing
being done on Tegra20 and Tegra30 which isn't a very cheap operation.
Replace all readl/writel occurrences in the code with the relaxed versions
since there is no need for the memory-access syncing.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 35 +++++++++++++++--------------
1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 739f83fdb318..55e9b3e1fbeb 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -61,9 +61,9 @@ static int tegra_timer_set_next_event(unsigned long cycles,
{
void __iomem *reg_base = timer_of_base(to_timer_of(evt));

- writel(TIMER_PTV_EN |
- ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
- reg_base + TIMER_PTV);
+ writel_relaxed(TIMER_PTV_EN |
+ ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
+ reg_base + TIMER_PTV);

return 0;
}
@@ -72,7 +72,7 @@ static int tegra_timer_shutdown(struct clock_event_device *evt)
{
void __iomem *reg_base = timer_of_base(to_timer_of(evt));

- writel(0, reg_base + TIMER_PTV);
+ writel_relaxed(0, reg_base + TIMER_PTV);

return 0;
}
@@ -81,9 +81,9 @@ static int tegra_timer_set_periodic(struct clock_event_device *evt)
{
void __iomem *reg_base = timer_of_base(to_timer_of(evt));

- writel(TIMER_PTV_EN | TIMER_PTV_PER |
- ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
- reg_base + TIMER_PTV);
+ writel_relaxed(TIMER_PTV_EN | TIMER_PTV_PER |
+ ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
+ reg_base + TIMER_PTV);

return 0;
}
@@ -93,7 +93,7 @@ static irqreturn_t tegra_timer_isr(int irq, void *dev_id)
struct clock_event_device *evt = (struct clock_event_device *)dev_id;
void __iomem *reg_base = timer_of_base(to_timer_of(evt));

- writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
+ writel_relaxed(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
evt->event_handler(evt);

return IRQ_HANDLED;
@@ -103,12 +103,12 @@ static void tegra_timer_suspend(struct clock_event_device *evt)
{
void __iomem *reg_base = timer_of_base(to_timer_of(evt));

- writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
+ writel_relaxed(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
}

static void tegra_timer_resume(struct clock_event_device *evt)
{
- writel(usec_config, timer_reg_base + TIMERUS_USEC_CFG);
+ writel_relaxed(usec_config, timer_reg_base + TIMERUS_USEC_CFG);
}

static DEFINE_PER_CPU(struct timer_of, tegra_to) = {
@@ -132,8 +132,8 @@ static int tegra_timer_setup(unsigned int cpu)
{
struct timer_of *to = per_cpu_ptr(&tegra_to, cpu);

- writel(0, timer_of_base(to) + TIMER_PTV);
- writel(TIMER_PCR_INTR_CLR, timer_of_base(to) + TIMER_PCR);
+ writel_relaxed(0, timer_of_base(to) + TIMER_PTV);
+ writel_relaxed(TIMER_PCR_INTR_CLR, timer_of_base(to) + TIMER_PCR);

irq_force_affinity(to->clkevt.irq, cpumask_of(cpu));
enable_irq(to->clkevt.irq);
@@ -157,13 +157,13 @@ static int tegra_timer_stop(unsigned int cpu)

static u64 notrace tegra_read_sched_clock(void)
{
- return readl(timer_reg_base + TIMERUS_CNTR_1US);
+ return readl_relaxed(timer_reg_base + TIMERUS_CNTR_1US);
}

#ifdef CONFIG_ARM
static unsigned long tegra_delay_timer_read_counter_long(void)
{
- return readl(timer_reg_base + TIMERUS_CNTR_1US);
+ return readl_relaxed(timer_reg_base + TIMERUS_CNTR_1US);
}

static struct delay_timer tegra_delay_timer = {
@@ -184,8 +184,9 @@ static struct timer_of suspend_rtc_to = {
*/
static u64 tegra_rtc_read_ms(struct clocksource *cs)
{
- u32 ms = readl(timer_of_base(&suspend_rtc_to) + RTC_MILLISECONDS);
- u32 s = readl(timer_of_base(&suspend_rtc_to) + RTC_SHADOW_SECONDS);
+ void __iomem *reg_base = timer_of_base(&suspend_rtc_to);
+ u32 ms = readl_relaxed(reg_base + RTC_MILLISECONDS);
+ u32 s = readl_relaxed(reg_base + RTC_SHADOW_SECONDS);
return (u64)s * MSEC_PER_SEC + ms;
}

@@ -270,7 +271,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
goto out;
}

- writel(usec_config, timer_reg_base + TIMERUS_USEC_CFG);
+ writel_relaxed(usec_config, timer_reg_base + TIMERUS_USEC_CFG);

for_each_possible_cpu(cpu) {
struct timer_of *cpu_to = per_cpu_ptr(&tegra_to, cpu);
--
2.21.0

2019-05-24 15:37:36

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 7/8] clocksource/drivers/tegra: Use SPDX identifier

Use SPDX tag for the license identification instead of a free form text
to aid license-checking automation and for brevity.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 12784a82fd57..1a3ee928e9a5 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -1,18 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (C) 2010 Google, Inc.
- *
- * Author:
- * Colin Cross <[email protected]>
- *
- * 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.
- *
+ * Author: Colin Cross <[email protected]>
*/

#define pr_fmt(fmt) "tegra-timer: " fmt
--
2.21.0

2019-05-24 15:37:36

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 5/8] clocksource/drivers/tegra: Release all IRQ's on request_irq() error

Release all requested IRQ's on the request error to properly clean up
allocated resources.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 55e9b3e1fbeb..18b81d814b3b 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -293,7 +293,7 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
pr_err("%s: can't map IRQ for CPU%d\n",
__func__, cpu);
ret = -EINVAL;
- goto out;
+ goto out_irq;
}

irq_set_status_flags(cpu_to->clkevt.irq, IRQ_NOAUTOEN);
@@ -303,7 +303,8 @@ static int __init tegra_init_timer(struct device_node *np, bool tegra20)
if (ret) {
pr_err("%s: cannot setup irq %d for CPU%d\n",
__func__, cpu_to->clkevt.irq, cpu);
- ret = -EINVAL;
+ irq_dispose_mapping(cpu_to->clkevt.irq);
+ cpu_to->clkevt.irq = 0;
goto out_irq;
}
}
--
2.21.0

2019-05-24 15:38:31

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 1/8] clocksource/drivers/tegra: Support per-CPU timers on all Tegra's

Assign TMR1-4 per-CPU core on 32bit Tegra's in a way it is done for
Tegra210. In a result each core can handle its own timer events, less
code is unique to ARM64 and Tegra's clock events driver now has higher
rating on all Tegra's, replacing the ARM's TWD timer which isn't very
accurate due to the clock rate jitter caused by CPU frequency scaling.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 120 ++++++++++------------------
1 file changed, 43 insertions(+), 77 deletions(-)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 919b3568c495..58e8bb6deac9 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -49,13 +49,18 @@
#define TIMER_PCR_INTR_CLR BIT(30)

#ifdef CONFIG_ARM
-#define TIMER_CPU0 0x50 /* TIMER3 */
+#define TIMER_CPU0 0x00 /* TIMER1 */
+#define TIMER_CPU2 0x50 /* TIMER3 */
+#define TIMER1_IRQ_IDX 0
+#define IRQ_IDX_FOR_CPU(cpu) (TIMER1_IRQ_IDX + cpu)
+#define TIMER_BASE_FOR_CPU(cpu) \
+ (((cpu) & 1) * 8 + ((cpu) < 2 ? TIMER_CPU0 : TIMER_CPU2))
#else
#define TIMER_CPU0 0x90 /* TIMER10 */
#define TIMER10_IRQ_IDX 10
#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
-#endif
#define TIMER_BASE_FOR_CPU(cpu) (TIMER_CPU0 + (cpu) * 8)
+#endif

static u32 usec_config;
static void __iomem *timer_reg_base;
@@ -118,7 +123,6 @@ static void tegra_timer_resume(struct clock_event_device *evt)
writel(usec_config, timer_reg_base + TIMERUS_USEC_CFG);
}

-#ifdef CONFIG_ARM64
static DEFINE_PER_CPU(struct timer_of, tegra_to) = {
.flags = TIMER_OF_CLOCK | TIMER_OF_BASE,

@@ -159,33 +163,8 @@ static int tegra_timer_stop(unsigned int cpu)

return 0;
}
-#else /* CONFIG_ARM */
-static struct timer_of tegra_to = {
- .flags = TIMER_OF_CLOCK | TIMER_OF_BASE | TIMER_OF_IRQ,
-
- .clkevt = {
- .name = "tegra_timer",
- .rating = 300,
- .features = CLOCK_EVT_FEAT_ONESHOT |
- CLOCK_EVT_FEAT_PERIODIC |
- CLOCK_EVT_FEAT_DYNIRQ,
- .set_next_event = tegra_timer_set_next_event,
- .set_state_shutdown = tegra_timer_shutdown,
- .set_state_periodic = tegra_timer_set_periodic,
- .set_state_oneshot = tegra_timer_shutdown,
- .tick_resume = tegra_timer_shutdown,
- .suspend = tegra_timer_suspend,
- .resume = tegra_timer_resume,
- .cpumask = cpu_possible_mask,
- },
-
- .of_irq = {
- .index = 2,
- .flags = IRQF_TIMER | IRQF_TRIGGER_HIGH,
- .handler = tegra_timer_isr,
- },
-};

+#ifdef CONFIG_ARM
static u64 notrace tegra_read_sched_clock(void)
{
return readl(timer_reg_base + TIMERUS_CNTR_1US);
@@ -222,10 +201,12 @@ static struct clocksource suspend_rtc_clocksource = {
};
#endif

-static int tegra_timer_common_init(struct device_node *np, struct timer_of *to)
+static int tegra_init_timer(struct device_node *np, bool tegra20)
{
- int ret = 0;
+ struct timer_of *to;
+ int cpu, ret;

+ to = this_cpu_ptr(&tegra_to);
ret = timer_of_init(np, to);
if (ret < 0)
goto out;
@@ -267,29 +248,19 @@ static int tegra_timer_common_init(struct device_node *np, struct timer_of *to)
goto out;
}

- writel(usec_config, timer_of_base(to) + TIMERUS_USEC_CFG);
-
-out:
- return ret;
-}
-
-#ifdef CONFIG_ARM64
-static int __init tegra_init_timer(struct device_node *np)
-{
- int cpu, ret = 0;
- struct timer_of *to;
-
- to = this_cpu_ptr(&tegra_to);
- ret = tegra_timer_common_init(np, to);
- if (ret < 0)
- goto out;
+ writel(usec_config, timer_reg_base + TIMERUS_USEC_CFG);

for_each_possible_cpu(cpu) {
- struct timer_of *cpu_to;
+ struct timer_of *cpu_to = per_cpu_ptr(&tegra_to, cpu);
+
+ /*
+ * TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the
+ * parent clock.
+ */
+ if (tegra20)
+ cpu_to->of_clk.rate = 1000000;

- cpu_to = per_cpu_ptr(&tegra_to, cpu);
cpu_to->of_base.base = timer_reg_base + TIMER_BASE_FOR_CPU(cpu);
- cpu_to->of_clk.rate = timer_of_rate(to);
cpu_to->clkevt.cpumask = cpumask_of(cpu);
cpu_to->clkevt.irq =
irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
@@ -331,43 +302,39 @@ static int __init tegra_init_timer(struct device_node *np)
timer_of_cleanup(to);
return ret;
}
+
+#ifdef CONFIG_ARM64
+static int __init tegra210_init_timer(struct device_node *np)
+{
+ return tegra_init_timer(np, false);
+}
+TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_init_timer);
#else /* CONFIG_ARM */
-static int __init tegra_init_timer(struct device_node *np)
+static int __init tegra20_init_timer(struct device_node *np)
{
- int ret = 0;
+ struct timer_of *to;
+ int err;

- ret = tegra_timer_common_init(np, &tegra_to);
- if (ret < 0)
- goto out;
+ err = tegra_init_timer(np, true);
+ if (err)
+ return err;

- tegra_to.of_base.base = timer_reg_base + TIMER_BASE_FOR_CPU(0);
- tegra_to.of_clk.rate = 1000000; /* microsecond timer */
+ to = this_cpu_ptr(&tegra_to);

sched_clock_register(tegra_read_sched_clock, 32,
- timer_of_rate(&tegra_to));
- ret = clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
- "timer_us", timer_of_rate(&tegra_to),
+ timer_of_rate(to));
+ err = clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
+ "timer_us", timer_of_rate(to),
300, 32, clocksource_mmio_readl_up);
- if (ret) {
- pr_err("Failed to register clocksource\n");
- goto out;
- }
+ if (err)
+ pr_err("Failed to register clocksource: %d\n", err);

tegra_delay_timer.read_current_timer =
tegra_delay_timer_read_counter_long;
- tegra_delay_timer.freq = timer_of_rate(&tegra_to);
+ tegra_delay_timer.freq = timer_of_rate(to);
register_current_timer_delay(&tegra_delay_timer);

- clockevents_config_and_register(&tegra_to.clkevt,
- timer_of_rate(&tegra_to),
- 0x1,
- 0x1fffffff);
-
- return ret;
-out:
- timer_of_cleanup(&tegra_to);
-
- return ret;
+ return 0;
}

static int __init tegra20_init_rtc(struct device_node *np)
@@ -383,6 +350,5 @@ static int __init tegra20_init_rtc(struct device_node *np)
return 0;
}
TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc);
+TIMER_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra20_init_timer);
#endif
-TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra_init_timer);
-TIMER_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra_init_timer);
--
2.21.0

2019-05-24 15:38:32

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 3/8] clocksource/drivers/tegra: Reset hardware state on init

Reset timer's hardware state to ensure that initially it is in a
predictable state.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 57e7aa2b80a3..739f83fdb318 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -132,6 +132,9 @@ static int tegra_timer_setup(unsigned int cpu)
{
struct timer_of *to = per_cpu_ptr(&tegra_to, cpu);

+ writel(0, timer_of_base(to) + TIMER_PTV);
+ writel(TIMER_PCR_INTR_CLR, timer_of_base(to) + TIMER_PCR);
+
irq_force_affinity(to->clkevt.irq, cpumask_of(cpu));
enable_irq(to->clkevt.irq);

--
2.21.0

2019-05-24 15:38:51

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 8/8] clocksource/drivers/tegra: Support COMPILE_TEST universally

Remove build dependency on ARM for compile-testing to allow non-arch
specific build-bots (like Intel's test robot) to compile the driver and
report about problems.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 3300739edce4..8c21e557181b 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -137,10 +137,10 @@ config SUN5I_HSTIMER
Enables support the Sun5i timer.

config TEGRA_TIMER
- bool "Tegra timer driver" if COMPILE_TEST
+ bool "Tegra timer driver"
select CLKSRC_MMIO
select TIMER_OF
- depends on ARM || ARM64
+ depends on ARCH_TEGRA || COMPILE_TEST
help
Enables support for the Tegra driver.

--
2.21.0

2019-05-24 15:39:18

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v3 2/8] clocksource/drivers/tegra: Unify timer code

Tegra132 is 64bit platform and it has the tegra20-timer hardware unit.
Right now the corresponding timer code isn't compiled for ARM64, remove
ifdef'iness from the code and compile tegra20-timer for both 32 and 64 bit
platforms. Also note that like the older generations, Tegra210 has the
microseconds counter, hence the timer_us clocksource is now made available
for Tegra210 as well.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clocksource/timer-tegra20.c | 111 +++++++++++++++-------------
1 file changed, 60 insertions(+), 51 deletions(-)

diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 58e8bb6deac9..57e7aa2b80a3 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -30,10 +30,6 @@

#include "timer-of.h"

-#ifdef CONFIG_ARM
-#include <asm/mach/time.h>
-#endif
-
#define RTC_SECONDS 0x08
#define RTC_SHADOW_SECONDS 0x0c
#define RTC_MILLISECONDS 0x10
@@ -48,25 +44,17 @@
#define TIMER_PCR 0x4
#define TIMER_PCR_INTR_CLR BIT(30)

-#ifdef CONFIG_ARM
-#define TIMER_CPU0 0x00 /* TIMER1 */
-#define TIMER_CPU2 0x50 /* TIMER3 */
+#define TIMER1_BASE 0x00
+#define TIMER2_BASE 0x08
+#define TIMER3_BASE 0x50
+#define TIMER4_BASE 0x58
+#define TIMER10_BASE 0x90
+
#define TIMER1_IRQ_IDX 0
-#define IRQ_IDX_FOR_CPU(cpu) (TIMER1_IRQ_IDX + cpu)
-#define TIMER_BASE_FOR_CPU(cpu) \
- (((cpu) & 1) * 8 + ((cpu) < 2 ? TIMER_CPU0 : TIMER_CPU2))
-#else
-#define TIMER_CPU0 0x90 /* TIMER10 */
#define TIMER10_IRQ_IDX 10
-#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
-#define TIMER_BASE_FOR_CPU(cpu) (TIMER_CPU0 + (cpu) * 8)
-#endif

static u32 usec_config;
static void __iomem *timer_reg_base;
-#ifdef CONFIG_ARM
-static struct delay_timer tegra_delay_timer;
-#endif

static int tegra_timer_set_next_event(unsigned long cycles,
struct clock_event_device *evt)
@@ -164,17 +152,23 @@ static int tegra_timer_stop(unsigned int cpu)
return 0;
}

-#ifdef CONFIG_ARM
static u64 notrace tegra_read_sched_clock(void)
{
return readl(timer_reg_base + TIMERUS_CNTR_1US);
}

+#ifdef CONFIG_ARM
static unsigned long tegra_delay_timer_read_counter_long(void)
{
return readl(timer_reg_base + TIMERUS_CNTR_1US);
}

+static struct delay_timer tegra_delay_timer = {
+ .read_current_timer = tegra_delay_timer_read_counter_long,
+ .freq = 1000000,
+};
+#endif
+
static struct timer_of suspend_rtc_to = {
.flags = TIMER_OF_BASE | TIMER_OF_CLOCK,
};
@@ -199,9 +193,34 @@ static struct clocksource suspend_rtc_clocksource = {
.mask = CLOCKSOURCE_MASK(32),
.flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
};
-#endif

-static int tegra_init_timer(struct device_node *np, bool tegra20)
+static inline unsigned int tegra_base_for_cpu(int cpu, bool tegra20)
+{
+ if (tegra20) {
+ switch (cpu) {
+ case 0:
+ return TIMER1_BASE;
+ case 1:
+ return TIMER2_BASE;
+ case 2:
+ return TIMER3_BASE;
+ default:
+ return TIMER4_BASE;
+ }
+ }
+
+ return TIMER10_BASE + cpu * 8;
+}
+
+static inline unsigned int tegra_irq_idx_for_cpu(int cpu, bool tegra20)
+{
+ if (tegra20)
+ return TIMER1_IRQ_IDX + cpu;
+
+ return TIMER10_IRQ_IDX + cpu;
+}
+
+static int __init tegra_init_timer(struct device_node *np, bool tegra20)
{
struct timer_of *to;
int cpu, ret;
@@ -252,6 +271,8 @@ static int tegra_init_timer(struct device_node *np, bool tegra20)

for_each_possible_cpu(cpu) {
struct timer_of *cpu_to = per_cpu_ptr(&tegra_to, cpu);
+ unsigned int base = tegra_base_for_cpu(cpu, tegra20);
+ unsigned int idx = tegra_irq_idx_for_cpu(cpu, tegra20);

/*
* TIMER1-9 are fixed to 1MHz, TIMER10-13 are running off the
@@ -260,10 +281,10 @@ static int tegra_init_timer(struct device_node *np, bool tegra20)
if (tegra20)
cpu_to->of_clk.rate = 1000000;

- cpu_to->of_base.base = timer_reg_base + TIMER_BASE_FOR_CPU(cpu);
+ cpu_to = per_cpu_ptr(&tegra_to, cpu);
+ cpu_to->of_base.base = timer_reg_base + base;
cpu_to->clkevt.cpumask = cpumask_of(cpu);
- cpu_to->clkevt.irq =
- irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
+ cpu_to->clkevt.irq = irq_of_parse_and_map(np, idx);
if (!cpu_to->clkevt.irq) {
pr_err("%s: can't map IRQ for CPU%d\n",
__func__, cpu);
@@ -283,6 +304,18 @@ static int tegra_init_timer(struct device_node *np, bool tegra20)
}
}

+ sched_clock_register(tegra_read_sched_clock, 32, 1000000);
+
+ ret = clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
+ "timer_us", 1000000,
+ 300, 32, clocksource_mmio_readl_up);
+ if (ret)
+ pr_err("failed to register clocksource: %d\n", ret);
+
+#ifdef CONFIG_ARM
+ register_current_timer_delay(&tegra_delay_timer);
+#endif
+
cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
"AP_TEGRA_TIMER_STARTING", tegra_timer_setup,
tegra_timer_stop);
@@ -303,39 +336,17 @@ static int tegra_init_timer(struct device_node *np, bool tegra20)
return ret;
}

-#ifdef CONFIG_ARM64
static int __init tegra210_init_timer(struct device_node *np)
{
return tegra_init_timer(np, false);
}
TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_init_timer);
-#else /* CONFIG_ARM */
+
static int __init tegra20_init_timer(struct device_node *np)
{
- struct timer_of *to;
- int err;
-
- err = tegra_init_timer(np, true);
- if (err)
- return err;
-
- to = this_cpu_ptr(&tegra_to);
-
- sched_clock_register(tegra_read_sched_clock, 32,
- timer_of_rate(to));
- err = clocksource_mmio_init(timer_reg_base + TIMERUS_CNTR_1US,
- "timer_us", timer_of_rate(to),
- 300, 32, clocksource_mmio_readl_up);
- if (err)
- pr_err("Failed to register clocksource: %d\n", err);
-
- tegra_delay_timer.read_current_timer =
- tegra_delay_timer_read_counter_long;
- tegra_delay_timer.freq = timer_of_rate(to);
- register_current_timer_delay(&tegra_delay_timer);
-
- return 0;
+ return tegra_init_timer(np, true);
}
+TIMER_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra20_init_timer);

static int __init tegra20_init_rtc(struct device_node *np)
{
@@ -350,5 +361,3 @@ static int __init tegra20_init_rtc(struct device_node *np)
return 0;
}
TIMER_OF_DECLARE(tegra20_rtc, "nvidia,tegra20-rtc", tegra20_init_rtc);
-TIMER_OF_DECLARE(tegra20_timer, "nvidia,tegra20-timer", tegra20_init_timer);
-#endif
--
2.21.0

2019-05-31 08:28:05

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
> Hello,
>
> This series primarily unifies the driver code across all Tegra SoC
> generations. In a result the clocksources are allocated per-CPU on
> older Tegra's and have a higher rating than the arch-timer, the newer
> Tegra210 is getting support for microsecond clocksource and the driver's
> code is getting much cleaner. Note that arch-timer usage is discouraged on
> all Tegra's due to the time jitter caused by the CPU frequency scaling.

I think the limitations are more as follows:

Chip timer suffers cpu dvfs jitter can wakeup from cc7
T20 us-timer No Yes
T20 twd timer Yes No?
T30 us-timer No Yes
T30 twd timer Yes No?
T114 us-timer No Yes
T114 arch timer No Yes
T124 us-timer No Yes
T124 arch timer No Yes
T210 us-timer No Yes
T210 arch timer No No
T210 clk_m timer No Yes

right?

Peter.

2019-05-31 12:36:30

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

31.05.2019 11:26, Peter De Schrijver пишет:
> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series primarily unifies the driver code across all Tegra SoC
>> generations. In a result the clocksources are allocated per-CPU on
>> older Tegra's and have a higher rating than the arch-timer, the newer
>> Tegra210 is getting support for microsecond clocksource and the driver's
>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>
> I think the limitations are more as follows:
>
> Chip timer suffers cpu dvfs jitter can wakeup from cc7
> T20 us-timer No Yes
> T20 twd timer Yes No?
> T30 us-timer No Yes
> T30 twd timer Yes No?
> T114 us-timer No Yes
> T114 arch timer No Yes
> T124 us-timer No Yes
> T124 arch timer No Yes
> T210 us-timer No Yes
> T210 arch timer No No
> T210 clk_m timer No Yes
>
> right?

Doesn't arch timer run off the CPU clock? If yes (that's what I
assumed), then it should be affected by the DVFS. Otherwise I'll lower
the clocksource's rating for T114/124/132.

TWD can't wake CPU from the power-down state, so it's a solid "No" for
TWD in the "can wakeup from cc7" column.

2019-05-31 20:32:35

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

On 31/05/2019 14:33, Dmitry Osipenko wrote:
> 31.05.2019 11:26, Peter De Schrijver пишет:
>> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>>> Hello,
>>>
>>> This series primarily unifies the driver code across all Tegra SoC
>>> generations. In a result the clocksources are allocated per-CPU on
>>> older Tegra's and have a higher rating than the arch-timer, the newer
>>> Tegra210 is getting support for microsecond clocksource and the driver's
>>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>>
>> I think the limitations are more as follows:
>>
>> Chip timer suffers cpu dvfs jitter can wakeup from cc7
>> T20 us-timer No Yes
>> T20 twd timer Yes No?
>> T30 us-timer No Yes
>> T30 twd timer Yes No?
>> T114 us-timer No Yes
>> T114 arch timer No Yes
>> T124 us-timer No Yes
>> T124 arch timer No Yes
>> T210 us-timer No Yes
>> T210 arch timer No No
>> T210 clk_m timer No Yes
>>
>> right?
>
> Doesn't arch timer run off the CPU clock? If yes (that's what I
> assumed), then it should be affected by the DVFS. Otherwise I'll lower
> the clocksource's rating for T114/124/132.
>
> TWD can't wake CPU from the power-down state, so it's a solid "No" for
> TWD in the "can wakeup from cc7" column.

Wouldn't make sense to rename the timer-tegra20.c to timer-tegra.c now ?


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

2019-06-01 13:04:07

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

31.05.2019 23:31, Daniel Lezcano пишет:
> On 31/05/2019 14:33, Dmitry Osipenko wrote:
>> 31.05.2019 11:26, Peter De Schrijver пишет:
>>> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>>>> Hello,
>>>>
>>>> This series primarily unifies the driver code across all Tegra SoC
>>>> generations. In a result the clocksources are allocated per-CPU on
>>>> older Tegra's and have a higher rating than the arch-timer, the newer
>>>> Tegra210 is getting support for microsecond clocksource and the driver's
>>>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>>>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>>>
>>> I think the limitations are more as follows:
>>>
>>> Chip timer suffers cpu dvfs jitter can wakeup from cc7
>>> T20 us-timer No Yes
>>> T20 twd timer Yes No?
>>> T30 us-timer No Yes
>>> T30 twd timer Yes No?
>>> T114 us-timer No Yes
>>> T114 arch timer No Yes
>>> T124 us-timer No Yes
>>> T124 arch timer No Yes
>>> T210 us-timer No Yes
>>> T210 arch timer No No
>>> T210 clk_m timer No Yes
>>>
>>> right?
>>
>> Doesn't arch timer run off the CPU clock? If yes (that's what I
>> assumed), then it should be affected by the DVFS. Otherwise I'll lower
>> the clocksource's rating for T114/124/132.
>>
>> TWD can't wake CPU from the power-down state, so it's a solid "No" for
>> TWD in the "can wakeup from cc7" column.
>
> Wouldn't make sense to rename the timer-tegra20.c to timer-tegra.c now ?

Wouldn't hurt, given the refreshment that driver is getting lately. I'll
include a patch for that in the next revision, thanks.

2019-06-03 09:49:34

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

On Fri, May 31, 2019 at 03:33:41PM +0300, Dmitry Osipenko wrote:
> 31.05.2019 11:26, Peter De Schrijver пишет:
> > On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
> >> Hello,
> >>
> >> This series primarily unifies the driver code across all Tegra SoC
> >> generations. In a result the clocksources are allocated per-CPU on
> >> older Tegra's and have a higher rating than the arch-timer, the newer
> >> Tegra210 is getting support for microsecond clocksource and the driver's
> >> code is getting much cleaner. Note that arch-timer usage is discouraged on
> >> all Tegra's due to the time jitter caused by the CPU frequency scaling.
> >
> > I think the limitations are more as follows:
> >
> > Chip timer suffers cpu dvfs jitter can wakeup from cc7
> > T20 us-timer No Yes
> > T20 twd timer Yes No?
> > T30 us-timer No Yes
> > T30 twd timer Yes No?
> > T114 us-timer No Yes
> > T114 arch timer No Yes
> > T124 us-timer No Yes
> > T124 arch timer No Yes
> > T210 us-timer No Yes
> > T210 arch timer No No
> > T210 clk_m timer No Yes
> >
> > right?
>
> Doesn't arch timer run off the CPU clock? If yes (that's what I
> assumed), then it should be affected by the DVFS. Otherwise I'll lower
> the clocksource's rating for T114/124/132.
>

No. It doesn't. This is the big change between A9 and later CPUs.

Peter.

> TWD can't wake CPU from the power-down state, so it's a solid "No" for
> TWD in the "can wakeup from cc7" column.

2019-06-03 12:15:38

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] NVIDIA Tegra clocksource driver improvements

03.06.2019 10:17, Peter De Schrijver пишет:
> On Fri, May 31, 2019 at 03:33:41PM +0300, Dmitry Osipenko wrote:
>> 31.05.2019 11:26, Peter De Schrijver пишет:
>>> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>>>> Hello,
>>>>
>>>> This series primarily unifies the driver code across all Tegra SoC
>>>> generations. In a result the clocksources are allocated per-CPU on
>>>> older Tegra's and have a higher rating than the arch-timer, the newer
>>>> Tegra210 is getting support for microsecond clocksource and the driver's
>>>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>>>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>>>
>>> I think the limitations are more as follows:
>>>
>>> Chip timer suffers cpu dvfs jitter can wakeup from cc7
>>> T20 us-timer No Yes
>>> T20 twd timer Yes No?
>>> T30 us-timer No Yes
>>> T30 twd timer Yes No?
>>> T114 us-timer No Yes
>>> T114 arch timer No Yes
>>> T124 us-timer No Yes
>>> T124 arch timer No Yes
>>> T210 us-timer No Yes
>>> T210 arch timer No No
>>> T210 clk_m timer No Yes
>>>
>>> right?
>>
>> Doesn't arch timer run off the CPU clock? If yes (that's what I
>> assumed), then it should be affected by the DVFS. Otherwise I'll lower
>> the clocksource's rating for T114/124/132.
>>
>
> No. It doesn't. This is the big change between A9 and later CPUs.

Thank you for the clarification, I'll add a patch to lower the rating
where appropriate.