From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: twd: Adjust localtimer frequency withcpufreqnotifiers
Date: Mon, 16 May 2011 22:13:19 +0530 [thread overview]
Message-ID: <4DD15427.7080506@ti.com> (raw)
In-Reply-To: <BANLkTimMSPmqD+kyz1EJUrH9sS=qWRZymw@mail.gmail.com>
On 5/16/2011 9:59 PM, Colin Cross wrote:
> On Mon, May 16, 2011 at 7:44 AM, Thomas Gleixner<tglx@linutronix.de> 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.
> 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().
Regards
Santosh
next prev parent reply other threads:[~2011-05-16 16:43 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-18 6:14 [PATCH] ARM: twd: Adjust localtimer frequency with cpufreq notifiers Colin Cross
2011-03-04 10:17 ` Linus Walleij
2011-03-04 10:27 ` martin persson
2011-03-04 20:11 ` Colin Cross
2011-03-04 20:31 ` Rob Herring
2011-03-04 21:33 ` Colin Cross
2011-03-05 8:19 ` [PATCH] ARM: twd: Adjust localtimer frequency with cpufreqnotifiers Santosh Shilimkar
2011-03-06 12:06 ` Linus Walleij
2011-03-06 14:20 ` [PATCH] ARM: twd: Adjust localtimer frequency withcpufreqnotifiers Santosh Shilimkar
2011-03-06 17:42 ` Colin Cross
2011-03-06 19:02 ` Linus Walleij
2011-05-12 15:14 ` Linus Walleij
2011-05-13 10:02 ` Thomas Gleixner
2011-05-13 10:59 ` Linus Walleij
2011-05-13 21:15 ` Russell King - ARM Linux
2011-05-13 21:22 ` Colin Cross
2011-05-13 21:24 ` Colin Cross
2011-05-14 15:51 ` Thomas Gleixner
2011-05-16 11:18 ` Santosh Shilimkar
2011-05-16 14:44 ` Thomas Gleixner
2011-05-16 16:29 ` Colin Cross
2011-05-16 16:33 ` Thomas Gleixner
2011-05-16 16:43 ` Santosh Shilimkar [this message]
2011-05-16 23:08 ` Colin Cross
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DD15427.7080506@ti.com \
--to=santosh.shilimkar@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).