linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] ARM: OMAP: counter: add locking to read_persistent_clock
Date: Mon, 8 Oct 2012 14:02:18 -0700	[thread overview]
Message-ID: <20121008210218.GD13011@atomide.com> (raw)
In-Reply-To: <20120925151755.GA4840@atomide.com>

* Tony Lindgren <tony@atomide.com> [120925 08:19]:
> * R, Sricharan <r.sricharan@ti.com> [120925 00:44]:
> > Hi Tony,
> > 
> > [snip..]
> > 
> > >> > index dbf1e03..2bc51fb 100644
> > >> > --- a/arch/arm/plat-omap/counter_32k.c
> > >> > +++ b/arch/arm/plat-omap/counter_32k.c
> > >> > @@ -55,22 +55,29 @@ static u32 notrace omap_32k_read_sched_clock(void)
> > >> >   * nsecs and adds to a monotonically increasing timespec.
> > >> >   */
> > >> >  static struct timespec persistent_ts;
> > >> > -static cycles_t cycles, last_cycles;
> > >> > +static cycles_t cycles;
> > >> >  static unsigned int persistent_mult, persistent_shift;
> > >> > +static DEFINE_SPINLOCK(read_persistent_clock_lock);
> > >> > +
> > >> >  static void omap_read_persistent_clock(struct timespec *ts)
> > >> >  {
> > >> >     unsigned long long nsecs;
> > >> > -   cycles_t delta;
> > >> > -   struct timespec *tsp = &persistent_ts;
> > >> > +   cycles_t last_cycles;
> > >> > +   unsigned long flags;
> > >> > +
> > >> > +   spin_lock_irqsave(&read_persistent_clock_lock, flags);
> > >> >
> > >> >     last_cycles = cycles;
> > >> >     cycles = sync32k_cnt_reg ? __raw_readl(sync32k_cnt_reg) : 0;
> > >> > -   delta = cycles - last_cycles;
> > >> >
> > >> > -   nsecs = clocksource_cyc2ns(delta, persistent_mult, persistent_shift);
> > >> > +   nsecs = clocksource_cyc2ns(cycles - last_cycles,
> > >> > +                                   persistent_mult, persistent_shift);
> > >
> > > ..I think there's another bug here where cycles - last_cycles
> > > returns wrong value when the timer wraps around as cycles_t is
> > > 64 bits and the counter is 32 bits. It seems it's been there
> > > since when the read_persistent_clock was added with commit
> > > d92cfcbe (OMAP: timekeeping: time should not stop during suspend)?
> > >
> > > It seems that after this patch cycles should not be cycles_t
> > > but u32, and the result of cycles - last_cycles should also
> > > be u32.
> > >
> >  cycles_t is defined as  typedef unsigned long cycles_t;
> >  Am i missing something here ?
> 
> Oh right, cycles_t is unsigned long, it's OK then. Must
> have grepped for it from some other arch.

Applying to fixes with Cc stable.

Regards,

Tony

      reply	other threads:[~2012-10-08 21:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-12  7:14 [PATCH V2] ARM: OMAP: counter: add locking to read_persistent_clock R Sricharan
2012-09-25  0:43 ` Kevin Hilman
2012-09-25  1:22   ` Tony Lindgren
2012-09-25  7:43     ` R, Sricharan
2012-09-25 15:17       ` Tony Lindgren
2012-10-08 21:02         ` Tony Lindgren [this message]

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=20121008210218.GD13011@atomide.com \
    --to=tony@atomide.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).