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.
