[PATCH 0/6] serial: Cleanup literals

[PATCH 0/6] serial: Cleanup literals

[Ilpo Järvinen]

Convert plenty of register write literals to use the proper defines. In
most of the cases the defines already exist but a few new defines are
added too for combined sets of flags.

Ilpo Järvinen (6):
  serial: 8250: Use defined IER bits
  serial: 8250: Name MSR literals
  serial: 8250: Cleanup MCR literals
  serial: 8250: Add IIR FIFOs enabled field properly
  serial: 8250: Define IIR 64 byte bit & cleanup related code
  serial: 8250_early: Convert literals to use defines

 drivers/tty/serial/8250/8250_early.c |  4 +--
 drivers/tty/serial/8250/8250_port.c  | 47 ++++++++++++++--------------
 include/linux/serial.h               | 10 ++++++
 include/uapi/linux/serial_reg.h      |  5 +++
 4 files changed, 41 insertions(+), 25 deletions(-)

-- 
2.30.2

[PATCH 1/6] serial: 8250: Use defined IER bits

[Ilpo Järvinen]

Instead of literal 0x0f, add a define for enabling all IER bits the
8250 driver is interested in.

Don't make the define for combined flags part of UAPI.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_port.c | 10 +++++-----
 include/linux/serial.h              |  5 +++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index beba8f38b3dc..8676f8b7f2e3 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1236,14 +1236,14 @@ static void autoconfig(struct uart_8250_port *up)
 		 * Mask out IER[7:4] bits for test as some UARTs (e.g. TL
 		 * 16C754B) allow only to modify them if an EFR bit is set.
 		 */
-		scratch2 = serial_in(up, UART_IER) & 0x0f;
-		serial_out(up, UART_IER, 0x0F);
+		scratch2 = serial_in(up, UART_IER) & UART_IER_ALL_INTR;
+		serial_out(up, UART_IER, UART_IER_ALL_INTR);
 #ifdef __i386__
 		outb(0, 0x080);
 #endif
-		scratch3 = serial_in(up, UART_IER) & 0x0f;
+		scratch3 = serial_in(up, UART_IER) & UART_IER_ALL_INTR;
 		serial_out(up, UART_IER, scratch);
-		if (scratch2 != 0 || scratch3 != 0x0F) {
+		if (scratch2 != 0 || scratch3 != UART_IER_ALL_INTR) {
 			/*
 			 * We failed; there's nothing here
 			 */
@@ -1394,7 +1394,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
 		serial8250_out_MCR(up,
 			UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2);
 	}
-	serial_out(up, UART_IER, 0x0f);	/* enable all intrs */
+	serial_out(up, UART_IER, UART_IER_ALL_INTR);
 	serial_in(up, UART_LSR);
 	serial_in(up, UART_RX);
 	serial_in(up, UART_IIR);
diff --git a/include/linux/serial.h b/include/linux/serial.h
index 3d6fe3ef92cf..ad6e1c37e2d5 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -12,6 +12,11 @@
 #include <uapi/linux/serial.h>
 #include <uapi/linux/serial_reg.h>
 
+#define UART_IER_ALL_INTR	(UART_IER_MSI | \
+				 UART_IER_RLSI | \
+				 UART_IER_THRI | \
+				 UART_IER_RDI)
+
 /* Helper for dealing with UART_LCR_WLEN* defines */
 #define UART_LCR_WLEN(x)	((x) - 5)
 
-- 
2.30.2

[PATCH 2/6] serial: 8250: Name MSR literals

[Ilpo Järvinen]

Add UART_MSR_STATUS_BITS for CD, RI, DSR & CTS. Use names for the
literal.

Don't make the define for combined flags part of UAPI.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_port.c | 4 ++--
 include/linux/serial.h              | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 8676f8b7f2e3..c870ee8e80b6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1268,9 +1268,9 @@ static void autoconfig(struct uart_8250_port *up)
 	 */
 	if (!(port->flags & UPF_SKIP_TEST)) {
 		serial8250_out_MCR(up, UART_MCR_LOOP | 0x0A);
-		status1 = serial_in(up, UART_MSR) & 0xF0;
+		status1 = serial_in(up, UART_MSR) & UART_MSR_STATUS_BITS;
 		serial8250_out_MCR(up, save_mcr);
-		if (status1 != 0x90) {
+		if (status1 != (UART_MSR_DCD | UART_MSR_CTS)) {
 			spin_unlock_irqrestore(&port->lock, flags);
 			DEBUG_AUTOCONF("LOOP test failed (%02x) ",
 				       status1);
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ad6e1c37e2d5..bfda927dde15 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -28,6 +28,11 @@ static inline bool uart_lsr_tx_empty(u16 lsr)
 	return (lsr & UART_LSR_BOTH_EMPTY) == UART_LSR_BOTH_EMPTY;
 }
 
+#define UART_MSR_STATUS_BITS	(UART_MSR_DCD | \
+				 UART_MSR_RI | \
+				 UART_MSR_DSR | \
+				 UART_MSR_CTS)
+
 /*
  * Counters of the input lines (CTS, DSR, RI, CD) interrupts
  */
-- 
2.30.2

[PATCH 3/6] serial: 8250: Cleanup MCR literals

[Ilpo Järvinen]

Use proper names from MCR bits.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_port.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index c870ee8e80b6..e79bf2afd9be 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1267,7 +1267,7 @@ static void autoconfig(struct uart_8250_port *up)
 	 * that conflicts with COM 1-4 --- we hope!
 	 */
 	if (!(port->flags & UPF_SKIP_TEST)) {
-		serial8250_out_MCR(up, UART_MCR_LOOP | 0x0A);
+		serial8250_out_MCR(up, UART_MCR_LOOP | UART_MCR_OUT2 | UART_MCR_RTS);
 		status1 = serial_in(up, UART_MSR) & UART_MSR_STATUS_BITS;
 		serial8250_out_MCR(up, save_mcr);
 		if (status1 != (UART_MSR_DCD | UART_MSR_CTS)) {
-- 
2.30.2

[PATCH 4/6] serial: 8250: Add IIR FIFOs enabled field properly

[Ilpo Järvinen]

Don't use magic literals & comments but define a real field instead
for UART_IIR_FIFO_ENABLED and name also the values.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_port.c | 17 +++++++----------
 include/uapi/linux/serial_reg.h     |  4 ++++
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index e79bf2afd9be..6d343b15bc58 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1293,22 +1293,19 @@ static void autoconfig(struct uart_8250_port *up)
 
 	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
 
-	/* Assign this as it is to truncate any bits above 7.  */
-	scratch = serial_in(up, UART_IIR);
-
-	switch (scratch >> 6) {
-	case 0:
+	switch (serial_in(up, UART_IIR) & UART_IIR_FIFO_ENABLED) {
+	case UART_IIR_FIFO_ENABLED_8250:
 		autoconfig_8250(up);
 		break;
-	case 1:
-		port->type = PORT_UNKNOWN;
-		break;
-	case 2:
+	case UART_IIR_FIFO_EAABLED_16550:
 		port->type = PORT_16550;
 		break;
-	case 3:
+	case UART_IIR_FIFO_ENABLED_16550A:
 		autoconfig_16550a(up);
 		break;
+	default:
+		port->type = PORT_UNKNOWN;
+		break;
 	}
 
 #ifdef CONFIG_SERIAL_8250_RSA
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index bab3b39266cc..a63c124b7fdc 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -44,6 +44,10 @@
 #define UART_IIR_RX_TIMEOUT	0x0c /* OMAP RX Timeout interrupt */
 #define UART_IIR_XOFF		0x10 /* OMAP XOFF/Special Character */
 #define UART_IIR_CTS_RTS_DSR	0x20 /* OMAP CTS/RTS/DSR Change */
+#define UART_IIR_FIFO_ENABLED	0xc0 /* FIFOs enabled / port type identification */
+#define  UART_IIR_FIFO_ENABLED_8250	0x00	/* 8250: no FIFO */
+#define  UART_IIR_FIFO_EAABLED_16550	0x80	/* 16550: (broken/unusable) FIFO */
+#define  UART_IIR_FIFO_ENABLED_16550A	0xc0	/* 16550A: FIFO enabled */
 
 #define UART_FCR	2	/* Out: FIFO Control Register */
 #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */
-- 
2.30.2

Re: [PATCH 4/6] serial: 8250: Add IIR FIFOs enabled field properly

[Jiri Slaby]

On 24. 11. 22, 13:59, Ilpo Järvinen wrote:
> Don't use magic literals & comments but define a real field instead
> for UART_IIR_FIFO_ENABLED and name also the values.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>   drivers/tty/serial/8250/8250_port.c | 17 +++++++----------
>   include/uapi/linux/serial_reg.h     |  4 ++++
>   2 files changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index e79bf2afd9be..6d343b15bc58 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1293,22 +1293,19 @@ static void autoconfig(struct uart_8250_port *up)
>   
>   	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
>   
> -	/* Assign this as it is to truncate any bits above 7.  */
> -	scratch = serial_in(up, UART_IIR);
> -
> -	switch (scratch >> 6) {
> -	case 0:
> +	switch (serial_in(up, UART_IIR) & UART_IIR_FIFO_ENABLED) {
> +	case UART_IIR_FIFO_ENABLED_8250:
>   		autoconfig_8250(up);
>   		break;
> -	case 1:
> -		port->type = PORT_UNKNOWN;
> -		break;
> -	case 2:
> +	case UART_IIR_FIFO_EAABLED_16550:

EAABLED :)?

> --- a/include/uapi/linux/serial_reg.h
> +++ b/include/uapi/linux/serial_reg.h
> @@ -44,6 +44,10 @@
>   #define UART_IIR_RX_TIMEOUT	0x0c /* OMAP RX Timeout interrupt */
>   #define UART_IIR_XOFF		0x10 /* OMAP XOFF/Special Character */
>   #define UART_IIR_CTS_RTS_DSR	0x20 /* OMAP CTS/RTS/DSR Change */
> +#define UART_IIR_FIFO_ENABLED	0xc0 /* FIFOs enabled / port type identification */
> +#define  UART_IIR_FIFO_ENABLED_8250	0x00	/* 8250: no FIFO */
> +#define  UART_IIR_FIFO_EAABLED_16550	0x80	/* 16550: (broken/unusable) FIFO */
> +#define  UART_IIR_FIFO_ENABLED_16550A	0xc0	/* 16550A: FIFO enabled */
>   
>   #define UART_FCR	2	/* Out: FIFO Control Register */
>   #define UART_FCR_ENABLE_FIFO	0x01 /* Enable the FIFO */

-- 
js
suse labs

[PATCH 5/6] serial: 8250: Define IIR 64 byte bit & cleanup related code

[Ilpo Järvinen]

16750 indicates 64 bytes FIFO with a IIR bit. Add define for it and
make related code more obvious.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_port.c | 14 +++++++++-----
 include/uapi/linux/serial_reg.h     |  1 +
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6d343b15bc58..764043931a99 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1050,11 +1050,12 @@ static void autoconfig_16550a(struct uart_8250_port *up)
 			serial_out(up, UART_LCR, 0);
 			serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO |
 				   UART_FCR7_64BYTE);
-			status1 = serial_in(up, UART_IIR) >> 5;
+			status1 = serial_in(up, UART_IIR) & (UART_IIR_64BYTE_FIFO |
+							     UART_IIR_FIFO_ENABLED);
 			serial_out(up, UART_FCR, 0);
 			serial_out(up, UART_LCR, 0);
 
-			if (status1 == 7)
+			if (status1 == (UART_IIR_64BYTE_FIFO | UART_IIR_FIFO_ENABLED))
 				up->port.type = PORT_16550A_FSL64;
 			else
 				DEBUG_AUTOCONF("Motorola 8xxx DUART ");
@@ -1122,17 +1123,20 @@ static void autoconfig_16550a(struct uart_8250_port *up)
 	 */
 	serial_out(up, UART_LCR, 0);
 	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
-	status1 = serial_in(up, UART_IIR) >> 5;
+	status1 = serial_in(up, UART_IIR) & (UART_IIR_64BYTE_FIFO | UART_IIR_FIFO_ENABLED);
 	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
+
 	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
 	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
-	status2 = serial_in(up, UART_IIR) >> 5;
+	status2 = serial_in(up, UART_IIR) & (UART_IIR_64BYTE_FIFO | UART_IIR_FIFO_ENABLED);
 	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO);
+
 	serial_out(up, UART_LCR, 0);
 
 	DEBUG_AUTOCONF("iir1=%d iir2=%d ", status1, status2);
 
-	if (status1 == 6 && status2 == 7) {
+	if (status1 == UART_IIR_FIFO_ENABLED_16550A &&
+	    status2 == (UART_IIR_64BYTE_FIFO | UART_IIR_FIFO_ENABLED_16550A)) {
 		up->port.type = PORT_16750;
 		up->capabilities |= UART_CAP_AFE | UART_CAP_SLEEP;
 		return;
diff --git a/include/uapi/linux/serial_reg.h b/include/uapi/linux/serial_reg.h
index a63c124b7fdc..21ae5b930c7b 100644
--- a/include/uapi/linux/serial_reg.h
+++ b/include/uapi/linux/serial_reg.h
@@ -44,6 +44,7 @@
 #define UART_IIR_RX_TIMEOUT	0x0c /* OMAP RX Timeout interrupt */
 #define UART_IIR_XOFF		0x10 /* OMAP XOFF/Special Character */
 #define UART_IIR_CTS_RTS_DSR	0x20 /* OMAP CTS/RTS/DSR Change */
+#define UART_IIR_64BYTE_FIFO	0x20 /* 16750 64 bytes FIFO */
 #define UART_IIR_FIFO_ENABLED	0xc0 /* FIFOs enabled / port type identification */
 #define  UART_IIR_FIFO_ENABLED_8250	0x00	/* 8250: no FIFO */
 #define  UART_IIR_FIFO_EAABLED_16550	0x80	/* 16550: (broken/unusable) FIFO */
-- 
2.30.2

[PATCH 6/6] serial: 8250_early: Convert literals to use defines

[Ilpo Järvinen]

Use existing defines for the serial register values in 8250_early.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_early.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c
index f271becfc46c..0ebde0ab8167 100644
--- a/drivers/tty/serial/8250/8250_early.c
+++ b/drivers/tty/serial/8250/8250_early.c
@@ -136,11 +136,11 @@ static void __init init_port(struct earlycon_device *device)
 	unsigned char c;
 	unsigned int ier;
 
-	serial8250_early_out(port, UART_LCR, 0x3);	/* 8n1 */
+	serial8250_early_out(port, UART_LCR, UART_LCR_WLEN8);		/* 8n1 */
 	ier = serial8250_early_in(port, UART_IER);
 	serial8250_early_out(port, UART_IER, ier & UART_IER_UUE); /* no interrupt */
 	serial8250_early_out(port, UART_FCR, 0);	/* no fifo */
-	serial8250_early_out(port, UART_MCR, 0x3);	/* DTR + RTS */
+	serial8250_early_out(port, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
 
 	if (port->uartclk) {
 		divisor = DIV_ROUND_CLOSEST(port->uartclk, 16 * device->baud);
-- 
2.30.2