* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo [not found] ` <20240405060826.2521-13-jirislaby@kernel.org> @ 2024-04-15 12:58 ` Marek Szyprowski 2024-04-15 13:28 ` Jiri Slaby 0 siblings, 1 reply; 9+ messages in thread From: Marek Szyprowski @ 2024-04-15 12:58 UTC (permalink / raw) To: Jiri Slaby (SUSE), gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org Cc: linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl 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: [ 8.763154] Run /sbin/init as init process NT [ 12.429776] platform cpufreq-dt: deferred probe pending: (reason unknown) [ 12.434259] platform regulator-vddcpu: deferred probe pending: pwm-regulator: Failed to get PWM [[6if;9]Uigmkfl-tl ocretbo nrnee .[[6if;9]Uigmkfl-tl ocretbo nrnee . [[6if;9]Uigmkfl-tl ocretbo nrnee . [[6if;9]Uigmkfl-tl ocretbo nrnee . [[6if;9]Uigmkfl-tl ocretbo nrnee . [[6if;9]Uigmkfl-tl ocretbo nrnee . [[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[2l[1[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. [.. yteiigteiiilhtlgeet dvcs..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. [.. yteiigteiiilhtlgeet dvcs..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. [.. yteiigteiiilhtlgeet dvcs..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. [.. yteiigteiiilhtlgeet dvcs..[[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. [.. yteiigteiiilhtlgeet dvcs..[ 105.613420] debugfs: Directory 'ff800280.cec' with parent 'regmap' already present! [[6if;9]Uigmkfl-tl ocretbo nrnee . [.. trightlgeet ipthr ytm-dv5c7[Gmo [94m8[2h[0. ..]Snhszn h nta opu vns(usses..[2l[11[[2 k;9?5[ 105.638809] mc: Linux media interface: v0.10 [.. atn o dvt eflyppltd.[ 105.707390] meson-vrtc ff8000a8.rtc: registered as rtc0 I found this patch by bisecting today's linux-next. I've checked the changes in the related UART drivers and I don't see any obvious issues though. Let me know if I can help debugging this issue somehow. I've trimmed recipient list due to my smtp server limitation. 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 2024-04-15 12:58 ` [PATCH 12/15] tty: serial: switch from circ_buf to kfifo Marek Szyprowski @ 2024-04-15 13:28 ` Jiri Slaby 2024-04-15 14:17 ` Marek Szyprowski 2024-04-17 10:08 ` [PATCH 12/15] tty: serial: switch from circ_buf to kfifo Anders Roxell 0 siblings, 2 replies; 9+ messages in thread From: Jiri Slaby @ 2024-04-15 13:28 UTC (permalink / raw) To: Marek Szyprowski, gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org Cc: linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl 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 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 2024-04-15 13:28 ` Jiri Slaby @ 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 1 sibling, 1 reply; 9+ messages in thread From: Marek Szyprowski @ 2024-04-15 14:17 UTC (permalink / raw) To: Jiri Slaby, gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org Cc: linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] serial: meson+qcom: don't advance the kfifo twice 2024-04-15 14:17 ` Marek Szyprowski @ 2024-04-16 5:48 ` Jiri Slaby (SUSE) 0 siblings, 0 replies; 9+ messages in thread From: Jiri Slaby (SUSE) @ 2024-04-16 5:48 UTC (permalink / raw) To: gregkh Cc: linux-serial, linux-kernel, Jiri Slaby (SUSE), Marek Szyprowski, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl, linux-arm-kernel, linux-amlogic, linux-arm-msm Marek reports, that the -next commit 1788cf6a91d9 (tty: serial: switch from circ_buf to kfifo) broke meson_uart and qcom_geni_serial. The commit mistakenly advanced the kfifo twice: once by uart_fifo_get()/kfifo_out() and second time by uart_xmit_advance(). To advance the fifo only once, drop the superfluous uart_xmit_advance() from both. To count the TX statistics properly, use uart_fifo_out() in qcom_geni_serial (meson_uart_start_tx() already uses that). I checked all other uses of uart_xmit_advance() and they appear correct: either they are finishing DMA transfers or are after peek/linear_ptr (i.e. they do not advance fifo). Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Cc: Neil Armstrong <neil.armstrong@linaro.org> Cc: Kevin Hilman <khilman@baylibre.com> Cc: Jerome Brunet <jbrunet@baylibre.com> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- Cc: linux-arm-kernel@lists.infradead.org Cc: linux-amlogic@lists.infradead.org Cc: linux-arm-msm@vger.kernel.org --- drivers/tty/serial/meson_uart.c | 1 - drivers/tty/serial/qcom_geni_serial.c | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c index 4587ed4d4d5d..8eb586ac3b0d 100644 --- a/drivers/tty/serial/meson_uart.c +++ b/drivers/tty/serial/meson_uart.c @@ -162,7 +162,6 @@ static void meson_uart_start_tx(struct uart_port *port) break; writel(ch, port->membase + AML_UART_WFIFO); - uart_xmit_advance(port, 1); } if (!kfifo_is_empty(&tport->xmit_fifo)) { diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 7814982f1921..2bd25afe0d92 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -855,7 +855,6 @@ static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport, unsigned int chunk) { struct qcom_geni_serial_port *port = to_dev_port(uport); - struct tty_port *tport = &uport->state->port; unsigned int tx_bytes, remaining = chunk; u8 buf[BYTES_PER_FIFO_WORD]; @@ -863,8 +862,7 @@ static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport, memset(buf, 0, sizeof(buf)); tx_bytes = min(remaining, BYTES_PER_FIFO_WORD); - tx_bytes = kfifo_out(&tport->xmit_fifo, buf, tx_bytes); - uart_xmit_advance(uport, tx_bytes); + tx_bytes = uart_fifo_out(uport, buf, tx_bytes); iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1); -- 2.44.0 _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 2024-04-15 13:28 ` Jiri Slaby 2024-04-15 14:17 ` Marek Szyprowski @ 2024-04-17 10:08 ` Anders Roxell 2024-04-17 10:20 ` Marek Szyprowski 1 sibling, 1 reply; 9+ messages in thread From: Anders Roxell @ 2024-04-17 10:08 UTC (permalink / raw) To: Jiri Slaby Cc: Marek Szyprowski, gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org, linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl On 2024-04-15 15:28, Jiri Slaby wrote: > 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. > I'm trying to run on two dragonboard devices db410c and db845c and both fails to boot see the boot failure from db845c [1], linux-next tag: next-20240415. I tried to apply the patch [2] (that you proposed in this thread) ontop of next-20240415. However, that didn't help bootlog on db845c [3]. Cheers, Anders [1] https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/lkft/tests/2f7sLxYtIQXQzsnTzE1Dye2xweg/logs?format=html [2] https://lore.kernel.org/lkml/20240416054825.6211-1-jirislaby@kernel.org/raw [3] https://tuxapi.tuxsuite.com/v1/groups/linaro/projects/anders/tests/2fDgvWnyEmFm9mqMCxOaruBOfTe/logs?format=html _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 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 0 siblings, 1 reply; 9+ messages in thread From: Marek Szyprowski @ 2024-04-17 10:20 UTC (permalink / raw) To: Anders Roxell, Jiri Slaby Cc: gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org, linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl On 17.04.2024 12:08, Anders Roxell wrote: > On 2024-04-15 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. >> > I'm trying to run on two dragonboard devices db410c and db845c and both > fails to boot see the boot failure from db845c [1], linux-next tag: next-20240415. > I tried to apply the patch [2] (that you proposed in this thread) ontop of next-20240415. However, that didn't > help bootlog on db845c [3]. This is a different issue, which I've reported 2 days ago. See the following thread: https://lore.kernel.org/all/d3eb9f21-f3e1-43ec-bf41-984c6aa5cfc8@samsung.com/ 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 2024-04-17 10:20 ` Marek Szyprowski @ 2024-04-17 11:19 ` Anders Roxell 2024-04-22 6:45 ` Jiri Slaby 0 siblings, 1 reply; 9+ messages in thread From: Anders Roxell @ 2024-04-17 11:19 UTC (permalink / raw) To: Marek Szyprowski Cc: Jiri Slaby, gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org, linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl On Wed, 17 Apr 2024 at 12:20, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > On 17.04.2024 12:08, Anders Roxell wrote: > > On 2024-04-15 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. > >> > > I'm trying to run on two dragonboard devices db410c and db845c and both > > fails to boot see the boot failure from db845c [1], linux-next tag: next-20240415. > > I tried to apply the patch [2] (that you proposed in this thread) ontop of next-20240415. However, that didn't > > help bootlog on db845c [3]. > > This is a different issue, which I've reported 2 days ago. See the > following thread: > > https://lore.kernel.org/all/d3eb9f21-f3e1-43ec-bf41-984c6aa5cfc8@samsung.com/ Oh ok, I did the bisection on db845v, and that led me to this patch 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo") Cheers, Anders _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 2024-04-17 11:19 ` Anders Roxell @ 2024-04-22 6:45 ` Jiri Slaby 2024-04-22 10:05 ` Anders Roxell 0 siblings, 1 reply; 9+ messages in thread From: Jiri Slaby @ 2024-04-22 6:45 UTC (permalink / raw) To: Anders Roxell, Marek Szyprowski Cc: gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org, linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl On 17. 04. 24, 13:19, Anders Roxell wrote: >> I'm trying to run on two dragonboard devices db410c and db845c and both >>> fails to boot see the boot failure from db845c [1], linux-next tag: next-20240415. >>> I tried to apply the patch [2] (that you proposed in this thread) ontop of next-20240415. However, that didn't >>> help bootlog on db845c [3]. >> >> This is a different issue, which I've reported 2 days ago. See the >> following thread: >> >> https://lore.kernel.org/all/d3eb9f21-f3e1-43ec-bf41-984c6aa5cfc8@samsung.com/ > > Oh ok, I did the bisection on db845v, and that led me to this > patch 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo") Could you re-test with the today's -next? In particular, with this commit: commit f70f95b485d78838ad28dbec804b986d11ad7bb0 Author: Jiri Slaby (SUSE) <jirislaby@kernel.org> Date: Fri Apr 19 10:09:31 2024 +0200 serial: msm: check dma_map_sg() return value properly thanks, -- js suse labs _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 12/15] tty: serial: switch from circ_buf to kfifo 2024-04-22 6:45 ` Jiri Slaby @ 2024-04-22 10:05 ` Anders Roxell 0 siblings, 0 replies; 9+ messages in thread From: Anders Roxell @ 2024-04-22 10:05 UTC (permalink / raw) To: Jiri Slaby Cc: Marek Szyprowski, gregkh, linux-amlogic, linux-arm-msm@vger.kernel.org, linux-serial, linux-kernel, Bjorn Andersson, Konrad Dybcio, Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl On Mon, 22 Apr 2024 at 08:45, Jiri Slaby <jirislaby@kernel.org> wrote: > > On 17. 04. 24, 13:19, Anders Roxell wrote: > >> I'm trying to run on two dragonboard devices db410c and db845c and both > >>> fails to boot see the boot failure from db845c [1], linux-next tag: next-20240415. > >>> I tried to apply the patch [2] (that you proposed in this thread) ontop of next-20240415. However, that didn't > >>> help bootlog on db845c [3]. > >> > >> This is a different issue, which I've reported 2 days ago. See the > >> following thread: > >> > >> https://lore.kernel.org/all/d3eb9f21-f3e1-43ec-bf41-984c6aa5cfc8@samsung.com/ > > > > Oh ok, I did the bisection on db845v, and that led me to this > > patch 1788cf6a91d9 ("tty: serial: switch from circ_buf to kfifo") > > Could you re-test with the today's -next? Tested todays next and it boots fine. Tested-by: Anders Roxell <anders.roxell@linaro.org> Cheers, Anders > > In particular, with this commit: > commit f70f95b485d78838ad28dbec804b986d11ad7bb0 > Author: Jiri Slaby (SUSE) <jirislaby@kernel.org> > Date: Fri Apr 19 10:09:31 2024 +0200 > > serial: msm: check dma_map_sg() return value properly > > > thanks, > -- > js > suse labs > _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-04-22 10:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240405060826.2521-1-jirislaby@kernel.org>
[not found] ` <CGME20240415125847eucas1p2bc180c35f40f9c490c713679871af9ae@eucas1p2.samsung.com>
[not found] ` <20240405060826.2521-13-jirislaby@kernel.org>
2024-04-15 12:58 ` [PATCH 12/15] tty: serial: switch from circ_buf to kfifo Marek Szyprowski
2024-04-15 13:28 ` Jiri Slaby
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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox