On 08/08/2015 02:28 AM, Peter Hurley wrote: >> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c >> index 0340ee6ba970..07a11e0935e4 100644 >> --- a/drivers/tty/serial/8250/8250_omap.c >> +++ b/drivers/tty/serial/8250/8250_omap.c >> @@ -769,7 +771,9 @@ static void omap_8250_rx_dma_flush(struct uart_8250_port *p) >> return; >> } >> >> - dmaengine_pause(dma->rxchan); >> + ret = dmaengine_pause(dma->rxchan); >> + if (WARN_ON_ONCE(ret)) >> + priv->rx_dma_broken = true; > > No offense, Sebastian, but it boggles my mind that anyone could defend this > as solid api design. We're in the middle of an interrupt handler and the > slave dma driver is /just/ telling us now that it doesn't implement this > functionality?!!? This is the best thing I could come up with. But to be fair: This happens once _very_ early in the process and is 100% reproducible. The WARN_ON should trigger user attention. > The dmaengine api has _so much_ setup and none of it contemplates telling the > consumer that critical functionality is missing? Lets say it is a missing feature which was not noticed / required until now. I have the missing functionality in patch #3. I don't see the point in adding a lookup API for this feature which has to be backported stable just to figure out that RX-DMA might not work. Again: the WARN_ON triggers on the first short RX read _or_ on shutdown of the port which is not after hours using the port. > Even dma_get_slave_caps() returns _true_ for cmd_pause support; ok, that > interface is pointless. > > Rather than losing /critical data/ here, the interrupt handler should just > busy-wait until dmaengine_tx_status() returns DMA_COMPLETE for the rx_cookie. This might not happen at all. At 115200 I *never* encouraged this. Once the FIFO is filled with less than RX-trigger size than the UART sends the time-out interrupt and the DMA *never* completes. Even if the FIFO reaches trigger size later. So you have to abort the DMA transfer and read data manually from the FIFO. That is why the omap enqueues the RX-transfer upfront (while the FIFO is still empty). It happens *sometimes* that the UART sends a timeout interrupt and the DMA transfer just is started and it has been only observed at 3mbaud (but there were no tests 115200 > x > 3mbaud that I know of). Therefor I think this is a fair fix without hiding anything. > Regards, > Peter Hurley Sebastian -- 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
