Re: [PATCH v2 2/2] msm: DMAEngine: Add DMAEngine driver based on old MSM DMA APIs

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Sat, Jan 07, 2012 at 04:13:56PM -0800, Daniel Walker wrote:
> On Sat, 2012-01-07 at 19:21 +0000, Russell King - ARM Linux wrote:
> > Please refer to the DMA engine documentation, found here:
> > 
> > Documentation/dmaengine.txt
> > 
> > section 3:
> > 
> >    Note:
> >         Although the async_tx API specifies that completion callback
> >         routines cannot submit any new operations, this is not the
> >         case for slave/cyclic DMA.
> > 
> >         For slave DMA, the subsequent transaction may not be available
> >         for submission prior to callback function being invoked, so
> >         slave DMA callbacks are permitted to prepare and submit a new
> >         transaction.
> > 
> >         For cyclic DMA, a callback function may wish to terminate the
> >         DMA via dmaengine_terminate_all().
> > 
> > *       Therefore, it is important that DMA engine drivers drop any
> > *       locks before calling the callback function which may cause a
> > *       deadlock.
> 
> Here's the comment from at_hdmac.c .
> 
>                 /*
>                  * The API requires that no submissions are done from a
>                  * callback, so we don't need to drop the lock here
>                  */
>                 if (callback)
>                         callback(param);
> 
> I don't know much about the DMA engine, but maybe there is some special
> case here that makes this ok.. (CC'ed Nicolas Ferre)

If you read the note fully, you'll notice that there's a difference
between the async_tx API and the slave/cyclic DMA API (it's covered
in the first paragraph.)

Plus that documentation was written by me, reviewed by Dan and Vinod
and accepted.  You can treat it as authoritive, and take from it that
at_hdmac.c is buggy.