Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
Cc: Thomas Gleixner
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo
Acked-by: Thierry Reding
---
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
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
On 2/2/19 9:30 PM, 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
Cc: Thomas Gleixner
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo
Acked-by: Thierry Reding
---
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.
Then shouldn't device tree look like this? Why TMR7-TMR0 are not defined there?
Yeah, they need to revisit and fix.
timer@60005000 {
compatible = "nvidia,tegra124-timer", "nvidia,tegra30-timer",
"nvidia,tegra20-timer";
reg = <0x0 0x60005000 0x0 0x400>;
interrupts = ,
,
,
,
,
;
;
;
;
;
clocks = <_car TEGRA124_CLK_TIMER>;
clock-names = "timer";
};
TMR 0,6,7,8,9 should define a shared interrupt as well, but seems the shared
interrupt provider is not supported in upstream.
Also note that seems T124/132 device tree has a typo (I'm looking at TK1 TRM),
TMR6 IRQ is 152 and not 122.
And T30 device tree looks incorrect, TRM says that TMR1-TMR5 have a "dedicated
interrupt bit", but not TMR6.
Yeah, noticed that as well. Because the wdt driver doesn't need IRQ
support and they (Tegra114/124/132) use arch timer. So everything just
works fine.
Thanks,
Joseph
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
> Cc: Thomas Gleixner
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo
> Acked-by: Thierry Reding
> ---
>>> 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.
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
>>> Cc: Thomas Gleixner
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo
>>> Acked-by: Thierry Reding
>>> ---
> 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.
Then shouldn't device tree look like this? Why TMR7-TMR0 are not defined there?
timer@60005000 {
compatible = "nvidia,tegra124-timer", "nvidia,tegra30-timer",
"nvidia,tegra20-timer";
reg = <0x0 0x60005000 0x0 0x400>;
interrupts = ,
,
,
,
,
;
;
;
;
;
clocks = <_car TEGRA124_CLK_TIMER>;
clock-names = "timer";
};
TMR 0,6,7,8,9 should define a shared interrupt as well, but seems the shared
interrupt provider is not supported in upstream.
Also note that seems T124/132 device tree has a typo (I'm looking at TK1 TRM),
TMR6 IRQ is 152 and not 122.
And T30 device tree looks incorrect, TRM says that TMR1-TMR5 have a "dedicated
interrupt bit", but not TMR6.
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
Cc: Thomas Gleixner
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo
Acked-by: Thierry Reding
---
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.)
> 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.
Thanks,
Joseph
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
>>> Cc: Thomas Gleixner
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo
>>> Acked-by: Thierry Reding
>>> ---
> 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?
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
On 2/1/19 11:43 PM, Jon Hunter wrote: On 01/02/2019 14:39, Joseph Lo wrote: 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 Cc: Thomas Gleixner Cc: linux-kernel@vger.kernel.org Signed-off-by: Joseph Lo Acked-by: Thierry Reding --- snip. 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) Ah I see. This doesn't need. +#define TIMER_FOR_CPU(cpu) (TIMER_CPU0 + cpu) How come? Don't you still need to know the timer index for a given CPU? Doesn't need. TIMER_BASE_FOR_CPU is enough. Other use cases are well handled by timer-of API. :) Thanks, Joseph
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
On 01/02/2019 14:39, Joseph Lo wrote: > 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 >>> Cc: Thomas Gleixner >>> Cc: linux-kernel@vger.kernel.org >>> Signed-off-by: Joseph Lo >>> Acked-by: Thierry Reding >>> --- >>> 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 >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> #include >>> -#include >>> #include >>> -#include >>> -#include >>> -#include >>> -#include >>> -#include >>> #include >>> #include >>> -#include >>> -#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include "timer-of.h" >>> +#ifdef CONFIG_ARM >>> #include >>> +#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) Ah I see. > This doesn't need. > +#define TIMER_FOR_CPU(cpu) (TIMER_CPU0 + cpu) How come? Don't you still need to know the timer index for a given CPU? Cheers Jon -- nvpublic
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
Cc: Thomas Gleixner
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo
Acked-by: Thierry Reding
---
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.
Thanks,
Joseph
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
> Cc: Thomas Gleixner
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo
> Acked-by: Thierry Reding
> ---
> 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
> +#include
> +#include
> +#include
> +#include
> +#include
> #include
> -#include
> #include
> -#include
> -#include
> -#include
> -#include
> -#include
> #include
> #include
> -#include
> -#include
> +#include
> +#include
> +#include
> +
> +#include "timer-of.h"
> +#ifdef CONFIG_ARM
> #include
> +#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)
> +
> +static u32 usec_config;
> static void __iomem *timer_reg_base;
> +#ifdef CONFIG_ARM
> static void __iomem *rtc_base;
> -
> static struct timespec64 persistent_ts;
> static u64 persistent_ms, last_persistent_ms;
> -
> static struct delay_timer tegra_delay_timer;
> -
> -#define timer_writel(value, reg) \
> - writel_relaxed(value, timer_reg_base + (reg))
> -#define timer_readl(reg) \
> - readl_relaxed(timer_reg_base + (reg))
> +#endif
> static int tegra_timer_set_next_event(unsigned long cycles,
> struct clock_event_device *evt)
> {
> - u32 reg;
> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
> - reg = 0x8000 | ((cycles > 1) ? (cycles-1) : 0);
> - timer_writel(reg, TIMER3_BASE + TIMER_PTV);
> + writel(TIMER_PTV_EN |
> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
> + reg_base + TIMER_PTV);
> return 0;
> }
> -static inline void timer_shutdown(struct clock_event_device *evt)
> +static int tegra_timer_shutdown(struct clock_event_device *evt)
> {
> - timer_writel(0, TIMER3_BASE + TIMER_PTV);
> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
> +
> + writel(0, reg_base + TIMER_PTV);
> +
> + return 0;
> }
> -static int tegra_timer_shutdown(struct clock_event_device *evt)
> +static int tegra_timer_set_periodic(struct clock_event_device *evt)
> {
> - timer_shutdown(evt);
> + void __iomem *reg_base =
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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 Cc: Thomas Gleixner Cc: linux-kernel@vger.kernel.org Signed-off-by: Joseph Lo Acked-by: Thierry Reding --- 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 +#include +#include +#include +#include +#include #include -#include #include -#include -#include -#include -#include -#include #include #include -#include -#include +#include +#include +#include + +#include "timer-of.h" +#ifdef CONFIG_ARM #include +#endif #define RTC_SECONDS0x08 #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_IDX10 +#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
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
Cc: Thomas Gleixner
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Joseph Lo
Acked-by: Thierry Reding
---
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
+#include
+#include
+#include
+#include
+#include
#include
-#include
#include
-#include
-#include
-#include
-#include
-#include
#include
#include
-#include
-#include
+#include
+#include
+#include
+
+#include "timer-of.h"
+#ifdef CONFIG_ARM
#include
+#endif
#define RTC_SECONDS0x08
#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_IDX10
+#define TIMER_FOR_CPU(cpu) (TIMER_BASE + (cpu) * 8)
+#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
+
+static u32 usec_config;
static void __iomem *timer_reg_base;
+#ifdef CONFIG_ARM
static void __iomem *rtc_base;
-
static struct timespec64 persistent_ts;
static u64 persistent_ms, last_persistent_ms;
-
static struct delay_timer tegra_delay_timer;
-
-#define timer_writel(value, reg) \
- writel_relaxed(value, timer_reg_base + (reg))
-#define timer_readl(reg) \
- readl_relaxed(timer_reg_base + (reg))
+#endif
static int tegra_timer_set_next_event(unsigned long cycles,
struct clock_event_device *evt)
{
- u32 reg;
+ void __iomem *reg_base = timer_of_base(to_timer_of(evt));
- reg = 0x8000 | ((cycles > 1) ? (cycles-1) : 0);
- timer_writel(reg, TIMER3_BASE + TIMER_PTV);
+ writel(TIMER_PTV_EN |
+ ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
+ reg_base + TIMER_PTV);
return 0;
}
-static inline void timer_shutdown(struct clock_event_device *evt)
+static int tegra_timer_shutdown(struct clock_event_device *evt)
{
- timer_writel(0, TIMER3_BASE + TIMER_PTV);
+ void __iomem *reg_base = timer_of_base(to_timer_of(evt));
+
+ writel(0, reg_base + TIMER_PTV);
+
+ return 0;
}
-static int tegra_timer_shutdown(struct clock_event_device *evt)
+static int tegra_timer_set_periodic(struct clock_event_device *evt)
{
- timer_shutdown(evt);
+ void __iomem *reg_base = timer_of_base(to_timer_of(evt));
+
+ writel(TIMER_PTV_EN | TIMER_PTV_PER |
+ ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
+ reg_base + TIMER_PTV);
+
return 0;
}
-static int tegra_timer_set_periodic(struct clock_event_device *evt)
+static irqreturn_t tegra_timer_isr(int irq, void *dev_id)
{
- u32 reg = 0xC000 | ((100 / HZ) - 1);
+ struct clock_event_device *evt = (struct clock_event_device *)dev_id;
+ void __iomem *reg_base = timer_of_base(to_timer_of(evt));
+
+ writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
+ evt->event_handler(evt);
+
+ return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_ARM64
+static DEFINE_PER_CPU(struct timer_of, tegra_to) = {
+ .flags = TIMER_OF_CLOCK | TIMER_OF_BASE,
+
+
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
>>> Cc: Thomas Gleixner
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Joseph Lo
>>> Acked-by: Thierry Reding
>>> ---
>>> 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
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> #include
>>> -#include
>>> #include
>>> -#include
>>> -#include
>>> -#include
>>> -#include
>>> -#include
>>> #include
>>> #include
>>> -#include
>>> -#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include "timer-of.h"
>>>
>>> +#ifdef CONFIG_ARM
>>> #include
>>> +#endif
>>>
>>> #define RTC_SECONDS0x08
>>> #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_IDX10
>>> +#define TIMER_FOR_CPU(cpu) (TIMER_BASE + (cpu) * 8)
>>> +#define IRQ_IDX_FOR_CPU(cpu) (TIMER10_IRQ_IDX + cpu)
>>> +
>>> +static u32 usec_config;
>>> static void __iomem *timer_reg_base;
>>> +#ifdef CONFIG_ARM
>>> static void __iomem *rtc_base;
>>> -
>>> static struct timespec64 persistent_ts;
>>> static u64 persistent_ms, last_persistent_ms;
>>> -
>>> static struct delay_timer tegra_delay_timer;
>>> -
>>> -#define timer_writel(value, reg) \
>>> - writel_relaxed(value, timer_reg_base + (reg))
>>> -#define timer_readl(reg) \
>>> - readl_relaxed(timer_reg_base + (reg))
>>> +#endif
>>>
>>> static int tegra_timer_set_next_event(unsigned long cycles,
>>> struct clock_event_device *evt)
>>> {
>>> - u32 reg;
>>> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>>>
>>> - reg = 0x8000 | ((cycles > 1) ? (cycles-1) : 0);
>>> - timer_writel(reg, TIMER3_BASE + TIMER_PTV);
>>> + writel(TIMER_PTV_EN |
>>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>>> + reg_base + TIMER_PTV);
>>>
>>> return 0;
>>> }
>>>
>>> -static inline void timer_shutdown(struct clock_event_device *evt)
>>> +static int tegra_timer_shutdown(struct clock_event_device *evt)
>>> {
>>> - timer_writel(0, TIMER3_BASE + TIMER_PTV);
>>> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>>> +
>>> + writel(0, reg_base + TIMER_PTV);
>>> +
>>> + return 0;
>>> }
>>>
>>> -static int tegra_timer_shutdown(struct clock_event_device *evt)
>>> +static int tegra_timer_set_periodic(struct clock_event_device *evt)
>>> {
>>> - timer_shutdown(evt);
>>> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>>> +
>>> + writel(TIMER_PTV_EN | TIMER_PTV_PER |
>>> + ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
>>> + reg_base + TIMER_PTV);
>>> +
>>> return 0;
>>> }
>>>
>>> -static int tegra_timer_set_periodic(struct clock_event_device *evt)
>>> +static irqreturn_t tegra_timer_isr(int irq, void *dev_id)
>>>
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
>> Cc: Thomas Gleixner
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Joseph Lo
>> Acked-by: Thierry Reding
>> ---
>> 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
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> #include
>> -#include
>> #include
>> -#include
>> -#include
>> -#include
>> -#include
>> -#include
>> #include
>> #include
>> -#include
>> -#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include "timer-of.h"
>>
>> +#ifdef CONFIG_ARM
>> #include
>> +#endif
>>
>> #define RTC_SECONDS0x08
>> #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_ENBIT(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)
>> +
>> +static u32 usec_config;
>> static void __iomem *timer_reg_base;
>> +#ifdef CONFIG_ARM
>> static void __iomem *rtc_base;
>> -
>> static struct timespec64 persistent_ts;
>> static u64 persistent_ms, last_persistent_ms;
>> -
>> static struct delay_timer tegra_delay_timer;
>> -
>> -#define timer_writel(value, reg) \
>> -writel_relaxed(value, timer_reg_base + (reg))
>> -#define timer_readl(reg) \
>> -readl_relaxed(timer_reg_base + (reg))
>> +#endif
>>
>> static int tegra_timer_set_next_event(unsigned long cycles,
>> struct clock_event_device *evt)
>> {
>> -u32 reg;
>> +void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>>
>> -reg = 0x8000 | ((cycles > 1) ? (cycles-1) : 0);
>> -timer_writel(reg, TIMER3_BASE + TIMER_PTV);
>> +writel(TIMER_PTV_EN |
>> + ((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
>> + reg_base + TIMER_PTV);
>>
>> return 0;
>> }
>>
>> -static inline void timer_shutdown(struct clock_event_device *evt)
>> +static int tegra_timer_shutdown(struct clock_event_device *evt)
>> {
>> -timer_writel(0, TIMER3_BASE + TIMER_PTV);
>> +void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>> +
>> +writel(0, reg_base + TIMER_PTV);
>> +
>> +return 0;
>> }
>>
>> -static int tegra_timer_shutdown(struct clock_event_device *evt)
>> +static int tegra_timer_set_periodic(struct clock_event_device *evt)
>> {
>> -timer_shutdown(evt);
>> +void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>> +
>> +writel(TIMER_PTV_EN | TIMER_PTV_PER |
>> + ((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
>> + reg_base + TIMER_PTV);
>> +
>> return 0;
>> }
>>
>> -static int tegra_timer_set_periodic(struct clock_event_device *evt)
>> +static irqreturn_t tegra_timer_isr(int irq, void *dev_id)
>> {
>> -u32 reg = 0xC000 | ((100 / HZ) - 1);
>> +struct clock_event_device *evt = (struct clock_event_device *)dev_id;
>> +void __iomem *reg_base =
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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
> Cc: Thomas Gleixner
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Joseph Lo
> Acked-by: Thierry Reding
> ---
> 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
> +#include
> +#include
> +#include
> +#include
> +#include
> #include
> -#include
> #include
> -#include
> -#include
> -#include
> -#include
> -#include
> #include
> #include
> -#include
> -#include
> +#include
> +#include
> +#include
> +
> +#include "timer-of.h"
>
> +#ifdef CONFIG_ARM
> #include
> +#endif
>
> #define RTC_SECONDS0x08
> #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_PTV0x0
> +#define TIMER_PTV_EN BIT(31)
> +#define TIMER_PTV_PERBIT(30)
> +#define TIMER_PCR0x4
> +#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)
> +
> +static u32 usec_config;
> static void __iomem *timer_reg_base;
> +#ifdef CONFIG_ARM
> static void __iomem *rtc_base;
> -
> static struct timespec64 persistent_ts;
> static u64 persistent_ms, last_persistent_ms;
> -
> static struct delay_timer tegra_delay_timer;
> -
> -#define timer_writel(value, reg) \
> - writel_relaxed(value, timer_reg_base + (reg))
> -#define timer_readl(reg) \
> - readl_relaxed(timer_reg_base + (reg))
> +#endif
>
> static int tegra_timer_set_next_event(unsigned long cycles,
>struct clock_event_device *evt)
> {
> - u32 reg;
> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
>
> - reg = 0x8000 | ((cycles > 1) ? (cycles-1) : 0);
> - timer_writel(reg, TIMER3_BASE + TIMER_PTV);
> + writel(TIMER_PTV_EN |
> +((cycles > 1) ? (cycles - 1) : 0), /* n+1 scheme */
> +reg_base + TIMER_PTV);
>
> return 0;
> }
>
> -static inline void timer_shutdown(struct clock_event_device *evt)
> +static int tegra_timer_shutdown(struct clock_event_device *evt)
> {
> - timer_writel(0, TIMER3_BASE + TIMER_PTV);
> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
> +
> + writel(0, reg_base + TIMER_PTV);
> +
> + return 0;
> }
>
> -static int tegra_timer_shutdown(struct clock_event_device *evt)
> +static int tegra_timer_set_periodic(struct clock_event_device *evt)
> {
> - timer_shutdown(evt);
> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
> +
> + writel(TIMER_PTV_EN | TIMER_PTV_PER |
> +((timer_of_rate(to_timer_of(evt)) / HZ) - 1),
> +reg_base + TIMER_PTV);
> +
> return 0;
> }
>
> -static int tegra_timer_set_periodic(struct clock_event_device *evt)
> +static irqreturn_t tegra_timer_isr(int irq, void *dev_id)
> {
> - u32 reg = 0xC000 | ((100 / HZ) - 1);
> + struct clock_event_device *evt = (struct clock_event_device *)dev_id;
> + void __iomem *reg_base = timer_of_base(to_timer_of(evt));
> +
> + writel(TIMER_PCR_INTR_CLR, reg_base + TIMER_PCR);
> + evt->event_handler(evt);
> +
> + return IRQ_HANDLED;
> +}
> +
> +#ifdef CONFIG_ARM64
>
Re: [PATCH V5 2/7] clocksource: tegra: add Tegra210 timer support
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 > Cc: Thomas Gleixner > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Joseph Lo > Acked-by: Thierry Reding > --- > 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 > +#include > +#include > +#include > +#include > +#include > #include > -#include > #include > -#include > -#include > -#include > -#include > -#include > #include > #include > -#include > -#include > +#include > +#include > +#include > + > +#include "timer-of.h" > > +#ifdef CONFIG_ARM > #include > +#endif > > #define RTC_SECONDS0x08 > #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_PTV0x0 > +#define TIMER_PTV_EN BIT(31) > +#define TIMER_PTV_PERBIT(30) > +#define TIMER_PCR0x4 > +#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. Furthermore, a lot of these TIMERx_BASE definitions are unused AFAICT. Would be good to get rid of these. 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) Otherwise looks good to me. Cheers Jon -- nvpublic
