Mem-2-Mem DMA - Generalized API

[Clifford Wolf]

Hi,

Thanks for your feedback. I will incorporate to my code and release a new
version of Tuesday when I'm back in the office..

On Sat, Jul 07, 2007 at 03:08:03PM +0200, Arnd Bergmann wrote:
> > +void dmatransfer_unregister_backend(struct dmatransfer_backend *backend)
> > +{
> > +	unsigned long irqflags;
> > +	write_lock_irqsave(&backend_list_lock, irqflags);
> > +	list_del(&backend->backend_list);
> > +	write_unlock_irqrestore(&backend_list_lock, irqflags);
> > +
> > +	// make sure all pending requests have finished before returning
> > +	down_write(&backend->unreg_sem);
> > +	up_write(&backend->unreg_sem);
> > +}
> 
> This usage of rw semaphores looks fishy. 

yep. do you have a better idea how to implement this easily?

> > +		if ((dmasr & (BIT(TE, 7) | BIT(EOCDI, 0))) != 0) {
> > +			if ((dmasr & BIT(TE, 7)) != 0) {
> > +				printk(KERN_ERR "MPC8349 DMA Transfer Error on DMA channel #%d.\n",
> > i); +				BUG();
> > +			} 
> 
> BUG() may be a little harsh here, especially since you are holding spinlocks.
> It would be better to try to recover here, unless you expect actual data
> corruption, in which case a full panic() might be more appropriate.

in this way dmatransfer() is designed to be used like memcpy(): you don't
try and check for errors afterwards, you only start a dmatransfer if you
know it is ok and can't fail.

This codepath triggeres if 'the impossible' happens: a dma transfer
actually fails. Since there is no datapath to communicate this error back
to the caller it is almost sure that we have lost data. I will change that
to use panic() instead.

yours,
 - clifford

-- 
Some languages are designed to solve a problem. Others are designed to
prove a point.