* [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-28 9:18 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-28 9:18 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter
Cc: Daniel Lezcano, linux-kernel, Joseph Lo, linux-tegra,
Thomas Gleixner, linux-arm-kernel
Add support for the Tegra210 timer that runs at oscillator clock
(TMR10-TMR13). We need these timers to work as clock event device and to
replace the ARMv8 architected timer due to it can't survive across the
power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
source when CPU suspends in power down state.
Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
v2:
* add error clean-up code
---
drivers/clocksource/Kconfig | 3 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 273 insertions(+)
create mode 100644 drivers/clocksource/timer-tegra210.c
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a9e26f6a81a1..e6e3e64b6320 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -135,6 +135,9 @@ config TEGRA_TIMER
help
Enables support for the Tegra driver.
+config TEGRA210_TIMER
+ def_bool ARCH_TEGRA_210_SOC
+
config VT8500_TIMER
bool "VT8500 timer driver" if COMPILE_TEST
depends on HAS_IOMEM
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index cdd210ff89ea..95de59c8a47b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
+obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
new file mode 100644
index 000000000000..477b164e540b
--- /dev/null
+++ b/drivers/clocksource/timer-tegra210.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/percpu.h>
+#include <linux/syscore_ops.h>
+
+static u32 tegra210_timer_freq;
+static void __iomem *tegra210_timer_reg_base;
+static u32 usec_config;
+
+#define TIMER_PTV 0x0
+#define TIMER_PTV_EN BIT(31)
+#define TIMER_PTV_PER BIT(30)
+#define TIMER_PCR 0x4
+#define TIMER_PCR_INTR_CLR BIT(30)
+#define TIMERUS_CNTR_1US 0x10
+#define TIMERUS_USEC_CFG 0x14
+
+#define TIMER10_OFFSET 0x90
+#define TIMER10_IRQ_IDX 10
+
+#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
+#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
+
+struct tegra210_clockevent {
+ struct clock_event_device evt;
+ char name[20];
+ void __iomem *reg_base;
+};
+#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
+
+static struct tegra210_clockevent __percpu *tegra210_evt;
+
+static int tegra210_timer_set_next_event(unsigned long cycles,
+ struct clock_event_device *evt)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = to_tegra_cevt(evt);
+ writel(TIMER_PTV_EN |
+ ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
+ tevt->reg_base + TIMER_PTV);
+
+ return 0;
+}
+
+static inline void timer_shutdown(struct tegra210_clockevent *tevt)
+{
+ writel(0, tevt->reg_base + TIMER_PTV);
+}
+
+static int tegra210_timer_shutdown(struct clock_event_device *evt)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = to_tegra_cevt(evt);
+ timer_shutdown(tevt);
+
+ return 0;
+}
+
+static int tegra210_timer_set_periodic(struct clock_event_device *evt)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = to_tegra_cevt(evt);
+ writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
+ tevt->reg_base + TIMER_PTV);
+
+ return 0;
+}
+
+static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = dev_id;
+ writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
+ tevt->evt.event_handler(&tevt->evt);
+
+ return IRQ_HANDLED;
+}
+
+static int tegra210_timer_setup(unsigned int cpu)
+{
+ struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
+
+ irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
+ enable_irq(tevt->evt.irq);
+
+ clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
+ 1, /* min */
+ 0x1fffffff); /* 29 bits */
+
+ return 0;
+}
+
+static int tegra210_timer_stop(unsigned int cpu)
+{
+ struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
+
+ tevt->evt.set_state_shutdown(&tevt->evt);
+ disable_irq_nosync(tevt->evt.irq);
+
+ return 0;
+}
+
+static int tegra_timer_suspend(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ void __iomem *reg_base = tegra210_timer_reg_base +
+ TIMER_FOR_CPU(cpu);
+ writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
+ }
+
+ return 0;
+}
+
+static void tegra_timer_resume(void)
+{
+ writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
+}
+
+static struct syscore_ops tegra_timer_syscore_ops = {
+ .suspend = tegra_timer_suspend,
+ .resume = tegra_timer_resume,
+};
+
+static int __init tegra210_timer_init(struct device_node *np)
+{
+ int cpu, ret = 0;
+ struct tegra210_clockevent *tevt;
+ struct clk *clk;
+
+ tegra210_evt = alloc_percpu(struct tegra210_clockevent);
+ if (!tegra210_evt) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ tegra210_timer_reg_base = of_iomap(np, 0);
+ if (!tegra210_timer_reg_base) {
+ ret = -ENXIO;
+ goto out_free_mem;
+ }
+
+ clk = of_clk_get(np, 0);
+ if (IS_ERR(clk)) {
+ ret = -EINVAL;
+ goto out_iounmap;
+ }
+
+ clk_prepare_enable(clk);
+ tegra210_timer_freq = clk_get_rate(clk);
+
+ for_each_possible_cpu(cpu) {
+ tevt = per_cpu_ptr(tegra210_evt, cpu);
+ tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
+ tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
+ if (!tevt->evt.irq) {
+ pr_err("%s: can't map IRQ for CPU%d\n",
+ __func__, cpu);
+ ret = -EINVAL;
+ goto out_clk;
+ }
+
+ snprintf(tevt->name, ARRAY_SIZE(tevt->name),
+ "tegra210_timer%d", cpu);
+ tevt->evt.name = tevt->name;
+ tevt->evt.cpumask = cpumask_of(cpu);
+ tevt->evt.set_next_event = tegra210_timer_set_next_event;
+ tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
+ tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
+ tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
+ tevt->evt.tick_resume = tegra210_timer_shutdown;
+ tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_ONESHOT;
+ tevt->evt.rating = 460;
+
+ irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
+ ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
+ IRQF_TIMER | IRQF_NOBALANCING,
+ tevt->name, tevt);
+ if (ret) {
+ pr_err("%s: cannot setup irq %d for CPU%d\n",
+ __func__, tevt->evt.irq, cpu);
+ ret = -EINVAL;
+ goto out_irq;
+ }
+ }
+
+ /*
+ * Configure microsecond timers to have 1MHz clock
+ * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
+ * Uses n+1 scheme
+ */
+ switch (tegra210_timer_freq) {
+ case 12000000:
+ usec_config = 0x000b; /* (11+1)/(0+1) */
+ break;
+ case 12800000:
+ usec_config = 0x043f; /* (63+1)/(4+1) */
+ break;
+ case 13000000:
+ usec_config = 0x000c; /* (12+1)/(0+1) */
+ break;
+ case 16800000:
+ usec_config = 0x0453; /* (83+1)/(4+1) */
+ break;
+ case 19200000:
+ usec_config = 0x045f; /* (95+1)/(4+1) */
+ break;
+ case 26000000:
+ usec_config = 0x0019; /* (25+1)/(0+1) */
+ break;
+ case 38400000:
+ usec_config = 0x04bf; /* (191+1)/(4+1) */
+ break;
+ case 48000000:
+ usec_config = 0x002f; /* (47+1)/(0+1) */
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_irq;
+ }
+
+ writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
+
+ cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
+ "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
+ tegra210_timer_stop);
+
+ register_syscore_ops(&tegra_timer_syscore_ops);
+
+ return ret;
+
+out_irq:
+ for_each_possible_cpu(cpu) {
+ tevt = per_cpu_ptr(tegra210_evt, cpu);
+ if (tevt->evt.irq) {
+ free_irq(tevt->evt.irq, tevt);
+ irq_dispose_mapping(tevt->evt.irq);
+ }
+ }
+out_clk:
+ clk_put(clk);
+out_iounmap:
+ iounmap(tegra210_timer_reg_base);
+out_free_mem:
+ free_percpu(tegra210_evt);
+out:
+ return ret;
+}
+
+TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index fd586d0301e7..e78281d07b70 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -121,6 +121,7 @@ enum cpuhp_state {
CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
CPUHP_AP_ARM_TWD_STARTING,
CPUHP_AP_QCOM_TIMER_STARTING,
+ CPUHP_AP_TEGRA_TIMER_STARTING,
CPUHP_AP_ARMADA_TIMER_STARTING,
CPUHP_AP_MARCO_TIMER_STARTING,
CPUHP_AP_MIPS_GIC_TIMER_STARTING,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 40+ messages in thread
* [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-28 9:18 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-28 9:18 UTC (permalink / raw)
To: Thierry Reding, Jonathan Hunter
Cc: linux-tegra, linux-arm-kernel, Joseph Lo, Daniel Lezcano,
Thomas Gleixner, linux-kernel
Add support for the Tegra210 timer that runs at oscillator clock
(TMR10-TMR13). We need these timers to work as clock event device and to
replace the ARMv8 architected timer due to it can't survive across the
power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
source when CPU suspends in power down state.
Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
v2:
* add error clean-up code
---
drivers/clocksource/Kconfig | 3 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
4 files changed, 273 insertions(+)
create mode 100644 drivers/clocksource/timer-tegra210.c
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a9e26f6a81a1..e6e3e64b6320 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -135,6 +135,9 @@ config TEGRA_TIMER
help
Enables support for the Tegra driver.
+config TEGRA210_TIMER
+ def_bool ARCH_TEGRA_210_SOC
+
config VT8500_TIMER
bool "VT8500 timer driver" if COMPILE_TEST
depends on HAS_IOMEM
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index cdd210ff89ea..95de59c8a47b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
+obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
new file mode 100644
index 000000000000..477b164e540b
--- /dev/null
+++ b/drivers/clocksource/timer-tegra210.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/percpu.h>
+#include <linux/syscore_ops.h>
+
+static u32 tegra210_timer_freq;
+static void __iomem *tegra210_timer_reg_base;
+static u32 usec_config;
+
+#define TIMER_PTV 0x0
+#define TIMER_PTV_EN BIT(31)
+#define TIMER_PTV_PER BIT(30)
+#define TIMER_PCR 0x4
+#define TIMER_PCR_INTR_CLR BIT(30)
+#define TIMERUS_CNTR_1US 0x10
+#define TIMERUS_USEC_CFG 0x14
+
+#define TIMER10_OFFSET 0x90
+#define TIMER10_IRQ_IDX 10
+
+#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
+#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
+
+struct tegra210_clockevent {
+ struct clock_event_device evt;
+ char name[20];
+ void __iomem *reg_base;
+};
+#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
+
+static struct tegra210_clockevent __percpu *tegra210_evt;
+
+static int tegra210_timer_set_next_event(unsigned long cycles,
+ struct clock_event_device *evt)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = to_tegra_cevt(evt);
+ writel(TIMER_PTV_EN |
+ ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
+ tevt->reg_base + TIMER_PTV);
+
+ return 0;
+}
+
+static inline void timer_shutdown(struct tegra210_clockevent *tevt)
+{
+ writel(0, tevt->reg_base + TIMER_PTV);
+}
+
+static int tegra210_timer_shutdown(struct clock_event_device *evt)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = to_tegra_cevt(evt);
+ timer_shutdown(tevt);
+
+ return 0;
+}
+
+static int tegra210_timer_set_periodic(struct clock_event_device *evt)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = to_tegra_cevt(evt);
+ writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
+ tevt->reg_base + TIMER_PTV);
+
+ return 0;
+}
+
+static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
+{
+ struct tegra210_clockevent *tevt;
+
+ tevt = dev_id;
+ writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
+ tevt->evt.event_handler(&tevt->evt);
+
+ return IRQ_HANDLED;
+}
+
+static int tegra210_timer_setup(unsigned int cpu)
+{
+ struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
+
+ irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
+ enable_irq(tevt->evt.irq);
+
+ clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
+ 1, /* min */
+ 0x1fffffff); /* 29 bits */
+
+ return 0;
+}
+
+static int tegra210_timer_stop(unsigned int cpu)
+{
+ struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
+
+ tevt->evt.set_state_shutdown(&tevt->evt);
+ disable_irq_nosync(tevt->evt.irq);
+
+ return 0;
+}
+
+static int tegra_timer_suspend(void)
+{
+ int cpu;
+
+ for_each_possible_cpu(cpu) {
+ void __iomem *reg_base = tegra210_timer_reg_base +
+ TIMER_FOR_CPU(cpu);
+ writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
+ }
+
+ return 0;
+}
+
+static void tegra_timer_resume(void)
+{
+ writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
+}
+
+static struct syscore_ops tegra_timer_syscore_ops = {
+ .suspend = tegra_timer_suspend,
+ .resume = tegra_timer_resume,
+};
+
+static int __init tegra210_timer_init(struct device_node *np)
+{
+ int cpu, ret = 0;
+ struct tegra210_clockevent *tevt;
+ struct clk *clk;
+
+ tegra210_evt = alloc_percpu(struct tegra210_clockevent);
+ if (!tegra210_evt) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ tegra210_timer_reg_base = of_iomap(np, 0);
+ if (!tegra210_timer_reg_base) {
+ ret = -ENXIO;
+ goto out_free_mem;
+ }
+
+ clk = of_clk_get(np, 0);
+ if (IS_ERR(clk)) {
+ ret = -EINVAL;
+ goto out_iounmap;
+ }
+
+ clk_prepare_enable(clk);
+ tegra210_timer_freq = clk_get_rate(clk);
+
+ for_each_possible_cpu(cpu) {
+ tevt = per_cpu_ptr(tegra210_evt, cpu);
+ tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
+ tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
+ if (!tevt->evt.irq) {
+ pr_err("%s: can't map IRQ for CPU%d\n",
+ __func__, cpu);
+ ret = -EINVAL;
+ goto out_clk;
+ }
+
+ snprintf(tevt->name, ARRAY_SIZE(tevt->name),
+ "tegra210_timer%d", cpu);
+ tevt->evt.name = tevt->name;
+ tevt->evt.cpumask = cpumask_of(cpu);
+ tevt->evt.set_next_event = tegra210_timer_set_next_event;
+ tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
+ tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
+ tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
+ tevt->evt.tick_resume = tegra210_timer_shutdown;
+ tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_ONESHOT;
+ tevt->evt.rating = 460;
+
+ irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
+ ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
+ IRQF_TIMER | IRQF_NOBALANCING,
+ tevt->name, tevt);
+ if (ret) {
+ pr_err("%s: cannot setup irq %d for CPU%d\n",
+ __func__, tevt->evt.irq, cpu);
+ ret = -EINVAL;
+ goto out_irq;
+ }
+ }
+
+ /*
+ * Configure microsecond timers to have 1MHz clock
+ * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
+ * Uses n+1 scheme
+ */
+ switch (tegra210_timer_freq) {
+ case 12000000:
+ usec_config = 0x000b; /* (11+1)/(0+1) */
+ break;
+ case 12800000:
+ usec_config = 0x043f; /* (63+1)/(4+1) */
+ break;
+ case 13000000:
+ usec_config = 0x000c; /* (12+1)/(0+1) */
+ break;
+ case 16800000:
+ usec_config = 0x0453; /* (83+1)/(4+1) */
+ break;
+ case 19200000:
+ usec_config = 0x045f; /* (95+1)/(4+1) */
+ break;
+ case 26000000:
+ usec_config = 0x0019; /* (25+1)/(0+1) */
+ break;
+ case 38400000:
+ usec_config = 0x04bf; /* (191+1)/(4+1) */
+ break;
+ case 48000000:
+ usec_config = 0x002f; /* (47+1)/(0+1) */
+ break;
+ default:
+ ret = -EINVAL;
+ goto out_irq;
+ }
+
+ writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
+
+ cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
+ "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
+ tegra210_timer_stop);
+
+ register_syscore_ops(&tegra_timer_syscore_ops);
+
+ return ret;
+
+out_irq:
+ for_each_possible_cpu(cpu) {
+ tevt = per_cpu_ptr(tegra210_evt, cpu);
+ if (tevt->evt.irq) {
+ free_irq(tevt->evt.irq, tevt);
+ irq_dispose_mapping(tevt->evt.irq);
+ }
+ }
+out_clk:
+ clk_put(clk);
+out_iounmap:
+ iounmap(tegra210_timer_reg_base);
+out_free_mem:
+ free_percpu(tegra210_evt);
+out:
+ return ret;
+}
+
+TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index fd586d0301e7..e78281d07b70 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -121,6 +121,7 @@ enum cpuhp_state {
CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
CPUHP_AP_ARM_TWD_STARTING,
CPUHP_AP_QCOM_TIMER_STARTING,
+ CPUHP_AP_TEGRA_TIMER_STARTING,
CPUHP_AP_ARMADA_TIMER_STARTING,
CPUHP_AP_MARCO_TIMER_STARTING,
CPUHP_AP_MIPS_GIC_TIMER_STARTING,
--
2.20.1
^ permalink raw reply related [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-28 9:18 ` Joseph Lo
@ 2019-01-28 13:00 ` Daniel Lezcano
-1 siblings, 0 replies; 40+ messages in thread
From: Daniel Lezcano @ 2019-01-28 13:00 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Jonathan Hunter
Cc: linux-tegra, linux-arm-kernel, Thomas Gleixner, linux-kernel
On 28/01/2019 10:18, Joseph Lo wrote:
> Add support for the Tegra210 timer that runs at oscillator clock
> (TMR10-TMR13). We need these timers to work as clock event device and to
> replace the ARMv8 architected timer due to it can't survive across the
> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
> source when CPU suspends in power down state.
>
> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> v2:
> * add error clean-up code
> ---
> drivers/clocksource/Kconfig | 3 +
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/clocksource/timer-tegra210.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a9e26f6a81a1..e6e3e64b6320 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -135,6 +135,9 @@ config TEGRA_TIMER
> help
> Enables support for the Tegra driver.
>
> +config TEGRA210_TIMER
> + def_bool ARCH_TEGRA_210_SOC
> +
Please make the option consistent with the option below (VT8500_TIMER or
similar).
> config VT8500_TIMER
> bool "VT8500 timer driver" if COMPILE_TEST
> depends on HAS_IOMEM
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index cdd210ff89ea..95de59c8a47b 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
> new file mode 100644
> index 000000000000..477b164e540b
> --- /dev/null
> +++ b/drivers/clocksource/timer-tegra210.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/percpu.h>
> +#include <linux/syscore_ops.h>
> +
> +static u32 tegra210_timer_freq;
> +static void __iomem *tegra210_timer_reg_base;
> +static u32 usec_config;
> +
> +#define TIMER_PTV 0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PER BIT(30)
> +#define TIMER_PCR 0x4
> +#define TIMER_PCR_INTR_CLR BIT(30)
> +#define TIMERUS_CNTR_1US 0x10
> +#define TIMERUS_USEC_CFG 0x14
> +
> +#define TIMER10_OFFSET 0x90
> +#define TIMER10_IRQ_IDX 10
> +
> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
> +
> +struct tegra210_clockevent {
> + struct clock_event_device evt;
> + char name[20];
> + void __iomem *reg_base;
> +};
> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
> +
> +static struct tegra210_clockevent __percpu *tegra210_evt;
> +
> +static int tegra210_timer_set_next_event(unsigned long cycles,
> + struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN |
> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
> +{
> + writel(0, tevt->reg_base + TIMER_PTV);
> +}
> +
> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + timer_shutdown(tevt);
> +
> + return 0;
> +}
> +
> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = dev_id;
> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
> + tevt->evt.event_handler(&tevt->evt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int tegra210_timer_setup(unsigned int cpu)
> +{
> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
> +
> + irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
> + enable_irq(tevt->evt.irq);
> +
> + clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
> + 1, /* min */
> + 0x1fffffff); /* 29 bits */
> +
> + return 0;
> +}
> +
> +static int tegra210_timer_stop(unsigned int cpu)
> +{
> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
> +
> + tevt->evt.set_state_shutdown(&tevt->evt);
> + disable_irq_nosync(tevt->evt.irq);
> +
> + return 0;
> +}
> +
> +static int tegra_timer_suspend(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + void __iomem *reg_base = tegra210_timer_reg_base +
> + TIMER_FOR_CPU(cpu);
> + writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
> + }
> +
> + return 0;
> +}
> +
> +static void tegra_timer_resume(void)
> +{
> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
> +}
> +
> +static struct syscore_ops tegra_timer_syscore_ops = {
> + .suspend = tegra_timer_suspend,
> + .resume = tegra_timer_resume,
> +};
> +
> +static int __init tegra210_timer_init(struct device_node *np)
> +{
> + int cpu, ret = 0;
> + struct tegra210_clockevent *tevt;
> + struct clk *clk;
> +
> + tegra210_evt = alloc_percpu(struct tegra210_clockevent);
> + if (!tegra210_evt) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + tegra210_timer_reg_base = of_iomap(np, 0);
> + if (!tegra210_timer_reg_base) {
> + ret = -ENXIO;
> + goto out_free_mem;
> + }
> +
> + clk = of_clk_get(np, 0);
> + if (IS_ERR(clk)) {
> + ret = -EINVAL;
> + goto out_iounmap;
> + }
> +
> + clk_prepare_enable(clk);
> + tegra210_timer_freq = clk_get_rate(clk);
Use the timer-of API for each CPU.
> + for_each_possible_cpu(cpu) {
No cpu loop for the initialization, use the tegra210_timer_setup for
this (cf. armada 370 timer).
> + tevt = per_cpu_ptr(tegra210_evt, cpu);
> + tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
> + tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
> + if (!tevt->evt.irq) {
> + pr_err("%s: can't map IRQ for CPU%d\n",
> + __func__, cpu);
> + ret = -EINVAL;
> + goto out_clk;
> + }
> +
> + snprintf(tevt->name, ARRAY_SIZE(tevt->name),
> + "tegra210_timer%d", cpu);
> + tevt->evt.name = tevt->name;
> + tevt->evt.cpumask = cpumask_of(cpu);
> + tevt->evt.set_next_event = tegra210_timer_set_next_event;
> + tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
> + tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
> + tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
> + tevt->evt.tick_resume = tegra210_timer_shutdown;
> + tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
> + CLOCK_EVT_FEAT_ONESHOT;
> + tevt->evt.rating = 460;
> + irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
Why do you need to prevent the interrupt to be enabled ? If the timer is
stopped and cleared, no spurious interrupt will occur no ?
> + ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
> + IRQF_TIMER | IRQF_NOBALANCING,
> + tevt->name, tevt);
> + if (ret) {
> + pr_err("%s: cannot setup irq %d for CPU%d\n",
> + __func__, tevt->evt.irq, cpu);
> + ret = -EINVAL;
> + goto out_irq;
> + }
> + }
> +
> + /*
> + * Configure microsecond timers to have 1MHz clock
> + * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
Did you mean 0xwwqq ?
> + * Uses n+1 scheme
> + */
> + switch (tegra210_timer_freq) {
> + case 12000000:
> + usec_config = 0x000b; /* (11+1)/(0+1) */
> + break;
> + case 12800000:
> + usec_config = 0x043f; /* (63+1)/(4+1) */
> + break;
> + case 13000000:
> + usec_config = 0x000c; /* (12+1)/(0+1) */
> + break;
> + case 16800000:
> + usec_config = 0x0453; /* (83+1)/(4+1) */
> + break;
> + case 19200000:
> + usec_config = 0x045f; /* (95+1)/(4+1) */
> + break;
> + case 26000000:
> + usec_config = 0x0019; /* (25+1)/(0+1) */
> + break;
> + case 38400000:
> + usec_config = 0x04bf; /* (191+1)/(4+1) */
> + break;
> + case 48000000:
> + usec_config = 0x002f; /* (47+1)/(0+1) */
> + break;
> + default:
> + ret = -EINVAL;
> + goto out_irq;
> + }
> +
> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
> +
> + cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
> + "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
> + tegra210_timer_stop);
> +
> + register_syscore_ops(&tegra_timer_syscore_ops);
> +
> + return ret;
> +
> +out_irq:
> + for_each_possible_cpu(cpu) {
> + tevt = per_cpu_ptr(tegra210_evt, cpu);
> + if (tevt->evt.irq) {
> + free_irq(tevt->evt.irq, tevt);
> + irq_dispose_mapping(tevt->evt.irq);
> + }
> + }
> +out_clk:
> + clk_put(clk);
> +out_iounmap:
> + iounmap(tegra210_timer_reg_base);
> +out_free_mem:
> + free_percpu(tegra210_evt);
> +out:
> + return ret;
> +}
> +
> +TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index fd586d0301e7..e78281d07b70 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -121,6 +121,7 @@ enum cpuhp_state {
> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
> CPUHP_AP_ARM_TWD_STARTING,
> CPUHP_AP_QCOM_TIMER_STARTING,
> + CPUHP_AP_TEGRA_TIMER_STARTING,
> CPUHP_AP_ARMADA_TIMER_STARTING,
> CPUHP_AP_MARCO_TIMER_STARTING,
> CPUHP_AP_MIPS_GIC_TIMER_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
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-28 13:00 ` Daniel Lezcano
0 siblings, 0 replies; 40+ messages in thread
From: Daniel Lezcano @ 2019-01-28 13:00 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding, Jonathan Hunter
Cc: linux-tegra, Thomas Gleixner, linux-kernel, linux-arm-kernel
On 28/01/2019 10:18, Joseph Lo wrote:
> Add support for the Tegra210 timer that runs at oscillator clock
> (TMR10-TMR13). We need these timers to work as clock event device and to
> replace the ARMv8 architected timer due to it can't survive across the
> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
> source when CPU suspends in power down state.
>
> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> v2:
> * add error clean-up code
> ---
> drivers/clocksource/Kconfig | 3 +
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/clocksource/timer-tegra210.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a9e26f6a81a1..e6e3e64b6320 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -135,6 +135,9 @@ config TEGRA_TIMER
> help
> Enables support for the Tegra driver.
>
> +config TEGRA210_TIMER
> + def_bool ARCH_TEGRA_210_SOC
> +
Please make the option consistent with the option below (VT8500_TIMER or
similar).
> config VT8500_TIMER
> bool "VT8500 timer driver" if COMPILE_TEST
> depends on HAS_IOMEM
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index cdd210ff89ea..95de59c8a47b 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
> new file mode 100644
> index 000000000000..477b164e540b
> --- /dev/null
> +++ b/drivers/clocksource/timer-tegra210.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/percpu.h>
> +#include <linux/syscore_ops.h>
> +
> +static u32 tegra210_timer_freq;
> +static void __iomem *tegra210_timer_reg_base;
> +static u32 usec_config;
> +
> +#define TIMER_PTV 0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PER BIT(30)
> +#define TIMER_PCR 0x4
> +#define TIMER_PCR_INTR_CLR BIT(30)
> +#define TIMERUS_CNTR_1US 0x10
> +#define TIMERUS_USEC_CFG 0x14
> +
> +#define TIMER10_OFFSET 0x90
> +#define TIMER10_IRQ_IDX 10
> +
> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
> +
> +struct tegra210_clockevent {
> + struct clock_event_device evt;
> + char name[20];
> + void __iomem *reg_base;
> +};
> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
> +
> +static struct tegra210_clockevent __percpu *tegra210_evt;
> +
> +static int tegra210_timer_set_next_event(unsigned long cycles,
> + struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN |
> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
> +{
> + writel(0, tevt->reg_base + TIMER_PTV);
> +}
> +
> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + timer_shutdown(tevt);
> +
> + return 0;
> +}
> +
> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = dev_id;
> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
> + tevt->evt.event_handler(&tevt->evt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int tegra210_timer_setup(unsigned int cpu)
> +{
> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
> +
> + irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
> + enable_irq(tevt->evt.irq);
> +
> + clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
> + 1, /* min */
> + 0x1fffffff); /* 29 bits */
> +
> + return 0;
> +}
> +
> +static int tegra210_timer_stop(unsigned int cpu)
> +{
> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
> +
> + tevt->evt.set_state_shutdown(&tevt->evt);
> + disable_irq_nosync(tevt->evt.irq);
> +
> + return 0;
> +}
> +
> +static int tegra_timer_suspend(void)
> +{
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + void __iomem *reg_base = tegra210_timer_reg_base +
> + TIMER_FOR_CPU(cpu);
> + writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
> + }
> +
> + return 0;
> +}
> +
> +static void tegra_timer_resume(void)
> +{
> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
> +}
> +
> +static struct syscore_ops tegra_timer_syscore_ops = {
> + .suspend = tegra_timer_suspend,
> + .resume = tegra_timer_resume,
> +};
> +
> +static int __init tegra210_timer_init(struct device_node *np)
> +{
> + int cpu, ret = 0;
> + struct tegra210_clockevent *tevt;
> + struct clk *clk;
> +
> + tegra210_evt = alloc_percpu(struct tegra210_clockevent);
> + if (!tegra210_evt) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + tegra210_timer_reg_base = of_iomap(np, 0);
> + if (!tegra210_timer_reg_base) {
> + ret = -ENXIO;
> + goto out_free_mem;
> + }
> +
> + clk = of_clk_get(np, 0);
> + if (IS_ERR(clk)) {
> + ret = -EINVAL;
> + goto out_iounmap;
> + }
> +
> + clk_prepare_enable(clk);
> + tegra210_timer_freq = clk_get_rate(clk);
Use the timer-of API for each CPU.
> + for_each_possible_cpu(cpu) {
No cpu loop for the initialization, use the tegra210_timer_setup for
this (cf. armada 370 timer).
> + tevt = per_cpu_ptr(tegra210_evt, cpu);
> + tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
> + tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
> + if (!tevt->evt.irq) {
> + pr_err("%s: can't map IRQ for CPU%d\n",
> + __func__, cpu);
> + ret = -EINVAL;
> + goto out_clk;
> + }
> +
> + snprintf(tevt->name, ARRAY_SIZE(tevt->name),
> + "tegra210_timer%d", cpu);
> + tevt->evt.name = tevt->name;
> + tevt->evt.cpumask = cpumask_of(cpu);
> + tevt->evt.set_next_event = tegra210_timer_set_next_event;
> + tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
> + tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
> + tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
> + tevt->evt.tick_resume = tegra210_timer_shutdown;
> + tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
> + CLOCK_EVT_FEAT_ONESHOT;
> + tevt->evt.rating = 460;
> + irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
Why do you need to prevent the interrupt to be enabled ? If the timer is
stopped and cleared, no spurious interrupt will occur no ?
> + ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
> + IRQF_TIMER | IRQF_NOBALANCING,
> + tevt->name, tevt);
> + if (ret) {
> + pr_err("%s: cannot setup irq %d for CPU%d\n",
> + __func__, tevt->evt.irq, cpu);
> + ret = -EINVAL;
> + goto out_irq;
> + }
> + }
> +
> + /*
> + * Configure microsecond timers to have 1MHz clock
> + * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
Did you mean 0xwwqq ?
> + * Uses n+1 scheme
> + */
> + switch (tegra210_timer_freq) {
> + case 12000000:
> + usec_config = 0x000b; /* (11+1)/(0+1) */
> + break;
> + case 12800000:
> + usec_config = 0x043f; /* (63+1)/(4+1) */
> + break;
> + case 13000000:
> + usec_config = 0x000c; /* (12+1)/(0+1) */
> + break;
> + case 16800000:
> + usec_config = 0x0453; /* (83+1)/(4+1) */
> + break;
> + case 19200000:
> + usec_config = 0x045f; /* (95+1)/(4+1) */
> + break;
> + case 26000000:
> + usec_config = 0x0019; /* (25+1)/(0+1) */
> + break;
> + case 38400000:
> + usec_config = 0x04bf; /* (191+1)/(4+1) */
> + break;
> + case 48000000:
> + usec_config = 0x002f; /* (47+1)/(0+1) */
> + break;
> + default:
> + ret = -EINVAL;
> + goto out_irq;
> + }
> +
> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
> +
> + cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
> + "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
> + tegra210_timer_stop);
> +
> + register_syscore_ops(&tegra_timer_syscore_ops);
> +
> + return ret;
> +
> +out_irq:
> + for_each_possible_cpu(cpu) {
> + tevt = per_cpu_ptr(tegra210_evt, cpu);
> + if (tevt->evt.irq) {
> + free_irq(tevt->evt.irq, tevt);
> + irq_dispose_mapping(tevt->evt.irq);
> + }
> + }
> +out_clk:
> + clk_put(clk);
> +out_iounmap:
> + iounmap(tegra210_timer_reg_base);
> +out_free_mem:
> + free_percpu(tegra210_evt);
> +out:
> + return ret;
> +}
> +
> +TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index fd586d0301e7..e78281d07b70 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -121,6 +121,7 @@ enum cpuhp_state {
> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
> CPUHP_AP_ARM_TWD_STARTING,
> CPUHP_AP_QCOM_TIMER_STARTING,
> + CPUHP_AP_TEGRA_TIMER_STARTING,
> CPUHP_AP_ARMADA_TIMER_STARTING,
> CPUHP_AP_MARCO_TIMER_STARTING,
> CPUHP_AP_MIPS_GIC_TIMER_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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-28 13:00 ` Daniel Lezcano
(?)
@ 2019-01-29 3:07 ` Joseph Lo
-1 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-29 3:07 UTC (permalink / raw)
To: Daniel Lezcano, Thierry Reding, Jonathan Hunter
Cc: linux-tegra, linux-arm-kernel, Thomas Gleixner, linux-kernel
Hi Daniel,
Thanks for your review.
On 1/28/19 9:00 PM, Daniel Lezcano wrote:
> On 28/01/2019 10:18, Joseph Lo wrote:
>> Add support for the Tegra210 timer that runs at oscillator clock
>> (TMR10-TMR13). We need these timers to work as clock event device and to
>> replace the ARMv8 architected timer due to it can't survive across the
>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
>> source when CPU suspends in power down state.
>>
>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v2:
>> * add error clean-up code
>> ---
>> drivers/clocksource/Kconfig | 3 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a9e26f6a81a1..e6e3e64b6320 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>> help
>> Enables support for the Tegra driver.
>>
>> +config TEGRA210_TIMER
>> + def_bool ARCH_TEGRA_210_SOC
>> +
>
> Please make the option consistent with the option below (VT8500_TIMER or
> similar).
Okay, will check.
>
>> config VT8500_TIMER
>> bool "VT8500 timer driver" if COMPILE_TEST
>> depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index cdd210ff89ea..95de59c8a47b 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
>> new file mode 100644
>> index 000000000000..477b164e540b
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-tegra210.c
>> @@ -0,0 +1,268 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/percpu.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +static u32 tegra210_timer_freq;
>> +static void __iomem *tegra210_timer_reg_base;
>> +static u32 usec_config;
>> +
>> +#define TIMER_PTV 0x0
>> +#define TIMER_PTV_EN BIT(31)
>> +#define TIMER_PTV_PER BIT(30)
>> +#define TIMER_PCR 0x4
>> +#define TIMER_PCR_INTR_CLR BIT(30)
>> +#define TIMERUS_CNTR_1US 0x10
>> +#define TIMERUS_USEC_CFG 0x14
>> +
>> +#define TIMER10_OFFSET 0x90
>> +#define TIMER10_IRQ_IDX 10
>> +
>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>> +
>> +struct tegra210_clockevent {
>> + struct clock_event_device evt;
>> + char name[20];
>> + void __iomem *reg_base;
>> +};
>> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
>> +
>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>> +
>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>> + struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>> +{
>> + writel(0, tevt->reg_base + TIMER_PTV);
>> +}
>> +
>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + timer_shutdown(tevt);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = dev_id;
>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>> + tevt->evt.event_handler(&tevt->evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int tegra210_timer_setup(unsigned int cpu)
>> +{
>> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
>> +
>> + irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
>> + enable_irq(tevt->evt.irq);
>> +
>> + clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
>> + 1, /* min */
>> + 0x1fffffff); /* 29 bits */
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_stop(unsigned int cpu)
>> +{
>> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
>> +
>> + tevt->evt.set_state_shutdown(&tevt->evt);
>> + disable_irq_nosync(tevt->evt.irq);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra_timer_suspend(void)
>> +{
>> + int cpu;
>> +
>> + for_each_possible_cpu(cpu) {
>> + void __iomem *reg_base = tegra210_timer_reg_base +
>> + TIMER_FOR_CPU(cpu);
>> + writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void tegra_timer_resume(void)
>> +{
>> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
>> +}
>> +
>> +static struct syscore_ops tegra_timer_syscore_ops = {
>> + .suspend = tegra_timer_suspend,
>> + .resume = tegra_timer_resume,
>> +};
>> +
>> +static int __init tegra210_timer_init(struct device_node *np)
>> +{
>> + int cpu, ret = 0;
>> + struct tegra210_clockevent *tevt;
>> + struct clk *clk;
>> +
>> + tegra210_evt = alloc_percpu(struct tegra210_clockevent);
>> + if (!tegra210_evt) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + tegra210_timer_reg_base = of_iomap(np, 0);
>> + if (!tegra210_timer_reg_base) {
>> + ret = -ENXIO;
>> + goto out_free_mem;
>> + }
>> +
>> + clk = of_clk_get(np, 0);
>> + if (IS_ERR(clk)) {
>> + ret = -EINVAL;
>> + goto out_iounmap;
>> + }
>> +
>> + clk_prepare_enable(clk);
>> + tegra210_timer_freq = clk_get_rate(clk);
>
>
> Use the timer-of API for each CPU.
Okay, I'll try to apply that for TIMER_OF_BASE and TIMER_OF_CLOCK. But
the TIMER_OF_IRQ, as you see below, it didn't have the ability to handle
the case that multiple CPUs with different timer IRQ lines. Which means
it's not PPI.
>
>> + for_each_possible_cpu(cpu) {
>
>
> No cpu loop for the initialization, use the tegra210_timer_setup for
> this (cf. armada 370 timer).
As mentioned above, I may still need this for IRQs initialization for
each CPU cores. Something like the IRQ init code in "exnos_mct.c".
>
>> + tevt = per_cpu_ptr(tegra210_evt, cpu);
>> + tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
>> + tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
>> + if (!tevt->evt.irq) {
>> + pr_err("%s: can't map IRQ for CPU%d\n",
>> + __func__, cpu);
>> + ret = -EINVAL;
>> + goto out_clk;
>> + }
>> +
>> + snprintf(tevt->name, ARRAY_SIZE(tevt->name),
>> + "tegra210_timer%d", cpu);
>> + tevt->evt.name = tevt->name;
>> + tevt->evt.cpumask = cpumask_of(cpu);
>> + tevt->evt.set_next_event = tegra210_timer_set_next_event;
>> + tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
>> + tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
>> + tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
>> + tevt->evt.tick_resume = tegra210_timer_shutdown;
>> + tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
>> + CLOCK_EVT_FEAT_ONESHOT;
>> + tevt->evt.rating = 460;
>> + irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
>
> Why do you need to prevent the interrupt to be enabled ? If the timer is
> stopped and cleared, no spurious interrupt will occur no ?
This is to prevent the IRQ will been enabled twice in the timer_setup
API with CPU hotplug event triggered. So we only enable the IRQ once the
CPU is online.
>
>> + ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
>> + IRQF_TIMER | IRQF_NOBALANCING,
>> + tevt->name, tevt);
>> + if (ret) {
>> + pr_err("%s: cannot setup irq %d for CPU%d\n",
>> + __func__, tevt->evt.irq, cpu);
>> + ret = -EINVAL;
>> + goto out_irq;
>> + }
>> + }
>> +
>> + /*
>> + * Configure microsecond timers to have 1MHz clock
>> + * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
>
> Did you mean 0xwwqq ?
Ah, because it's microsecond timer, that means the fraction of 1
microsecond each clk represents. For ex, if the clk is running at 12MHz,
then each clk represents 1/12 of a microsecond.
Thanks,
Joseph
>
>> + * Uses n+1 scheme
>> + */
>> + switch (tegra210_timer_freq) {
>> + case 12000000:
>> + usec_config = 0x000b; /* (11+1)/(0+1) */
>> + break;
>> + case 12800000:
>> + usec_config = 0x043f; /* (63+1)/(4+1) */
>> + break;
>> + case 13000000:
>> + usec_config = 0x000c; /* (12+1)/(0+1) */
>> + break;
>> + case 16800000:
>> + usec_config = 0x0453; /* (83+1)/(4+1) */
>> + break;
>> + case 19200000:
>> + usec_config = 0x045f; /* (95+1)/(4+1) */
>> + break;
>> + case 26000000:
>> + usec_config = 0x0019; /* (25+1)/(0+1) */
>> + break;
>> + case 38400000:
>> + usec_config = 0x04bf; /* (191+1)/(4+1) */
>> + break;
>> + case 48000000:
>> + usec_config = 0x002f; /* (47+1)/(0+1) */
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + goto out_irq;
>> + }
>> +
>> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
>> +
>> + cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
>> + "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
>> + tegra210_timer_stop);
>> +
>> + register_syscore_ops(&tegra_timer_syscore_ops);
>> +
>> + return ret;
>> +
>> +out_irq:
>> + for_each_possible_cpu(cpu) {
>> + tevt = per_cpu_ptr(tegra210_evt, cpu);
>> + if (tevt->evt.irq) {
>> + free_irq(tevt->evt.irq, tevt);
>> + irq_dispose_mapping(tevt->evt.irq);
>> + }
>> + }
>> +out_clk:
>> + clk_put(clk);
>> +out_iounmap:
>> + iounmap(tegra210_timer_reg_base);
>> +out_free_mem:
>> + free_percpu(tegra210_evt);
>> +out:
>> + return ret;
>> +}
>> +
>> +TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index fd586d0301e7..e78281d07b70 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -121,6 +121,7 @@ enum cpuhp_state {
>> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>> CPUHP_AP_ARM_TWD_STARTING,
>> CPUHP_AP_QCOM_TIMER_STARTING,
>> + CPUHP_AP_TEGRA_TIMER_STARTING,
>> CPUHP_AP_ARMADA_TIMER_STARTING,
>> CPUHP_AP_MARCO_TIMER_STARTING,
>> CPUHP_AP_MIPS_GIC_TIMER_STARTING,
>>
>
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 3:07 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-29 3:07 UTC (permalink / raw)
To: Daniel Lezcano, Thierry Reding, Jonathan Hunter
Cc: linux-tegra, Thomas Gleixner, linux-kernel, linux-arm-kernel
Hi Daniel,
Thanks for your review.
On 1/28/19 9:00 PM, Daniel Lezcano wrote:
> On 28/01/2019 10:18, Joseph Lo wrote:
>> Add support for the Tegra210 timer that runs at oscillator clock
>> (TMR10-TMR13). We need these timers to work as clock event device and to
>> replace the ARMv8 architected timer due to it can't survive across the
>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
>> source when CPU suspends in power down state.
>>
>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v2:
>> * add error clean-up code
>> ---
>> drivers/clocksource/Kconfig | 3 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a9e26f6a81a1..e6e3e64b6320 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>> help
>> Enables support for the Tegra driver.
>>
>> +config TEGRA210_TIMER
>> + def_bool ARCH_TEGRA_210_SOC
>> +
>
> Please make the option consistent with the option below (VT8500_TIMER or
> similar).
Okay, will check.
>
>> config VT8500_TIMER
>> bool "VT8500 timer driver" if COMPILE_TEST
>> depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index cdd210ff89ea..95de59c8a47b 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
>> new file mode 100644
>> index 000000000000..477b164e540b
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-tegra210.c
>> @@ -0,0 +1,268 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/percpu.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +static u32 tegra210_timer_freq;
>> +static void __iomem *tegra210_timer_reg_base;
>> +static u32 usec_config;
>> +
>> +#define TIMER_PTV 0x0
>> +#define TIMER_PTV_EN BIT(31)
>> +#define TIMER_PTV_PER BIT(30)
>> +#define TIMER_PCR 0x4
>> +#define TIMER_PCR_INTR_CLR BIT(30)
>> +#define TIMERUS_CNTR_1US 0x10
>> +#define TIMERUS_USEC_CFG 0x14
>> +
>> +#define TIMER10_OFFSET 0x90
>> +#define TIMER10_IRQ_IDX 10
>> +
>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>> +
>> +struct tegra210_clockevent {
>> + struct clock_event_device evt;
>> + char name[20];
>> + void __iomem *reg_base;
>> +};
>> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
>> +
>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>> +
>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>> + struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>> +{
>> + writel(0, tevt->reg_base + TIMER_PTV);
>> +}
>> +
>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + timer_shutdown(tevt);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = dev_id;
>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>> + tevt->evt.event_handler(&tevt->evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int tegra210_timer_setup(unsigned int cpu)
>> +{
>> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
>> +
>> + irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
>> + enable_irq(tevt->evt.irq);
>> +
>> + clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
>> + 1, /* min */
>> + 0x1fffffff); /* 29 bits */
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_stop(unsigned int cpu)
>> +{
>> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
>> +
>> + tevt->evt.set_state_shutdown(&tevt->evt);
>> + disable_irq_nosync(tevt->evt.irq);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra_timer_suspend(void)
>> +{
>> + int cpu;
>> +
>> + for_each_possible_cpu(cpu) {
>> + void __iomem *reg_base = tegra210_timer_reg_base +
>> + TIMER_FOR_CPU(cpu);
>> + writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void tegra_timer_resume(void)
>> +{
>> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
>> +}
>> +
>> +static struct syscore_ops tegra_timer_syscore_ops = {
>> + .suspend = tegra_timer_suspend,
>> + .resume = tegra_timer_resume,
>> +};
>> +
>> +static int __init tegra210_timer_init(struct device_node *np)
>> +{
>> + int cpu, ret = 0;
>> + struct tegra210_clockevent *tevt;
>> + struct clk *clk;
>> +
>> + tegra210_evt = alloc_percpu(struct tegra210_clockevent);
>> + if (!tegra210_evt) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + tegra210_timer_reg_base = of_iomap(np, 0);
>> + if (!tegra210_timer_reg_base) {
>> + ret = -ENXIO;
>> + goto out_free_mem;
>> + }
>> +
>> + clk = of_clk_get(np, 0);
>> + if (IS_ERR(clk)) {
>> + ret = -EINVAL;
>> + goto out_iounmap;
>> + }
>> +
>> + clk_prepare_enable(clk);
>> + tegra210_timer_freq = clk_get_rate(clk);
>
>
> Use the timer-of API for each CPU.
Okay, I'll try to apply that for TIMER_OF_BASE and TIMER_OF_CLOCK. But
the TIMER_OF_IRQ, as you see below, it didn't have the ability to handle
the case that multiple CPUs with different timer IRQ lines. Which means
it's not PPI.
>
>> + for_each_possible_cpu(cpu) {
>
>
> No cpu loop for the initialization, use the tegra210_timer_setup for
> this (cf. armada 370 timer).
As mentioned above, I may still need this for IRQs initialization for
each CPU cores. Something like the IRQ init code in "exnos_mct.c".
>
>> + tevt = per_cpu_ptr(tegra210_evt, cpu);
>> + tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
>> + tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
>> + if (!tevt->evt.irq) {
>> + pr_err("%s: can't map IRQ for CPU%d\n",
>> + __func__, cpu);
>> + ret = -EINVAL;
>> + goto out_clk;
>> + }
>> +
>> + snprintf(tevt->name, ARRAY_SIZE(tevt->name),
>> + "tegra210_timer%d", cpu);
>> + tevt->evt.name = tevt->name;
>> + tevt->evt.cpumask = cpumask_of(cpu);
>> + tevt->evt.set_next_event = tegra210_timer_set_next_event;
>> + tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
>> + tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
>> + tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
>> + tevt->evt.tick_resume = tegra210_timer_shutdown;
>> + tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
>> + CLOCK_EVT_FEAT_ONESHOT;
>> + tevt->evt.rating = 460;
>> + irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
>
> Why do you need to prevent the interrupt to be enabled ? If the timer is
> stopped and cleared, no spurious interrupt will occur no ?
This is to prevent the IRQ will been enabled twice in the timer_setup
API with CPU hotplug event triggered. So we only enable the IRQ once the
CPU is online.
>
>> + ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
>> + IRQF_TIMER | IRQF_NOBALANCING,
>> + tevt->name, tevt);
>> + if (ret) {
>> + pr_err("%s: cannot setup irq %d for CPU%d\n",
>> + __func__, tevt->evt.irq, cpu);
>> + ret = -EINVAL;
>> + goto out_irq;
>> + }
>> + }
>> +
>> + /*
>> + * Configure microsecond timers to have 1MHz clock
>> + * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
>
> Did you mean 0xwwqq ?
Ah, because it's microsecond timer, that means the fraction of 1
microsecond each clk represents. For ex, if the clk is running at 12MHz,
then each clk represents 1/12 of a microsecond.
Thanks,
Joseph
>
>> + * Uses n+1 scheme
>> + */
>> + switch (tegra210_timer_freq) {
>> + case 12000000:
>> + usec_config = 0x000b; /* (11+1)/(0+1) */
>> + break;
>> + case 12800000:
>> + usec_config = 0x043f; /* (63+1)/(4+1) */
>> + break;
>> + case 13000000:
>> + usec_config = 0x000c; /* (12+1)/(0+1) */
>> + break;
>> + case 16800000:
>> + usec_config = 0x0453; /* (83+1)/(4+1) */
>> + break;
>> + case 19200000:
>> + usec_config = 0x045f; /* (95+1)/(4+1) */
>> + break;
>> + case 26000000:
>> + usec_config = 0x0019; /* (25+1)/(0+1) */
>> + break;
>> + case 38400000:
>> + usec_config = 0x04bf; /* (191+1)/(4+1) */
>> + break;
>> + case 48000000:
>> + usec_config = 0x002f; /* (47+1)/(0+1) */
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + goto out_irq;
>> + }
>> +
>> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
>> +
>> + cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
>> + "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
>> + tegra210_timer_stop);
>> +
>> + register_syscore_ops(&tegra_timer_syscore_ops);
>> +
>> + return ret;
>> +
>> +out_irq:
>> + for_each_possible_cpu(cpu) {
>> + tevt = per_cpu_ptr(tegra210_evt, cpu);
>> + if (tevt->evt.irq) {
>> + free_irq(tevt->evt.irq, tevt);
>> + irq_dispose_mapping(tevt->evt.irq);
>> + }
>> + }
>> +out_clk:
>> + clk_put(clk);
>> +out_iounmap:
>> + iounmap(tegra210_timer_reg_base);
>> +out_free_mem:
>> + free_percpu(tegra210_evt);
>> +out:
>> + return ret;
>> +}
>> +
>> +TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index fd586d0301e7..e78281d07b70 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -121,6 +121,7 @@ enum cpuhp_state {
>> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>> CPUHP_AP_ARM_TWD_STARTING,
>> CPUHP_AP_QCOM_TIMER_STARTING,
>> + CPUHP_AP_TEGRA_TIMER_STARTING,
>> CPUHP_AP_ARMADA_TIMER_STARTING,
>> CPUHP_AP_MARCO_TIMER_STARTING,
>> CPUHP_AP_MIPS_GIC_TIMER_STARTING,
>>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 3:07 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-29 3:07 UTC (permalink / raw)
To: Daniel Lezcano, Thierry Reding, Jonathan Hunter
Cc: linux-tegra, linux-arm-kernel, Thomas Gleixner, linux-kernel
Hi Daniel,
Thanks for your review.
On 1/28/19 9:00 PM, Daniel Lezcano wrote:
> On 28/01/2019 10:18, Joseph Lo wrote:
>> Add support for the Tegra210 timer that runs at oscillator clock
>> (TMR10-TMR13). We need these timers to work as clock event device and to
>> replace the ARMv8 architected timer due to it can't survive across the
>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
>> source when CPU suspends in power down state.
>>
>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v2:
>> * add error clean-up code
>> ---
>> drivers/clocksource/Kconfig | 3 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a9e26f6a81a1..e6e3e64b6320 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>> help
>> Enables support for the Tegra driver.
>>
>> +config TEGRA210_TIMER
>> + def_bool ARCH_TEGRA_210_SOC
>> +
>
> Please make the option consistent with the option below (VT8500_TIMER or
> similar).
Okay, will check.
>
>> config VT8500_TIMER
>> bool "VT8500 timer driver" if COMPILE_TEST
>> depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index cdd210ff89ea..95de59c8a47b 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
>> new file mode 100644
>> index 000000000000..477b164e540b
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-tegra210.c
>> @@ -0,0 +1,268 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/percpu.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +static u32 tegra210_timer_freq;
>> +static void __iomem *tegra210_timer_reg_base;
>> +static u32 usec_config;
>> +
>> +#define TIMER_PTV 0x0
>> +#define TIMER_PTV_EN BIT(31)
>> +#define TIMER_PTV_PER BIT(30)
>> +#define TIMER_PCR 0x4
>> +#define TIMER_PCR_INTR_CLR BIT(30)
>> +#define TIMERUS_CNTR_1US 0x10
>> +#define TIMERUS_USEC_CFG 0x14
>> +
>> +#define TIMER10_OFFSET 0x90
>> +#define TIMER10_IRQ_IDX 10
>> +
>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>> +
>> +struct tegra210_clockevent {
>> + struct clock_event_device evt;
>> + char name[20];
>> + void __iomem *reg_base;
>> +};
>> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
>> +
>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>> +
>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>> + struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>> +{
>> + writel(0, tevt->reg_base + TIMER_PTV);
>> +}
>> +
>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + timer_shutdown(tevt);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = dev_id;
>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>> + tevt->evt.event_handler(&tevt->evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int tegra210_timer_setup(unsigned int cpu)
>> +{
>> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
>> +
>> + irq_force_affinity(tevt->evt.irq, cpumask_of(cpu));
>> + enable_irq(tevt->evt.irq);
>> +
>> + clockevents_config_and_register(&tevt->evt, tegra210_timer_freq,
>> + 1, /* min */
>> + 0x1fffffff); /* 29 bits */
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_stop(unsigned int cpu)
>> +{
>> + struct tegra210_clockevent *tevt = per_cpu_ptr(tegra210_evt, cpu);
>> +
>> + tevt->evt.set_state_shutdown(&tevt->evt);
>> + disable_irq_nosync(tevt->evt.irq);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra_timer_suspend(void)
>> +{
>> + int cpu;
>> +
>> + for_each_possible_cpu(cpu) {
>> + void __iomem *reg_base = tegra210_timer_reg_base +
>> + TIMER_FOR_CPU(cpu);
>> + writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void tegra_timer_resume(void)
>> +{
>> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
>> +}
>> +
>> +static struct syscore_ops tegra_timer_syscore_ops = {
>> + .suspend = tegra_timer_suspend,
>> + .resume = tegra_timer_resume,
>> +};
>> +
>> +static int __init tegra210_timer_init(struct device_node *np)
>> +{
>> + int cpu, ret = 0;
>> + struct tegra210_clockevent *tevt;
>> + struct clk *clk;
>> +
>> + tegra210_evt = alloc_percpu(struct tegra210_clockevent);
>> + if (!tegra210_evt) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + tegra210_timer_reg_base = of_iomap(np, 0);
>> + if (!tegra210_timer_reg_base) {
>> + ret = -ENXIO;
>> + goto out_free_mem;
>> + }
>> +
>> + clk = of_clk_get(np, 0);
>> + if (IS_ERR(clk)) {
>> + ret = -EINVAL;
>> + goto out_iounmap;
>> + }
>> +
>> + clk_prepare_enable(clk);
>> + tegra210_timer_freq = clk_get_rate(clk);
>
>
> Use the timer-of API for each CPU.
Okay, I'll try to apply that for TIMER_OF_BASE and TIMER_OF_CLOCK. But
the TIMER_OF_IRQ, as you see below, it didn't have the ability to handle
the case that multiple CPUs with different timer IRQ lines. Which means
it's not PPI.
>
>> + for_each_possible_cpu(cpu) {
>
>
> No cpu loop for the initialization, use the tegra210_timer_setup for
> this (cf. armada 370 timer).
As mentioned above, I may still need this for IRQs initialization for
each CPU cores. Something like the IRQ init code in "exnos_mct.c".
>
>> + tevt = per_cpu_ptr(tegra210_evt, cpu);
>> + tevt->reg_base = tegra210_timer_reg_base + TIMER_FOR_CPU(cpu);
>> + tevt->evt.irq = irq_of_parse_and_map(np, IRQ_IDX_FOR_CPU(cpu));
>> + if (!tevt->evt.irq) {
>> + pr_err("%s: can't map IRQ for CPU%d\n",
>> + __func__, cpu);
>> + ret = -EINVAL;
>> + goto out_clk;
>> + }
>> +
>> + snprintf(tevt->name, ARRAY_SIZE(tevt->name),
>> + "tegra210_timer%d", cpu);
>> + tevt->evt.name = tevt->name;
>> + tevt->evt.cpumask = cpumask_of(cpu);
>> + tevt->evt.set_next_event = tegra210_timer_set_next_event;
>> + tevt->evt.set_state_shutdown = tegra210_timer_shutdown;
>> + tevt->evt.set_state_periodic = tegra210_timer_set_periodic;
>> + tevt->evt.set_state_oneshot = tegra210_timer_shutdown;
>> + tevt->evt.tick_resume = tegra210_timer_shutdown;
>> + tevt->evt.features = CLOCK_EVT_FEAT_PERIODIC |
>> + CLOCK_EVT_FEAT_ONESHOT;
>> + tevt->evt.rating = 460;
>> + irq_set_status_flags(tevt->evt.irq, IRQ_NOAUTOEN);
>
> Why do you need to prevent the interrupt to be enabled ? If the timer is
> stopped and cleared, no spurious interrupt will occur no ?
This is to prevent the IRQ will been enabled twice in the timer_setup
API with CPU hotplug event triggered. So we only enable the IRQ once the
CPU is online.
>
>> + ret = request_irq(tevt->evt.irq, tegra210_timer_isr,
>> + IRQF_TIMER | IRQF_NOBALANCING,
>> + tevt->name, tevt);
>> + if (ret) {
>> + pr_err("%s: cannot setup irq %d for CPU%d\n",
>> + __func__, tevt->evt.irq, cpu);
>> + ret = -EINVAL;
>> + goto out_irq;
>> + }
>> + }
>> +
>> + /*
>> + * Configure microsecond timers to have 1MHz clock
>> + * Config register is 0xqqww, where qq is "dividend", ww is "divisor"
>
> Did you mean 0xwwqq ?
Ah, because it's microsecond timer, that means the fraction of 1
microsecond each clk represents. For ex, if the clk is running at 12MHz,
then each clk represents 1/12 of a microsecond.
Thanks,
Joseph
>
>> + * Uses n+1 scheme
>> + */
>> + switch (tegra210_timer_freq) {
>> + case 12000000:
>> + usec_config = 0x000b; /* (11+1)/(0+1) */
>> + break;
>> + case 12800000:
>> + usec_config = 0x043f; /* (63+1)/(4+1) */
>> + break;
>> + case 13000000:
>> + usec_config = 0x000c; /* (12+1)/(0+1) */
>> + break;
>> + case 16800000:
>> + usec_config = 0x0453; /* (83+1)/(4+1) */
>> + break;
>> + case 19200000:
>> + usec_config = 0x045f; /* (95+1)/(4+1) */
>> + break;
>> + case 26000000:
>> + usec_config = 0x0019; /* (25+1)/(0+1) */
>> + break;
>> + case 38400000:
>> + usec_config = 0x04bf; /* (191+1)/(4+1) */
>> + break;
>> + case 48000000:
>> + usec_config = 0x002f; /* (47+1)/(0+1) */
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + goto out_irq;
>> + }
>> +
>> + writel(usec_config, tegra210_timer_reg_base + TIMERUS_USEC_CFG);
>> +
>> + cpuhp_setup_state(CPUHP_AP_TEGRA_TIMER_STARTING,
>> + "AP_TEGRA_TIMER_STARTING", tegra210_timer_setup,
>> + tegra210_timer_stop);
>> +
>> + register_syscore_ops(&tegra_timer_syscore_ops);
>> +
>> + return ret;
>> +
>> +out_irq:
>> + for_each_possible_cpu(cpu) {
>> + tevt = per_cpu_ptr(tegra210_evt, cpu);
>> + if (tevt->evt.irq) {
>> + free_irq(tevt->evt.irq, tevt);
>> + irq_dispose_mapping(tevt->evt.irq);
>> + }
>> + }
>> +out_clk:
>> + clk_put(clk);
>> +out_iounmap:
>> + iounmap(tegra210_timer_reg_base);
>> +out_free_mem:
>> + free_percpu(tegra210_evt);
>> +out:
>> + return ret;
>> +}
>> +
>> +TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>> index fd586d0301e7..e78281d07b70 100644
>> --- a/include/linux/cpuhotplug.h
>> +++ b/include/linux/cpuhotplug.h
>> @@ -121,6 +121,7 @@ enum cpuhp_state {
>> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
>> CPUHP_AP_ARM_TWD_STARTING,
>> CPUHP_AP_QCOM_TIMER_STARTING,
>> + CPUHP_AP_TEGRA_TIMER_STARTING,
>> CPUHP_AP_ARMADA_TIMER_STARTING,
>> CPUHP_AP_MARCO_TIMER_STARTING,
>> CPUHP_AP_MIPS_GIC_TIMER_STARTING,
>>
>
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-28 9:18 ` Joseph Lo
@ 2019-01-28 15:09 ` Thierry Reding
-1 siblings, 0 replies; 40+ messages in thread
From: Thierry Reding @ 2019-01-28 15:09 UTC (permalink / raw)
To: Joseph Lo
Cc: Jonathan Hunter, linux-tegra, linux-arm-kernel, Daniel Lezcano,
Thomas Gleixner, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5452 bytes --]
On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
> Add support for the Tegra210 timer that runs at oscillator clock
> (TMR10-TMR13). We need these timers to work as clock event device and to
> replace the ARMv8 architected timer due to it can't survive across the
> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
> source when CPU suspends in power down state.
>
> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> v2:
> * add error clean-up code
> ---
> drivers/clocksource/Kconfig | 3 +
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/clocksource/timer-tegra210.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a9e26f6a81a1..e6e3e64b6320 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -135,6 +135,9 @@ config TEGRA_TIMER
> help
> Enables support for the Tegra driver.
>
> +config TEGRA210_TIMER
> + def_bool ARCH_TEGRA_210_SOC
> +
> config VT8500_TIMER
> bool "VT8500 timer driver" if COMPILE_TEST
> depends on HAS_IOMEM
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index cdd210ff89ea..95de59c8a47b 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
> new file mode 100644
> index 000000000000..477b164e540b
> --- /dev/null
> +++ b/drivers/clocksource/timer-tegra210.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/percpu.h>
> +#include <linux/syscore_ops.h>
> +
> +static u32 tegra210_timer_freq;
> +static void __iomem *tegra210_timer_reg_base;
> +static u32 usec_config;
> +
> +#define TIMER_PTV 0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PER BIT(30)
> +#define TIMER_PCR 0x4
> +#define TIMER_PCR_INTR_CLR BIT(30)
> +#define TIMERUS_CNTR_1US 0x10
> +#define TIMERUS_USEC_CFG 0x14
> +
> +#define TIMER10_OFFSET 0x90
> +#define TIMER10_IRQ_IDX 10
> +
> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
> +
> +struct tegra210_clockevent {
> + struct clock_event_device evt;
> + char name[20];
> + void __iomem *reg_base;
> +};
> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
> +
> +static struct tegra210_clockevent __percpu *tegra210_evt;
> +
> +static int tegra210_timer_set_next_event(unsigned long cycles,
> + struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN |
> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
> +{
> + writel(0, tevt->reg_base + TIMER_PTV);
> +}
> +
> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + timer_shutdown(tevt);
> +
> + return 0;
> +}
> +
> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = dev_id;
> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
> + tevt->evt.event_handler(&tevt->evt);
> +
> + return IRQ_HANDLED;
> +}
Up to here this is a duplicate of timer-tegra20.c. And a lot of
tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
we unify the two drivers instead?
The power cycle restrictions of the architected timer, do they not apply
to chips earlier than Tegra210 either? So don't we need all of these
additional features on the timer-tegra20.c driver as well? If so that
would increase the code duplication even more. I think we should avoid
that, unless there are any strong arguments that would justify it.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-28 15:09 ` Thierry Reding
0 siblings, 0 replies; 40+ messages in thread
From: Thierry Reding @ 2019-01-28 15:09 UTC (permalink / raw)
To: Joseph Lo
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 5452 bytes --]
On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
> Add support for the Tegra210 timer that runs at oscillator clock
> (TMR10-TMR13). We need these timers to work as clock event device and to
> replace the ARMv8 architected timer due to it can't survive across the
> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
> source when CPU suspends in power down state.
>
> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
> ---
> v2:
> * add error clean-up code
> ---
> drivers/clocksource/Kconfig | 3 +
> drivers/clocksource/Makefile | 1 +
> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
> include/linux/cpuhotplug.h | 1 +
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/clocksource/timer-tegra210.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a9e26f6a81a1..e6e3e64b6320 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -135,6 +135,9 @@ config TEGRA_TIMER
> help
> Enables support for the Tegra driver.
>
> +config TEGRA210_TIMER
> + def_bool ARCH_TEGRA_210_SOC
> +
> config VT8500_TIMER
> bool "VT8500 timer driver" if COMPILE_TEST
> depends on HAS_IOMEM
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index cdd210ff89ea..95de59c8a47b 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
> new file mode 100644
> index 000000000000..477b164e540b
> --- /dev/null
> +++ b/drivers/clocksource/timer-tegra210.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/percpu.h>
> +#include <linux/syscore_ops.h>
> +
> +static u32 tegra210_timer_freq;
> +static void __iomem *tegra210_timer_reg_base;
> +static u32 usec_config;
> +
> +#define TIMER_PTV 0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PER BIT(30)
> +#define TIMER_PCR 0x4
> +#define TIMER_PCR_INTR_CLR BIT(30)
> +#define TIMERUS_CNTR_1US 0x10
> +#define TIMERUS_USEC_CFG 0x14
> +
> +#define TIMER10_OFFSET 0x90
> +#define TIMER10_IRQ_IDX 10
> +
> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
> +
> +struct tegra210_clockevent {
> + struct clock_event_device evt;
> + char name[20];
> + void __iomem *reg_base;
> +};
> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
> +
> +static struct tegra210_clockevent __percpu *tegra210_evt;
> +
> +static int tegra210_timer_set_next_event(unsigned long cycles,
> + struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN |
> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
> +{
> + writel(0, tevt->reg_base + TIMER_PTV);
> +}
> +
> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + timer_shutdown(tevt);
> +
> + return 0;
> +}
> +
> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = to_tegra_cevt(evt);
> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
> + tevt->reg_base + TIMER_PTV);
> +
> + return 0;
> +}
> +
> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
> +{
> + struct tegra210_clockevent *tevt;
> +
> + tevt = dev_id;
> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
> + tevt->evt.event_handler(&tevt->evt);
> +
> + return IRQ_HANDLED;
> +}
Up to here this is a duplicate of timer-tegra20.c. And a lot of
tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
we unify the two drivers instead?
The power cycle restrictions of the architected timer, do they not apply
to chips earlier than Tegra210 either? So don't we need all of these
additional features on the timer-tegra20.c driver as well? If so that
would increase the code duplication even more. I think we should avoid
that, unless there are any strong arguments that would justify it.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-28 15:09 ` Thierry Reding
(?)
@ 2019-01-29 3:35 ` Joseph Lo
-1 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-29 3:35 UTC (permalink / raw)
To: Thierry Reding
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
On 1/28/19 11:09 PM, Thierry Reding wrote:
> On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
>> Add support for the Tegra210 timer that runs at oscillator clock
>> (TMR10-TMR13). We need these timers to work as clock event device and to
>> replace the ARMv8 architected timer due to it can't survive across the
>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
>> source when CPU suspends in power down state.
>>
>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v2:
>> * add error clean-up code
>> ---
>> drivers/clocksource/Kconfig | 3 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a9e26f6a81a1..e6e3e64b6320 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>> help
>> Enables support for the Tegra driver.
>>
>> +config TEGRA210_TIMER
>> + def_bool ARCH_TEGRA_210_SOC
>> +
>> config VT8500_TIMER
>> bool "VT8500 timer driver" if COMPILE_TEST
>> depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index cdd210ff89ea..95de59c8a47b 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
>> new file mode 100644
>> index 000000000000..477b164e540b
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-tegra210.c
>> @@ -0,0 +1,268 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/percpu.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +static u32 tegra210_timer_freq;
>> +static void __iomem *tegra210_timer_reg_base;
>> +static u32 usec_config;
>> +
>> +#define TIMER_PTV 0x0
>> +#define TIMER_PTV_EN BIT(31)
>> +#define TIMER_PTV_PER BIT(30)
>> +#define TIMER_PCR 0x4
>> +#define TIMER_PCR_INTR_CLR BIT(30)
>> +#define TIMERUS_CNTR_1US 0x10
>> +#define TIMERUS_USEC_CFG 0x14
>> +
>> +#define TIMER10_OFFSET 0x90
>> +#define TIMER10_IRQ_IDX 10
>> +
>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>> +
>> +struct tegra210_clockevent {
>> + struct clock_event_device evt;
>> + char name[20];
>> + void __iomem *reg_base;
>> +};
>> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
>> +
>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>> +
>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>> + struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>> +{
>> + writel(0, tevt->reg_base + TIMER_PTV);
>> +}
>> +
>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + timer_shutdown(tevt);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = dev_id;
>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>> + tevt->evt.event_handler(&tevt->evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>
> Up to here this is a duplicate of timer-tegra20.c. And a lot of
> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> we unify the two drivers instead?
I still prefer to remove the timer-tegra20 driver, because it didn't
been used for Tegra 32 bit chips for quite a long time. So currently it
just compiles OK, I also doubt the functionality still can achieve the
same what I want to do for Tegra210.
My personal opinion is to remove the old and unused driver and enhance
the new one.
>
> The power cycle restrictions of the architected timer, do they not apply
> to chips earlier than Tegra210 either? So don't we need all of these
> additional features on the timer-tegra20.c driver as well? If so that
> would increase the code duplication even more. I think we should avoid
> that, unless there are any strong arguments that would justify it.
>
As far as I know, this was a issue on some specific version of Cortex-a57.
For timer-tegra20, which was designed for the very early stage that TWD
driver was not ready yet for Tegra 32 bit chips (Tegra20/30). Currently,
it has been replaced by ARM TWD and V7 timer driver. It didn't been used
and maintained for quite a long time.
BTW, we had very similar issue with idle power-down state with Tegra
32bit chips. But it's related to GIC not timer. See below two changes
for reference.
For Tegra20,
d4b92fb2535a ARM: tegra: add pending SGI checking API
For Tegra114/124,
7e8b15dbc392 ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on
CPU PM entry
Thanks,
Joseph
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 3:35 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-29 3:35 UTC (permalink / raw)
To: Thierry Reding
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
On 1/28/19 11:09 PM, Thierry Reding wrote:
> On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
>> Add support for the Tegra210 timer that runs at oscillator clock
>> (TMR10-TMR13). We need these timers to work as clock event device and to
>> replace the ARMv8 architected timer due to it can't survive across the
>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
>> source when CPU suspends in power down state.
>>
>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v2:
>> * add error clean-up code
>> ---
>> drivers/clocksource/Kconfig | 3 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a9e26f6a81a1..e6e3e64b6320 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>> help
>> Enables support for the Tegra driver.
>>
>> +config TEGRA210_TIMER
>> + def_bool ARCH_TEGRA_210_SOC
>> +
>> config VT8500_TIMER
>> bool "VT8500 timer driver" if COMPILE_TEST
>> depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index cdd210ff89ea..95de59c8a47b 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
>> new file mode 100644
>> index 000000000000..477b164e540b
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-tegra210.c
>> @@ -0,0 +1,268 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/percpu.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +static u32 tegra210_timer_freq;
>> +static void __iomem *tegra210_timer_reg_base;
>> +static u32 usec_config;
>> +
>> +#define TIMER_PTV 0x0
>> +#define TIMER_PTV_EN BIT(31)
>> +#define TIMER_PTV_PER BIT(30)
>> +#define TIMER_PCR 0x4
>> +#define TIMER_PCR_INTR_CLR BIT(30)
>> +#define TIMERUS_CNTR_1US 0x10
>> +#define TIMERUS_USEC_CFG 0x14
>> +
>> +#define TIMER10_OFFSET 0x90
>> +#define TIMER10_IRQ_IDX 10
>> +
>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>> +
>> +struct tegra210_clockevent {
>> + struct clock_event_device evt;
>> + char name[20];
>> + void __iomem *reg_base;
>> +};
>> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
>> +
>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>> +
>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>> + struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>> +{
>> + writel(0, tevt->reg_base + TIMER_PTV);
>> +}
>> +
>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + timer_shutdown(tevt);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = dev_id;
>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>> + tevt->evt.event_handler(&tevt->evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>
> Up to here this is a duplicate of timer-tegra20.c. And a lot of
> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> we unify the two drivers instead?
I still prefer to remove the timer-tegra20 driver, because it didn't
been used for Tegra 32 bit chips for quite a long time. So currently it
just compiles OK, I also doubt the functionality still can achieve the
same what I want to do for Tegra210.
My personal opinion is to remove the old and unused driver and enhance
the new one.
>
> The power cycle restrictions of the architected timer, do they not apply
> to chips earlier than Tegra210 either? So don't we need all of these
> additional features on the timer-tegra20.c driver as well? If so that
> would increase the code duplication even more. I think we should avoid
> that, unless there are any strong arguments that would justify it.
>
As far as I know, this was a issue on some specific version of Cortex-a57.
For timer-tegra20, which was designed for the very early stage that TWD
driver was not ready yet for Tegra 32 bit chips (Tegra20/30). Currently,
it has been replaced by ARM TWD and V7 timer driver. It didn't been used
and maintained for quite a long time.
BTW, we had very similar issue with idle power-down state with Tegra
32bit chips. But it's related to GIC not timer. See below two changes
for reference.
For Tegra20,
d4b92fb2535a ARM: tegra: add pending SGI checking API
For Tegra114/124,
7e8b15dbc392 ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on
CPU PM entry
Thanks,
Joseph
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 3:35 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-29 3:35 UTC (permalink / raw)
To: Thierry Reding
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
On 1/28/19 11:09 PM, Thierry Reding wrote:
> On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
>> Add support for the Tegra210 timer that runs at oscillator clock
>> (TMR10-TMR13). We need these timers to work as clock event device and to
>> replace the ARMv8 architected timer due to it can't survive across the
>> power cycle of the CPU core or CPUPORESET signal. So it can't be a wake-up
>> source when CPU suspends in power down state.
>>
>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>
>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>> ---
>> v2:
>> * add error clean-up code
>> ---
>> drivers/clocksource/Kconfig | 3 +
>> drivers/clocksource/Makefile | 1 +
>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>> include/linux/cpuhotplug.h | 1 +
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index a9e26f6a81a1..e6e3e64b6320 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>> help
>> Enables support for the Tegra driver.
>>
>> +config TEGRA210_TIMER
>> + def_bool ARCH_TEGRA_210_SOC
>> +
>> config VT8500_TIMER
>> bool "VT8500 timer driver" if COMPILE_TEST
>> depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index cdd210ff89ea..95de59c8a47b 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>> diff --git a/drivers/clocksource/timer-tegra210.c b/drivers/clocksource/timer-tegra210.c
>> new file mode 100644
>> index 000000000000..477b164e540b
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-tegra210.c
>> @@ -0,0 +1,268 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/percpu.h>
>> +#include <linux/syscore_ops.h>
>> +
>> +static u32 tegra210_timer_freq;
>> +static void __iomem *tegra210_timer_reg_base;
>> +static u32 usec_config;
>> +
>> +#define TIMER_PTV 0x0
>> +#define TIMER_PTV_EN BIT(31)
>> +#define TIMER_PTV_PER BIT(30)
>> +#define TIMER_PCR 0x4
>> +#define TIMER_PCR_INTR_CLR BIT(30)
>> +#define TIMERUS_CNTR_1US 0x10
>> +#define TIMERUS_USEC_CFG 0x14
>> +
>> +#define TIMER10_OFFSET 0x90
>> +#define TIMER10_IRQ_IDX 10
>> +
>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>> +
>> +struct tegra210_clockevent {
>> + struct clock_event_device evt;
>> + char name[20];
>> + void __iomem *reg_base;
>> +};
>> +#define to_tegra_cevt(p) (container_of(p, struct tegra210_clockevent, evt))
>> +
>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>> +
>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>> + struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>> +{
>> + writel(0, tevt->reg_base + TIMER_PTV);
>> +}
>> +
>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + timer_shutdown(tevt);
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = to_tegra_cevt(evt);
>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq / HZ) - 1),
>> + tevt->reg_base + TIMER_PTV);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>> +{
>> + struct tegra210_clockevent *tevt;
>> +
>> + tevt = dev_id;
>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>> + tevt->evt.event_handler(&tevt->evt);
>> +
>> + return IRQ_HANDLED;
>> +}
>
> Up to here this is a duplicate of timer-tegra20.c. And a lot of
> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> we unify the two drivers instead?
I still prefer to remove the timer-tegra20 driver, because it didn't
been used for Tegra 32 bit chips for quite a long time. So currently it
just compiles OK, I also doubt the functionality still can achieve the
same what I want to do for Tegra210.
My personal opinion is to remove the old and unused driver and enhance
the new one.
>
> The power cycle restrictions of the architected timer, do they not apply
> to chips earlier than Tegra210 either? So don't we need all of these
> additional features on the timer-tegra20.c driver as well? If so that
> would increase the code duplication even more. I think we should avoid
> that, unless there are any strong arguments that would justify it.
>
As far as I know, this was a issue on some specific version of Cortex-a57.
For timer-tegra20, which was designed for the very early stage that TWD
driver was not ready yet for Tegra 32 bit chips (Tegra20/30). Currently,
it has been replaced by ARM TWD and V7 timer driver. It didn't been used
and maintained for quite a long time.
BTW, we had very similar issue with idle power-down state with Tegra
32bit chips. But it's related to GIC not timer. See below two changes
for reference.
For Tegra20,
d4b92fb2535a ARM: tegra: add pending SGI checking API
For Tegra114/124,
7e8b15dbc392 ARM: tegra114: Reprogram GIC CPU interface to bypass IRQ on
CPU PM entry
Thanks,
Joseph
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-29 3:35 ` Joseph Lo
(?)
@ 2019-01-29 9:16 ` Jon Hunter
-1 siblings, 0 replies; 40+ messages in thread
From: Jon Hunter @ 2019-01-29 9:16 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding
Cc: Daniel Lezcano, linux-kernel, linux-tegra, Thomas Gleixner,
linux-arm-kernel
On 29/01/2019 03:35, Joseph Lo wrote:
> On 1/28/19 11:09 PM, Thierry Reding wrote:
>> On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
>>> Add support for the Tegra210 timer that runs at oscillator clock
>>> (TMR10-TMR13). We need these timers to work as clock event device and to
>>> replace the ARMv8 architected timer due to it can't survive across the
>>> power cycle of the CPU core or CPUPORESET signal. So it can't be a
>>> wake-up
>>> source when CPU suspends in power down state.
>>>
>>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>> v2:
>>> * add error clean-up code
>>> ---
>>> drivers/clocksource/Kconfig | 3 +
>>> drivers/clocksource/Makefile | 1 +
>>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>>> include/linux/cpuhotplug.h | 1 +
>>> 4 files changed, 273 insertions(+)
>>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index a9e26f6a81a1..e6e3e64b6320 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>>> help
>>> Enables support for the Tegra driver.
>>> +config TEGRA210_TIMER
>>> + def_bool ARCH_TEGRA_210_SOC
>>> +
>>> config VT8500_TIMER
>>> bool "VT8500 timer driver" if COMPILE_TEST
>>> depends on HAS_IOMEM
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index cdd210ff89ea..95de59c8a47b 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>>> diff --git a/drivers/clocksource/timer-tegra210.c
>>> b/drivers/clocksource/timer-tegra210.c
>>> new file mode 100644
>>> index 000000000000..477b164e540b
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-tegra210.c
>>> @@ -0,0 +1,268 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/cpu.h>
>>> +#include <linux/cpumask.h>
>>> +#include <linux/err.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/percpu.h>
>>> +#include <linux/syscore_ops.h>
>>> +
>>> +static u32 tegra210_timer_freq;
>>> +static void __iomem *tegra210_timer_reg_base;
>>> +static u32 usec_config;
>>> +
>>> +#define TIMER_PTV 0x0
>>> +#define TIMER_PTV_EN BIT(31)
>>> +#define TIMER_PTV_PER BIT(30)
>>> +#define TIMER_PCR 0x4
>>> +#define TIMER_PCR_INTR_CLR BIT(30)
>>> +#define TIMERUS_CNTR_1US 0x10
>>> +#define TIMERUS_USEC_CFG 0x14
>>> +
>>> +#define TIMER10_OFFSET 0x90
>>> +#define TIMER10_IRQ_IDX 10
>>> +
>>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>>> +
>>> +struct tegra210_clockevent {
>>> + struct clock_event_device evt;
>>> + char name[20];
>>> + void __iomem *reg_base;
>>> +};
>>> +#define to_tegra_cevt(p) (container_of(p, struct
>>> tegra210_clockevent, evt))
>>> +
>>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>>> +
>>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>>> + struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + writel(TIMER_PTV_EN |
>>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>>> + tevt->reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>>> +{
>>> + writel(0, tevt->reg_base + TIMER_PTV);
>>> +}
>>> +
>>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + timer_shutdown(tevt);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq /
>>> HZ) - 1),
>>> + tevt->reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = dev_id;
>>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>>> + tevt->evt.event_handler(&tevt->evt);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>
>> Up to here this is a duplicate of timer-tegra20.c. And a lot of
>> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
>> we unify the two drivers instead?
>
> I still prefer to remove the timer-tegra20 driver, because it didn't
> been used for Tegra 32 bit chips for quite a long time. So currently it
> just compiles OK, I also doubt the functionality still can achieve the
> same what I want to do for Tegra210.
>
> My personal opinion is to remove the old and unused driver and enhance
> the new one.
Although it may not be used it is still a valid clocksource and could be
used. Our preference is to keeping the existing timer driver and enhance
to support T210 (unless there is some fundamental issue that would
prevent this that I am overlooking).
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 9:16 ` Jon Hunter
0 siblings, 0 replies; 40+ messages in thread
From: Jon Hunter @ 2019-01-29 9:16 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding
Cc: linux-tegra, Thomas Gleixner, Daniel Lezcano, linux-kernel,
linux-arm-kernel
On 29/01/2019 03:35, Joseph Lo wrote:
> On 1/28/19 11:09 PM, Thierry Reding wrote:
>> On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
>>> Add support for the Tegra210 timer that runs at oscillator clock
>>> (TMR10-TMR13). We need these timers to work as clock event device and to
>>> replace the ARMv8 architected timer due to it can't survive across the
>>> power cycle of the CPU core or CPUPORESET signal. So it can't be a
>>> wake-up
>>> source when CPU suspends in power down state.
>>>
>>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>> v2:
>>> * add error clean-up code
>>> ---
>>> drivers/clocksource/Kconfig | 3 +
>>> drivers/clocksource/Makefile | 1 +
>>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>>> include/linux/cpuhotplug.h | 1 +
>>> 4 files changed, 273 insertions(+)
>>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index a9e26f6a81a1..e6e3e64b6320 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>>> help
>>> Enables support for the Tegra driver.
>>> +config TEGRA210_TIMER
>>> + def_bool ARCH_TEGRA_210_SOC
>>> +
>>> config VT8500_TIMER
>>> bool "VT8500 timer driver" if COMPILE_TEST
>>> depends on HAS_IOMEM
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index cdd210ff89ea..95de59c8a47b 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>>> diff --git a/drivers/clocksource/timer-tegra210.c
>>> b/drivers/clocksource/timer-tegra210.c
>>> new file mode 100644
>>> index 000000000000..477b164e540b
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-tegra210.c
>>> @@ -0,0 +1,268 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/cpu.h>
>>> +#include <linux/cpumask.h>
>>> +#include <linux/err.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/percpu.h>
>>> +#include <linux/syscore_ops.h>
>>> +
>>> +static u32 tegra210_timer_freq;
>>> +static void __iomem *tegra210_timer_reg_base;
>>> +static u32 usec_config;
>>> +
>>> +#define TIMER_PTV 0x0
>>> +#define TIMER_PTV_EN BIT(31)
>>> +#define TIMER_PTV_PER BIT(30)
>>> +#define TIMER_PCR 0x4
>>> +#define TIMER_PCR_INTR_CLR BIT(30)
>>> +#define TIMERUS_CNTR_1US 0x10
>>> +#define TIMERUS_USEC_CFG 0x14
>>> +
>>> +#define TIMER10_OFFSET 0x90
>>> +#define TIMER10_IRQ_IDX 10
>>> +
>>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>>> +
>>> +struct tegra210_clockevent {
>>> + struct clock_event_device evt;
>>> + char name[20];
>>> + void __iomem *reg_base;
>>> +};
>>> +#define to_tegra_cevt(p) (container_of(p, struct
>>> tegra210_clockevent, evt))
>>> +
>>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>>> +
>>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>>> + struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + writel(TIMER_PTV_EN |
>>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>>> + tevt->reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>>> +{
>>> + writel(0, tevt->reg_base + TIMER_PTV);
>>> +}
>>> +
>>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + timer_shutdown(tevt);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq /
>>> HZ) - 1),
>>> + tevt->reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = dev_id;
>>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>>> + tevt->evt.event_handler(&tevt->evt);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>
>> Up to here this is a duplicate of timer-tegra20.c. And a lot of
>> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
>> we unify the two drivers instead?
>
> I still prefer to remove the timer-tegra20 driver, because it didn't
> been used for Tegra 32 bit chips for quite a long time. So currently it
> just compiles OK, I also doubt the functionality still can achieve the
> same what I want to do for Tegra210.
>
> My personal opinion is to remove the old and unused driver and enhance
> the new one.
Although it may not be used it is still a valid clocksource and could be
used. Our preference is to keeping the existing timer driver and enhance
to support T210 (unless there is some fundamental issue that would
prevent this that I am overlooking).
Cheers
Jon
--
nvpublic
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 9:16 ` Jon Hunter
0 siblings, 0 replies; 40+ messages in thread
From: Jon Hunter @ 2019-01-29 9:16 UTC (permalink / raw)
To: Joseph Lo, Thierry Reding
Cc: Daniel Lezcano, linux-kernel, linux-tegra, Thomas Gleixner,
linux-arm-kernel
On 29/01/2019 03:35, Joseph Lo wrote:
> On 1/28/19 11:09 PM, Thierry Reding wrote:
>> On Mon, Jan 28, 2019 at 05:18:11PM +0800, Joseph Lo wrote:
>>> Add support for the Tegra210 timer that runs at oscillator clock
>>> (TMR10-TMR13). We need these timers to work as clock event device and to
>>> replace the ARMv8 architected timer due to it can't survive across the
>>> power cycle of the CPU core or CPUPORESET signal. So it can't be a
>>> wake-up
>>> source when CPU suspends in power down state.
>>>
>>> Based on the work of Antti P Miettinen <amiettinen@nvidia.com>
>>>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo <josephl@nvidia.com>
>>> ---
>>> v2:
>>> * add error clean-up code
>>> ---
>>> drivers/clocksource/Kconfig | 3 +
>>> drivers/clocksource/Makefile | 1 +
>>> drivers/clocksource/timer-tegra210.c | 268 +++++++++++++++++++++++++++
>>> include/linux/cpuhotplug.h | 1 +
>>> 4 files changed, 273 insertions(+)
>>> create mode 100644 drivers/clocksource/timer-tegra210.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index a9e26f6a81a1..e6e3e64b6320 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -135,6 +135,9 @@ config TEGRA_TIMER
>>> help
>>> Enables support for the Tegra driver.
>>> +config TEGRA210_TIMER
>>> + def_bool ARCH_TEGRA_210_SOC
>>> +
>>> config VT8500_TIMER
>>> bool "VT8500 timer driver" if COMPILE_TEST
>>> depends on HAS_IOMEM
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index cdd210ff89ea..95de59c8a47b 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -36,6 +36,7 @@ obj-$(CONFIG_SUN4I_TIMER) += timer-sun4i.o
>>> obj-$(CONFIG_SUN5I_HSTIMER) += timer-sun5i.o
>>> obj-$(CONFIG_MESON6_TIMER) += timer-meson6.o
>>> obj-$(CONFIG_TEGRA_TIMER) += timer-tegra20.o
>>> +obj-$(CONFIG_TEGRA210_TIMER) += timer-tegra210.o
>>> obj-$(CONFIG_VT8500_TIMER) += timer-vt8500.o
>>> obj-$(CONFIG_NSPIRE_TIMER) += timer-zevio.o
>>> obj-$(CONFIG_BCM_KONA_TIMER) += bcm_kona_timer.o
>>> diff --git a/drivers/clocksource/timer-tegra210.c
>>> b/drivers/clocksource/timer-tegra210.c
>>> new file mode 100644
>>> index 000000000000..477b164e540b
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-tegra210.c
>>> @@ -0,0 +1,268 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2014-2019, NVIDIA CORPORATION. All rights reserved.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/cpu.h>
>>> +#include <linux/cpumask.h>
>>> +#include <linux/err.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/percpu.h>
>>> +#include <linux/syscore_ops.h>
>>> +
>>> +static u32 tegra210_timer_freq;
>>> +static void __iomem *tegra210_timer_reg_base;
>>> +static u32 usec_config;
>>> +
>>> +#define TIMER_PTV 0x0
>>> +#define TIMER_PTV_EN BIT(31)
>>> +#define TIMER_PTV_PER BIT(30)
>>> +#define TIMER_PCR 0x4
>>> +#define TIMER_PCR_INTR_CLR BIT(30)
>>> +#define TIMERUS_CNTR_1US 0x10
>>> +#define TIMERUS_USEC_CFG 0x14
>>> +
>>> +#define TIMER10_OFFSET 0x90
>>> +#define TIMER10_IRQ_IDX 10
>>> +
>>> +#define TIMER_FOR_CPU(cpu) (TIMER10_OFFSET + (cpu) * 8)
>>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>>> +
>>> +struct tegra210_clockevent {
>>> + struct clock_event_device evt;
>>> + char name[20];
>>> + void __iomem *reg_base;
>>> +};
>>> +#define to_tegra_cevt(p) (container_of(p, struct
>>> tegra210_clockevent, evt))
>>> +
>>> +static struct tegra210_clockevent __percpu *tegra210_evt;
>>> +
>>> +static int tegra210_timer_set_next_event(unsigned long cycles,
>>> + struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + writel(TIMER_PTV_EN |
>>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>>> + tevt->reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static inline void timer_shutdown(struct tegra210_clockevent *tevt)
>>> +{
>>> + writel(0, tevt->reg_base + TIMER_PTV);
>>> +}
>>> +
>>> +static int tegra210_timer_shutdown(struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + timer_shutdown(tevt);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra210_timer_set_periodic(struct clock_event_device *evt)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = to_tegra_cevt(evt);
>>> + writel(TIMER_PTV_EN | TIMER_PTV_PER | ((tegra210_timer_freq /
>>> HZ) - 1),
>>> + tevt->reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static irqreturn_t tegra210_timer_isr(int irq, void *dev_id)
>>> +{
>>> + struct tegra210_clockevent *tevt;
>>> +
>>> + tevt = dev_id;
>>> + writel(TIMER_PCR_INTR_CLR, tevt->reg_base + TIMER_PCR);
>>> + tevt->evt.event_handler(&tevt->evt);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>
>> Up to here this is a duplicate of timer-tegra20.c. And a lot of
>> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
>> we unify the two drivers instead?
>
> I still prefer to remove the timer-tegra20 driver, because it didn't
> been used for Tegra 32 bit chips for quite a long time. So currently it
> just compiles OK, I also doubt the functionality still can achieve the
> same what I want to do for Tegra210.
>
> My personal opinion is to remove the old and unused driver and enhance
> the new one.
Although it may not be used it is still a valid clocksource and could be
used. Our preference is to keeping the existing timer driver and enhance
to support T210 (unless there is some fundamental issue that would
prevent this that I am overlooking).
Cheers
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-28 15:09 ` Thierry Reding
(?)
@ 2019-01-29 8:41 ` Peter De Schrijver
-1 siblings, 0 replies; 40+ messages in thread
From: Peter De Schrijver @ 2019-01-29 8:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Joseph Lo, Daniel Lezcano, linux-kernel, Jonathan Hunter,
linux-tegra, Thomas Gleixner, linux-arm-kernel
On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
...
>
> Up to here this is a duplicate of timer-tegra20.c. And a lot of
> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> we unify the two drivers instead?
>
> The power cycle restrictions of the architected timer, do they not apply
> to chips earlier than Tegra210 either? So don't we need all of these
> additional features on the timer-tegra20.c driver as well? If so that
No. Chips prior to Tegra114 do not have an arch timer and the arch timer
does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
It's broken on Cortex-A57 though, so we can't use it as a wakeup source
on Tegra210.
Peter.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 8:41 ` Peter De Schrijver
0 siblings, 0 replies; 40+ messages in thread
From: Peter De Schrijver @ 2019-01-29 8:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, Joseph Lo,
linux-tegra, Thomas Gleixner, linux-arm-kernel
On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
...
>
> Up to here this is a duplicate of timer-tegra20.c. And a lot of
> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> we unify the two drivers instead?
>
> The power cycle restrictions of the architected timer, do they not apply
> to chips earlier than Tegra210 either? So don't we need all of these
> additional features on the timer-tegra20.c driver as well? If so that
No. Chips prior to Tegra114 do not have an arch timer and the arch timer
does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
It's broken on Cortex-A57 though, so we can't use it as a wakeup source
on Tegra210.
Peter.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 8:41 ` Peter De Schrijver
0 siblings, 0 replies; 40+ messages in thread
From: Peter De Schrijver @ 2019-01-29 8:41 UTC (permalink / raw)
To: Thierry Reding
Cc: Joseph Lo, Daniel Lezcano, linux-kernel, Jonathan Hunter,
linux-tegra, Thomas Gleixner, linux-arm-kernel
On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
...
>
> Up to here this is a duplicate of timer-tegra20.c. And a lot of
> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> we unify the two drivers instead?
>
> The power cycle restrictions of the architected timer, do they not apply
> to chips earlier than Tegra210 either? So don't we need all of these
> additional features on the timer-tegra20.c driver as well? If so that
No. Chips prior to Tegra114 do not have an arch timer and the arch timer
does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
It's broken on Cortex-A57 though, so we can't use it as a wakeup source
on Tegra210.
Peter.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-29 8:41 ` Peter De Schrijver
@ 2019-01-29 10:29 ` Thierry Reding
-1 siblings, 0 replies; 40+ messages in thread
From: Thierry Reding @ 2019-01-29 10:29 UTC (permalink / raw)
To: Peter De Schrijver
Cc: Joseph Lo, Daniel Lezcano, linux-kernel, Jonathan Hunter,
linux-tegra, Thomas Gleixner, linux-arm-kernel
[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]
On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
> On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
>
> ...
>
> >
> > Up to here this is a duplicate of timer-tegra20.c. And a lot of
> > tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> > we unify the two drivers instead?
> >
> > The power cycle restrictions of the architected timer, do they not apply
> > to chips earlier than Tegra210 either? So don't we need all of these
> > additional features on the timer-tegra20.c driver as well? If so that
>
> No. Chips prior to Tegra114 do not have an arch timer and the arch timer
> does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
> It's broken on Cortex-A57 though, so we can't use it as a wakeup source
> on Tegra210.
If chips prior to Tegra114 don't have an architected timer, then we
can't remove the timer-tegra20 driver, because we still need it on
Tegra20 and Tegra30, right?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-29 10:29 ` Thierry Reding
0 siblings, 0 replies; 40+ messages in thread
From: Thierry Reding @ 2019-01-29 10:29 UTC (permalink / raw)
To: Peter De Schrijver
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, Joseph Lo,
linux-tegra, Thomas Gleixner, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1000 bytes --]
On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
> On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
>
> ...
>
> >
> > Up to here this is a duplicate of timer-tegra20.c. And a lot of
> > tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> > we unify the two drivers instead?
> >
> > The power cycle restrictions of the architected timer, do they not apply
> > to chips earlier than Tegra210 either? So don't we need all of these
> > additional features on the timer-tegra20.c driver as well? If so that
>
> No. Chips prior to Tegra114 do not have an arch timer and the arch timer
> does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
> It's broken on Cortex-A57 though, so we can't use it as a wakeup source
> on Tegra210.
If chips prior to Tegra114 don't have an architected timer, then we
can't remove the timer-tegra20 driver, because we still need it on
Tegra20 and Tegra30, right?
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-29 10:29 ` Thierry Reding
(?)
@ 2019-01-30 2:40 ` Joseph Lo
-1 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-30 2:40 UTC (permalink / raw)
To: Thierry Reding, Peter De Schrijver
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
On 1/29/19 6:29 PM, Thierry Reding wrote:
> On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
>> On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
>>
>> ...
>>
>>>
>>> Up to here this is a duplicate of timer-tegra20.c. And a lot of
>>> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
>>> we unify the two drivers instead?
>>>
>>> The power cycle restrictions of the architected timer, do they not apply
>>> to chips earlier than Tegra210 either? So don't we need all of these
>>> additional features on the timer-tegra20.c driver as well? If so that
>>
>> No. Chips prior to Tegra114 do not have an arch timer and the arch timer
>> does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
>> It's broken on Cortex-A57 though, so we can't use it as a wakeup source
>> on Tegra210.
>
> If chips prior to Tegra114 don't have an architected timer, then we
> can't remove the timer-tegra20 driver, because we still need it on
> Tegra20 and Tegra30, right?
>
For Tegra20/30, it's Cortext-A9 with TWD timer. (arch/arm/kernel/smp_twd.c)
Originally, I thought the functionality of timer-tegra20 would be fully
replaced by TWD timer driver. But from the log in the kernelci test
farm[1][2], it looks to me the timer-tegra20 driver still works as
clocksource driver for Tegra20/30. I cannot confirm if the clock event
device has been replaced by TWD timer in the log. It could be replaced
in the background. And by looking into the driver, it should be.
Compare to the log of Tegra124[3], it has been fully replaced by arch
timer driver.
Note, "timer_us" is the name of Tegra20 timer.
[1]:
https://storage.kernelci.org/mainline/master/v5.0-rc4-1-g4aa9fc2a435a/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra30-beaver.html
[2]:
https://storage.kernelci.org/stable-rc/linux-4.9.y/v4.9.153-43-g6674590d15d2/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra20-iris-512.html
[3]:
https://storage.kernelci.org/lsk/linux-linaro-lsk-v4.9/lsk-v4.9-18.09-1494-gde3059d32f93/arm/tegra_defconfig/lab-collabora/boot-tegra124-nyan-big.html
Thanks,
Joseph
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-30 2:40 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-30 2:40 UTC (permalink / raw)
To: Thierry Reding, Peter De Schrijver
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
On 1/29/19 6:29 PM, Thierry Reding wrote:
> On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
>> On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
>>
>> ...
>>
>>>
>>> Up to here this is a duplicate of timer-tegra20.c. And a lot of
>>> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
>>> we unify the two drivers instead?
>>>
>>> The power cycle restrictions of the architected timer, do they not apply
>>> to chips earlier than Tegra210 either? So don't we need all of these
>>> additional features on the timer-tegra20.c driver as well? If so that
>>
>> No. Chips prior to Tegra114 do not have an arch timer and the arch timer
>> does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
>> It's broken on Cortex-A57 though, so we can't use it as a wakeup source
>> on Tegra210.
>
> If chips prior to Tegra114 don't have an architected timer, then we
> can't remove the timer-tegra20 driver, because we still need it on
> Tegra20 and Tegra30, right?
>
For Tegra20/30, it's Cortext-A9 with TWD timer. (arch/arm/kernel/smp_twd.c)
Originally, I thought the functionality of timer-tegra20 would be fully
replaced by TWD timer driver. But from the log in the kernelci test
farm[1][2], it looks to me the timer-tegra20 driver still works as
clocksource driver for Tegra20/30. I cannot confirm if the clock event
device has been replaced by TWD timer in the log. It could be replaced
in the background. And by looking into the driver, it should be.
Compare to the log of Tegra124[3], it has been fully replaced by arch
timer driver.
Note, "timer_us" is the name of Tegra20 timer.
[1]:
https://storage.kernelci.org/mainline/master/v5.0-rc4-1-g4aa9fc2a435a/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra30-beaver.html
[2]:
https://storage.kernelci.org/stable-rc/linux-4.9.y/v4.9.153-43-g6674590d15d2/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra20-iris-512.html
[3]:
https://storage.kernelci.org/lsk/linux-linaro-lsk-v4.9/lsk-v4.9-18.09-1494-gde3059d32f93/arm/tegra_defconfig/lab-collabora/boot-tegra124-nyan-big.html
Thanks,
Joseph
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-30 2:40 ` Joseph Lo
0 siblings, 0 replies; 40+ messages in thread
From: Joseph Lo @ 2019-01-30 2:40 UTC (permalink / raw)
To: Thierry Reding, Peter De Schrijver
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, linux-tegra,
Thomas Gleixner, linux-arm-kernel
On 1/29/19 6:29 PM, Thierry Reding wrote:
> On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
>> On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
>>
>> ...
>>
>>>
>>> Up to here this is a duplicate of timer-tegra20.c. And a lot of
>>> tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
>>> we unify the two drivers instead?
>>>
>>> The power cycle restrictions of the architected timer, do they not apply
>>> to chips earlier than Tegra210 either? So don't we need all of these
>>> additional features on the timer-tegra20.c driver as well? If so that
>>
>> No. Chips prior to Tegra114 do not have an arch timer and the arch timer
>> does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
>> It's broken on Cortex-A57 though, so we can't use it as a wakeup source
>> on Tegra210.
>
> If chips prior to Tegra114 don't have an architected timer, then we
> can't remove the timer-tegra20 driver, because we still need it on
> Tegra20 and Tegra30, right?
>
For Tegra20/30, it's Cortext-A9 with TWD timer. (arch/arm/kernel/smp_twd.c)
Originally, I thought the functionality of timer-tegra20 would be fully
replaced by TWD timer driver. But from the log in the kernelci test
farm[1][2], it looks to me the timer-tegra20 driver still works as
clocksource driver for Tegra20/30. I cannot confirm if the clock event
device has been replaced by TWD timer in the log. It could be replaced
in the background. And by looking into the driver, it should be.
Compare to the log of Tegra124[3], it has been fully replaced by arch
timer driver.
Note, "timer_us" is the name of Tegra20 timer.
[1]:
https://storage.kernelci.org/mainline/master/v5.0-rc4-1-g4aa9fc2a435a/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra30-beaver.html
[2]:
https://storage.kernelci.org/stable-rc/linux-4.9.y/v4.9.153-43-g6674590d15d2/arm/tegra_defconfig/lab-baylibre-seattle/boot-tegra20-iris-512.html
[3]:
https://storage.kernelci.org/lsk/linux-linaro-lsk-v4.9/lsk-v4.9-18.09-1494-gde3059d32f93/arm/tegra_defconfig/lab-collabora/boot-tegra124-nyan-big.html
Thanks,
Joseph
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
2019-01-30 2:40 ` Joseph Lo
(?)
@ 2019-01-31 9:01 ` Peter De Schrijver
-1 siblings, 0 replies; 40+ messages in thread
From: Peter De Schrijver @ 2019-01-31 9:01 UTC (permalink / raw)
To: Joseph Lo
Cc: Thierry Reding, Daniel Lezcano, linux-kernel, Jonathan Hunter,
linux-tegra, Thomas Gleixner, linux-arm-kernel
On Wed, Jan 30, 2019 at 10:40:06AM +0800, Joseph Lo wrote:
> On 1/29/19 6:29 PM, Thierry Reding wrote:
> > On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
> > > On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
> > >
> > > ...
> > >
> > > >
> > > > Up to here this is a duplicate of timer-tegra20.c. And a lot of
> > > > tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> > > > we unify the two drivers instead?
> > > >
> > > > The power cycle restrictions of the architected timer, do they not apply
> > > > to chips earlier than Tegra210 either? So don't we need all of these
> > > > additional features on the timer-tegra20.c driver as well? If so that
> > >
> > > No. Chips prior to Tegra114 do not have an arch timer and the arch timer
> > > does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
> > > It's broken on Cortex-A57 though, so we can't use it as a wakeup source
> > > on Tegra210.
> >
> > If chips prior to Tegra114 don't have an architected timer, then we
> > can't remove the timer-tegra20 driver, because we still need it on
> > Tegra20 and Tegra30, right?
> >
>
> For Tegra20/30, it's Cortext-A9 with TWD timer. (arch/arm/kernel/smp_twd.c)
>
> Originally, I thought the functionality of timer-tegra20 would be fully
> replaced by TWD timer driver. But from the log in the kernelci test
> farm[1][2], it looks to me the timer-tegra20 driver still works as
> clocksource driver for Tegra20/30. I cannot confirm if the clock event
> device has been replaced by TWD timer in the log. It could be replaced in
> the background. And by looking into the driver, it should be.
The TWD timer runs from the CPU clock so its frequency changes with
CPU DVFS. That makes it difficult to use.
Peter.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-31 9:01 ` Peter De Schrijver
0 siblings, 0 replies; 40+ messages in thread
From: Peter De Schrijver @ 2019-01-31 9:01 UTC (permalink / raw)
To: Joseph Lo
Cc: Daniel Lezcano, linux-kernel, Jonathan Hunter, Thierry Reding,
linux-tegra, Thomas Gleixner, linux-arm-kernel
On Wed, Jan 30, 2019 at 10:40:06AM +0800, Joseph Lo wrote:
> On 1/29/19 6:29 PM, Thierry Reding wrote:
> > On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
> > > On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
> > >
> > > ...
> > >
> > > >
> > > > Up to here this is a duplicate of timer-tegra20.c. And a lot of
> > > > tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> > > > we unify the two drivers instead?
> > > >
> > > > The power cycle restrictions of the architected timer, do they not apply
> > > > to chips earlier than Tegra210 either? So don't we need all of these
> > > > additional features on the timer-tegra20.c driver as well? If so that
> > >
> > > No. Chips prior to Tegra114 do not have an arch timer and the arch timer
> > > does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
> > > It's broken on Cortex-A57 though, so we can't use it as a wakeup source
> > > on Tegra210.
> >
> > If chips prior to Tegra114 don't have an architected timer, then we
> > can't remove the timer-tegra20 driver, because we still need it on
> > Tegra20 and Tegra30, right?
> >
>
> For Tegra20/30, it's Cortext-A9 with TWD timer. (arch/arm/kernel/smp_twd.c)
>
> Originally, I thought the functionality of timer-tegra20 would be fully
> replaced by TWD timer driver. But from the log in the kernelci test
> farm[1][2], it looks to me the timer-tegra20 driver still works as
> clocksource driver for Tegra20/30. I cannot confirm if the clock event
> device has been replaced by TWD timer in the log. It could be replaced in
> the background. And by looking into the driver, it should be.
The TWD timer runs from the CPU clock so its frequency changes with
CPU DVFS. That makes it difficult to use.
Peter.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver
@ 2019-01-31 9:01 ` Peter De Schrijver
0 siblings, 0 replies; 40+ messages in thread
From: Peter De Schrijver @ 2019-01-31 9:01 UTC (permalink / raw)
To: Joseph Lo
Cc: Thierry Reding, Daniel Lezcano, linux-kernel, Jonathan Hunter,
linux-tegra, Thomas Gleixner, linux-arm-kernel
On Wed, Jan 30, 2019 at 10:40:06AM +0800, Joseph Lo wrote:
> On 1/29/19 6:29 PM, Thierry Reding wrote:
> > On Tue, Jan 29, 2019 at 10:41:55AM +0200, Peter De Schrijver wrote:
> > > On Mon, Jan 28, 2019 at 04:09:08PM +0100, Thierry Reding wrote:
> > >
> > > ...
> > >
> > > >
> > > > Up to here this is a duplicate of timer-tegra20.c. And a lot of
> > > > tegra210_timer_init() is the same as tegra20_timer_init() as well. Can't
> > > > we unify the two drivers instead?
> > > >
> > > > The power cycle restrictions of the architected timer, do they not apply
> > > > to chips earlier than Tegra210 either? So don't we need all of these
> > > > additional features on the timer-tegra20.c driver as well? If so that
> > >
> > > No. Chips prior to Tegra114 do not have an arch timer and the arch timer
> > > does work correctly on Cortex-A15 so Tegra114 and Tegra124 can use it.
> > > It's broken on Cortex-A57 though, so we can't use it as a wakeup source
> > > on Tegra210.
> >
> > If chips prior to Tegra114 don't have an architected timer, then we
> > can't remove the timer-tegra20 driver, because we still need it on
> > Tegra20 and Tegra30, right?
> >
>
> For Tegra20/30, it's Cortext-A9 with TWD timer. (arch/arm/kernel/smp_twd.c)
>
> Originally, I thought the functionality of timer-tegra20 would be fully
> replaced by TWD timer driver. But from the log in the kernelci test
> farm[1][2], it looks to me the timer-tegra20 driver still works as
> clocksource driver for Tegra20/30. I cannot confirm if the clock event
> device has been replaced by TWD timer in the log. It could be replaced in
> the background. And by looking into the driver, it should be.
The TWD timer runs from the CPU clock so its frequency changes with
CPU DVFS. That makes it difficult to use.
Peter.
^ permalink raw reply [flat|nested] 40+ messages in thread