From: Johan Hovold <johan+linaro@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Jiri Slaby" <jirislaby@kernel.org>,
"Bjorn Andersson" <andersson@kernel.org>,
"Konrad Dybcio" <konradybcio@kernel.org>,
"Douglas Anderson" <dianders@chromium.org>,
"Nícolas F . R . A . Prado" <nfraprado@collabora.com>,
linux-arm-msm@vger.kernel.org, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Johan Hovold" <johan+linaro@kernel.org>,
stable@vger.kernel.org
Subject: [PATCH v2 1/8] serial: qcom-geni: fix fifo polling timeout
Date: Fri, 6 Sep 2024 15:13:29 +0200 [thread overview]
Message-ID: <20240906131336.23625-2-johan+linaro@kernel.org> (raw)
In-Reply-To: <20240906131336.23625-1-johan+linaro@kernel.org>
The qcom_geni_serial_poll_bit() can be used to wait for events like
command completion and is supposed to wait for the time it takes to
clear a full fifo before timing out.
As noted by Doug, the current implementation does not account for start,
stop and parity bits when determining the timeout. The helper also does
not currently account for the shift register and the two-word
intermediate transfer register.
A too short timeout can specifically lead to lost characters when
waiting for a transfer to complete as the transfer is cancelled on
timeout.
Instead of determining the poll timeout on every call, store the fifo
timeout when updating it in set_termios() and make sure to take the
shift and intermediate registers into account. Note that serial core has
already added a 20 ms margin to the fifo timeout.
Also note that the current uart_fifo_timeout() interface does
unnecessary calculations on every call and did not exist in earlier
kernels so only store its result once. This facilitates backports too as
earlier kernels can derive the timeout from uport->timeout, which has
since been removed.
Fixes: c4f528795d1a ("tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP")
Cc: stable@vger.kernel.org # 4.17
Reported-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
drivers/tty/serial/qcom_geni_serial.c | 31 +++++++++++++++------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 69a632fefc41..309c0bddf26a 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -124,7 +124,7 @@ struct qcom_geni_serial_port {
dma_addr_t tx_dma_addr;
dma_addr_t rx_dma_addr;
bool setup;
- unsigned int baud;
+ unsigned long poll_timeout_us;
unsigned long clk_rate;
void *rx_buf;
u32 loopback;
@@ -270,22 +270,13 @@ static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
{
u32 reg;
struct qcom_geni_serial_port *port;
- unsigned int baud;
- unsigned int fifo_bits;
unsigned long timeout_us = 20000;
struct qcom_geni_private_data *private_data = uport->private_data;
if (private_data->drv) {
port = to_dev_port(uport);
- baud = port->baud;
- if (!baud)
- baud = 115200;
- fifo_bits = port->tx_fifo_depth * port->tx_fifo_width;
- /*
- * Total polling iterations based on FIFO worth of bytes to be
- * sent at current baud. Add a little fluff to the wait.
- */
- timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500;
+ if (port->poll_timeout_us)
+ timeout_us = port->poll_timeout_us;
}
/*
@@ -1244,11 +1235,11 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
unsigned long clk_rate;
u32 ver, sampling_rate;
unsigned int avg_bw_core;
+ unsigned long timeout;
qcom_geni_serial_stop_rx(uport);
/* baud rate */
baud = uart_get_baud_rate(uport, termios, old, 300, 4000000);
- port->baud = baud;
sampling_rate = UART_OVERSAMPLING;
/* Sampling rate is halved for IP versions >= 2.5 */
@@ -1326,9 +1317,21 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
else
tx_trans_cfg |= UART_CTS_MASK;
- if (baud)
+ if (baud) {
uart_update_timeout(uport, termios->c_cflag, baud);
+ /*
+ * Make sure that qcom_geni_serial_poll_bitfield() waits for
+ * the FIFO, two-word intermediate transfer register and shift
+ * register to clear.
+ *
+ * Note that uart_fifo_timeout() also adds a 20 ms margin.
+ */
+ timeout = jiffies_to_usecs(uart_fifo_timeout(uport));
+ timeout += 3 * timeout / port->tx_fifo_depth;
+ WRITE_ONCE(port->poll_timeout_us, timeout);
+ }
+
if (!uart_console(uport))
writel(port->loopback,
uport->membase + SE_UART_LOOPBACK_CFG);
--
2.44.2
next prev parent reply other threads:[~2024-09-06 13:14 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-06 13:13 [PATCH v2 0/8] serial: qcom-geni: fix console corruption Johan Hovold
2024-09-06 13:13 ` Johan Hovold [this message]
2024-09-11 20:19 ` [PATCH v2 1/8] serial: qcom-geni: fix fifo polling timeout Doug Anderson
2024-09-06 13:13 ` [PATCH v2 2/8] serial: qcom-geni: fix false console tx restart Johan Hovold
2024-09-06 13:13 ` [PATCH v2 3/8] soc: qcom: geni-se: add GP_LENGTH/IRQ_EN_SET/IRQ_EN_CLEAR registers Johan Hovold
2024-09-06 13:13 ` [PATCH v2 4/8] serial: qcom-geni: fix arg types for qcom_geni_serial_poll_bit() Johan Hovold
2024-09-06 13:13 ` [PATCH v2 5/8] serial: qcom-geni: introduce qcom_geni_serial_poll_bitfield() Johan Hovold
2024-09-06 13:13 ` [PATCH v2 6/8] serial: qcom-geni: fix console corruption Johan Hovold
2024-09-06 13:13 ` [PATCH v2 7/8] serial: qcom-geni: disable interrupts during console writes Johan Hovold
2024-09-06 13:13 ` [PATCH v2 8/8] serial: qcom-geni: fix polled console corruption Johan Hovold
2024-09-11 6:19 ` [PATCH v2 0/8] serial: qcom-geni: fix " Johan Hovold
2024-09-11 7:45 ` Greg Kroah-Hartman
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=20240906131336.23625-2-johan+linaro@kernel.org \
--to=johan+linaro@kernel.org \
--cc=andersson@kernel.org \
--cc=dianders@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=konradybcio@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=nfraprado@collabora.com \
--cc=stable@vger.kernel.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