The driver has some follow-up comments right after it got merged. This
patch addresses those comments that got missed out.
* Document reason for newline character counting in console_write
* Document reason for disabling IRQ in the system resume operation
* Use min3 to find the minimum of 3 values
* Remove unnecessary casting while using min_t
* Use iowrite32_rep to write to the hardware FIFO
* Initialize the console port statically
* Fine-tune memory barrier usage
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@xxxxxxxxxxxxxx>
---
drivers/tty/serial/qcom_geni_serial.c | 84 ++++++++++++++++++-----------------
1 file changed, 44 insertions(+), 40 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 1442777..f5b9cb8 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -105,7 +105,7 @@ struct qcom_geni_serial_port {
bool brk;
};
-static const struct uart_ops qcom_geni_serial_pops;
+static const struct uart_ops qcom_geni_console_pops;
static struct uart_driver qcom_geni_console_driver;
static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop);
static unsigned int qcom_geni_serial_tx_empty(struct uart_port *port);
@@ -118,7 +118,14 @@ struct qcom_geni_serial_port {
#define to_dev_port(ptr, member) \
container_of(ptr, struct qcom_geni_serial_port, member)
-static struct qcom_geni_serial_port qcom_geni_console_port;
+static struct qcom_geni_serial_port qcom_geni_console_port = {
+ .uport = {
+ .iotype = UPIO_MEM,
+ .ops = &qcom_geni_console_pops,
+ .flags = UPF_BOOT_AUTOCONF,
+ .line = 0,
+ },
+};
static int qcom_geni_serial_request_port(struct uart_port *uport)
{
@@ -173,8 +180,12 @@ static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
unsigned int fifo_bits;
unsigned long timeout_us = 20000;
- /* Ensure polling is not re-ordered before the prior writes/reads */
- mb();
+ /*
+ * Ensure polling is not re-ordered before the prior writes/reads.
+ * Just invoke the write memory barrier here, since readl_poll*
+ * performs readl which invokes the read memory barrier.
+ */
+ wmb();
if (uport->private_data) {
port = to_dev_port(uport, uport);
@@ -286,6 +297,10 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch)
u32 bytes_to_send = count;
for (i = 0; i < count; i++) {
+ /*
+ * uart_console_write() adds a carriage return for each newline.
+ * Account for additional bytes to be written.
+ */
if (s[i] == '\n')
bytes_to_send++;
}
@@ -305,7 +320,7 @@ static void qcom_geni_serial_wr_char(struct uart_port *uport, int ch)
if (!qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
M_TX_FIFO_WATERMARK_EN, true))
break;
- chars_to_write = min_t(size_t, (size_t)(count - i), avail / 2);
+ chars_to_write = min_t(size_t, count - i, avail / 2);
uart_console_write(uport, s + i, chars_to_write,
qcom_geni_serial_wr_char);
writel_relaxed(M_TX_FIFO_WATERMARK_EN, uport->membase +
@@ -406,20 +421,18 @@ static void qcom_geni_serial_start_tx(struct uart_port *uport)
u32 status;
if (port->xfer_mode == GENI_SE_FIFO) {
- status = readl_relaxed(uport->membase + SE_GENI_STATUS);
+ /*
+ * readl ensures reading & writing of IRQ_EN register
+ * is not re-ordered before checking the status of the
+ * Serial Engine.
+ */
+ status = readl(uport->membase + SE_GENI_STATUS);
if (status & M_GENI_CMD_ACTIVE)
return;
if (!qcom_geni_serial_tx_empty(uport))
return;
- /*
- * Ensure writing to IRQ_EN & watermark registers are not
- * re-ordered before checking the status of the Serial
- * Engine and TX FIFO
- */
- mb();
-
irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;
@@ -582,11 +595,7 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport)
avail = (port->tx_fifo_depth - port->tx_wm) * port->tx_bytes_pw;
tail = (xmit->tail + port->xmit_size) & (UART_XMIT_SIZE - 1);
- if (chunk > (UART_XMIT_SIZE - tail))
- chunk = UART_XMIT_SIZE - tail;
- if (chunk > avail)
- chunk = avail;
-
+ chunk = min3((size_t)chunk, UART_XMIT_SIZE - tail, avail);
if (!chunk)
goto out_write_wakeup;
@@ -595,15 +604,15 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport)
remaining = chunk;
for (i = 0; i < chunk; ) {
unsigned int tx_bytes;
- unsigned int buf = 0;
+ u8 buf[sizeof(u32)];
int c;
+ memset(buf, 0, ARRAY_SIZE(buf));
tx_bytes = min_t(size_t, remaining, (size_t)port->tx_bytes_pw);
for (c = 0; c < tx_bytes ; c++)
- buf |= (xmit->buf[tail + c] << (c * BITS_PER_BYTE));
-
- writel_relaxed(buf, uport->membase + SE_GENI_TX_FIFOn);
+ buf[c] = xmit->buf[tail + c];
+ iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1);
i += tx_bytes;
tail = (tail + tx_bytes) & (UART_XMIT_SIZE - 1);
uport->icount.tx += tx_bytes;
@@ -627,7 +636,7 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
if (uport->suspended)
- return IRQ_HANDLED;
+ return IRQ_NONE;
spin_lock_irqsave(&uport->lock, flags);
m_irq_status = readl_relaxed(uport->membase + SE_GENI_M_IRQ_STATUS);
@@ -671,9 +680,6 @@ static int get_tx_fifo_size(struct qcom_geni_serial_port *port)
{
struct uart_port *uport;
- if (!port)
- return -ENODEV;
-
uport = &port->uport;
port->tx_fifo_depth = geni_se_get_tx_fifo_depth(&port->se);
port->tx_fifo_width = geni_se_get_tx_fifo_width(&port->se);
@@ -702,7 +708,6 @@ static void qcom_geni_serial_shutdown(struct uart_port *uport)
/* Stop the console before stopping the current tx */
console_stop(uport->cons);
- disable_irq(uport->irq);
free_irq(uport->irq, uport);
spin_lock_irqsave(&uport->lock, flags);
qcom_geni_serial_stop_tx(uport);
@@ -892,7 +897,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
{
- return !readl_relaxed(uport->membase + SE_GENI_TX_FIFO_STATUS);
+ return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
}
#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
@@ -910,7 +915,7 @@ static int __init qcom_geni_console_setup(struct console *co, char *options)
port = get_port_from_line(co->index);
if (IS_ERR(port)) {
- pr_err("Invalid line %d(%d)\n", co->index, (int)PTR_ERR(port));
+ pr_err("Invalid line %d\n", co->index);
return PTR_ERR(port);
}
@@ -1025,16 +1030,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
if (pdev->dev.of_node)
line = of_alias_get_id(pdev->dev.of_node, "serial");
- else
- line = pdev->id;
if (line < 0 || line >= GENI_UART_CONS_PORTS)
return -ENXIO;
port = get_port_from_line(line);
if (IS_ERR(port)) {
- ret = PTR_ERR(port);
- dev_err(&pdev->dev, "Invalid line %d(%d)\n", line, ret);
- return ret;
+ dev_err(&pdev->dev, "Invalid line %d\n", line);
+ return PTR_ERR(port);
}
uport = &port->uport;
@@ -1070,7 +1072,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
uport->private_data = &qcom_geni_console_driver;
platform_set_drvdata(pdev, port);
port->handle_rx = handle_rx_console;
- port->setup = false;
return uart_add_one_port(&qcom_geni_console_driver, uport);
}
@@ -1101,6 +1102,14 @@ static int __maybe_unused qcom_geni_serial_sys_resume_noirq(struct device *dev)
if (console_suspend_enabled && uport->suspended) {
uart_resume_port(uport->private_data, uport);
+ /*
+ * uart_suspend_port() invokes port shutdown which in turn
+ * frees the irq. uart_resume_port invokes port startup which
+ * performs request_irq. The request_irq auto-enables the IRQ.
+ * In addition, resume_noirq implicitly enables the IRQ and
+ * leads to an unbalanced IRQ enable warning. Disable the IRQ
+ * before returning so that the warning is suppressed.
+ */
disable_irq(uport->irq);
}
return 0;
@@ -1131,11 +1140,6 @@ static int __init qcom_geni_serial_init(void)
{
int ret;
- qcom_geni_console_port.uport.iotype = UPIO_MEM;
- qcom_geni_console_port.uport.ops = &qcom_geni_console_pops;
- qcom_geni_console_port.uport.flags = UPF_BOOT_AUTOCONF;
- qcom_geni_console_port.uport.line = 0;
-
ret = console_register(&qcom_geni_console_driver);
if (ret)
return ret;
--
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
