lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ