[PATCH 0/5] UART_LCR_WLEN cleanups

[PATCH 0/5] UART_LCR_WLEN cleanups

[Jiri Slaby]

Many drivers currently do:
   switch (cflag & CSIZE) {
   case CS5:
           lcr = UART_LCR_WLEN5;
           break;
   case CS6:
           lcr = UART_LCR_WLEN6;
           break;
   case CS7:
           lcr = UART_LCR_WLEN7;
           break;
   default:
   case CS8:
           lcr = UART_LCR_WLEN8;
           break;
   }

We can simplify it to:
   lcr = UART_LCR_WLEN(tty_get_char_size(cflag));
if we define UART_LCR_WLEN() properly first.

So UART_LCR_WLEN is defined in this series and all such drivers are
converted too.

We could go even further: to define something like uart_compute_LCR()
and compute there whole LCR based even on parity+stop fields in cflag. I
will try if it is worth it later. But this series on its own drops some
duplicated lines already.

Jiri Slaby (5):
  tty: serial: define UART_LCR_WLEN() macro
  tty: serial: make use of UART_LCR_WLEN() + tty_get_char_size()
  USB: serial: make use of UART_LCR_WLEN() + tty_get_char_size()
  sdio_uart: make use of UART_LCR_WLEN() + tty_get_char_size()
  mxser: make use of UART_LCR_WLEN() + tty_get_char_size()

 drivers/mmc/core/sdio_uart.c        | 16 +---------------
 drivers/tty/mxser.c                 | 16 +---------------
 drivers/tty/serial/8250/8250_omap.c | 16 +---------------
 drivers/tty/serial/8250/8250_port.c | 16 +---------------
 drivers/tty/serial/jsm/jsm_cls.c    | 16 +---------------
 drivers/tty/serial/jsm/jsm_neo.c    | 16 +---------------
 drivers/tty/serial/omap-serial.c    | 16 +---------------
 drivers/tty/serial/pxa.c            | 16 +---------------
 drivers/tty/serial/serial-tegra.c   | 22 ++++------------------
 drivers/tty/serial/vr41xx_siu.c     | 15 +--------------
 drivers/usb/serial/ark3116.c        | 17 ++---------------
 drivers/usb/serial/f81232.c         | 16 +---------------
 drivers/usb/serial/f81534.c         | 16 +---------------
 drivers/usb/serial/mos7720.c        | 20 +-------------------
 drivers/usb/serial/quatech2.c       | 16 +---------------
 drivers/usb/serial/ssu100.c         | 16 +---------------
 include/uapi/linux/serial_reg.h     |  1 +
 17 files changed, 21 insertions(+), 246 deletions(-)

-- 
2.35.1

[PATCH 1/5] tty: serial: define UART_LCR_WLEN() macro

[Jiri Slaby]

Define a generic UART_LCR_WLEN() macro with a size argument. It can be
used to encode byte size into an LCR value. Therefore we can use it to
simplify the drivers using tty_get_char_size() in the next patches.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 include/uapi/linux/serial_reg.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index f51bc8f36813..c9d5ff6dd4c6 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -111,6 +111,7 @@
 #define UART_LCR_WLEN6		0x01 /* Wordlength: 6 bits */
 #define UART_LCR_WLEN7		0x02 /* Wordlength: 7 bits */
 #define UART_LCR_WLEN8		0x03 /* Wordlength: 8 bits */
+#define UART_LCR_WLEN(x)	((x) - 5)
 
 /*
  * Access to some registers depends on register access / configuration
-- 
2.35.1

[PATCH 2/5] tty: serial: make use of UART_LCR_WLEN() + tty_get_char_size()

[Jiri Slaby]

Having a generic UART_LCR_WLEN() macro and the tty_get_char_size()
helper, we can remove all those repeated switch-cases in drivers.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Laxman Dewangan <ldewangan@nvidia.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: linux-tegra@vger.kernel.org
---
 drivers/tty/serial/8250/8250_omap.c | 16 +---------------
 drivers/tty/serial/8250/8250_port.c | 16 +---------------
 drivers/tty/serial/jsm/jsm_cls.c    | 16 +---------------
 drivers/tty/serial/jsm/jsm_neo.c    | 16 +---------------
 drivers/tty/serial/omap-serial.c    | 16 +---------------
 drivers/tty/serial/pxa.c            | 16 +---------------
 drivers/tty/serial/serial-tegra.c   | 22 ++++------------------
 drivers/tty/serial/vr41xx_siu.c     | 15 +--------------
 8 files changed, 11 insertions(+), 122 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 73e5f1dbd075..ac8bfa042391 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -357,21 +357,7 @@ static void omap_8250_set_termios(struct uart_port *port,
 	unsigned char cval = 0;
 	unsigned int baud;
 
-	switch (termios->c_cflag & CSIZE) {
-	case CS5:
-		cval = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		cval = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		cval = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		cval = UART_LCR_WLEN8;
-		break;
-	}
+	cval = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag));
 
 	if (termios->c_cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 863f38e036a7..8642f02e4f05 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2590,21 +2590,7 @@ static unsigned char serial8250_compute_lcr(struct uart_8250_port *up,
 {
 	unsigned char cval;
 
-	switch (c_cflag & CSIZE) {
-	case CS5:
-		cval = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		cval = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		cval = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		cval = UART_LCR_WLEN8;
-		break;
-	}
+	cval = UART_LCR_WLEN(tty_get_char_size(c_cflag));
 
 	if (c_cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
diff --git a/drivers/tty/serial/jsm/jsm_cls.c b/drivers/tty/serial/jsm/jsm_cls.c
index b507a2cec926..b280da50290c 100644
--- a/drivers/tty/serial/jsm/jsm_cls.c
+++ b/drivers/tty/serial/jsm/jsm_cls.c
@@ -737,21 +737,7 @@ static void cls_param(struct jsm_channel *ch)
 	if (ch->ch_c_cflag & CSTOPB)
 		lcr |= UART_LCR_STOP;
 
-	switch (ch->ch_c_cflag & CSIZE) {
-	case CS5:
-		lcr |= UART_LCR_WLEN5;
-		break;
-	case CS6:
-		lcr |= UART_LCR_WLEN6;
-		break;
-	case CS7:
-		lcr |= UART_LCR_WLEN7;
-		break;
-	case CS8:
-	default:
-		lcr |= UART_LCR_WLEN8;
-		break;
-	}
+	lcr |= UART_LCR_WLEN(tty_get_char_size(ch->ch_c_cflag));
 
 	ier = readb(&ch->ch_cls_uart->ier);
 	uart_lcr = readb(&ch->ch_cls_uart->lcr);
diff --git a/drivers/tty/serial/jsm/jsm_neo.c b/drivers/tty/serial/jsm/jsm_neo.c
index c6f927a76c3b..c4fd31de04b4 100644
--- a/drivers/tty/serial/jsm/jsm_neo.c
+++ b/drivers/tty/serial/jsm/jsm_neo.c
@@ -1008,21 +1008,7 @@ static void neo_param(struct jsm_channel *ch)
 	if (ch->ch_c_cflag & CSTOPB)
 		lcr |= UART_LCR_STOP;
 
-	switch (ch->ch_c_cflag & CSIZE) {
-	case CS5:
-		lcr |= UART_LCR_WLEN5;
-		break;
-	case CS6:
-		lcr |= UART_LCR_WLEN6;
-		break;
-	case CS7:
-		lcr |= UART_LCR_WLEN7;
-		break;
-	case CS8:
-	default:
-		lcr |= UART_LCR_WLEN8;
-	break;
-	}
+	lcr |= UART_LCR_WLEN(tty_get_char_size(ch->ch_c_cflag));
 
 	ier = readb(&ch->ch_neo_uart->ier);
 	uart_lcr = readb(&ch->ch_neo_uart->lcr);
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 7074670cf81d..8d5ffa196097 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -808,21 +808,7 @@ serial_omap_set_termios(struct uart_port *port, struct ktermios *termios,
 	unsigned long flags;
 	unsigned int baud, quot;
 
-	switch (termios->c_cflag & CSIZE) {
-	case CS5:
-		cval = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		cval = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		cval = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		cval = UART_LCR_WLEN8;
-		break;
-	}
+	cval = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag));
 
 	if (termios->c_cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 5d7b7e56661f..e80ba8e10407 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -430,21 +430,7 @@ serial_pxa_set_termios(struct uart_port *port, struct ktermios *termios,
 	unsigned int baud, quot;
 	unsigned int dll;
 
-	switch (termios->c_cflag & CSIZE) {
-	case CS5:
-		cval = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		cval = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		cval = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		cval = UART_LCR_WLEN8;
-		break;
-	}
+	cval = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag));
 
 	if (termios->c_cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
index b6223fab0687..d942ab152f5a 100644
--- a/drivers/tty/serial/serial-tegra.c
+++ b/drivers/tty/serial/serial-tegra.c
@@ -1277,6 +1277,7 @@ static void tegra_uart_set_termios(struct uart_port *u,
 	unsigned int baud;
 	unsigned long flags;
 	unsigned int lcr;
+	unsigned char char_bits;
 	int symb_bit = 1;
 	struct clk *parent_clk = clk_get_parent(tup->uart_clk);
 	unsigned long parent_clk_rate = clk_get_rate(parent_clk);
@@ -1316,25 +1317,10 @@ static void tegra_uart_set_termios(struct uart_port *u,
 		}
 	}
 
+	char_bits = tty_get_char_size(termios->c_cflag);
+	symb_bit += char_bits;
 	lcr &= ~UART_LCR_WLEN8;
-	switch (termios->c_cflag & CSIZE) {
-	case CS5:
-		lcr |= UART_LCR_WLEN5;
-		symb_bit += 5;
-		break;
-	case CS6:
-		lcr |= UART_LCR_WLEN6;
-		symb_bit += 6;
-		break;
-	case CS7:
-		lcr |= UART_LCR_WLEN7;
-		symb_bit += 7;
-		break;
-	default:
-		lcr |= UART_LCR_WLEN8;
-		symb_bit += 8;
-		break;
-	}
+	lcr |= UART_LCR_WLEN(char_bits);
 
 	/* Stop bits */
 	if (termios->c_cflag & CSTOPB) {
diff --git a/drivers/tty/serial/vr41xx_siu.c b/drivers/tty/serial/vr41xx_siu.c
index 6b303af5ee54..e0bf003ca3a1 100644
--- a/drivers/tty/serial/vr41xx_siu.c
+++ b/drivers/tty/serial/vr41xx_siu.c
@@ -504,20 +504,7 @@ static void siu_set_termios(struct uart_port *port, struct ktermios *new,
 	unsigned long flags;
 
 	c_cflag = new->c_cflag;
-	switch (c_cflag & CSIZE) {
-	case CS5:
-		lcr = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		lcr = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		lcr = UART_LCR_WLEN7;
-		break;
-	default:
-		lcr = UART_LCR_WLEN8;
-		break;
-	}
+	lcr = UART_LCR_WLEN(tty_get_char_size(c_cflag));
 
 	if (c_cflag & CSTOPB)
 		lcr |= UART_LCR_STOP;
-- 
2.35.1

[PATCH 3/5] USB: serial: make use of UART_LCR_WLEN() + tty_get_char_size()

[Jiri Slaby]

Having a generic UART_LCR_WLEN() macro and the tty_get_char_size()
helper, we can remove all those repeated switch-cases in drivers.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Johan Hovold <johan@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/serial/ark3116.c  | 17 ++---------------
 drivers/usb/serial/f81232.c   | 16 +---------------
 drivers/usb/serial/f81534.c   | 16 +---------------
 drivers/usb/serial/mos7720.c  | 20 +-------------------
 drivers/usb/serial/quatech2.c | 16 +---------------
 drivers/usb/serial/ssu100.c   | 16 +---------------
 6 files changed, 7 insertions(+), 94 deletions(-)

diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 5dd710e9fe7d..c0e4df87ff22 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -200,21 +200,8 @@ static void ark3116_set_termios(struct tty_struct *tty,
 	__u8 lcr, hcr, eval;
 
 	/* set data bit count */
-	switch (cflag & CSIZE) {
-	case CS5:
-		lcr = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		lcr = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		lcr = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		lcr = UART_LCR_WLEN8;
-		break;
-	}
+	lcr = UART_LCR_WLEN(tty_get_char_size(cflag));
+
 	if (cflag & CSTOPB)
 		lcr |= UART_LCR_STOP;
 	if (cflag & PARENB)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 3ad1f515fb68..d9f20256a6a8 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -643,21 +643,7 @@ static void f81232_set_termios(struct tty_struct *tty,
 	if (C_CSTOPB(tty))
 		new_lcr |= UART_LCR_STOP;
 
-	switch (C_CSIZE(tty)) {
-	case CS5:
-		new_lcr |= UART_LCR_WLEN5;
-		break;
-	case CS6:
-		new_lcr |= UART_LCR_WLEN6;
-		break;
-	case CS7:
-		new_lcr |= UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		new_lcr |= UART_LCR_WLEN8;
-		break;
-	}
+	new_lcr |= UART_LCR_WLEN(tty_get_char_size(tty->termios.c_cflag));
 
 	mutex_lock(&priv->lock);
 
diff --git a/drivers/usb/serial/f81534.c b/drivers/usb/serial/f81534.c
index c0bca52ef92a..d789c1ec87b3 100644
--- a/drivers/usb/serial/f81534.c
+++ b/drivers/usb/serial/f81534.c
@@ -970,21 +970,7 @@ static void f81534_set_termios(struct tty_struct *tty,
 	if (C_CSTOPB(tty))
 		new_lcr |= UART_LCR_STOP;
 
-	switch (C_CSIZE(tty)) {
-	case CS5:
-		new_lcr |= UART_LCR_WLEN5;
-		break;
-	case CS6:
-		new_lcr |= UART_LCR_WLEN6;
-		break;
-	case CS7:
-		new_lcr |= UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		new_lcr |= UART_LCR_WLEN8;
-		break;
-	}
+	new_lcr |= UART_LCR_WLEN(tty_get_char_size(tty->termios.c_cflag));
 
 	baud = tty_get_baud_rate(tty);
 	if (!baud)
diff --git a/drivers/usb/serial/mos7720.c b/drivers/usb/serial/mos7720.c
index 227f43d2bd56..1e12b5f30dcc 100644
--- a/drivers/usb/serial/mos7720.c
+++ b/drivers/usb/serial/mos7720.c
@@ -1380,30 +1380,12 @@ static void change_port_settings(struct tty_struct *tty,
 		return;
 	}
 
-	lData = UART_LCR_WLEN8;
 	lStop = 0x00;	/* 1 stop bit */
 	lParity = 0x00;	/* No parity */
 
 	cflag = tty->termios.c_cflag;
 
-	/* Change the number of bits */
-	switch (cflag & CSIZE) {
-	case CS5:
-		lData = UART_LCR_WLEN5;
-		break;
-
-	case CS6:
-		lData = UART_LCR_WLEN6;
-		break;
-
-	case CS7:
-		lData = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		lData = UART_LCR_WLEN8;
-		break;
-	}
+	lData = UART_LCR_WLEN(tty_get_char_size(cflag));
 
 	/* Change the Parity bit */
 	if (cflag & PARENB) {
diff --git a/drivers/usb/serial/quatech2.c b/drivers/usb/serial/quatech2.c
index 971907f083a3..36b1e064e51f 100644
--- a/drivers/usb/serial/quatech2.c
+++ b/drivers/usb/serial/quatech2.c
@@ -281,21 +281,7 @@ static void qt2_set_termios(struct tty_struct *tty,
 			new_lcr |= SERIAL_EVEN_PARITY;
 	}
 
-	switch (cflag & CSIZE) {
-	case CS5:
-		new_lcr |= UART_LCR_WLEN5;
-		break;
-	case CS6:
-		new_lcr |= UART_LCR_WLEN6;
-		break;
-	case CS7:
-		new_lcr |= UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		new_lcr |= UART_LCR_WLEN8;
-		break;
-	}
+	new_lcr |= UART_LCR_WLEN(tty_get_char_size(cflag));
 
 	baud = tty_get_baud_rate(tty);
 	if (!baud)
diff --git a/drivers/usb/serial/ssu100.c b/drivers/usb/serial/ssu100.c
index 3baf7c0f5a98..181e302136a5 100644
--- a/drivers/usb/serial/ssu100.c
+++ b/drivers/usb/serial/ssu100.c
@@ -231,21 +231,7 @@ static void ssu100_set_termios(struct tty_struct *tty,
 			urb_value |= SERIAL_EVEN_PARITY;
 	}
 
-	switch (cflag & CSIZE) {
-	case CS5:
-		urb_value |= UART_LCR_WLEN5;
-		break;
-	case CS6:
-		urb_value |= UART_LCR_WLEN6;
-		break;
-	case CS7:
-		urb_value |= UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		urb_value |= UART_LCR_WLEN8;
-		break;
-	}
+	urb_value |= UART_LCR_WLEN(tty_get_char_size(cflag));
 
 	baud = tty_get_baud_rate(tty);
 	if (!baud)
-- 
2.35.1

[PATCH 4/5] sdio_uart: make use of UART_LCR_WLEN() + tty_get_char_size()

[Jiri Slaby]

Having a generic UART_LCR_WLEN() macro and the tty_get_char_size()
helper, we can remove all those repeated switch-cases in drivers.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-mmc@vger.kernel.org
---
 drivers/mmc/core/sdio_uart.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/mmc/core/sdio_uart.c b/drivers/mmc/core/sdio_uart.c
index 04c0823e0359..e6eb5bd6e440 100644
--- a/drivers/mmc/core/sdio_uart.c
+++ b/drivers/mmc/core/sdio_uart.c
@@ -250,21 +250,7 @@ static void sdio_uart_change_speed(struct sdio_uart_port *port,
 	unsigned char cval, fcr = 0;
 	unsigned int baud, quot;
 
-	switch (termios->c_cflag & CSIZE) {
-	case CS5:
-		cval = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		cval = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		cval = UART_LCR_WLEN7;
-		break;
-	default:
-	case CS8:
-		cval = UART_LCR_WLEN8;
-		break;
-	}
+	cval = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag));
 
 	if (termios->c_cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
-- 
2.35.1

[PATCH 5/5] mxser: make use of UART_LCR_WLEN() + tty_get_char_size()

[Jiri Slaby]

Having a generic UART_LCR_WLEN() macro and the tty_get_char_size()
helper, we can remove all those repeated switch-cases in drivers.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/mxser.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/tty/mxser.c b/drivers/tty/mxser.c
index 836c9eca2946..6ebd3e4ed859 100644
--- a/drivers/tty/mxser.c
+++ b/drivers/tty/mxser.c
@@ -588,21 +588,7 @@ static void mxser_change_speed(struct tty_struct *tty, struct ktermios *old_term
 	}
 
 	/* byte size and parity */
-	switch (cflag & CSIZE) {
-	default:
-	case CS5:
-		cval = UART_LCR_WLEN5;
-		break;
-	case CS6:
-		cval = UART_LCR_WLEN6;
-		break;
-	case CS7:
-		cval = UART_LCR_WLEN7;
-		break;
-	case CS8:
-		cval = UART_LCR_WLEN8;
-		break;
-	}
+	cval = UART_LCR_WLEN(tty_get_char_size(tty->termios.c_cflag));
 
 	if (cflag & CSTOPB)
 		cval |= UART_LCR_STOP;
-- 
2.35.1

Re: [PATCH 1/5] tty: serial: define UART_LCR_WLEN() macro

[Greg KH]

On Thu, Feb 24, 2022 at 10:55:54AM +0100, Jiri Slaby wrote:
> Define a generic UART_LCR_WLEN() macro with a size argument. It can be
> used to encode byte size into an LCR value. Therefore we can use it to
> simplify the drivers using tty_get_char_size() in the next patches.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  include/uapi/linux/serial_reg.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
> index f51bc8f36813..c9d5ff6dd4c6 100644
> --- a/include/uapi/linux/serial_reg.h
> +++ b/include/uapi/linux/serial_reg.h
> @@ -111,6 +111,7 @@
>  #define UART_LCR_WLEN6		0x01 /* Wordlength: 6 bits */
>  #define UART_LCR_WLEN7		0x02 /* Wordlength: 7 bits */
>  #define UART_LCR_WLEN8		0x03 /* Wordlength: 8 bits */
> +#define UART_LCR_WLEN(x)	((x) - 5)

I'm all for this, but why does it need to be in a uapi .h file?

thanks,

greg k-h

Re: [PATCH 1/5] tty: serial: define UART_LCR_WLEN() macro

[Jiri Slaby]

On 24. 02. 22, 11:10, Greg KH wrote:
> On Thu, Feb 24, 2022 at 10:55:54AM +0100, Jiri Slaby wrote:
>> Define a generic UART_LCR_WLEN() macro with a size argument. It can be
>> used to encode byte size into an LCR value. Therefore we can use it to
>> simplify the drivers using tty_get_char_size() in the next patches.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> ---
>>   include/uapi/linux/serial_reg.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
>> index f51bc8f36813..c9d5ff6dd4c6 100644
>> --- a/include/uapi/linux/serial_reg.h
>> +++ b/include/uapi/linux/serial_reg.h
>> @@ -111,6 +111,7 @@
>>   #define UART_LCR_WLEN6		0x01 /* Wordlength: 6 bits */
>>   #define UART_LCR_WLEN7		0x02 /* Wordlength: 7 bits */
>>   #define UART_LCR_WLEN8		0x03 /* Wordlength: 8 bits */
>> +#define UART_LCR_WLEN(x)	((x) - 5)
> 
> I'm all for this, but why does it need to be in a uapi .h file?

I'd love to put it somewhere else. But I didn't find an appropriate 
place. Should I put it simply to
include/linux/serial_core.h
or
include/linux/serial.h
? Or create a new:
include/linux/serial_reg.h
to contain only this def?

thanks,
-- 
js
suse labs

Re: [PATCH 1/5] tty: serial: define UART_LCR_WLEN() macro

[Greg KH]

On Thu, Feb 24, 2022 at 11:42:20AM +0100, Jiri Slaby wrote:
> On 24. 02. 22, 11:10, Greg KH wrote:
> > On Thu, Feb 24, 2022 at 10:55:54AM +0100, Jiri Slaby wrote:
> > > Define a generic UART_LCR_WLEN() macro with a size argument. It can be
> > > used to encode byte size into an LCR value. Therefore we can use it to
> > > simplify the drivers using tty_get_char_size() in the next patches.
> > > 
> > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > > ---
> > >   include/uapi/linux/serial_reg.h | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
> > > index f51bc8f36813..c9d5ff6dd4c6 100644
> > > --- a/include/uapi/linux/serial_reg.h
> > > +++ b/include/uapi/linux/serial_reg.h
> > > @@ -111,6 +111,7 @@
> > >   #define UART_LCR_WLEN6		0x01 /* Wordlength: 6 bits */
> > >   #define UART_LCR_WLEN7		0x02 /* Wordlength: 7 bits */
> > >   #define UART_LCR_WLEN8		0x03 /* Wordlength: 8 bits */
> > > +#define UART_LCR_WLEN(x)	((x) - 5)
> > 
> > I'm all for this, but why does it need to be in a uapi .h file?
> 
> I'd love to put it somewhere else. But I didn't find an appropriate place.
> Should I put it simply to
> include/linux/serial_core.h
> or
> include/linux/serial.h

This one would make sense.

> ? Or create a new:
> include/linux/serial_reg.h
> to contain only this def?

Nah, no need to create a whole .h file for a single macro :)

thanks,

greg k-h

Re: [PATCH 2/5] tty: serial: make use of UART_LCR_WLEN() + tty_get_char_size()

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On Thu, Feb 24, 2022 at 10:55:55AM +0100, Jiri Slaby wrote:
> Having a generic UART_LCR_WLEN() macro and the tty_get_char_size()
> helper, we can remove all those repeated switch-cases in drivers.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Laxman Dewangan <ldewangan@nvidia.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Cc: linux-tegra@vger.kernel.org
> ---
>  drivers/tty/serial/8250/8250_omap.c | 16 +---------------
>  drivers/tty/serial/8250/8250_port.c | 16 +---------------
>  drivers/tty/serial/jsm/jsm_cls.c    | 16 +---------------
>  drivers/tty/serial/jsm/jsm_neo.c    | 16 +---------------
>  drivers/tty/serial/omap-serial.c    | 16 +---------------
>  drivers/tty/serial/pxa.c            | 16 +---------------
>  drivers/tty/serial/serial-tegra.c   | 22 ++++------------------
>  drivers/tty/serial/vr41xx_siu.c     | 15 +--------------
>  8 files changed, 11 insertions(+), 122 deletions(-)

This would've been easier to review if I had this in my inbox, but I did
find it on lkml.org and this does look correct, so:

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] tty: serial: define UART_LCR_WLEN() macro

[Greg KH]

On Thu, Feb 24, 2022 at 12:23:41PM +0100, Greg KH wrote:
> On Thu, Feb 24, 2022 at 11:42:20AM +0100, Jiri Slaby wrote:
> > On 24. 02. 22, 11:10, Greg KH wrote:
> > > On Thu, Feb 24, 2022 at 10:55:54AM +0100, Jiri Slaby wrote:
> > > > Define a generic UART_LCR_WLEN() macro with a size argument. It can be
> > > > used to encode byte size into an LCR value. Therefore we can use it to
> > > > simplify the drivers using tty_get_char_size() in the next patches.
> > > > 
> > > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > > > ---
> > > >   include/uapi/linux/serial_reg.h | 1 +
> > > >   1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
> > > > index f51bc8f36813..c9d5ff6dd4c6 100644
> > > > --- a/include/uapi/linux/serial_reg.h
> > > > +++ b/include/uapi/linux/serial_reg.h
> > > > @@ -111,6 +111,7 @@
> > > >   #define UART_LCR_WLEN6		0x01 /* Wordlength: 6 bits */
> > > >   #define UART_LCR_WLEN7		0x02 /* Wordlength: 7 bits */
> > > >   #define UART_LCR_WLEN8		0x03 /* Wordlength: 8 bits */
> > > > +#define UART_LCR_WLEN(x)	((x) - 5)
> > > 
> > > I'm all for this, but why does it need to be in a uapi .h file?
> > 
> > I'd love to put it somewhere else. But I didn't find an appropriate place.
> > Should I put it simply to
> > include/linux/serial_core.h
> > or
> > include/linux/serial.h
> 
> This one would make sense.

I'll fix this up on my own, no need to resend this...

thanks

greg k-h

Re: [PATCH 4/5] sdio_uart: make use of UART_LCR_WLEN() + tty_get_char_size()

[Ulf Hansson]

On Thu, 24 Feb 2022 at 10:56, Jiri Slaby <jslaby@suse.cz> wrote:
>
> Having a generic UART_LCR_WLEN() macro and the tty_get_char_size()
> helper, we can remove all those repeated switch-cases in drivers.
>
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: linux-mmc@vger.kernel.org

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
>  drivers/mmc/core/sdio_uart.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio_uart.c b/drivers/mmc/core/sdio_uart.c
> index 04c0823e0359..e6eb5bd6e440 100644
> --- a/drivers/mmc/core/sdio_uart.c
> +++ b/drivers/mmc/core/sdio_uart.c
> @@ -250,21 +250,7 @@ static void sdio_uart_change_speed(struct sdio_uart_port *port,
>         unsigned char cval, fcr = 0;
>         unsigned int baud, quot;
>
> -       switch (termios->c_cflag & CSIZE) {
> -       case CS5:
> -               cval = UART_LCR_WLEN5;
> -               break;
> -       case CS6:
> -               cval = UART_LCR_WLEN6;
> -               break;
> -       case CS7:
> -               cval = UART_LCR_WLEN7;
> -               break;
> -       default:
> -       case CS8:
> -               cval = UART_LCR_WLEN8;
> -               break;
> -       }
> +       cval = UART_LCR_WLEN(tty_get_char_size(termios->c_cflag));
>
>         if (termios->c_cflag & CSTOPB)
>                 cval |= UART_LCR_STOP;
> --
> 2.35.1
>