Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers

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

On Fri, Aug 07, 2015 at 02:21:59PM -0400, Peter Hurley wrote:
> [ + Heikki ]
> 
> On 08/07/2015 12:33 PM, Russell King - ARM Linux wrote:
> > What you have is a race condition in the code you a responsible for
> > maintaining, caused by poorly implemented code.  Fix it, rather than
> > whinging about drivers outside of your subsystem having never implemented
> > _optional_ things that you choose to merge broken code which relied upon
> > it _without_ checking that the operation succeeded.
> > 
> > It is _entirely_ your code which is wrong here.
> > 
> > I will wait for that to be fixed before acking the omap-dma change since
> > you obviously need something to test with.
> 
> I'm not sure to what you're referring here.
> 
> A WARNing fixes nothing.

The warning can wait.

> If you mean some patch, as yet unwritten, that handles the dma cases when
> dmaengine_pause() is unimplemented without data loss, ok, but please confirm
> that's what you mean.

But the regression needs fixing.

> However, at some point one must look at the api and wonder if the separation
> of concern has been drawn in the right place.

It _is_ in the right place.  dmaengine_pause() always has been permitted
to fail.  It's the responsibility of the user of this API to _check_ the
return code to find out whether it had the desired effect.  Not checking
the return code is a bug in the caller's code.

If that wasn't the case, dmaengine_pause() would have a void return type.
It doesn't.  It has an 'int' to allow failure, or to allow non-
implementation for cases where the underlying hardware can't pause the
channel without causing data loss.

What would you think is better: an API which silently loses data, or
one which refuses to stop the transfer and reports an error code back
to the caller.

You seem to be arguing for the former, and as such, there's no way I
can take you seriously.

In any case, Greg has now commented on the patch adding the feature,
basically refusing it for stable tree inclusion.  So the matter is
settled: omap-dma isn't going to get the pause feature added in stable
trees any time soon.  So a different solution now needs to be found,
which is what I've been saying all along...

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.