Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/1/19 8:44 PM, Jon Hunter wrote:

On 01/02/2019 03:36, 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.

Also convert the original driver to use timer-of API.

It may have been nice to split this into 2 patches to make it easier to
see what is going on but not a big deal.

Cc: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx
Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
Acked-by: Thierry Reding <treding@xxxxxxxxxx>
---
v5:
  * add ack tag from Thierry
v4:
  * merge timer-tegra210.c in previous version into timer-tegra20.c
v3:
  * use timer-of API
v2:
  * add error clean-up code
---
  drivers/clocksource/Kconfig         |   2 +-
  drivers/clocksource/timer-tegra20.c | 369 ++++++++++++++++++++--------
  include/linux/cpuhotplug.h          |   1 +
  3 files changed, 272 insertions(+), 100 deletions(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a9e26f6a81a1..6af78534a285 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -131,7 +131,7 @@ config SUN5I_HSTIMER
  config TEGRA_TIMER
  	bool "Tegra timer driver" if COMPILE_TEST
  	select CLKSRC_MMIO
-	depends on ARM
+	select TIMER_OF
  	help
  	  Enables support for the Tegra driver.
diff --git a/drivers/clocksource/timer-tegra20.c b/drivers/clocksource/timer-tegra20.c
index 4293943f4e2b..96a809341c9b 100644
--- a/drivers/clocksource/timer-tegra20.c
+++ b/drivers/clocksource/timer-tegra20.c
@@ -15,21 +15,24 @@
   *
   */
-#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/delay.h>
  #include <linux/err.h>
-#include <linux/time.h>
  #include <linux/interrupt.h>
-#include <linux/irq.h>
-#include <linux/clockchips.h>
-#include <linux/clocksource.h>
-#include <linux/clk.h>
-#include <linux/io.h>
  #include <linux/of_address.h>
  #include <linux/of_irq.h>
-#include <linux/sched_clock.h>
-#include <linux/delay.h>
+#include <linux/percpu.h>
+#include <linux/syscore_ops.h>
+#include <linux/time.h>
+
+#include "timer-of.h"
+#ifdef CONFIG_ARM
  #include <asm/mach/time.h>
+#endif
#define RTC_SECONDS 0x08
  #define RTC_SHADOW_SECONDS     0x0c
@@ -43,70 +46,147 @@
  #define TIMER2_BASE 0x8
  #define TIMER3_BASE 0x50
  #define TIMER4_BASE 0x58
-
-#define TIMER_PTV 0x0
-#define TIMER_PCR 0x4
-
+#define TIMER10_BASE 0x90
+
+#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)
+
+#ifdef CONFIG_ARM
+#define TIMER_BASE TIMER3_BASE
+#else
+#define TIMER_BASE TIMER10_BASE
+#endif
+#define TIMER10_IRQ_IDX		10
+#define TIMER_FOR_CPU(cpu) (TIMER_BASE + (cpu) * 8)
+#define IRQ_IDX_FOR_CPU(cpu)	(TIMER10_IRQ_IDX + cpu)

TIMER10_IRQ_IDX and IRQ_IDX_FOR_CPU are only applicable to ARM64 and so
we should probably not defined for ARM to avoid any confusion.
Okay, will do.

Furthermore, a lot of these TIMERx_BASE definitions are unused AFAICT.
Would be good to get rid of these.

Okay.

Maybe we could just have ...

  +#ifdef CONFIG_ARM
  +#define TIMER_CPU0 3
  +#else
  +#define TIMER_CPU0 10
  +#endif
  +#define TIMER_BASE_FOR_CPU(cpu) ((TIMER_CPU0 + cpu) * 8)
  +#define TIMER_FOR_CPU(cpu) (TIMER_CPU0 + cpu)

This can't get the timer base address. I think you mean ...

+#ifdef CONFIG_ARM
+#define TIMER_CPU0 0x50 /* TIMER3 */
+#else
+#define TIMER_CPU0 0x90 /* TIMER10 */
+#endif
+#define TIMER_BASE_FOR_CPU(cpu) (TIMER_CPU0 + (cpu) * 8)

This doesn't need.
+#define TIMER_FOR_CPU(cpu) (TIMER_CPU0 + cpu)

Will fix above accordingly and adding your ack tag.

Thanks,
Joseph

Otherwise looks good to me.

Cheers
Jon




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux