linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] ARM: SMP_TWD: make setup()/stop() reentrant
Date: Mon, 22 Oct 2012 12:21:16 +0530	[thread overview]
Message-ID: <5084ECE4.5020608@ti.com> (raw)
In-Reply-To: <20121022051319.GA19107@S2101-09.ap.freescale.net>

Linus,

On Monday 22 October 2012 10:43 AM, Shawn Guo wrote:
> On Fri, Oct 19, 2012 at 11:56:29AM +0200, Linus Walleij wrote:
>> This makes the SMP_TWD clock .setup()/.stop() pair reentrant by
>> not re-fetching the clk and re-registering the clock event every
>> time .setup() is called. We also make sure to call the
>> clk_enable()/clk_disable() pair on subsequent calls.
>>
>> As it has been brought to my knowledge that this pair is going
>> to be called from atomic contexts for CPU clusters coming and
>> going, the clk_prepare()/clk_unprepare() calls cannot be called
>> on subsequent .setup()/.stop() iterations.
>>
>> The patch assumes that the code will make sure that
>> twd_set_mode() is called through .set_mode() on the clock
>> event *after* the .setup() call, so that the timer registers
>> are fully re-programmed after a new .setup() cycle.
>>
>> Cc: Shawn Guo <shawn.guo@linaro.org>
>> Reported-by: Peter Chen <peter.chen@freescale.com>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> Peter/Shawn: can you please respond with a Tested-by from your
>> system(s) to indicate if this works as expected?
>
> I have seen two problems with the patch.
>
> 1. twd_timer_setup() is called on per-cpu path, and initial_setup_called
>     should be a per-cpu variable.
>
> 2. When secondary cores get off-line, the clockevent devices for
>     the cores will be released.  So when they become online, the
>     clockevent devices should be registered again.
>
> I can only have my system work as expected with the following changes.
>

How about moving twd_get_clock() outside setup and do that in onetime
init code like register etc. Its is pointless to keep doing
re-registration, clock_enable/disable() since you can not really
disable this clock. Remember we added this clock node for CPUFREQ
and it is just CPU clock with an additional fixed divider.

With above, you neither see the sleep while atomic warning nor
there is any need to fiddle with clock node after boot.

Let me know if I have missed any other consideration here.

Regards,
Santosh

  reply	other threads:[~2012-10-22  6:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-19  9:56 [PATCH v2] ARM: SMP_TWD: make setup()/stop() reentrant Linus Walleij
2012-10-19 16:15 ` Shawn Guo
2012-10-22  5:13 ` Shawn Guo
2012-10-22  6:51   ` Santosh Shilimkar [this message]
2012-10-22 10:14   ` Linus Walleij
2012-10-22 14:14     ` Shawn Guo

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=5084ECE4.5020608@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).