[Letux-kernel] Timers (was Re: [PATCH 00/20] A bunch of JZ4730 fixups for letux-kernel)

[H. Nikolaus Schaller]

Hi Paul,

> Am 19.01.2021 um 19:25 schrieb Paul Boddie <paul at boddie.org.uk>:
> 
> On Tuesday, 19 January 2021 17:14:08 CET H. Nikolaus Schaller wrote:
>>> Am 18.01.2021 um 01:03 schrieb Paul Boddie <paul at boddie.org.uk>:
>>> 
>>> I was just reviewing the timer-related code, and I can only suggest the
>>> improvements covered by the patch included here, with the casting to
>>> 16-bit values removed, and with an adjustment setting the counter upon
>>> reload appropriately remembering that the OST counts down, not up, unlike
>>> the TCU on the JZ4740.
>> 
>> Ah, sorry. I didn't understand that you essentially had the same finding as
>> I had...
>> 
>> So you deserve appreciation of the first to have found the key issue (which
>> is setting both registers to "next").
>> 
>> Expanding to 32 bit turned out to be not necessary.
> 
> Is the setting in ingenic_tcu_cevt_set_next really the issue after all?

Yes.

> I 
> thought that this would only be called once and that any erroneous setting of 
> TCNT would cause a single but inconsequential discrepancy. If it is called 
> many times, however, not setting the correct value would be rather disruptive.

From my analysis the hrtimers work as follows:

- There are one-shot timers and repeating timers.
- These are put into a queue sorted by ascending expiration time.
- Each time a timer expires it triggers its activity (a callback function or waking
  up a process/thread)
- It is then either removed from the queue or requeued at the next expiration time.
- Then, the first expiration event is used in a call to ingenic_tcu_cevt_set_next()
  to request an interrupt after that time.
- What I have not analysed is what happens if there are no active timers or if the
  next one is beyond timer resolution. But there are certainly mechanisms.

Now, the first OST is used for these hrtimers and did interrupt not after "next"
but after 0xffff clock ticks. In most cases this is slower than expected. This is
"relative" time.

The second OST is used by sched_clock() to give a notion of "absolute" time with
high precision. This requires to regularly check the timer before it overflows. This
is done by a repeating hrtimer. If that is triggering too late we loose overflows
and printk timing gets wrong. Maybe this also happens for the jz4780 SMP+Cache
updates?

> 
> Still, it is good that you reached a similar conclusion, even if it was mostly 
> speculative on my part, not having looked into the mechanisms more deeply this 
> time round. I also think it was very helpful of Lubomir to have modified the 
> ingenic-timer.c file to subtract TCNT from TRDR to get a counter value 
> appropriate for the timer. (Here, I'm assuming it was Lubomir and not 
> something I did a while ago and forgot about!)

Yes it was him to add this in "[WIP] clocksource/drivers/ingenic: Add support for JZ4730"
He also added the initial code for ingenic_tcu_cevt_set_next().

BR and thanks,
Nikolaus