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

From: Joseph Lo
Date: Sat Feb 02 2019 - 11:07:17 EST


On 2/2/19 9:38 PM, Dmitry Osipenko wrote:
02.02.2019 2:53, Joseph Lo ÐÐÑÐÑ:
On 2/2/19 2:08 AM, Dmitry Osipenko wrote:
01.02.2019 18:37, Joseph Lo ÐÐÑÐÑ:
On 2/1/19 11:13 PM, Dmitry Osipenko wrote:
01.02.2019 17:13, Joseph Lo ÐÐÑÐÑ:
On 2/1/19 9:54 PM, Jon Hunter wrote:

On 01/02/2019 13:11, Dmitry Osipenko wrote:
01.02.2019 16:06, Dmitry Osipenko ÐÐÑÐÑ:
01.02.2019 6:36, 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.

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

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>
---
snip.
+}
+TIMER_OF_DECLARE(tegra210_timer, "nvidia,tegra210-timer", tegra210_timer_init);
+#else /* CONFIG_ARM */
+static int __init tegra20_init_timer(struct device_node *np)
+{
What about T132? Isn't it ARM64 which uses tegra20-timer IP? At least T132 DT suggests so and seems this change will break it.

[snip]


Ah, noticed the "depends on ARM" in Kconfig.. Seems okay then.



This is a good point, because even though we had 'depends on ARM', this
still means that the Tegra132 DT is incorrect.

Joseph, can you take a quick look at Tegra132?

Hi Jon and Dmitry,

No worry about T132, T132 uses arch timer (v7). The tegra20 timer driver has never been used. We should fix the dtsi file later.

Hi Joseph,

So is T132 HW actually incompatible with the tegra20-timer? If it's compatible, then I think the driver's code should be made more universal to support T132.


ÂFrom HW point of view, the TIMER1 ~ TIMER4 is compatible with "nvidia,tegra20-timer". But Tegra132 actually has 10 timers which are exactly the same as Tegra30. So it should backward compatible with "nvidia,tegra30-timer", which is tegra_wdt driver now. And Tegra132 should never use this driver.

The Tegra timer driver should only be used on Tegra20/30/210, three platforms only. Others use arch timer driver for system timer driver.

So we don't really need to take care the usage on other Tegra platforms.

Doesn't Linux kernel put in use all of available timers? If yes, then we probably would want to expose all available timers. It looks to me that right now tegra20-timer exposes only a single-shared timer to the system [please correct me if I'm wrong]. Wouldn't make sense at least to give a timer per CPU core?


No, only one timer driver works at a time. ( see /proc/timer_list to check which timer is working.)

Okay, thanks for the clarification.

It looks to me that right now tegra20-timer exposes only a single-shared timer to the system [please correct me if I'm wrong]. Wouldn't make sense at least to give a timer per CPU core?

Yes, it's correct. the timer-tegra20 only provides a single-shared timer. And yes, ,it should provide a timer per CPU core. But that is another task, this patch only introduce the timer support for Tegra210. Others that originally from timer-tegra20 driver still remain the same.

I may take a look at it. Could be better for older Tegra's to use tegra20-timer for the per-CPU timer since TWD timer has some time-jitter due to DVFS.


That would be great, thank you.
Joseph