Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo

From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Jiri Slaby <jirislaby@kernel.org>,
	gregkh@linuxfoundation.org, linux-amlogic@lists.infradead.org,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>
Cc: linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konrad.dybcio@linaro.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Subject: Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo
Date: Mon, 15 Apr 2024 16:17:44 +0200	[thread overview]
Message-ID: <1dd9e3a1-29d0-4628-9b6a-b7e9fc09bf0f@samsung.com> (raw)
In-Reply-To: <d70049d5-d0fe-465f-a558-45b6785f6014@kernel.org>

On 15.04.2024 15:28, Jiri Slaby wrote:
> On 15. 04. 24, 14:58, Marek Szyprowski wrote:
>> On 05.04.2024 08:08, Jiri Slaby (SUSE) wrote:
>>> Switch from struct circ_buf to proper kfifo. kfifo provides much better
>>> API, esp. when wrap-around of the buffer needs to be taken into 
>>> account.
>>> Look at pl011_dma_tx_refill() or cpm_uart_tx_pump() changes for 
>>> example.
>>>
>>> Kfifo API can also fill in scatter-gather DMA structures, so it easier
>>> for that use case too. Look at lpuart_dma_tx() for example. Note that
>>> not all drivers can be converted to that (like atmel_serial), they
>>> handle DMA specially.
>>>
>>> Note that usb-serial uses kfifo for TX for ages.
>>>
>>> omap needed a bit more care as it needs to put a char into FIFO to 
>>> start
>>> the DMA transfer when OMAP_DMA_TX_KICK is set. In that case, we have to
>>> do kfifo_dma_out_prepare twice: once to find out the tx_size (to find
>>> out if it is worths to do DMA at all -- size >= 4), the second time for
>>> the actual transfer.
>>>
>>> All traces of circ_buf are removed from serial_core.h (and its struct
>>> uart_state).
>>>
>>> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
>>> ...
>>
>> This patch landed in linux-next as commit 1788cf6a91d9 ("tty: serial:
>> switch from circ_buf to kfifo"). Unfortunately it breaks UART operation
>> on thr Amlogic Meson based boards (drivers/tty/serial/meson_uart.c
>> driver) and Qualcomm RB5 board (drivers/tty/serial/qcom_geni_serial.c).
>> Once the init process is started, a complete garbage is printed to the
>> serial console. Here is an example how it looks:
>
> Oh my!
>
> Both drivers move the tail using both kfifo and uart_xmit_advance() 
> interfaces. Bah. Does it help to remove that uart_xmit_advance() for 
> both of them? (TX stats will be broken.)
>
> Users of uart_port_tx() are not affected.
>
> This is my fault when merging uart_xmit_advance() with this series.

Yes, removing uart_xmit_advance() from both drivers seems to be fixing 
the console output.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic