Re: [PATCH] ARM: twd: Adjust localtimer frequency withcpufreqnotifiers

[Colin Cross]

On Mon, May 16, 2011 at 9:43 AM, Santosh Shilimkar
<santosh.shilimkar@xxxxxx> wrote:
> On 5/16/2011 9:59 PM, Colin Cross wrote:
>>
>> On Mon, May 16, 2011 at 7:44 AM, Thomas Gleixner<tglx@xxxxxxxxxxxxx>
>>  wrote:
>>>
>>> On Mon, 16 May 2011, Santosh Shilimkar wrote:
>>>>
>>>> On 5/14/2011 9:21 PM, Thomas Gleixner wrote:
>>>> Just for my understanding, the clockevents_reconfigure() needs to
>>>> be called with interrupts disabled on that CPU as part of
>>>> the CPUFREQ notifiers. I assume the right place is do it
>>>> in POST notifier after the CPU clock and hence TWD clock is
>>>> updated. Is that right ?
>>>
>>> Yes.
>>
>> Is it safe to only call it in POST?  If the frequency is increasing,
>> and the TWD is not updated until after the CPU frequency has changed,
>> it is possible for a clockevent to fire too early.  Will that cause
>> problems, or does the clockevent code check against a clocksource to
>> ensure the desired time has been reached?  If that is OK, it
>> drastically simplifies the code, because the driver only needs to know
>> the current TWD frequency, not predict a future TWD frequency.
>>
> This was the exact reason I asked this question. As discussed
> earlier on this thread, we observed drift in ticks especially
> at lowest and highest clock-points. But they way I understood
> is clockevents_reconfigure() will block those additional
> ticks at least during the reconfiguration of the clock-event.
>
>
>>>> Since there is need to call this API in interrupt
>>>> disable context, does it make sense to take care of it
>>>> inside the API itself instead of relying on caller fn ?
>>>
>>> Hmm, no strong opinion
>>
>> For SMP TWD, the caller will always be in interrupt disabled mode,
>> because the cpufreq notifier will get called on a random cpu, so
>> smp_call_function_single will be used to transition to the correct
>> cpu, which disables interrupts.
>>
> Ok. So it's indirectly taken care then.
>
>>>> The arch's where the per CPU TWD's share clock, per-cpu
>>>> clock-events should be reconfigured on all CPUs, whenever
>>>> the parent(CPU) clock has changed using some thing like
>>>> smp_call_function_any() etc. Is that right understanding?
>>>
>>> Yes. If that's a common requirement we should move that to core code.
>>
>> Santosh, are you suggesting the TWD be updated from the clock
>> framework instead of the cpufreq notifier?
>>
> That's where I was kind of leaning to. Basically doing this in common
> core code at one place and possibly outside the ARM TWD library. You
> might get same requirement on other arch's in future.

But right now, there is no common clock place, and even if there was,
there is no plan that I know of for a clock notifier, which is what
would be required here.

>> I believe ARMv7 requires all CPUs to run at the same frequency, so it
>> would be possible to do this in the core code somewhere, but A15 has
>> fixed frequency counters, and all SMP Cortex-A9s I've seen use the SMP
>> TWD driver, so in practice this may end up being the only user.
>>
> Yes but the code managing the architectural timer(A15) and TWD(A9) is
> different. But I understand your point about the usage and it
> might be limited to CA9 at this point of time.
>
>> It would be possible for the clockevent to have a flag
>> CLOCKEVENT_EVT_FEAT_SCALES_WITH_CPU, which registers a cpufreq
>> notifier, if there were any other users.
>
>>
> Something like this is better to get better clarity on the
> hardware behavior. O.w we will have piece of code in TWD
> library which needs proper documentation about the
> usage of likes of smp_call_function_single().

This is harder than I made it sound, unless the clockevent code took a
struct clk as well.  Something needs to get the frequency of the
smp_twd clock and pass it in to clockevents_reconfigure.

I will post an RFC patch that uses a clock to call
clockevents_reconfigure from a cpufreq notifier in smp_twd.c.
--
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/