Hi Loic, On Wed, Apr 16, 2014 at 04:34:58PM +0000, Poulain, Loic wrote: > After analysis, here is my status: > > The Bluetooth chip generates hardware errors due to unexpected > data. These unexpected data are caused by 8250 concurrent TX. > > - Concurrent DMA TX (8250_dma.c): > > After each DMA transfer, __dma_tx_complete() is called. > This function updates the circular buffer tail and calls > serial8250_tx_dma() if data remains. > > However these operations are not protected against concurrent > call of serial8250_tx_dma() (via uart start_tx). So, this concurrent > call may use non-updated tail index and may be called in parallel > of an other serial8250_tx_dma(). > > > - Concurrent Chars TX (8250_corec.c): > > In the serial8250_handle_irq(), serial8250_tx_chars is called if > UART_LSR_THRE. However we should not call this function > if we are using the DMA. Moreover, for the same reason as above, > serial8250_tx_chars could be called with bad tail index or in parallel > of a serial8250_tx_dma. Nice job! Thanks a lot for the analysis! > I think two fixes are necessary: > > to avoid tx chars usage: <snip> > The last one fixes TX issues but causes random freeze of my > platform (when uploading file via bluetooth). It seems to be a deadlock. > I have no kernel trace in my console, system is stuck. > > To temporally workaround this, I simply moved "tx_running=0" after updating > tail index (in __dma_tx_complete) so that no dma_tx can happen before > this update. > > xmit->tail += dma->tx_size; > xmit->tail &= UART_XMIT_SIZE - 1; > p->port.icount.tx += dma->tx_size; > > dma->tx_running = 0; As a side note. Since you are in any case playing with the tx_running flag, could you check if it's possible to drop it and just rely on the status from the dma engine (with dmaengine_tx_status())? Thanks, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
