Re: [PATCH 3/3] serial: qcom-geni: fix garbage output after buffer flush

[Johan Hovold <johan@kernel.org>]

On Mon, Jun 24, 2024 at 03:19:33PM -0700, Doug Anderson wrote:
> On Mon, Jun 24, 2024 at 6:31 AM Johan Hovold <johan+linaro@kernel.org> wrote:
> >
> > The Qualcomm GENI serial driver does not handle buffer flushing and
> > outputs garbage (or NUL) characters for the remainder of any active TX
> > command after the write buffer has been cleared.
> >
> > Implement the flush_buffer() callback and use it to cancel any active TX
> > command when the write buffer has been emptied.
> 
> I could be reading it wrong, but in the kernel-doc of `struct
> tty_ldisc_ops` it seems to indicate that flush_buffer() is for the
> other direction. Specifically, it says:
> 
> This function instructs the line discipline to clear its buffers of
> any input characters it may have queued to be delivered to the user
> mode process.

Yes, but this a uart op (i.e. not tty_ldisc_ops), for which the doc
states:

	Flush any write buffers, reset any DMA state and stop any
	ongoing DMA transfers.

> I guess the underlying worry I have is that there's no guarantee that
> the flush function will be called when the kfifo loses bytes. If it
> ever happens we'll fall back to writing NUL bytes out and that doesn't
> seem amazing to me. To me it feels like
> qcom_geni_serial_send_chunk_fifo() should detect this situation and
> then it should be responsible for canceling, though better (in my
> mind) is if we never initiate any big transfers if we can get away
> with that and still be performant.

The flush buffer callback is called from the uart_flush_buffer() tty
operation (again, not tty_ldisc_ops) when the FIFO is reset.

Johan