Re: [PATCH] tty: serial: 8250_omap: fix TX with DMA for am33xx

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, May 14, 2025 at 09:41:16AM +0100, Måns Rullgård wrote:
> "Jiri Slaby (SUSE)" <jirislaby@kernel.org> writes:
> 
> > Commit 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> > introduced an error in the TX DMA handling for 8250_omap.
> >
> > When the OMAP_DMA_TX_KICK flag is set, the "skip_byte" is pulled from
> > the kfifo and emitted directly in order to start the DMA. While the
> > kfifo is updated, dma->tx_size is not decreased. This leads to
> > uart_xmit_advance() called in omap_8250_dma_tx_complete() advancing the
> > kfifo by one too much.
> >
> > In practice, transmitting N bytes has been seen to result in the last
> > N-1 bytes being sent repeatedly.
> >
> > This change fixes the problem by moving all of the dma setup after the
> > OMAP_DMA_TX_KICK handling and using kfifo_len() instead of the DMA size
> > for the 4-byte cutoff check. This slightly changes the behaviour at
> > buffer wraparound, but it still transmits the correct bytes somehow.
> >
> > Now, the "skip_byte" would no longer be accounted to the stats. As
> > previously, dma->tx_size included also this skip byte, up->icount.tx was
> > updated by aforementioned uart_xmit_advance() in
> > omap_8250_dma_tx_complete(). Fix this by using the uart_fifo_out()
> > helper instead of bare kfifo_get().
> >
> > Based on patch by Mans Rullgard <mans@mansr.com>
> >
> > Fixes: 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo")
> > Reported-by: Mans Rullgard <mans@mansr.com>
> > Cc: stable@vger.kernel.org
> >
> > ---
> > The same as for the original patch, I would appreaciate if someone
> > actually tests this one on a real HW too.
> >
> > A patch to optimize the driver to use 2 sgls is still welcome. I will
> > not add it without actually having the HW.
> 
> Are you seriously expecting me to waste even more time on this?
> Do your damn job like you should have to begin with.

That type of response is not allowed here.  Please refrain from this in
the future.

greg k-h