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*? 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). > 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? > 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. > > Thanks > edma.patch > diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c > index 898eb02..557da31 100644 > --- a/arch/arm/mach-davinci/dma.c > +++ b/arch/arm/mach-davinci/dma.c > @@ -1077,10 +1077,11 @@ void edma_stop(unsigned channel) > unsigned int mask = (1 << (channel & 0x1f)); > > edma_shadow0_write_array(SH_EECR, j, mask); > - edma_shadow0_write_array(SH_ECR, j, mask); > + edma_shadow0_write_array(SH_IECR, j, mask); > edma_shadow0_write_array(SH_SECR, j, mask); > edma_write_array(EDMA_EMCR, j, mask); > - > + edma_write_array(EDMA_EEVAL, j, mask); This EEVAL write looks wrong. Only BIT(0) is valid, and zeroing it is a NOP. What were you trying to do? > + > dev_dbg(&edma_dev.dev, "EER%d %08x\n", j, > edma_shadow0_read_array(SH_EER, j)); _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
