Re: [PATCH] clocksource: register persistent clock for arm arch_timer

[Lei Wen]

Hi Stephen,

On Thu, Apr 3, 2014 at 2:09 AM, Stephen Boyd <sboyd@xxxxxxxxxxxxxx> wrote:
> On 04/02/14 04:02, Lei Wen wrote:
>> Since arm's arch_timer's counter would keep accumulated even in the
>> low power mode, including suspend state, it is very suitable to be
>> the persistent clock instead of RTC.
>>
>> While read_persistent_clock calling place shall be rare, like only
>> suspend/resume place? So we shall don't care for its performance
>> very much, so use direclty divided by frequency should be accepted
>> for this reason. Actually archtimer's counter read performance already
>> be very good, since it is directly access from core's bus, not from
>> soc, so this is another reason why we choose use divide here.
>>
>> Final reason for why we don't use multi+shift way is for we may not
>> call read_persistent_clock for long time, like system long time
>> not enter into suspend, so that the accumulated cycle difference value
>> may larger than we used for calculate the multi+shift, thus precise
>> would be highly affected in such corner case.
>>
>> Signed-off-by: Lei Wen <leiwen@xxxxxxxxxxx>
>> ---
>>
>> I am not sure whether it is good to add something like
>> generic_persistent_clock_read in the new added kernel/time/sched_clock.c?
>> Since from arch timer's perspective, all it need to do is to pick
>> the suspend period from the place where sched_clock being stopped/restarted.
>>
>> Any idea for make the persistent clock reading as one generic function,
>> like current sched_clock do?
>
> Why do we need this? Don't we put the CLOCK_SOURCE_SUSPEND_NONSTOP flag
> on the arm_arch_timer clocksource to handle this? The only reason I can
> think of would be that you're calling read_persistent_clock() from
> somewhere else besides the timekeeping core. If that's why, please use
> the time functionality like ktime_get_boottime() or
> get_monotonic_boottime().

You are right for the CLOCK_SOURCE_SUSPEND_NONSTOP flag.
I haven't noticed that latest kernel already has such patch for arch_timer,
so provide additional such implementation shall no need then.

Another question may not related with this patch:
After 3.10, seem sched_clock also would get suspend by default during
suspend, and thus printk message's timestamp would get freezed
during the suspend, and later resume print message appears suspend
never happen, for the sched_clock already deduce the suspend period.

It makes the kernel log hard to align with other cores' generated message.
For other cores, I mean like communication processor.
So it make the debug process painful...
Any idea to solve it? Replace the local_clock to ktime_get_boottime in printk.c?

Thanks,
Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/