David Brownell wrote:
On Monday 26 January 2009, Brian G Rhodes wrote:
I think someone forgot an 'I' in edma_stop. Writing to ECR seems
pointless to me. I am assuming they meant to do SH_IECR instead of
SH_ECR. This also happens to fix the problem I was having with audio
stalls on dm6446.
ECR == Event Clear Register, scrubbing out pending events
IECR == Interrupt Enable Clear Register, disabling completion IRQs
Wouldn't it make sense to clear *both*?
To me, it makes more sense to leave the event for the channel
controller. But I just started looking at this yesterday.
I guess the theory here is that an IRQ would sometimes trigger
because the TC was busy with a transfer when edma_stop() was
called, so the audio driver got called and then updated the
reload slot ... causing the "stall". Yes?
However, you're then relying on something to release the
channel and then re-acquire it after edma_stop(), since
IESR (Interrupt Enable Set Register) is only written in
edma_alloc_channel(). That seems likely to break any
EDMA user (say, MMC) that doesn't constantly re-alloc
the channels but still relies on completion IRQs (unlike
MMC).
It would probably be better to have edma_free_channel clear the
Interrupt Enable Register bits.
Tested using.
while [ 1 ] ; do speaker-test ; done
while [ 1 ]; do killall speaker-test ; usleep 200000 ; done
Those would be in separate shells, I guess?
Indeed. Can you reproduce this?
Let me know if I am overlooking something here. This is against current
davinci-git kernel. I have merged the current GIT code back into our
2.6.26 stable branch and tested. Perhaps something else is fixing it in
davinci-git.
I'm still stuck at wondering whether edma_clear_channel()
should be needed ... edma_stop() could scrub channel error
status too.
edma_clean_channel may be a good place to _reset_ bits in the Interrupt
Enable Register.
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
