Re: Re: [linux-sunxi] [PATCH 0/8] clocksource: sunxi: Timer fixes and cleanup

From: "maxime.ripard" <maxime.ripard@free-electrons.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: 张猛 <kevin@allwinnertech.com>,
	"Siarhei Siamashka" <siarhei.siamashka@gmail.com>,
	"linux-sunxi@googlegroups.com" <linux-sunxi@googlegroups.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"John Stultz" <john.stultz@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Emilio Lopez" <emilio@elopez.com.ar>,
	孙彦邦 <sunny@allwinnertech.com>, 吴书耕 <shuge@allwinnertech.com>
Subject: Re: Re: [linux-sunxi] [PATCH 0/8] clocksource: sunxi: Timer fixes and cleanup
Date: Fri, 28 Jun 2013 19:03:01 +0200	[thread overview]
Message-ID: <20130628170301.GN4319@lukather> (raw)
In-Reply-To: <alpine.DEB.2.02.1306281559490.4013@ionos.tec.linutronix.de>

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

Hi Thomas,

On Fri, Jun 28, 2013 at 04:02:23PM +0200, Thomas Gleixner wrote:
> On Fri, 28 Jun 2013, 张猛 wrote:
> > > The A10 manual from http://free-electrons.com/~maxime/pub/datasheet/
> > > does not seem to contain any details about what bad things may happen
> > > if we try to read CNT64_LO_REG while latching is still in progress and
> > > CNT64_RL_EN bit in CNT64_CTRL_REG has not changed to zero yet.
> > > I can imagine the following possible scenarios:
> > >  1. We read either the old stale CNT64_LO_REG value or the new
> > >     correct value.
> > >  2. We read either the old stale CNT64_LO_REG value or the new
> > >     correct value, or some random garbage.
> > >  3. The processor may deadlock, eat your dog, or do some other
> > >     nasty thing.
> > >
> > > In the case of 1, we probably can get away without using any spinlocks?
> > 
> > About the 64bits counter, the latch bit is needed because of the asynchronous circuit.
> > The internal circuit of 64bits counter is working under 24Mhz clock, and CNT_LO/HI
> > is read with APB clock. So the clock synchronize is needed. The function of the latch
> > is synchronous the 64bits counter from 24Mhz clock domain to APB clock domain.
> > So, if the latch is not completely, value of the CNT_LO/HI maybe a random value, because
> > some bits are latched, but others are not. So, the CNT_LO/HI should be read after
> > latch is completely.
> > The latch just takes 3 cycles of 24Mhz clock, the time is nearly 0.125 micro-second.
> > 
> 
> I really wonder why we're trying to use that timer. AFAICT the A10 has
> another six 32bit timers which do not have this restriction and the
> clocksoure/sched_clock implementation works nicely with 32 bits. So
> what's the point of using that 64 bit counter if it's horrible to
> access?

Yes, you're right. I actually wanted at first not to use a timer for
this since we had a counter to do just that.

But that's true that actually using a timer would make the code simpler
and presumably faster as well.

I'll change this in the v2.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]