Thread (29 messages) 29 messages, 5 authors, 2015-08-08

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

From: Russell King - ARM Linux <hidden>
Date: 2015-08-07 13:57:44
Also in: linux-omap, lkml

On Fri, Aug 07, 2015 at 03:42:06PM +0200, Sebastian Andrzej Siewior wrote:
On 08/07/2015 03:22 PM, Russell King - ARM Linux wrote:
quoted
On Fri, Aug 07, 2015 at 12:36:14PM +0200, Sebastian Andrzej Siewior wrote:
quoted
On 08/07/2015 11:44 AM, Peter Ujfalusi wrote:
quoted
with a short testing audio did not broke (the only user of pause/resume)
Some comments embedded.
quoted
Cc: <redacted>
Why stable? This is not fixing any bugs since the PAUSE was not allowed for
non cyclic transfers.
Hmmm. The DRA7x was using pause before for UART. I just did not see it
coming that it was not allowed here. John made a similar change to the
edma driver and I assumed it went stable but now I see that it was just
cherry-picked into the ti tree.
This is *NOT* stable material.

Pause of these channels is something that omap-dma has *never* supported.
Therefore, it is *not* a regression.  What you are doing is *adding* a
feature to the omap-dma driver.  That is not stable material in any sense.
Stable is for bug fixes to existing code, not feature enhancements.
I didn't consider this as a feature.
As it is something that the driver has _not_ supported, you are clearly
adding a feature to an existing driver.  It's not a bug fix.
quoted
If something else has been converted to pause channels and that is causing
a problem, then _that_ conversion is where the bug lies, not the lack of
support in the omap-dma.
So we had the 8250-DMA doing pause and all its current users implement
it. We have a DMA driver tree which is not used and it not implementing
pause (not implementing pause at all). Later we get a combo of 8250-DMA
+ DMA driver that is broken because the lack of pause and this is
noticed a few kernel releases later.
Right, so the patch which caused the regression is the one which arranged
for the 8250-dma + omap-dma combination to work together, not the missing
pause support in omap-dma.

It doesn't matter that it's several releases old, it's that change caused
the regression you have today.  That change is incorrect today, and it was
just as incorrect at the time that it was merged.
The only way of fixing the bug is by implementing the pause feature.
That's not the only way of fixing the bug.

As the binding of drivers is controlled by DT, you can disable the binding
of these two drivers there and 8250 will go back to using non-DMA mode -
which is the situation which existed prior to the change which coupled the
two drivers together.  That's an acceptable change for -stable trees,
because it's reverting the change which caused the regression, taking us
back to a situation we _know_ worked previously.

Then, in mainline during the next merge window, we can introduce the pause
feature to omap-dma, and re-enable the 8250 driver to use it.  _Once_ that's
proven stable, we can then take a view whether those changes should _then_
be backported to stable kernels with greater confidence that backporting
the feature addition won't itself cause a new regression.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help