[ + Greg KH ] On 08/07/2015 09:57 AM, Russell King - ARM Linux wrote: > 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: >>> On Fri, Aug 07, 2015 at 12:36:14PM +0200, Sebastian Andrzej Siewior wrote: >>>> On 08/07/2015 11:44 AM, Peter Ujfalusi wrote: >>>>> with a short testing audio did not broke (the only user of pause/resume) >>>>> Some comments embedded. >>>>> >>>>>> Cc: <stable@xxxxxxxxxxxxxxx> >>>>> >>>>> 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. > >>> 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. FWIW, the actual bug is the api that silently does nothing. >> 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. That would be the original submission patch set for an entire driver, the 8250_omap driver. > 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 No. 8250 dma is not a stand-alone driver. Even if it were, how would you go back and fix DTs in the wild? The "binding" is built-in with a CONFIG_ switch. > 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. What you're suggesting here is only possible if the entire 8250_omap driver is removed, so that's a non-starter. I suggest to wait on any solution until the correct fix is mainlined and backported, as you note below. Regards, Peter Hurley > 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. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
