[v3,0/8] NVIDIA Tegra clocksource driver improvements
mbox series

Message ID 20190524153253.28564-1-digetx@gmail.com
Headers show
Series
  • NVIDIA Tegra clocksource driver improvements
Related show

Message

Dmitry Osipenko May 24, 2019, 3:32 p.m. UTC
Hello,

This series primarily unifies the driver code across all Tegra SoC
generations. In a result the clocksources are allocated per-CPU on
older Tegra's and have a higher rating than the arch-timer, the newer
Tegra210 is getting support for microsecond clocksource and the driver's
code is getting much cleaner. Note that arch-timer usage is discouraged on
all Tegra's due to the time jitter caused by the CPU frequency scaling.

The series was extensively tested on Tegra20 and Tegra30.

Changelog:

v3: Fixed compilation on ARM64. Turned out that it doesn't have the
    delay-timer, thanks to Nicolas Chauvet for the report.

    Added new "Support COMPILE_TEST universally" patch for better
    compile-test coverage.

v2: Rebased on recent linux-next. Now all of #ifdef's are removed from the
    code due to the recent patch that generalized persistent clocksource.

    Couple other minor cosmetic changes.

Dmitry Osipenko (8):
  clocksource/drivers/tegra: Support per-CPU timers on all Tegra's
  clocksource/drivers/tegra: Unify timer code
  clocksource/drivers/tegra: Reset hardware state on init
  clocksource/drivers/tegra: Replace readl/writel with relaxed versions
  clocksource/drivers/tegra: Release all IRQ's on request_irq() error
  clocksource/drivers/tegra: Minor code clean up
  clocksource/drivers/tegra: Use SPDX identifier
  clocksource/drivers/tegra: Support COMPILE_TEST universally

 drivers/clocksource/Kconfig         |   4 +-
 drivers/clocksource/timer-tegra20.c | 276 +++++++++++++---------------
 2 files changed, 127 insertions(+), 153 deletions(-)

Comments

Peter De Schrijver May 31, 2019, 8:26 a.m. UTC | #1
On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> This series primarily unifies the driver code across all Tegra SoC
> generations. In a result the clocksources are allocated per-CPU on
> older Tegra's and have a higher rating than the arch-timer, the newer
> Tegra210 is getting support for microsecond clocksource and the driver's
> code is getting much cleaner. Note that arch-timer usage is discouraged on
> all Tegra's due to the time jitter caused by the CPU frequency scaling.

I think the limitations are more as follows:

Chip	timer		suffers cpu dvfs jitter		can wakeup from cc7
T20	us-timer	No				Yes
T20	twd timer	Yes				No?
T30	us-timer	No				Yes
T30	twd timer	Yes				No?
T114	us-timer	No				Yes
T114	arch timer	No				Yes
T124	us-timer	No				Yes
T124	arch timer	No				Yes
T210	us-timer	No				Yes
T210	arch timer	No				No
T210	clk_m timer	No				Yes

right?

Peter.
Dmitry Osipenko May 31, 2019, 12:33 p.m. UTC | #2
31.05.2019 11:26, Peter De Schrijver пишет:
> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series primarily unifies the driver code across all Tegra SoC
>> generations. In a result the clocksources are allocated per-CPU on
>> older Tegra's and have a higher rating than the arch-timer, the newer
>> Tegra210 is getting support for microsecond clocksource and the driver's
>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
> 
> I think the limitations are more as follows:
> 
> Chip	timer		suffers cpu dvfs jitter		can wakeup from cc7
> T20	us-timer	No				Yes
> T20	twd timer	Yes				No?
> T30	us-timer	No				Yes
> T30	twd timer	Yes				No?
> T114	us-timer	No				Yes
> T114	arch timer	No				Yes
> T124	us-timer	No				Yes
> T124	arch timer	No				Yes
> T210	us-timer	No				Yes
> T210	arch timer	No				No
> T210	clk_m timer	No				Yes
> 
> right?

Doesn't arch timer run off the CPU clock? If yes (that's what I
assumed), then it should be affected by the DVFS. Otherwise I'll lower
the clocksource's rating for T114/124/132.

TWD can't wake CPU from the power-down state, so it's a solid "No" for
TWD in the "can wakeup from cc7" column.
Daniel Lezcano May 31, 2019, 8:31 p.m. UTC | #3
On 31/05/2019 14:33, Dmitry Osipenko wrote:
> 31.05.2019 11:26, Peter De Schrijver пишет:
>> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>>> Hello,
>>>
>>> This series primarily unifies the driver code across all Tegra SoC
>>> generations. In a result the clocksources are allocated per-CPU on
>>> older Tegra's and have a higher rating than the arch-timer, the newer
>>> Tegra210 is getting support for microsecond clocksource and the driver's
>>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>>
>> I think the limitations are more as follows:
>>
>> Chip	timer		suffers cpu dvfs jitter		can wakeup from cc7
>> T20	us-timer	No				Yes
>> T20	twd timer	Yes				No?
>> T30	us-timer	No				Yes
>> T30	twd timer	Yes				No?
>> T114	us-timer	No				Yes
>> T114	arch timer	No				Yes
>> T124	us-timer	No				Yes
>> T124	arch timer	No				Yes
>> T210	us-timer	No				Yes
>> T210	arch timer	No				No
>> T210	clk_m timer	No				Yes
>>
>> right?
> 
> Doesn't arch timer run off the CPU clock? If yes (that's what I
> assumed), then it should be affected by the DVFS. Otherwise I'll lower
> the clocksource's rating for T114/124/132.
> 
> TWD can't wake CPU from the power-down state, so it's a solid "No" for
> TWD in the "can wakeup from cc7" column.

Wouldn't make sense to rename the timer-tegra20.c to timer-tegra.c now ?
Dmitry Osipenko June 1, 2019, 1 p.m. UTC | #4
31.05.2019 23:31, Daniel Lezcano пишет:
> On 31/05/2019 14:33, Dmitry Osipenko wrote:
>> 31.05.2019 11:26, Peter De Schrijver пишет:
>>> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>>>> Hello,
>>>>
>>>> This series primarily unifies the driver code across all Tegra SoC
>>>> generations. In a result the clocksources are allocated per-CPU on
>>>> older Tegra's and have a higher rating than the arch-timer, the newer
>>>> Tegra210 is getting support for microsecond clocksource and the driver's
>>>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>>>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>>>
>>> I think the limitations are more as follows:
>>>
>>> Chip	timer		suffers cpu dvfs jitter		can wakeup from cc7
>>> T20	us-timer	No				Yes
>>> T20	twd timer	Yes				No?
>>> T30	us-timer	No				Yes
>>> T30	twd timer	Yes				No?
>>> T114	us-timer	No				Yes
>>> T114	arch timer	No				Yes
>>> T124	us-timer	No				Yes
>>> T124	arch timer	No				Yes
>>> T210	us-timer	No				Yes
>>> T210	arch timer	No				No
>>> T210	clk_m timer	No				Yes
>>>
>>> right?
>>
>> Doesn't arch timer run off the CPU clock? If yes (that's what I
>> assumed), then it should be affected by the DVFS. Otherwise I'll lower
>> the clocksource's rating for T114/124/132.
>>
>> TWD can't wake CPU from the power-down state, so it's a solid "No" for
>> TWD in the "can wakeup from cc7" column.
> 
> Wouldn't make sense to rename the timer-tegra20.c to timer-tegra.c now ?

Wouldn't hurt, given the refreshment that driver is getting lately. I'll
include a patch for that in the next revision, thanks.
Peter De Schrijver June 3, 2019, 7:17 a.m. UTC | #5
On Fri, May 31, 2019 at 03:33:41PM +0300, Dmitry Osipenko wrote:
> 31.05.2019 11:26, Peter De Schrijver пишет:
> > On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
> >> Hello,
> >>
> >> This series primarily unifies the driver code across all Tegra SoC
> >> generations. In a result the clocksources are allocated per-CPU on
> >> older Tegra's and have a higher rating than the arch-timer, the newer
> >> Tegra210 is getting support for microsecond clocksource and the driver's
> >> code is getting much cleaner. Note that arch-timer usage is discouraged on
> >> all Tegra's due to the time jitter caused by the CPU frequency scaling.
> > 
> > I think the limitations are more as follows:
> > 
> > Chip	timer		suffers cpu dvfs jitter		can wakeup from cc7
> > T20	us-timer	No				Yes
> > T20	twd timer	Yes				No?
> > T30	us-timer	No				Yes
> > T30	twd timer	Yes				No?
> > T114	us-timer	No				Yes
> > T114	arch timer	No				Yes
> > T124	us-timer	No				Yes
> > T124	arch timer	No				Yes
> > T210	us-timer	No				Yes
> > T210	arch timer	No				No
> > T210	clk_m timer	No				Yes
> > 
> > right?
> 
> Doesn't arch timer run off the CPU clock? If yes (that's what I
> assumed), then it should be affected by the DVFS. Otherwise I'll lower
> the clocksource's rating for T114/124/132.
> 

No. It doesn't. This is the big change between A9 and later CPUs. 

Peter.

> TWD can't wake CPU from the power-down state, so it's a solid "No" for
> TWD in the "can wakeup from cc7" column.
Dmitry Osipenko June 3, 2019, 11:14 a.m. UTC | #6
03.06.2019 10:17, Peter De Schrijver пишет:
> On Fri, May 31, 2019 at 03:33:41PM +0300, Dmitry Osipenko wrote:
>> 31.05.2019 11:26, Peter De Schrijver пишет:
>>> On Fri, May 24, 2019 at 06:32:45PM +0300, Dmitry Osipenko wrote:
>>>> Hello,
>>>>
>>>> This series primarily unifies the driver code across all Tegra SoC
>>>> generations. In a result the clocksources are allocated per-CPU on
>>>> older Tegra's and have a higher rating than the arch-timer, the newer
>>>> Tegra210 is getting support for microsecond clocksource and the driver's
>>>> code is getting much cleaner. Note that arch-timer usage is discouraged on
>>>> all Tegra's due to the time jitter caused by the CPU frequency scaling.
>>>
>>> I think the limitations are more as follows:
>>>
>>> Chip	timer		suffers cpu dvfs jitter		can wakeup from cc7
>>> T20	us-timer	No				Yes
>>> T20	twd timer	Yes				No?
>>> T30	us-timer	No				Yes
>>> T30	twd timer	Yes				No?
>>> T114	us-timer	No				Yes
>>> T114	arch timer	No				Yes
>>> T124	us-timer	No				Yes
>>> T124	arch timer	No				Yes
>>> T210	us-timer	No				Yes
>>> T210	arch timer	No				No
>>> T210	clk_m timer	No				Yes
>>>
>>> right?
>>
>> Doesn't arch timer run off the CPU clock? If yes (that's what I
>> assumed), then it should be affected by the DVFS. Otherwise I'll lower
>> the clocksource's rating for T114/124/132.
>>
> 
> No. It doesn't. This is the big change between A9 and later CPUs. 

Thank you for the clarification, I'll add a patch to lower the rating
where appropriate.