* [PATCH v3 01/13] tty: serial: qcom-geni-serial: drop unneeded forward definitions
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 02/13] tty: serial: qcom-geni-serial: remove unused symbols Bartosz Golaszewski
` (11 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
If we shuffle the code a bit, we can drop all forward definitions of
various static functions.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 79 +++++++++++++--------------
1 file changed, 37 insertions(+), 42 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 83b66b73303a..9f2212e7b5ec 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -147,11 +147,6 @@ static const struct uart_ops qcom_geni_console_pops;
static const struct uart_ops qcom_geni_uart_pops;
static struct uart_driver qcom_geni_console_driver;
static struct uart_driver qcom_geni_uart_driver;
-static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop);
-static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop);
-static unsigned int qcom_geni_serial_tx_empty(struct uart_port *port);
-static void qcom_geni_serial_stop_rx(struct uart_port *uport);
-static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop);
#define to_dev_port(ptr, member) \
container_of(ptr, struct qcom_geni_serial_port, member)
@@ -590,6 +585,11 @@ static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
return ret;
}
+static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
+{
+ return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
+}
+
static void qcom_geni_serial_start_tx(struct uart_port *uport)
{
u32 irq_en;
@@ -635,25 +635,29 @@ static void qcom_geni_serial_stop_tx(struct uart_port *uport)
writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
}
-static void qcom_geni_serial_start_rx(struct uart_port *uport)
+static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
{
- u32 irq_en;
u32 status;
+ u32 word_cnt;
+ u32 last_word_byte_cnt;
+ u32 last_word_partial;
+ u32 total_bytes;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- status = readl(uport->membase + SE_GENI_STATUS);
- if (status & S_GENI_CMD_ACTIVE)
- qcom_geni_serial_stop_rx(uport);
-
- geni_se_setup_s_cmd(&port->se, UART_START_READ, 0);
-
- irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
- irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
- writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+ status = readl(uport->membase + SE_GENI_RX_FIFO_STATUS);
+ word_cnt = status & RX_FIFO_WC_MSK;
+ last_word_partial = status & RX_LAST;
+ last_word_byte_cnt = (status & RX_LAST_BYTE_VALID_MSK) >>
+ RX_LAST_BYTE_VALID_SHFT;
- irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
- irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
- writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
+ if (!word_cnt)
+ return;
+ total_bytes = BYTES_PER_FIFO_WORD * (word_cnt - 1);
+ if (last_word_partial && last_word_byte_cnt)
+ total_bytes += last_word_byte_cnt;
+ else
+ total_bytes += BYTES_PER_FIFO_WORD;
+ port->handle_rx(uport, total_bytes, drop);
}
static void qcom_geni_serial_stop_rx(struct uart_port *uport)
@@ -694,29 +698,25 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
qcom_geni_serial_abort_rx(uport);
}
-static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
+static void qcom_geni_serial_start_rx(struct uart_port *uport)
{
+ u32 irq_en;
u32 status;
- u32 word_cnt;
- u32 last_word_byte_cnt;
- u32 last_word_partial;
- u32 total_bytes;
struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
- status = readl(uport->membase + SE_GENI_RX_FIFO_STATUS);
- word_cnt = status & RX_FIFO_WC_MSK;
- last_word_partial = status & RX_LAST;
- last_word_byte_cnt = (status & RX_LAST_BYTE_VALID_MSK) >>
- RX_LAST_BYTE_VALID_SHFT;
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (status & S_GENI_CMD_ACTIVE)
+ qcom_geni_serial_stop_rx(uport);
- if (!word_cnt)
- return;
- total_bytes = BYTES_PER_FIFO_WORD * (word_cnt - 1);
- if (last_word_partial && last_word_byte_cnt)
- total_bytes += last_word_byte_cnt;
- else
- total_bytes += BYTES_PER_FIFO_WORD;
- port->handle_rx(uport, total_bytes, drop);
+ geni_se_setup_s_cmd(&port->se, UART_START_READ, 0);
+
+ irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
+ irq_en |= S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN;
+ writel(irq_en, uport->membase + SE_GENI_S_IRQ_EN);
+
+ irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ irq_en |= M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN;
+ writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
@@ -1122,11 +1122,6 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
qcom_geni_serial_start_rx(uport);
}
-static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
-{
- return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
-}
-
#ifdef CONFIG_SERIAL_QCOM_GENI_CONSOLE
static int qcom_geni_console_setup(struct console *co, char *options)
{
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 02/13] tty: serial: qcom-geni-serial: remove unused symbols
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 01/13] tty: serial: qcom-geni-serial: drop unneeded forward definitions Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 03/13] tty: serial: qcom-geni-serial: align #define values Bartosz Golaszewski
` (10 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Drop all unused symbols from the driver.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 9f2212e7b5ec..7af5df6833c7 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -42,20 +42,11 @@
#define UART_TX_PAR_EN BIT(0)
#define UART_CTS_MASK BIT(1)
-/* SE_UART_TX_WORD_LEN */
-#define TX_WORD_LEN_MSK GENMASK(9, 0)
-
/* SE_UART_TX_STOP_BIT_LEN */
-#define TX_STOP_BIT_LEN_MSK GENMASK(23, 0)
#define TX_STOP_BIT_LEN_1 0
-#define TX_STOP_BIT_LEN_1_5 1
#define TX_STOP_BIT_LEN_2 2
-/* SE_UART_TX_TRANS_LEN */
-#define TX_TRANS_LEN_MSK GENMASK(23, 0)
-
/* SE_UART_RX_TRANS_CFG */
-#define UART_RX_INS_STATUS_BIT BIT(2)
#define UART_RX_PAR_EN BIT(3)
/* SE_UART_RX_WORD_LEN */
@@ -66,12 +57,9 @@
/* SE_UART_TX_PARITY_CFG/RX_PARITY_CFG */
#define PAR_CALC_EN BIT(0)
-#define PAR_MODE_MSK GENMASK(2, 1)
-#define PAR_MODE_SHFT 1
#define PAR_EVEN 0x00
#define PAR_ODD 0x01
#define PAR_SPACE 0x10
-#define PAR_MARK 0x11
/* SE_UART_MANUAL_RFR register fields */
#define UART_MANUAL_RFR_EN BIT(31)
@@ -80,11 +68,8 @@
/* UART M_CMD OP codes */
#define UART_START_TX 0x1
-#define UART_START_BREAK 0x4
-#define UART_STOP_BREAK 0x5
/* UART S_CMD OP codes */
#define UART_START_READ 0x1
-#define UART_PARAM 0x1
#define UART_OVERSAMPLING 32
#define STALE_TIMEOUT 16
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 03/13] tty: serial: qcom-geni-serial: align #define values
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 01/13] tty: serial: qcom-geni-serial: drop unneeded forward definitions Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 02/13] tty: serial: qcom-geni-serial: remove unused symbols Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 04/13] tty: serial: qcom-geni-serial: improve the to_dev_port() macro Bartosz Golaszewski
` (9 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Keep the #define symbols aligned for better readability.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 62 +++++++++++++--------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 7af5df6833c7..97ee7c074b79 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -39,57 +39,57 @@
#define SE_UART_MANUAL_RFR 0x2ac
/* SE_UART_TRANS_CFG */
-#define UART_TX_PAR_EN BIT(0)
-#define UART_CTS_MASK BIT(1)
+#define UART_TX_PAR_EN BIT(0)
+#define UART_CTS_MASK BIT(1)
/* SE_UART_TX_STOP_BIT_LEN */
-#define TX_STOP_BIT_LEN_1 0
-#define TX_STOP_BIT_LEN_2 2
+#define TX_STOP_BIT_LEN_1 0
+#define TX_STOP_BIT_LEN_2 2
/* SE_UART_RX_TRANS_CFG */
-#define UART_RX_PAR_EN BIT(3)
+#define UART_RX_PAR_EN BIT(3)
/* SE_UART_RX_WORD_LEN */
-#define RX_WORD_LEN_MASK GENMASK(9, 0)
+#define RX_WORD_LEN_MASK GENMASK(9, 0)
/* SE_UART_RX_STALE_CNT */
-#define RX_STALE_CNT GENMASK(23, 0)
+#define RX_STALE_CNT GENMASK(23, 0)
/* SE_UART_TX_PARITY_CFG/RX_PARITY_CFG */
-#define PAR_CALC_EN BIT(0)
-#define PAR_EVEN 0x00
-#define PAR_ODD 0x01
-#define PAR_SPACE 0x10
+#define PAR_CALC_EN BIT(0)
+#define PAR_EVEN 0x00
+#define PAR_ODD 0x01
+#define PAR_SPACE 0x10
/* SE_UART_MANUAL_RFR register fields */
-#define UART_MANUAL_RFR_EN BIT(31)
-#define UART_RFR_NOT_READY BIT(1)
-#define UART_RFR_READY BIT(0)
+#define UART_MANUAL_RFR_EN BIT(31)
+#define UART_RFR_NOT_READY BIT(1)
+#define UART_RFR_READY BIT(0)
/* UART M_CMD OP codes */
-#define UART_START_TX 0x1
+#define UART_START_TX 0x1
/* UART S_CMD OP codes */
-#define UART_START_READ 0x1
-
-#define UART_OVERSAMPLING 32
-#define STALE_TIMEOUT 16
-#define DEFAULT_BITS_PER_CHAR 10
-#define GENI_UART_CONS_PORTS 1
-#define GENI_UART_PORTS 3
-#define DEF_FIFO_DEPTH_WORDS 16
-#define DEF_TX_WM 2
-#define DEF_FIFO_WIDTH_BITS 32
-#define UART_RX_WM 2
+#define UART_START_READ 0x1
+
+#define UART_OVERSAMPLING 32
+#define STALE_TIMEOUT 16
+#define DEFAULT_BITS_PER_CHAR 10
+#define GENI_UART_CONS_PORTS 1
+#define GENI_UART_PORTS 3
+#define DEF_FIFO_DEPTH_WORDS 16
+#define DEF_TX_WM 2
+#define DEF_FIFO_WIDTH_BITS 32
+#define UART_RX_WM 2
/* SE_UART_LOOPBACK_CFG */
-#define RX_TX_SORTED BIT(0)
-#define CTS_RTS_SORTED BIT(1)
-#define RX_TX_CTS_RTS_SORTED (RX_TX_SORTED | CTS_RTS_SORTED)
+#define RX_TX_SORTED BIT(0)
+#define CTS_RTS_SORTED BIT(1)
+#define RX_TX_CTS_RTS_SORTED (RX_TX_SORTED | CTS_RTS_SORTED)
/* UART pin swap value */
-#define DEFAULT_IO_MACRO_IO0_IO1_MASK GENMASK(3, 0)
+#define DEFAULT_IO_MACRO_IO0_IO1_MASK GENMASK(3, 0)
#define IO_MACRO_IO0_SEL 0x3
-#define DEFAULT_IO_MACRO_IO2_IO3_MASK GENMASK(15, 4)
+#define DEFAULT_IO_MACRO_IO2_IO3_MASK GENMASK(15, 4)
#define IO_MACRO_IO2_IO3_SWAP 0x4640
/* We always configure 4 bytes per FIFO word */
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 04/13] tty: serial: qcom-geni-serial: improve the to_dev_port() macro
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (2 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 03/13] tty: serial: qcom-geni-serial: align #define values Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 05/13] tty: serial: qcom-geni-serial: remove stray newlines Bartosz Golaszewski
` (8 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The member we want to resolve in struct qcom_geni_serial_port is called
uport so we don't need an additional redundant parameter in this macro.
While at it: turn the macro into a static inline function.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 36 ++++++++++++++-------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 97ee7c074b79..e4139718e084 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -133,8 +133,10 @@ static const struct uart_ops qcom_geni_uart_pops;
static struct uart_driver qcom_geni_console_driver;
static struct uart_driver qcom_geni_uart_driver;
-#define to_dev_port(ptr, member) \
- container_of(ptr, struct qcom_geni_serial_port, member)
+static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
+{
+ return container_of(uport, struct qcom_geni_serial_port, uport);
+}
static struct qcom_geni_serial_port qcom_geni_uart_ports[GENI_UART_PORTS] = {
[0] = {
@@ -175,7 +177,7 @@ static struct qcom_geni_serial_port qcom_geni_console_port = {
static int qcom_geni_serial_request_port(struct uart_port *uport)
{
struct platform_device *pdev = to_platform_device(uport->dev);
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
uport->membase = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(uport->membase))
@@ -212,7 +214,7 @@ static void qcom_geni_serial_set_mctrl(struct uart_port *uport,
unsigned int mctrl)
{
u32 uart_manual_rfr = 0;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
if (uart_console(uport))
return;
@@ -253,7 +255,7 @@ static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
struct qcom_geni_private_data *private_data = uport->private_data;
if (private_data->drv) {
- port = to_dev_port(uport, uport);
+ port = to_dev_port(uport);
baud = port->baud;
if (!baud)
baud = 115200;
@@ -506,7 +508,7 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
u32 i;
unsigned char buf[sizeof(u32)];
struct tty_port *tport;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
tport = &uport->state->port;
for (i = 0; i < bytes; ) {
@@ -549,7 +551,7 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
{
struct tty_port *tport;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
u32 num_bytes_pw = port->tx_fifo_width / BITS_PER_BYTE;
u32 words = ALIGN(bytes, num_bytes_pw) / num_bytes_pw;
int ret;
@@ -598,7 +600,7 @@ static void qcom_geni_serial_stop_tx(struct uart_port *uport)
{
u32 irq_en;
u32 status;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
irq_en &= ~(M_CMD_DONE_EN | M_TX_FIFO_WATERMARK_EN);
@@ -627,7 +629,7 @@ static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
u32 last_word_byte_cnt;
u32 last_word_partial;
u32 total_bytes;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
status = readl(uport->membase + SE_GENI_RX_FIFO_STATUS);
word_cnt = status & RX_FIFO_WC_MSK;
@@ -649,7 +651,7 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
{
u32 irq_en;
u32 status;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
u32 s_irq_status;
irq_en = readl(uport->membase + SE_GENI_S_IRQ_EN);
@@ -687,7 +689,7 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
{
u32 irq_en;
u32 status;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
status = readl(uport->membase + SE_GENI_STATUS);
if (status & S_GENI_CMD_ACTIVE)
@@ -707,7 +709,7 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
bool active)
{
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
struct circ_buf *xmit = &uport->state->xmit;
size_t avail;
size_t remaining;
@@ -803,7 +805,7 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
struct uart_port *uport = dev;
bool drop_rx = false;
struct tty_port *tport = &uport->state->port;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
if (uport->suspended)
return IRQ_NONE;
@@ -869,7 +871,7 @@ static void qcom_geni_serial_shutdown(struct uart_port *uport)
static int qcom_geni_serial_port_setup(struct uart_port *uport)
{
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
u32 rxstale = DEFAULT_BITS_PER_CHAR * STALE_TIMEOUT;
u32 proto;
u32 pin_swap;
@@ -917,7 +919,7 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
static int qcom_geni_serial_startup(struct uart_port *uport)
{
int ret;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
if (!port->setup) {
ret = qcom_geni_serial_port_setup(uport);
@@ -1003,7 +1005,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport,
u32 stop_bit_len;
unsigned int clk_div;
u32 ser_clk_cfg;
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
unsigned long clk_rate;
u32 ver, sampling_rate;
unsigned int avg_bw_core;
@@ -1288,7 +1290,7 @@ static struct uart_driver qcom_geni_uart_driver = {
static void qcom_geni_serial_pm(struct uart_port *uport,
unsigned int new_state, unsigned int old_state)
{
- struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
/* If we've never been called, treat it as off */
if (old_state == UART_PM_STATE_UNDEFINED)
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 05/13] tty: serial: qcom-geni-serial: remove stray newlines
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (3 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 04/13] tty: serial: qcom-geni-serial: improve the to_dev_port() macro Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 06/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_isr() Bartosz Golaszewski
` (7 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Remove stray newlines around #ifdefs for consistency with the rest
of the driver code.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index e4139718e084..ec2523736e99 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -320,7 +320,6 @@ static void qcom_geni_serial_abort_rx(struct uart_port *uport)
}
#ifdef CONFIG_CONSOLE_POLL
-
static int qcom_geni_serial_get_char(struct uart_port *uport)
{
struct qcom_geni_private_data *private_data = uport->private_data;
@@ -545,7 +544,6 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
{
return -EPERM;
}
-
#endif /* CONFIG_SERIAL_QCOM_GENI_CONSOLE */
static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 06/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_isr()
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (4 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 05/13] tty: serial: qcom-geni-serial: remove stray newlines Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 07/13] tty: serial: qcom-geni-serial: remove unneeded tabs Bartosz Golaszewski
` (6 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Simplify the conditions in qcom_geni_serial_isr() and fix indentation.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/serial/qcom_geni_serial.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index ec2523736e99..fba02f71a874 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -827,20 +827,18 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
if (m_irq_status & m_irq_en & (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
qcom_geni_serial_handle_tx(uport, m_irq_status & M_CMD_DONE_EN,
- geni_status & M_GENI_CMD_ACTIVE);
+ geni_status & M_GENI_CMD_ACTIVE);
- if (s_irq_status & S_GP_IRQ_0_EN || s_irq_status & S_GP_IRQ_1_EN) {
+ if (s_irq_status & (S_GP_IRQ_0_EN | S_GP_IRQ_1_EN)) {
if (s_irq_status & S_GP_IRQ_0_EN)
uport->icount.parity++;
drop_rx = true;
- } else if (s_irq_status & S_GP_IRQ_2_EN ||
- s_irq_status & S_GP_IRQ_3_EN) {
+ } else if (s_irq_status & (S_GP_IRQ_2_EN | S_GP_IRQ_3_EN)) {
uport->icount.brk++;
port->brk = true;
}
- if (s_irq_status & S_RX_FIFO_WATERMARK_EN ||
- s_irq_status & S_RX_FIFO_LAST_EN)
+ if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN))
qcom_geni_serial_handle_rx(uport, drop_rx);
out_unlock:
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 07/13] tty: serial: qcom-geni-serial: remove unneeded tabs
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (5 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 06/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_isr() Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 08/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_handle_tx() Bartosz Golaszewski
` (5 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Remove redundant indentation in struct member assignment.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index fba02f71a874..68a1402fbe58 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -141,26 +141,26 @@ static inline struct qcom_geni_serial_port *to_dev_port(struct uart_port *uport)
static struct qcom_geni_serial_port qcom_geni_uart_ports[GENI_UART_PORTS] = {
[0] = {
.uport = {
- .iotype = UPIO_MEM,
- .ops = &qcom_geni_uart_pops,
- .flags = UPF_BOOT_AUTOCONF,
- .line = 0,
+ .iotype = UPIO_MEM,
+ .ops = &qcom_geni_uart_pops,
+ .flags = UPF_BOOT_AUTOCONF,
+ .line = 0,
},
},
[1] = {
.uport = {
- .iotype = UPIO_MEM,
- .ops = &qcom_geni_uart_pops,
- .flags = UPF_BOOT_AUTOCONF,
- .line = 1,
+ .iotype = UPIO_MEM,
+ .ops = &qcom_geni_uart_pops,
+ .flags = UPF_BOOT_AUTOCONF,
+ .line = 1,
},
},
[2] = {
.uport = {
- .iotype = UPIO_MEM,
- .ops = &qcom_geni_uart_pops,
- .flags = UPF_BOOT_AUTOCONF,
- .line = 2,
+ .iotype = UPIO_MEM,
+ .ops = &qcom_geni_uart_pops,
+ .flags = UPF_BOOT_AUTOCONF,
+ .line = 2,
},
},
};
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 08/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_handle_tx()
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (6 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 07/13] tty: serial: qcom-geni-serial: remove unneeded tabs Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-24 7:18 ` Jiri Slaby
2022-11-23 11:07 ` [PATCH v3 09/13] tty: serial: qcom-geni-serial: drop the return value from handle_rx Bartosz Golaszewski
` (4 subsequent siblings)
12 siblings, 1 reply; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
qcom_geni_serial_handle_tx() is pretty big, let's move the code that
handles the actual writing of data to a separate function which makes
sense in preparation for introducing a dma variant of handle_tx().
Let's also shuffle the code a bit, drop unneeded variables and use
uart_xmit_advance() instead of handling tail->xmit manually.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 54 +++++++++++++--------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 68a1402fbe58..658b6d596f58 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -704,19 +704,42 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
+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 circ_buf *xmit = &uport->state->xmit;
+ u8 buf[BYTES_PER_FIFO_WORD];
+ size_t remaining = chunk;
+ unsigned int tx_bytes;
+ int c;
+
+ while (remaining) {
+ memset(buf, 0, sizeof(buf));
+ tx_bytes = min_t(size_t, remaining, BYTES_PER_FIFO_WORD);
+
+ for (c = 0; c < tx_bytes ; c++) {
+ buf[c] = xmit->buf[xmit->tail];
+ uart_xmit_advance(uport, 1);
+ }
+
+ iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1);
+
+ remaining -= tx_bytes;
+ port->tx_remaining -= tx_bytes;
+ }
+}
+
static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
bool active)
{
struct qcom_geni_serial_port *port = to_dev_port(uport);
struct circ_buf *xmit = &uport->state->xmit;
size_t avail;
- size_t remaining;
size_t pending;
- int i;
u32 status;
u32 irq_en;
unsigned int chunk;
- int tail;
status = readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
@@ -735,7 +758,6 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
avail = port->tx_fifo_depth - (status & TX_FIFO_WC);
avail *= BYTES_PER_FIFO_WORD;
- tail = xmit->tail;
chunk = min(avail, pending);
if (!chunk)
goto out_write_wakeup;
@@ -750,29 +772,7 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
uport->membase + SE_GENI_M_IRQ_EN);
}
- remaining = chunk;
- for (i = 0; i < chunk; ) {
- unsigned int tx_bytes;
- u8 buf[sizeof(u32)];
- int c;
-
- memset(buf, 0, sizeof(buf));
- tx_bytes = min_t(size_t, remaining, BYTES_PER_FIFO_WORD);
-
- for (c = 0; c < tx_bytes ; c++) {
- buf[c] = xmit->buf[tail++];
- tail &= UART_XMIT_SIZE - 1;
- }
-
- iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1);
-
- i += tx_bytes;
- uport->icount.tx += tx_bytes;
- remaining -= tx_bytes;
- port->tx_remaining -= tx_bytes;
- }
-
- xmit->tail = tail;
+ qcom_geni_serial_send_chunk_fifo(uport, chunk);
/*
* The tx fifo watermark is level triggered and latched. Though we had
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 08/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_handle_tx()
2022-11-23 11:07 ` [PATCH v3 08/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_handle_tx() Bartosz Golaszewski
@ 2022-11-24 7:18 ` Jiri Slaby
2022-11-24 14:51 ` Bartosz Golaszewski
0 siblings, 1 reply; 19+ messages in thread
From: Jiri Slaby @ 2022-11-24 7:18 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Greg Kroah-Hartman, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski
On 23. 11. 22, 12:07, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> qcom_geni_serial_handle_tx() is pretty big, let's move the code that
> handles the actual writing of data to a separate function which makes
> sense in preparation for introducing a dma variant of handle_tx().
>
> Let's also shuffle the code a bit, drop unneeded variables and use
> uart_xmit_advance() instead of handling tail->xmit manually.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 54 +++++++++++++--------------
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 68a1402fbe58..658b6d596f58 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -704,19 +704,42 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
> writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> }
I know you just shuffle the code, but:
> +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 circ_buf *xmit = &uport->state->xmit;
> + u8 buf[BYTES_PER_FIFO_WORD];
> + size_t remaining = chunk;
Why size_t when the others are uints? Well, BYTES_PER_FIFO_WORD should
be defined as 4U.
> + unsigned int tx_bytes;
> + int c;
> +
> + while (remaining) {
> + memset(buf, 0, sizeof(buf));
> + tx_bytes = min_t(size_t, remaining, BYTES_PER_FIFO_WORD);
Then, no need for min_t.
> +
> + for (c = 0; c < tx_bytes ; c++) {
> + buf[c] = xmit->buf[xmit->tail];
> + uart_xmit_advance(uport, 1);
> + }
> +
> + iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1);
I wonder, why is _rep variant used to transfer a single word? Only to
hide the cast?
thanks,
--
js
suse labs
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 08/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_handle_tx()
2022-11-24 7:18 ` Jiri Slaby
@ 2022-11-24 14:51 ` Bartosz Golaszewski
0 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-24 14:51 UTC (permalink / raw)
To: Jiri Slaby
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Srinivas Kandagatla, Vinod Koul, Alex Elder, Ilpo Järvinen,
linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski
On Thu, Nov 24, 2022 at 8:18 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 23. 11. 22, 12:07, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > qcom_geni_serial_handle_tx() is pretty big, let's move the code that
> > handles the actual writing of data to a separate function which makes
> > sense in preparation for introducing a dma variant of handle_tx().
> >
> > Let's also shuffle the code a bit, drop unneeded variables and use
> > uart_xmit_advance() instead of handling tail->xmit manually.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> > drivers/tty/serial/qcom_geni_serial.c | 54 +++++++++++++--------------
> > 1 file changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> > index 68a1402fbe58..658b6d596f58 100644
> > --- a/drivers/tty/serial/qcom_geni_serial.c
> > +++ b/drivers/tty/serial/qcom_geni_serial.c
> > @@ -704,19 +704,42 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
> > writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> > }
>
> I know you just shuffle the code, but:
>
> > +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 circ_buf *xmit = &uport->state->xmit;
> > + u8 buf[BYTES_PER_FIFO_WORD];
> > + size_t remaining = chunk;
>
> Why size_t when the others are uints? Well, BYTES_PER_FIFO_WORD should
> be defined as 4U.
Good point.
>
> > + unsigned int tx_bytes;
> > + int c;
> > +
> > + while (remaining) {
> > + memset(buf, 0, sizeof(buf));
> > + tx_bytes = min_t(size_t, remaining, BYTES_PER_FIFO_WORD);
>
> Then, no need for min_t.
>
Same.
> > +
> > + for (c = 0; c < tx_bytes ; c++) {
> > + buf[c] = xmit->buf[xmit->tail];
> > + uart_xmit_advance(uport, 1);
> > + }
> > +
> > + iowrite32_rep(uport->membase + SE_GENI_TX_FIFOn, buf, 1);
>
> I wonder, why is _rep variant used to transfer a single word? Only to
> hide the cast?
>
Even if - using writel() with a cast doesn't seem to improve the
performance and this one looks prettier IMO.
Bartosz
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 09/13] tty: serial: qcom-geni-serial: drop the return value from handle_rx
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (7 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 08/13] tty: serial: qcom-geni-serial: refactor qcom_geni_serial_handle_tx() Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 10/13] tty: serial: qcom-geni-serial: use of_device_id data Bartosz Golaszewski
` (3 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The return value of the handle_rx() callback is never checked. Drop it.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 658b6d596f58..d5c343b06c23 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -114,7 +114,7 @@ struct qcom_geni_serial_port {
u32 tx_fifo_width;
u32 rx_fifo_depth;
bool setup;
- int (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
+ void (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
unsigned int baud;
void *rx_fifo;
u32 loopback;
@@ -502,7 +502,7 @@ static void qcom_geni_serial_console_write(struct console *co, const char *s,
spin_unlock_irqrestore(&uport->lock, flags);
}
-static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
+static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
{
u32 i;
unsigned char buf[sizeof(u32)];
@@ -537,16 +537,15 @@ static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
}
if (!drop)
tty_flip_buffer_push(tport);
- return 0;
}
#else
-static int handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
+static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
{
- return -EPERM;
+
}
#endif /* CONFIG_SERIAL_QCOM_GENI_CONSOLE */
-static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
+static void handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
{
struct tty_port *tport;
struct qcom_geni_serial_port *port = to_dev_port(uport);
@@ -557,7 +556,7 @@ static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
tport = &uport->state->port;
ioread32_rep(uport->membase + SE_GENI_RX_FIFOn, port->rx_fifo, words);
if (drop)
- return 0;
+ return;
ret = tty_insert_flip_string(tport, port->rx_fifo, bytes);
if (ret != bytes) {
@@ -567,7 +566,6 @@ static int handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
}
uport->icount.rx += ret;
tty_flip_buffer_push(tport);
- return ret;
}
static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 10/13] tty: serial: qcom-geni-serial: use of_device_id data
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (8 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 09/13] tty: serial: qcom-geni-serial: drop the return value from handle_rx Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 11/13] tty: serial: qcom-geni-serial: stop operations in progress at shutdown Bartosz Golaszewski
` (2 subsequent siblings)
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Instead of checking the device compatible in probe(), assign the
device-specific data to struct of_device_id. We'll use it later when
providing SE DMA support.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 46 ++++++++++++++++++++-------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index d5c343b06c23..036231106321 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -95,6 +95,11 @@
/* We always configure 4 bytes per FIFO word */
#define BYTES_PER_FIFO_WORD 4
+struct qcom_geni_device_data {
+ bool console;
+ void (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
+};
+
struct qcom_geni_private_data {
/* NOTE: earlycon port will have NULL here */
struct uart_driver *drv;
@@ -114,7 +119,6 @@ struct qcom_geni_serial_port {
u32 tx_fifo_width;
u32 rx_fifo_depth;
bool setup;
- void (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
unsigned int baud;
void *rx_fifo;
u32 loopback;
@@ -126,6 +130,7 @@ struct qcom_geni_serial_port {
bool cts_rts_swap;
struct qcom_geni_private_data private_data;
+ const struct qcom_geni_device_data *dev_data;
};
static const struct uart_ops qcom_geni_console_pops;
@@ -640,7 +645,7 @@ static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
total_bytes += last_word_byte_cnt;
else
total_bytes += BYTES_PER_FIFO_WORD;
- port->handle_rx(uport, total_bytes, drop);
+ port->dev_data->handle_rx(uport, total_bytes, drop);
}
static void qcom_geni_serial_stop_rx(struct uart_port *uport)
@@ -1345,13 +1350,14 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
struct uart_port *uport;
struct resource *res;
int irq;
- bool console = false;
struct uart_driver *drv;
+ const struct qcom_geni_device_data *data;
- if (of_device_is_compatible(pdev->dev.of_node, "qcom,geni-debug-uart"))
- console = true;
+ data = of_device_get_match_data(&pdev->dev);
+ if (!data)
+ return -EINVAL;
- if (console) {
+ if (data->console) {
drv = &qcom_geni_console_driver;
line = of_alias_get_id(pdev->dev.of_node, "serial");
} else {
@@ -1361,7 +1367,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
line = of_alias_get_id(pdev->dev.of_node, "hsuart");
}
- port = get_port_from_line(line, console);
+ port = get_port_from_line(line, data->console);
if (IS_ERR(port)) {
dev_err(&pdev->dev, "Invalid line %d\n", line);
return PTR_ERR(port);
@@ -1373,6 +1379,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
return -ENODEV;
uport->dev = &pdev->dev;
+ port->dev_data = data;
port->se.dev = &pdev->dev;
port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
port->se.clk = devm_clk_get(&pdev->dev, "se");
@@ -1391,7 +1398,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
port->rx_fifo_depth = DEF_FIFO_DEPTH_WORDS;
port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
- if (!console) {
+ if (!data->console) {
port->rx_fifo = devm_kcalloc(uport->dev,
port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
if (!port->rx_fifo)
@@ -1421,7 +1428,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
uport->irq = irq;
uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
- if (!console)
+ if (!data->console)
port->wakeup_irq = platform_get_irq_optional(pdev, 1);
if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap"))
@@ -1443,7 +1450,6 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
port->private_data.drv = drv;
uport->private_data = &port->private_data;
platform_set_drvdata(pdev, port);
- port->handle_rx = console ? handle_rx_console : handle_rx_uart;
ret = uart_add_one_port(drv, uport);
if (ret)
@@ -1523,14 +1529,30 @@ static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
return ret;
}
+static const struct qcom_geni_device_data qcom_geni_console_data = {
+ .console = true,
+ .handle_rx = handle_rx_console,
+};
+
+static const struct qcom_geni_device_data qcom_geni_uart_data = {
+ .console = false,
+ .handle_rx = handle_rx_uart,
+};
+
static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(qcom_geni_serial_sys_suspend,
qcom_geni_serial_sys_resume)
};
static const struct of_device_id qcom_geni_serial_match_table[] = {
- { .compatible = "qcom,geni-debug-uart", },
- { .compatible = "qcom,geni-uart", },
+ {
+ .compatible = "qcom,geni-debug-uart",
+ .data = &qcom_geni_console_data,
+ },
+ {
+ .compatible = "qcom,geni-uart",
+ .data = &qcom_geni_uart_data,
+ },
{}
};
MODULE_DEVICE_TABLE(of, qcom_geni_serial_match_table);
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 11/13] tty: serial: qcom-geni-serial: stop operations in progress at shutdown
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (9 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 10/13] tty: serial: qcom-geni-serial: use of_device_id data Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 12/13] soc: qcom-geni-se: add more symbol definitions Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA Bartosz Golaszewski
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We don't stop transmissions in progress at shutdown. This is fine with
FIFO SE mode but with DMA it causes trouble so fix it now.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 036231106321..82242a40a95a 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -865,6 +865,9 @@ static void get_tx_fifo_size(struct qcom_geni_serial_port *port)
static void qcom_geni_serial_shutdown(struct uart_port *uport)
{
+ qcom_geni_serial_stop_tx(uport);
+ qcom_geni_serial_stop_rx(uport);
+
disable_irq(uport->irq);
}
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 12/13] soc: qcom-geni-se: add more symbol definitions
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (10 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 11/13] tty: serial: qcom-geni-serial: stop operations in progress at shutdown Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-23 11:07 ` [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA Bartosz Golaszewski
12 siblings, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski,
Konrad Dybcio
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The following symbols will be used when adding support for SE DMA in
the qcom geni serial driver.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
include/linux/qcom-geni-se.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h
index f5672785c0c4..400213daa461 100644
--- a/include/linux/qcom-geni-se.h
+++ b/include/linux/qcom-geni-se.h
@@ -103,6 +103,7 @@ struct geni_se {
#define SE_DMA_TX_FSM_RST 0xc58
#define SE_DMA_RX_IRQ_STAT 0xd40
#define SE_DMA_RX_IRQ_CLR 0xd44
+#define SE_DMA_RX_LEN_IN 0xd54
#define SE_DMA_RX_FSM_RST 0xd58
#define SE_HW_PARAM_0 0xe24
#define SE_HW_PARAM_1 0xe28
@@ -235,6 +236,8 @@ struct geni_se {
#define RX_SBE BIT(2)
#define RX_RESET_DONE BIT(3)
#define RX_FLUSH_DONE BIT(4)
+#define RX_DMA_PARITY_ERR BIT(5)
+#define RX_DMA_BREAK GENMASK(8, 7)
#define RX_GENI_GP_IRQ GENMASK(10, 5)
#define RX_GENI_CANCEL_IRQ BIT(11)
#define RX_GENI_GP_IRQ_EXT GENMASK(13, 12)
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA
2022-11-23 11:07 [PATCH v3 00/13] serial: qcom-geni-serial: implement support for SE DMA Bartosz Golaszewski
` (11 preceding siblings ...)
2022-11-23 11:07 ` [PATCH v3 12/13] soc: qcom-geni-se: add more symbol definitions Bartosz Golaszewski
@ 2022-11-23 11:07 ` Bartosz Golaszewski
2022-11-25 14:37 ` Srinivas Kandagatla
12 siblings, 1 reply; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-23 11:07 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Srinivas Kandagatla, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The qcom-geni-serial driver currently only works in SE FIFO mode. This
limits the UART speed to around 180 kB/s. In order to achieve higher
speeds we need to use SE DMA mode.
Keep the console port working in FIFO mode but extend the code to use DMA
for the high-speed port.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/tty/serial/qcom_geni_serial.c | 289 ++++++++++++++++++++++----
1 file changed, 247 insertions(+), 42 deletions(-)
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 82242a40a95a..0f4ba909c792 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -70,6 +70,8 @@
#define UART_START_TX 0x1
/* UART S_CMD OP codes */
#define UART_START_READ 0x1
+#define UART_PARAM 0x1
+#define UART_PARAM_RFR_OPEN BIT(7)
#define UART_OVERSAMPLING 32
#define STALE_TIMEOUT 16
@@ -95,9 +97,11 @@
/* We always configure 4 bytes per FIFO word */
#define BYTES_PER_FIFO_WORD 4
+#define DMA_RX_BUF_SIZE 2048
+
struct qcom_geni_device_data {
bool console;
- void (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
+ enum geni_se_xfer_mode mode;
};
struct qcom_geni_private_data {
@@ -118,9 +122,11 @@ struct qcom_geni_serial_port {
u32 tx_fifo_depth;
u32 tx_fifo_width;
u32 rx_fifo_depth;
+ dma_addr_t tx_dma_addr;
+ dma_addr_t rx_dma_addr;
bool setup;
unsigned int baud;
- void *rx_fifo;
+ void *rx_buf;
u32 loopback;
bool brk;
@@ -552,18 +558,11 @@ static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
static void handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
{
- struct tty_port *tport;
struct qcom_geni_serial_port *port = to_dev_port(uport);
- u32 num_bytes_pw = port->tx_fifo_width / BITS_PER_BYTE;
- u32 words = ALIGN(bytes, num_bytes_pw) / num_bytes_pw;
+ struct tty_port *tport = &uport->state->port;
int ret;
- tport = &uport->state->port;
- ioread32_rep(uport->membase + SE_GENI_RX_FIFOn, port->rx_fifo, words);
- if (drop)
- return;
-
- ret = tty_insert_flip_string(tport, port->rx_fifo, bytes);
+ ret = tty_insert_flip_string(tport, port->rx_buf, bytes);
if (ret != bytes) {
dev_err(uport->dev, "%s:Unable to push data ret %d_bytes %d\n",
__func__, ret, bytes);
@@ -578,7 +577,70 @@ static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
}
-static void qcom_geni_serial_start_tx(struct uart_port *uport)
+static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport)
+{
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
+ bool done;
+ u32 status;
+ u32 m_irq_en;
+
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (!(status & M_GENI_CMD_ACTIVE))
+ return;
+
+ if (port->rx_dma_addr) {
+ geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr,
+ port->tx_remaining);
+ port->tx_dma_addr = (dma_addr_t)NULL;
+ port->tx_remaining = 0;
+ }
+
+ m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
+ writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
+ geni_se_cancel_m_cmd(&port->se);
+
+ done = qcom_geni_serial_poll_bit(uport, SE_GENI_S_IRQ_STATUS,
+ S_CMD_CANCEL_EN, true);
+ if (!done) {
+ geni_se_abort_m_cmd(&port->se);
+ qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
+ M_CMD_ABORT_EN, true);
+ writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
+ }
+
+ writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
+}
+
+static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
+{
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
+ struct circ_buf *xmit = &uport->state->xmit;
+ unsigned int xmit_size;
+ int ret;
+
+ if (port->tx_dma_addr)
+ return;
+
+ xmit_size = uart_circ_chars_pending(xmit);
+ if (xmit_size < WAKEUP_CHARS)
+ uart_write_wakeup(uport);
+
+ xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
+
+ qcom_geni_serial_setup_tx(uport, xmit_size);
+
+ ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
+ xmit_size, &port->tx_dma_addr);
+ if (ret) {
+ dev_err(uport->dev, "unable to start TX SE DMA: %d\n", ret);
+ qcom_geni_serial_stop_tx_dma(uport);
+ return;
+ }
+
+ port->tx_remaining = xmit_size;
+}
+
+static void qcom_geni_serial_start_tx_fifo(struct uart_port *uport)
{
u32 irq_en;
u32 status;
@@ -597,7 +659,7 @@ static void qcom_geni_serial_start_tx(struct uart_port *uport)
writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
-static void qcom_geni_serial_stop_tx(struct uart_port *uport)
+static void qcom_geni_serial_stop_tx_fifo(struct uart_port *uport)
{
u32 irq_en;
u32 status;
@@ -623,14 +685,13 @@ static void qcom_geni_serial_stop_tx(struct uart_port *uport)
writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
}
-static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
+static void qcom_geni_serial_handle_rx_fifo(struct uart_port *uport, bool drop)
{
u32 status;
u32 word_cnt;
u32 last_word_byte_cnt;
u32 last_word_partial;
u32 total_bytes;
- struct qcom_geni_serial_port *port = to_dev_port(uport);
status = readl(uport->membase + SE_GENI_RX_FIFO_STATUS);
word_cnt = status & RX_FIFO_WC_MSK;
@@ -645,10 +706,10 @@ static void qcom_geni_serial_handle_rx(struct uart_port *uport, bool drop)
total_bytes += last_word_byte_cnt;
else
total_bytes += BYTES_PER_FIFO_WORD;
- port->dev_data->handle_rx(uport, total_bytes, drop);
+ handle_rx_console(uport, total_bytes, drop);
}
-static void qcom_geni_serial_stop_rx(struct uart_port *uport)
+static void qcom_geni_serial_stop_rx_fifo(struct uart_port *uport)
{
u32 irq_en;
u32 status;
@@ -678,7 +739,7 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
/* Flush the Rx buffer */
if (s_irq_status & S_RX_FIFO_LAST_EN)
- qcom_geni_serial_handle_rx(uport, true);
+ qcom_geni_serial_handle_rx_fifo(uport, true);
writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
status = readl(uport->membase + SE_GENI_STATUS);
@@ -686,7 +747,7 @@ static void qcom_geni_serial_stop_rx(struct uart_port *uport)
qcom_geni_serial_abort_rx(uport);
}
-static void qcom_geni_serial_start_rx(struct uart_port *uport)
+static void qcom_geni_serial_start_rx_fifo(struct uart_port *uport)
{
u32 irq_en;
u32 status;
@@ -694,7 +755,7 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
status = readl(uport->membase + SE_GENI_STATUS);
if (status & S_GENI_CMD_ACTIVE)
- qcom_geni_serial_stop_rx(uport);
+ qcom_geni_serial_stop_rx_fifo(uport);
geni_se_setup_s_cmd(&port->se, UART_START_READ, 0);
@@ -707,6 +768,101 @@ static void qcom_geni_serial_start_rx(struct uart_port *uport)
writel(irq_en, uport->membase + SE_GENI_M_IRQ_EN);
}
+static void qcom_geni_serial_stop_rx_dma(struct uart_port *uport)
+{
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
+ u32 status;
+
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (!(status & S_GENI_CMD_ACTIVE))
+ return;
+
+ geni_se_cancel_s_cmd(&port->se);
+ qcom_geni_serial_poll_bit(uport, SE_GENI_S_IRQ_STATUS,
+ S_CMD_CANCEL_EN, true);
+
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (status & S_GENI_CMD_ACTIVE)
+ qcom_geni_serial_abort_rx(uport);
+
+ if (port->rx_dma_addr) {
+ geni_se_rx_dma_unprep(&port->se, port->rx_dma_addr,
+ DMA_RX_BUF_SIZE);
+ port->rx_dma_addr = (dma_addr_t)NULL;
+ }
+}
+
+static void qcom_geni_serial_start_rx_dma(struct uart_port *uport)
+{
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
+ u32 status;
+ int ret;
+
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (status & S_GENI_CMD_ACTIVE)
+ qcom_geni_serial_stop_rx_dma(uport);
+
+ geni_se_setup_s_cmd(&port->se, UART_START_READ, UART_PARAM_RFR_OPEN);
+
+ ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
+ DMA_RX_BUF_SIZE,
+ &port->rx_dma_addr);
+ if (ret) {
+ dev_err(uport->dev, "unable to start RX SE DMA: %d\n", ret);
+ qcom_geni_serial_stop_rx_dma(uport);
+ }
+}
+
+static void qcom_geni_serial_handle_rx_dma(struct uart_port *uport, bool drop)
+{
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
+ u32 status;
+ u32 rx_in;
+ int ret;
+
+ status = readl(uport->membase + SE_GENI_STATUS);
+ if (!(status & S_GENI_CMD_ACTIVE))
+ return;
+
+ if (!port->rx_dma_addr)
+ return;
+
+ geni_se_rx_dma_unprep(&port->se, port->rx_dma_addr, DMA_RX_BUF_SIZE);
+ port->rx_dma_addr = (dma_addr_t)NULL;
+
+ rx_in = readl(uport->membase + SE_DMA_RX_LEN_IN);
+ if (!rx_in) {
+ dev_warn(uport->dev, "serial engine reports 0 RX bytes in!\n");
+ return;
+ }
+
+ if (!drop)
+ handle_rx_uart(uport, rx_in, drop);
+
+ ret = geni_se_rx_dma_prep(&port->se, port->rx_buf,
+ DMA_RX_BUF_SIZE,
+ &port->rx_dma_addr);
+ if (ret) {
+ dev_err(uport->dev, "unable to start RX SE DMA: %d\n", ret);
+ qcom_geni_serial_stop_rx_dma(uport);
+ }
+}
+
+static void qcom_geni_serial_start_rx(struct uart_port *uport)
+{
+ uport->ops->start_rx(uport);
+}
+
+static void qcom_geni_serial_stop_rx(struct uart_port *uport)
+{
+ uport->ops->stop_rx(uport);
+}
+
+static void qcom_geni_serial_stop_tx(struct uart_port *uport)
+{
+ uport->ops->stop_tx(uport);
+}
+
static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport,
unsigned int chunk)
{
@@ -733,8 +889,8 @@ static void qcom_geni_serial_send_chunk_fifo(struct uart_port *uport,
}
}
-static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
- bool active)
+static void qcom_geni_serial_handle_tx_fifo(struct uart_port *uport,
+ bool done, bool active)
{
struct qcom_geni_serial_port *port = to_dev_port(uport);
struct circ_buf *xmit = &uport->state->xmit;
@@ -754,7 +910,7 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
/* All data has been transmitted and acknowledged as received */
if (!pending && !status && done) {
- qcom_geni_serial_stop_tx(uport);
+ qcom_geni_serial_stop_tx_fifo(uport);
goto out_write_wakeup;
}
@@ -797,12 +953,32 @@ static void qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
uart_write_wakeup(uport);
}
+static void qcom_geni_serial_handle_tx_dma(struct uart_port *uport)
+{
+ struct qcom_geni_serial_port *port = to_dev_port(uport);
+ struct circ_buf *xmit = &uport->state->xmit;
+
+ uart_xmit_advance(uport, port->tx_remaining);
+ geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr, port->tx_remaining);
+ port->tx_dma_addr = (dma_addr_t)NULL;
+ port->tx_remaining = 0;
+
+ if (!uart_circ_empty(xmit))
+ qcom_geni_serial_start_tx_dma(uport);
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(uport);
+}
+
static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
{
u32 m_irq_en;
u32 m_irq_status;
u32 s_irq_status;
u32 geni_status;
+ u32 dma;
+ u32 dma_tx_status;
+ u32 dma_rx_status;
struct uart_port *uport = dev;
bool drop_rx = false;
struct tty_port *tport = &uport->state->port;
@@ -815,10 +991,15 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
m_irq_status = readl(uport->membase + SE_GENI_M_IRQ_STATUS);
s_irq_status = readl(uport->membase + SE_GENI_S_IRQ_STATUS);
+ dma_tx_status = readl(uport->membase + SE_DMA_TX_IRQ_STAT);
+ dma_rx_status = readl(uport->membase + SE_DMA_RX_IRQ_STAT);
geni_status = readl(uport->membase + SE_GENI_STATUS);
+ dma = readl(uport->membase + SE_GENI_DMA_MODE_EN);
m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
writel(m_irq_status, uport->membase + SE_GENI_M_IRQ_CLEAR);
writel(s_irq_status, uport->membase + SE_GENI_S_IRQ_CLEAR);
+ writel(dma_tx_status, uport->membase + SE_DMA_TX_IRQ_CLR);
+ writel(dma_rx_status, uport->membase + SE_DMA_RX_IRQ_CLR);
if (WARN_ON(m_irq_status & M_ILLEGAL_CMD_EN))
goto out_unlock;
@@ -828,10 +1009,6 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
tty_insert_flip_char(tport, 0, TTY_OVERRUN);
}
- if (m_irq_status & m_irq_en & (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
- qcom_geni_serial_handle_tx(uport, m_irq_status & M_CMD_DONE_EN,
- geni_status & M_GENI_CMD_ACTIVE);
-
if (s_irq_status & (S_GP_IRQ_0_EN | S_GP_IRQ_1_EN)) {
if (s_irq_status & S_GP_IRQ_0_EN)
uport->icount.parity++;
@@ -841,8 +1018,35 @@ static irqreturn_t qcom_geni_serial_isr(int isr, void *dev)
port->brk = true;
}
- if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN))
- qcom_geni_serial_handle_rx(uport, drop_rx);
+ if (dma) {
+ if (dma_tx_status & TX_DMA_DONE)
+ qcom_geni_serial_handle_tx_dma(uport);
+
+ if (dma_rx_status) {
+ if (dma_rx_status & RX_RESET_DONE)
+ goto out_unlock;
+
+ if (dma_rx_status & RX_DMA_PARITY_ERR) {
+ uport->icount.parity++;
+ drop_rx = true;
+ }
+
+ if (dma_rx_status & RX_DMA_BREAK)
+ uport->icount.brk++;
+
+ if (dma_rx_status & (RX_DMA_DONE | RX_EOT))
+ qcom_geni_serial_handle_rx_dma(uport, drop_rx);
+ }
+ } else {
+ if (m_irq_status & m_irq_en &
+ (M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN))
+ qcom_geni_serial_handle_tx_fifo(uport,
+ m_irq_status & M_CMD_DONE_EN,
+ geni_status & M_GENI_CMD_ACTIVE);
+
+ if (s_irq_status & (S_RX_FIFO_WATERMARK_EN | S_RX_FIFO_LAST_EN))
+ qcom_geni_serial_handle_rx_fifo(uport, drop_rx);
+ }
out_unlock:
uart_unlock_and_check_sysrq(uport);
@@ -912,7 +1116,7 @@ static int qcom_geni_serial_port_setup(struct uart_port *uport)
geni_se_config_packing(&port->se, BITS_PER_BYTE, BYTES_PER_FIFO_WORD,
false, true, true);
geni_se_init(&port->se, UART_RX_WM, port->rx_fifo_depth - 2);
- geni_se_select_mode(&port->se, GENI_SE_FIFO);
+ geni_se_select_mode(&port->se, port->dev_data->mode);
port->setup = true;
return 0;
@@ -1310,10 +1514,10 @@ static void qcom_geni_serial_pm(struct uart_port *uport,
static const struct uart_ops qcom_geni_console_pops = {
.tx_empty = qcom_geni_serial_tx_empty,
- .stop_tx = qcom_geni_serial_stop_tx,
- .start_tx = qcom_geni_serial_start_tx,
- .stop_rx = qcom_geni_serial_stop_rx,
- .start_rx = qcom_geni_serial_start_rx,
+ .stop_tx = qcom_geni_serial_stop_tx_fifo,
+ .start_tx = qcom_geni_serial_start_tx_fifo,
+ .stop_rx = qcom_geni_serial_stop_rx_fifo,
+ .start_rx = qcom_geni_serial_start_rx_fifo,
.set_termios = qcom_geni_serial_set_termios,
.startup = qcom_geni_serial_startup,
.request_port = qcom_geni_serial_request_port,
@@ -1331,9 +1535,10 @@ static const struct uart_ops qcom_geni_console_pops = {
static const struct uart_ops qcom_geni_uart_pops = {
.tx_empty = qcom_geni_serial_tx_empty,
- .stop_tx = qcom_geni_serial_stop_tx,
- .start_tx = qcom_geni_serial_start_tx,
- .stop_rx = qcom_geni_serial_stop_rx,
+ .stop_tx = qcom_geni_serial_stop_tx_dma,
+ .start_tx = qcom_geni_serial_start_tx_dma,
+ .start_rx = qcom_geni_serial_start_rx_dma,
+ .stop_rx = qcom_geni_serial_stop_rx_dma,
.set_termios = qcom_geni_serial_set_termios,
.startup = qcom_geni_serial_startup,
.request_port = qcom_geni_serial_request_port,
@@ -1402,9 +1607,9 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
port->tx_fifo_width = DEF_FIFO_WIDTH_BITS;
if (!data->console) {
- port->rx_fifo = devm_kcalloc(uport->dev,
- port->rx_fifo_depth, sizeof(u32), GFP_KERNEL);
- if (!port->rx_fifo)
+ port->rx_buf = devm_kzalloc(uport->dev,
+ DMA_RX_BUF_SIZE, GFP_KERNEL);
+ if (!port->rx_buf)
return -ENOMEM;
}
@@ -1534,12 +1739,12 @@ static int __maybe_unused qcom_geni_serial_sys_resume(struct device *dev)
static const struct qcom_geni_device_data qcom_geni_console_data = {
.console = true,
- .handle_rx = handle_rx_console,
+ .mode = GENI_SE_FIFO,
};
static const struct qcom_geni_device_data qcom_geni_uart_data = {
.console = false,
- .handle_rx = handle_rx_uart,
+ .mode = GENI_SE_DMA,
};
static const struct dev_pm_ops qcom_geni_serial_pm_ops = {
--
2.37.2
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA
2022-11-23 11:07 ` [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA Bartosz Golaszewski
@ 2022-11-25 14:37 ` Srinivas Kandagatla
2022-11-28 11:03 ` Bartosz Golaszewski
2022-11-29 10:13 ` Bartosz Golaszewski
0 siblings, 2 replies; 19+ messages in thread
From: Srinivas Kandagatla @ 2022-11-25 14:37 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Greg Kroah-Hartman, Jiri Slaby, Vinod Koul, Alex Elder,
Ilpo Järvinen
Cc: linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski
Thanks Bartosz for the patch,
On 23/11/2022 11:07, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> The qcom-geni-serial driver currently only works in SE FIFO mode. This
> limits the UART speed to around 180 kB/s. In order to achieve higher
> speeds we need to use SE DMA mode.
>
> Keep the console port working in FIFO mode but extend the code to use DMA
> for the high-speed port.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> drivers/tty/serial/qcom_geni_serial.c | 289 ++++++++++++++++++++++----
> 1 file changed, 247 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 82242a40a95a..0f4ba909c792 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -70,6 +70,8 @@
> #define UART_START_TX 0x1
> /* UART S_CMD OP codes */
> #define UART_START_READ 0x1
> +#define UART_PARAM 0x1
> +#define UART_PARAM_RFR_OPEN BIT(7)
>
> #define UART_OVERSAMPLING 32
> #define STALE_TIMEOUT 16
> @@ -95,9 +97,11 @@
> /* We always configure 4 bytes per FIFO word */
> #define BYTES_PER_FIFO_WORD 4
>
> +#define DMA_RX_BUF_SIZE 2048
> +
> struct qcom_geni_device_data {
> bool console;
> - void (*handle_rx)(struct uart_port *uport, u32 bytes, bool drop);
> + enum geni_se_xfer_mode mode;
> };
>
> struct qcom_geni_private_data {
> @@ -118,9 +122,11 @@ struct qcom_geni_serial_port {
> u32 tx_fifo_depth;
> u32 tx_fifo_width;
> u32 rx_fifo_depth;
> + dma_addr_t tx_dma_addr;
> + dma_addr_t rx_dma_addr;
> bool setup;
> unsigned int baud;
> - void *rx_fifo;
> + void *rx_buf;
> u32 loopback;
> bool brk;
>
> @@ -552,18 +558,11 @@ static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
>
> static void handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
> {
> - struct tty_port *tport;
> struct qcom_geni_serial_port *port = to_dev_port(uport);
> - u32 num_bytes_pw = port->tx_fifo_width / BITS_PER_BYTE;
> - u32 words = ALIGN(bytes, num_bytes_pw) / num_bytes_pw;
> + struct tty_port *tport = &uport->state->port;
> int ret;
>
> - tport = &uport->state->port;
> - ioread32_rep(uport->membase + SE_GENI_RX_FIFOn, port->rx_fifo, words);
> - if (drop)
> - return;
> -
Are we removing FIFO support for uart?
It it not a overhead to use dma for small transfers that are less than
fifo size?
> - ret = tty_insert_flip_string(tport, port->rx_fifo, bytes);
> + ret = tty_insert_flip_string(tport, port->rx_buf, bytes);
> if (ret != bytes) {
> dev_err(uport->dev, "%s:Unable to push data ret %d_bytes %d\n",
> __func__, ret, bytes);
> @@ -578,7 +577,70 @@ static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
> return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
> }
>
> -static void qcom_geni_serial_start_tx(struct uart_port *uport)
> +static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport)
> +{
> + struct qcom_geni_serial_port *port = to_dev_port(uport);
> + bool done;
-->
> + u32 status;
...
> +
> + status = readl(uport->membase + SE_GENI_STATUS);
> + if (!(status & M_GENI_CMD_ACTIVE))
> + return;
<---
this code snippet is repeating more than few times in the patches, looks
like it could be made to a inline helper.
> +
> + if (port->rx_dma_addr) {
> + geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr,
> + port->tx_remaining);
> + port->tx_dma_addr = (dma_addr_t)NULL;
> + port->tx_remaining = 0;
> + }
> +
> + m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> + writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> + geni_se_cancel_m_cmd(&port->se);
> +
> + done = qcom_geni_serial_poll_bit(uport, SE_GENI_S_IRQ_STATUS,
> + S_CMD_CANCEL_EN, true);
> + if (!done) {
> + geni_se_abort_m_cmd(&port->se);
> + qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> + M_CMD_ABORT_EN, true);
return is not checked, there are few more such instances in the patch.
> + writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
> + }
> +
> + writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
> +}
> +
> +static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
> +{
> + struct qcom_geni_serial_port *port = to_dev_port(uport);
> + struct circ_buf *xmit = &uport->state->xmit;
> + unsigned int xmit_size;
> + int ret;
> +
> + if (port->tx_dma_addr)
> + return;
Is this condition actually possible?
> +
> + xmit_size = uart_circ_chars_pending(xmit);
> + if (xmit_size < WAKEUP_CHARS)
> + uart_write_wakeup(uport);
> +
> + xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
> +
> + qcom_geni_serial_setup_tx(uport, xmit_size);
> +
> + ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
> + xmit_size, &port->tx_dma_addr);
> + if (ret) {
> + dev_err(uport->dev, "unable to start TX SE DMA: %d\n", ret);
> + qcom_geni_serial_stop_tx_dma(uport);
> + return;
> + }
> +
> + port->tx_remaining = xmit_size;
> +}
> +
...
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA
2022-11-25 14:37 ` Srinivas Kandagatla
@ 2022-11-28 11:03 ` Bartosz Golaszewski
2022-11-29 10:13 ` Bartosz Golaszewski
1 sibling, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-28 11:03 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Bartosz Golaszewski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Greg Kroah-Hartman, Jiri Slaby, Vinod Koul, Alex Elder,
Ilpo Järvinen, linux-kernel, linux-arm-msm, linux-serial
On Fri, 25 Nov 2022 at 15:37, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
<snip>
> >
> > @@ -552,18 +558,11 @@ static void handle_rx_console(struct uart_port *uport, u32 bytes, bool drop)
> >
> > static void handle_rx_uart(struct uart_port *uport, u32 bytes, bool drop)
> > {
> > - struct tty_port *tport;
> > struct qcom_geni_serial_port *port = to_dev_port(uport);
> > - u32 num_bytes_pw = port->tx_fifo_width / BITS_PER_BYTE;
> > - u32 words = ALIGN(bytes, num_bytes_pw) / num_bytes_pw;
> > + struct tty_port *tport = &uport->state->port;
> > int ret;
> >
> > - tport = &uport->state->port;
> > - ioread32_rep(uport->membase + SE_GENI_RX_FIFOn, port->rx_fifo, words);
> > - if (drop)
> > - return;
> > -
>
> Are we removing FIFO support for uart?
>
> It it not a overhead to use dma for small transfers that are less than
> fifo size?
>
You made me think about it but no - while I haven't measured it yet, I
don't think it's worth the code complexity behind it. The i2c driver
doesn't do it this way for short transfers either. If you insist I can
test it and come forward with numbers but unless we encounter a
real-life example of the need for lots of very short reads/writes in
short succession, I don't think we should overcomplicate this.
>
> > - ret = tty_insert_flip_string(tport, port->rx_fifo, bytes);
> > + ret = tty_insert_flip_string(tport, port->rx_buf, bytes);
> > if (ret != bytes) {
> > dev_err(uport->dev, "%s:Unable to push data ret %d_bytes %d\n",
> > __func__, ret, bytes);
> > @@ -578,7 +577,70 @@ static unsigned int qcom_geni_serial_tx_empty(struct uart_port *uport)
> > return !readl(uport->membase + SE_GENI_TX_FIFO_STATUS);
> > }
> >
> > -static void qcom_geni_serial_start_tx(struct uart_port *uport)
> > +static void qcom_geni_serial_stop_tx_dma(struct uart_port *uport)
> > +{
> > + struct qcom_geni_serial_port *port = to_dev_port(uport);
> > + bool done;
>
> -->
> > + u32 status;
> ...
> > +
> > + status = readl(uport->membase + SE_GENI_STATUS);
> > + if (!(status & M_GENI_CMD_ACTIVE))
> > + return;
> <---
>
> this code snippet is repeating more than few times in the patches, looks
> like it could be made to a inline helper.
>
Makes sense.
>
> > +
> > + if (port->rx_dma_addr) {
> > + geni_se_tx_dma_unprep(&port->se, port->tx_dma_addr,
> > + port->tx_remaining);
> > + port->tx_dma_addr = (dma_addr_t)NULL;
> > + port->tx_remaining = 0;
> > + }
> > +
> > + m_irq_en = readl(uport->membase + SE_GENI_M_IRQ_EN);
> > + writel(m_irq_en, uport->membase + SE_GENI_M_IRQ_EN);
> > + geni_se_cancel_m_cmd(&port->se);
> > +
> > + done = qcom_geni_serial_poll_bit(uport, SE_GENI_S_IRQ_STATUS,
> > + S_CMD_CANCEL_EN, true);
> > + if (!done) {
> > + geni_se_abort_m_cmd(&port->se);
> > + qcom_geni_serial_poll_bit(uport, SE_GENI_M_IRQ_STATUS,
> > + M_CMD_ABORT_EN, true);
>
> return is not checked, there are few more such instances in the patch.
>
Yes, but this is an error path. What would I do if the cancel failed
to go through and then the abort failed as well? I can at best emit an
error message.
> > + writel(M_CMD_ABORT_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
> > + }
> > +
> > + writel(M_CMD_CANCEL_EN, uport->membase + SE_GENI_M_IRQ_CLEAR);
> > +}
> > +
> > +static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
> > +{
> > + struct qcom_geni_serial_port *port = to_dev_port(uport);
> > + struct circ_buf *xmit = &uport->state->xmit;
> > + unsigned int xmit_size;
> > + int ret;
> > +
> > + if (port->tx_dma_addr)
> > + return;
> Is this condition actually possible?
>
It should never happen but I wanted to be sure. Maybe a BUG_ON() or
WARN_ON() would be better?
>
> > +
> > + xmit_size = uart_circ_chars_pending(xmit);
> > + if (xmit_size < WAKEUP_CHARS)
> > + uart_write_wakeup(uport);
> > +
> > + xmit_size = CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
> > +
> > + qcom_geni_serial_setup_tx(uport, xmit_size);
> > +
> > + ret = geni_se_tx_dma_prep(&port->se, &xmit->buf[xmit->tail],
> > + xmit_size, &port->tx_dma_addr);
> > + if (ret) {
> > + dev_err(uport->dev, "unable to start TX SE DMA: %d\n", ret);
> > + qcom_geni_serial_stop_tx_dma(uport);
> > + return;
> > + }
> > +
> > + port->tx_remaining = xmit_size;
> > +}
> > +
>
> ...
Bart
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 13/13] tty: serial: qcom-geni-serial: add support for serial engine DMA
2022-11-25 14:37 ` Srinivas Kandagatla
2022-11-28 11:03 ` Bartosz Golaszewski
@ 2022-11-29 10:13 ` Bartosz Golaszewski
1 sibling, 0 replies; 19+ messages in thread
From: Bartosz Golaszewski @ 2022-11-29 10:13 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Greg Kroah-Hartman,
Jiri Slaby, Vinod Koul, Alex Elder, Ilpo Järvinen,
linux-kernel, linux-arm-msm, linux-serial, Bartosz Golaszewski
On Fri, Nov 25, 2022 at 3:37 PM Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
[snip]
> > +
> > +static void qcom_geni_serial_start_tx_dma(struct uart_port *uport)
> > +{
> > + struct qcom_geni_serial_port *port = to_dev_port(uport);
> > + struct circ_buf *xmit = &uport->state->xmit;
> > + unsigned int xmit_size;
> > + int ret;
> > +
> > + if (port->tx_dma_addr)
> > + return;
> Is this condition actually possible?
>
So it turns out, it's possible that the subsystem calls start_tx when
this is already set but the main engine is not yet in active state (so
we can't simply test that bit). This needs to stay then.
Bart
^ permalink raw reply [flat|nested] 19+ messages in thread