[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <2197faa3-0217-41e0-8ff0-b5396561c623@www.fastmail.com>
Date: Wed, 07 Sep 2022 16:56:03 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Russell King" <linux@...linux.org.uk>,
"Greg Kroah-Hartman" <gregkh@...uxfoundation.org>
Cc: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
"Jiri Slaby" <jirislaby@...nel.org>,
"Johan Hovold" <johan@...nel.org>,
linux-serial <linux-serial@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
"Tobias Klauser" <tklauser@...tanz.ch>,
"Richard Genoud" <richard.genoud@...il.com>,
"Nicolas Ferre" <nicolas.ferre@...rochip.com>,
"Alexandre Belloni" <alexandre.belloni@...tlin.com>,
"Claudiu Beznea" <claudiu.beznea@...rochip.com>,
"Vladimir Zapolskiy" <vz@...ia.com>,
"Liviu Dudau" <liviu.dudau@....com>,
"Sudeep Holla" <sudeep.holla@....com>,
"Lorenzo Pieralisi" <lorenzo.pieralisi@....com>,
"Shawn Guo" <shawnguo@...nel.org>,
"Sascha Hauer" <s.hauer@...gutronix.de>,
"Pengutronix Kernel Team" <kernel@...gutronix.de>,
"Fabio Estevam" <festevam@...il.com>,
"NXP Linux Team" <linux-imx@....com>,
Andreas Färber <afaerber@...e.de>,
"Manivannan Sadhasivam" <mani@...nel.org>,
"Florian Fainelli" <f.fainelli@...il.com>,
bcm-kernel-feedback-list@...adcom.com,
Pali Rohár <pali@...nel.org>,
"Kevin Cernekee" <cernekee@...il.com>,
"Palmer Dabbelt" <palmer@...belt.com>,
"Paul Walmsley" <paul.walmsley@...ive.com>,
"Orson Zhai" <orsonzhai@...il.com>,
"Baolin Wang" <baolin.wang7@...il.com>,
"Chunyan Zhang" <zhang.lyra@...il.com>,
"Patrice Chotard" <patrice.chotard@...s.st.com>,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v3 0/4] tty: TX helpers
On Wed, Sep 7, 2022, at 3:52 PM, Russell King (Oracle) wrote:
> On Wed, Sep 07, 2022 at 02:36:37PM +0200, Greg Kroah-Hartman wrote:
>
> Of course, it would have been nicer to see the definition of this
> macro, because then we can understand what the "ch" argument is to
> this macro, and how that relates to the macro argument that is
> shown in the example as a writel().
I pulled out the 'ch' variable from the macro to avoid having
the macro define local variables that are then passed to the
inner expressions.
> Maybe a more complete example would help clear up the confusion?
> Arnd?
Here is a patch on top of the series that would implement the
uart_port_tx_helper_limited() and uart_port_tx_helper()
macros that can be used directly from drivers in place of defining
local functions, with the (alphabetically) first two drivers
converted to that.
I left the (now trivial) DEFINE_UART_PORT_TX_HELPER_LIMITED() and
DEFINE_UART_PORT_TX_HELPER() macros in place to keep it building,
but they would get removed if we decide to use something like
my suggested approach for all drivers.
Arnd
diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 40cf1bb534f3..a0f5c59d6128 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -151,16 +151,14 @@ static irqreturn_t serial21285_rx_chars(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static DEFINE_UART_PORT_TX_HELPER_LIMITED(serial21285_do_tx_chars,
- !(*CSR_UARTFLG & 0x20),
- *CSR_UARTDR = ch,
- ({}));
-
static irqreturn_t serial21285_tx_chars(int irq, void *dev_id)
{
struct uart_port *port = dev_id;
+ unsigned int count = 256;
+ unsigned char ch;
- serial21285_do_tx_chars(port, 256);
+ uart_port_tx_helper_limited(port, !(*CSR_UARTFLG & 0x20),
+ *CSR_UARTDR = ch, ({}), count, ch);
return IRQ_HANDLED;
}
diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 70c0ad431cf9..f81dd950cd39 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -246,10 +246,14 @@ static void altera_uart_rx_chars(struct altera_uart *pp)
tty_flip_buffer_push(&port->state->port);
}
-static DEFINE_UART_PORT_TX_HELPER(altera_uart_tx_chars,
- altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
- ALTERA_UART_STATUS_TRDY_MSK,
- altera_uart_writel(port, ch, ALTERA_UART_TXDATA_REG));
+static int altera_uart_tx_chars(struct uart_port *port)
+{
+ u8 ch;
+
+ return uart_port_tx_helper(port,
+ altera_uart_readl(port, ALTERA_UART_STATUS_REG) & ALTERA_UART_STATUS_TRDY_MSK,
+ altera_uart_writel(port, ch, ALTERA_UART_TXDATA_REG), ch);
+}
static irqreturn_t altera_uart_interrupt(int irq, void *data)
{
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 7236fc76ba22..d48d2301d1b7 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -638,13 +638,11 @@ struct uart_driver {
void uart_write_wakeup(struct uart_port *port);
-#define __DEFINE_UART_PORT_TX_HELPER(name, tx_ready, put_char, tx_done, \
- for_test, for_post, ...) \
-unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__) \
-{ \
+#define __uart_port_tx_helper(port, tx_ready, put_char, tx_done, \
+ for_test, for_post, ch) \
+({ \
struct circ_buf *xmit = &port->state->xmit; \
unsigned int pending; \
- u8 ch; \
\
for (; (for_test) && (tx_ready); (for_post), port->icount.tx++) { \
if (port->x_char) { \
@@ -672,8 +670,15 @@ unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__) \
port->ops->stop_tx(port); \
} \
\
- return pending; \
-}
+ pending; \
+})
+
+#define uart_port_tx_helper_limited(port, tx_ready, put_char, tx_done, count, ch) \
+ __uart_port_tx_helper(port, tx_ready, put_char, tx_done, count, count--, ch)
+
+#define uart_port_tx_helper(port, tx_ready, put_char, ch) \
+ __uart_port_tx_helper(port, tx_ready, put_char, ({}), true, ({}), ch)
+
/**
* DEFINE_UART_PORT_TX_HELPER_LIMITED -- generate transmit helper for uart_port
@@ -703,9 +708,13 @@ unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__) \
* For all of them, @port->lock is held, interrupts are locally disabled and
* the expressions must not sleep.
*/
-#define DEFINE_UART_PORT_TX_HELPER_LIMITED(name, tx_ready, put_char, tx_done) \
- __DEFINE_UART_PORT_TX_HELPER(name, tx_ready, put_char, tx_done, \
- count, count--, unsigned int count)
+#define DEFINE_UART_PORT_TX_HELPER_LIMITED(name, tx_ready, put_char, tx_done) \
+unsigned int name(struct uart_port *port, unsigned int count) \
+{ \
+ u8 ch; \
+ return uart_port_tx_helper_limited(port, tx_ready, put_char, tx_done, \
+ count, ch); \
+}
/**
* DEFINE_UART_PORT_TX_HELPER -- generate transmit helper for uart_port
@@ -715,9 +724,12 @@ unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__) \
*
* See DEFINE_UART_PORT_TX_HELPER_LIMITED() for more details.
*/
-#define DEFINE_UART_PORT_TX_HELPER(name, tx_ready, put_char) \
- __DEFINE_UART_PORT_TX_HELPER(name, tx_ready, put_char, ({}), \
- true, ({}))
+#define DEFINE_UART_PORT_TX_HELPER(name, tx_ready, put_char, ...) \
+unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__) \
+{ \
+ u8 ch; \
+ return uart_port_tx_helper(port, tx_ready, put_char, ch); \
+}
/*
* Baud rate helpers.
Powered by blists - more mailing lists