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

From: Dmitry Osipenko
Date: Fri Feb 01 2019 - 13:08:35 EST

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?