Linux-Amlogic Archive on lore.kernel.org
 help / color / mirror / Atom feed
* 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