Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-31 Thread Peter De Schrijver
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.


Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-29 Thread Joseph Lo

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


Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-29 Thread Thierry Reding
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


signature.asc
Description: PGP signature


Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-29 Thread Jon Hunter


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 
>>>
>>> Cc: Daniel Lezcano 
>>> Cc: Thomas Gleixner 
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo 
>>> ---
>>> 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 ..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 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +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);
>>> +    

Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-29 Thread Peter De Schrijver
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.



Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-28 Thread Joseph Lo

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 

Cc: Daniel Lezcano 
Cc: Thomas Gleixner 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo 
---
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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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_IDX10
+
+#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(>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 

Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-28 Thread Joseph Lo

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 

Cc: Daniel Lezcano 
Cc: Thomas Gleixner 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo 
---
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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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_IDX10
+
+#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(>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(>evt, tegra210_timer_freq,
+ 

Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-28 Thread Thierry Reding
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 
> 
> Cc: Daniel Lezcano 
> Cc: Thomas Gleixner 
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo 
> ---
> 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 ..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static u32 tegra210_timer_freq;
> +static void __iomem *tegra210_timer_reg_base;
> +static u32 usec_config;
> +
> +#define TIMER_PTV0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PERBIT(30)
> +#define TIMER_PCR0x4
> +#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(>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 

Re: [PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-28 Thread Daniel Lezcano
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 
> 
> Cc: Daniel Lezcano 
> Cc: Thomas Gleixner 
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo 
> ---
> 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 ..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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static u32 tegra210_timer_freq;
> +static void __iomem *tegra210_timer_reg_base;
> +static u32 usec_config;
> +
> +#define TIMER_PTV0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PERBIT(30)
> +#define TIMER_PCR0x4
> +#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(>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));
> + 

[PATCH V2 2/6] clocksource: tegra: add Tegra210 timer driver

2019-01-28 Thread Joseph Lo
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 

Cc: Daniel Lezcano 
Cc: Thomas Gleixner 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo 
---
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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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_IDX10
+
+#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(>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(>evt, tegra210_timer_freq,
+   1, /* min */
+   0x1fff); /* 29 bits */
+
+   return 0;
+}
+
+static int tegra210_timer_stop(unsigned int cpu)
+{
+   struct tegra210_clockevent *tevt =