Thread (22 messages) 22 messages, 4 authors, 2009-05-28

Re: [PATCH V3 2/4] AC97 driver for mpc5200

From: Grant Likely <hidden>
Date: 2009-05-25 15:59:22
Also in: alsa-devel

On Mon, May 25, 2009 at 9:15 AM, Jon Smirl [off-list ref] wrote:
On Mon, May 25, 2009 at 2:16 AM, Grant Likely [off-list ref]=
 wrote:
quoted
On Sun, May 24, 2009 at 7:38 PM, Jon Smirl [off-list ref] wrote:
quoted
+static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned sh=
ort reg)
quoted
quoted
+{
+ =A0 =A0 =A0 int timeout;
+ =A0 =A0 =A0 unsigned int val;
+
+ =A0 =A0 =A0 spin_lock(&psc_dma->lock);
+
+ =A0 =A0 =A0 /* Wait for it to be ready */
+ =A0 =A0 =A0 timeout =3D 1000;
+ =A0 =A0 =A0 while ((--timeout) && (in_be16(&psc_dma->psc_regs->sr_csr=
.status) &
quoted
quoted
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 MPC52xx_PSC_SR_CMDSEND))
quoted
quoted
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 udelay(10);
Holy unbounded latency Batman! =A0This code waits up to 10ms for a regis=
ter read!
quoted
I hate spinning, but if it must be done; I'd like to see it small.
What is the worst case latency? 125us for 8000Hz bus speed? =A0If you
must spin; can a cpu_relax() be used instead of the udelay() while
watch the timebase? =A0Timur recently posted a patch which makes this
easier.

http://patchwork.ozlabs.org/patch/27414/

They *should* be appearing in Ben's -next branch soon.

The link always runs at 12.288Mhz. Each frame is 256 bits. Worst case
you wait for two frames, 42us. If it doesn't respond in 42us the codec
clock is not ticking ( a recurring problem I am running into). These
codecs may be going into a sleep mode I don't understand, but this is
not the right place to try and wake them up. I'll lower the retry
counts to 10 instead of 1000.
That still leaves the problem of unecessarily burning time.  udelay
shouldn't be passed any value larger than 1.  In fact, I think udelay
itself is too coarse grained.  Plus, I'd rather see the timebase used
as the exit condition (as mentioned in previous email).
I played around with implementing this on a kernel thread with
interrupts. It can be done but the code is a lot more complex.
A kernel thread is definitely the wrong approach.  However, if this is
non-atomic context and IRQs are available, then a wait queue can be
used.  42us is about 16k processor clocks.  I'm not sure what the IRQ
and scheduling overhead is so I don't know whether it would be a net
gain or loss in performance.  However, it would be a net gain in worst
case latency.
BTW, 8000Hz is implemented by slot stuffing. The link always runs at
12.288Mhz. The DACs are double buffered. When a sample is transfered
between buffers it sets a bit on the link back to the host, and the
host sends the next sample in the appropriate slot.
ok.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help