public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jiri Slaby <jirislaby@kernel.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>,
	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 15:28:43 +0200	[thread overview]
Message-ID: <d70049d5-d0fe-465f-a558-45b6785f6014@kernel.org> (raw)
In-Reply-To: <91ac609b-0fae-4856-a2a6-636908d7ad3c@samsung.com>

On 15. 04. 24, 14:58, Marek Szyprowski wrote:
> Dear All,
> 
> 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.

thanks,
-- 
js
suse labs


  reply	other threads:[~2024-04-15 13:28 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-05  6:08 [PATCH 00/15] tty: serial: switch from circ_buf to kfifo Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 01/15] kfifo: drop __kfifo_dma_out_finish_r() Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 02/15] kfifo: introduce and use kfifo_skip_count() Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 03/15] kfifo: add kfifo_out_linear{,_ptr}() Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 04/15] kfifo: remove support for physically non-contiguous memory Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 05/15] kfifo: rename l to len_to_end in setup_sgl() Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 06/15] kfifo: pass offset to setup_sgl_buf() instead of a pointer Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 07/15] kfifo: add kfifo_dma_out_prepare_mapped() Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 08/15] kfifo: fix typos in kernel-doc Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 09/15] tty: 8250_dma: use dmaengine_prep_slave_sg() Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 10/15] tty: 8250_omap: " Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 11/15] tty: msm_serial: " Jiri Slaby (SUSE)
2024-04-15 21:17   ` Marek Szyprowski
2024-04-16 10:23     ` Jiri Slaby
2024-04-17 10:15       ` Marek Szyprowski
2024-04-17 10:50         ` Jiri Slaby
2024-04-17 12:45           ` Marek Szyprowski
2024-04-19  7:17             ` Jiri Slaby
2024-04-19  7:43             ` Jiri Slaby
2024-04-19  7:53               ` Jiri Slaby
2024-04-19  8:00                 ` Marek Szyprowski
2024-04-19  8:09                   ` Jiri Slaby
2024-04-19  8:09                   ` [PATCH] serial: msm: check dma_map_sg() return value properly Jiri Slaby (SUSE)
2024-04-19  9:03                     ` Marek Szyprowski
2024-04-05  6:08 ` [PATCH 12/15] tty: serial: switch from circ_buf to kfifo Jiri Slaby (SUSE)
2024-04-15 12:58   ` Marek Szyprowski
2024-04-15 13:28     ` Jiri Slaby [this message]
2024-04-15 14:17       ` Marek Szyprowski
2024-04-16  5:48         ` [PATCH] serial: meson+qcom: don't advance the kfifo twice Jiri Slaby (SUSE)
2024-04-17 10:08       ` [PATCH 12/15] tty: serial: switch from circ_buf to kfifo Anders Roxell
2024-04-17 10:20         ` Marek Szyprowski
2024-04-17 11:19           ` Anders Roxell
2024-04-22  6:45             ` Jiri Slaby
2024-04-22 10:05               ` Anders Roxell
2024-04-16  3:24   ` Pengfei Xu
2024-04-16  7:04     ` Jiri Slaby
2024-04-16  7:19     ` [PATCH] serial: drop debugging WARN_ON_ONCE() from uart_put_char() Jiri Slaby (SUSE)
2024-05-28 15:05   ` [PATCH] serial: drop debugging WARN_ON_ONCE() from uart_write() Tetsuo Handa
2024-06-03  7:10     ` Jiri Slaby
2024-04-05  6:08 ` [PATCH 13/15] tty: atmel_serial: use single DMA mapping for TX Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 14/15] tty: atmel_serial: define macro for RX size Jiri Slaby (SUSE)
2024-04-05  6:08 ` [PATCH 15/15] tty: atmel_serial: use single DMA mapping for RX Jiri Slaby (SUSE)
2024-04-19 15:12 ` [PATCH 00/15] tty: serial: switch from circ_buf to kfifo Neil Armstrong
2024-04-20  5:42   ` Greg KH
2024-04-22  7:50     ` Neil Armstrong
2024-04-22  5:51   ` Jiri Slaby
2024-04-22  7:43     ` neil.armstrong
2024-06-07 20:32     ` Ferry Toth
2024-06-10 20:16       ` Ferry Toth
2024-06-11  7:36         ` Jiri Slaby
2024-06-12 13:13         ` Ilpo Järvinen
2024-06-16 20:55           ` Ferry Toth
2024-06-17  6:23             ` Ilpo Järvinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d70049d5-d0fe-465f-a558-45b6785f6014@kernel.org \
    --to=jirislaby@kernel.org \
    --cc=andersson@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=konrad.dybcio@linaro.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=neil.armstrong@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox