* [PATCH v4 0/6] Cleanup AMBA PL011 driver
@ 2023-11-23 13:42 Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 1/6] tty: serial: amba: cleanup whitespace Théo Lebrun
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
Hi,
While adding upstream support to a new platform (Mobileye EyeQ5[1]) that
uses the AMBA PL011 driver, I took some time to look at the PL011
driver and ended up with a few patches that cleanup parts of it. The
line-diff is big mostly because of the checkpatch-fixing commits.
The driver hadn't received any love for quite some time. See commit
messages for more information.
We can thank Ilpo Järvinen for the V3 & V4. V4 fixes an inversed if
expression bug added in [PATCH V3 6/6].
[1]: https://lore.kernel.org/all/202310050726.GDpZbMDO-lkp@intel.com/T/
Have a nice day,
Théo Lebrun
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v4:
- Fix reverse if logic bug in [PATCH V3 6/6].
- Link to v3: https://lore.kernel.org/r/20231120-mbly-uart-v3-0-07ae35979f1f@bootlin.com
Changes in v3:
- Replace magic constants in linux/amba/serial.h by FIELD_PREP_CONST calls
- Refactor QDF2400 SoC erratum 44 handling out of probe in a new patch
- A nit in "unindent pl011_console_get_options function body"
- Link to v2: https://lore.kernel.org/r/20231116-mbly-uart-v2-0-863f665ce520@bootlin.com
Changes in v2:
- [PATCH 2]: add #include <linux/bits.h> in include/linux/amba/serial.h
as we use the BIT() macro.
- Move one whitespace cleanup from [PATCH 4/6] to [PATCH v2 3/5] where
it belongs.
- Drop [PATCH 6/6]: console will never have a word length of 5 or 6.
- Link to v1: https://lore.kernel.org/r/20231026-mbly-uart-v1-0-9258eea297d3@bootlin.com
---
Théo Lebrun (6):
tty: serial: amba: cleanup whitespace
tty: serial: amba: Use linux/bits.h macros for constant declarations
tty: serial: amba-pl011: cleanup driver
tty: serial: amba-pl011: replace TIOCMBIT macros by static functions
tty: serial: amba-pl011: unindent pl011_console_get_options function body
tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe
drivers/tty/serial/amba-pl011.c | 260 +++++++++++++++++++++-------------------
include/linux/amba/serial.h | 257 ++++++++++++++++++++-------------------
2 files changed, 265 insertions(+), 252 deletions(-)
---
base-commit: 82251bf0515a7c32121b0e833922708d4dd70da9
change-id: 20231023-mbly-uart-afcacbb98f8b
Best regards,
--
Théo Lebrun <theo.lebrun@bootlin.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/6] tty: serial: amba: cleanup whitespace
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
@ 2023-11-23 13:42 ` Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 2/6] tty: serial: amba: Use linux/bits.h macros for constant declarations Théo Lebrun
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
Fix whitespace in include/linux/amba/serial.h to match current kernel
coding standards. Fixes about:
- CHECK: spaces preferred around that '|' (ctx:VxV)
- ERROR: code indent should use tabs where possible
- WARNING: Unnecessary space before function pointer arguments
- WARNING: please, no spaces at the start of a line
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
include/linux/amba/serial.h | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/amba/serial.h b/include/linux/amba/serial.h
index a1307b58cc2c..27003ec52114 100644
--- a/include/linux/amba/serial.h
+++ b/include/linux/amba/serial.h
@@ -75,10 +75,10 @@
#define UART011_DR_PE (1 << 9)
#define UART011_DR_FE (1 << 8)
-#define UART01x_RSR_OE 0x08
-#define UART01x_RSR_BE 0x04
-#define UART01x_RSR_PE 0x02
-#define UART01x_RSR_FE 0x01
+#define UART01x_RSR_OE 0x08
+#define UART01x_RSR_BE 0x04
+#define UART01x_RSR_PE 0x02
+#define UART01x_RSR_FE 0x01
#define UART011_FR_RI 0x100
#define UART011_FR_TXFE 0x080
@@ -86,9 +86,9 @@
#define UART01x_FR_TXFF 0x020
#define UART01x_FR_RXFE 0x010
#define UART01x_FR_BUSY 0x008
-#define UART01x_FR_DCD 0x004
-#define UART01x_FR_DSR 0x002
-#define UART01x_FR_CTS 0x001
+#define UART01x_FR_DCD 0x004
+#define UART01x_FR_DSR 0x002
+#define UART01x_FR_CTS 0x001
#define UART01x_FR_TMSK (UART01x_FR_TXFF + UART01x_FR_BUSY)
/*
@@ -110,14 +110,14 @@
#define UART011_CR_TXE 0x0100 /* transmit enable */
#define UART011_CR_LBE 0x0080 /* loopback enable */
#define UART010_CR_RTIE 0x0040
-#define UART010_CR_TIE 0x0020
-#define UART010_CR_RIE 0x0010
+#define UART010_CR_TIE 0x0020
+#define UART010_CR_RIE 0x0010
#define UART010_CR_MSIE 0x0008
#define ST_UART011_CR_OVSFACT 0x0008 /* Oversampling factor */
#define UART01x_CR_IIRLP 0x0004 /* SIR low power mode */
#define UART01x_CR_SIREN 0x0002 /* SIR enable */
#define UART01x_CR_UARTEN 0x0001 /* UART enable */
-
+
#define UART011_LCRH_SPS 0x80
#define UART01x_LCRH_WLEN_8 0x60
#define UART01x_LCRH_WLEN_7 0x40
@@ -203,8 +203,8 @@
#define UART011_TXDMAE (1 << 1) /* enable transmit dma */
#define UART011_RXDMAE (1 << 0) /* enable receive dma */
-#define UART01x_RSR_ANY (UART01x_RSR_OE|UART01x_RSR_BE|UART01x_RSR_PE|UART01x_RSR_FE)
-#define UART01x_FR_MODEM_ANY (UART01x_FR_DCD|UART01x_FR_DSR|UART01x_FR_CTS)
+#define UART01x_RSR_ANY (UART01x_RSR_OE | UART01x_RSR_BE | UART01x_RSR_PE | UART01x_RSR_FE)
+#define UART01x_FR_MODEM_ANY (UART01x_FR_DCD | UART01x_FR_DSR | UART01x_FR_CTS)
#ifndef __ASSEMBLY__
struct amba_device; /* in uncompress this is included but amba/bus.h is not */
@@ -220,8 +220,8 @@ struct amba_pl011_data {
bool dma_rx_poll_enable;
unsigned int dma_rx_poll_rate;
unsigned int dma_rx_poll_timeout;
- void (*init) (void);
- void (*exit) (void);
+ void (*init)(void);
+ void (*exit)(void);
};
#endif
--
2.42.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/6] tty: serial: amba: Use linux/bits.h macros for constant declarations
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 1/6] tty: serial: amba: cleanup whitespace Théo Lebrun
@ 2023-11-23 13:42 ` Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 3/6] tty: serial: amba-pl011: cleanup driver Théo Lebrun
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
The driver uses bit shifts and hexadecimal expressions to declare
constants. Replace that with the BIT() & FIELD_PREP_CONST() macros to
clarify intent.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
include/linux/amba/serial.h | 247 ++++++++++++++++++++++----------------------
1 file changed, 126 insertions(+), 121 deletions(-)
diff --git a/include/linux/amba/serial.h b/include/linux/amba/serial.h
index 27003ec52114..d179f7161110 100644
--- a/include/linux/amba/serial.h
+++ b/include/linux/amba/serial.h
@@ -10,6 +10,7 @@
#ifndef ASM_ARM_HARDWARE_SERIAL_AMBA_H
#define ASM_ARM_HARDWARE_SERIAL_AMBA_H
+#include <linux/bits.h>
#include <linux/types.h>
/* -------------------------------------------------------------------------------
@@ -70,138 +71,142 @@
#define ZX_UART011_ICR 0x4c
#define ZX_UART011_DMACR 0x50
-#define UART011_DR_OE (1 << 11)
-#define UART011_DR_BE (1 << 10)
-#define UART011_DR_PE (1 << 9)
-#define UART011_DR_FE (1 << 8)
-
-#define UART01x_RSR_OE 0x08
-#define UART01x_RSR_BE 0x04
-#define UART01x_RSR_PE 0x02
-#define UART01x_RSR_FE 0x01
-
-#define UART011_FR_RI 0x100
-#define UART011_FR_TXFE 0x080
-#define UART011_FR_RXFF 0x040
-#define UART01x_FR_TXFF 0x020
-#define UART01x_FR_RXFE 0x010
-#define UART01x_FR_BUSY 0x008
-#define UART01x_FR_DCD 0x004
-#define UART01x_FR_DSR 0x002
-#define UART01x_FR_CTS 0x001
+#define UART011_DR_OE BIT(11)
+#define UART011_DR_BE BIT(10)
+#define UART011_DR_PE BIT(9)
+#define UART011_DR_FE BIT(8)
+
+#define UART01x_RSR_OE BIT(3)
+#define UART01x_RSR_BE BIT(2)
+#define UART01x_RSR_PE BIT(1)
+#define UART01x_RSR_FE BIT(0)
+
+#define UART011_FR_RI BIT(8)
+#define UART011_FR_TXFE BIT(7)
+#define UART011_FR_RXFF BIT(6)
+#define UART01x_FR_TXFF BIT(5)
+#define UART01x_FR_RXFE BIT(4)
+#define UART01x_FR_BUSY BIT(3)
+#define UART01x_FR_DCD BIT(2)
+#define UART01x_FR_DSR BIT(1)
+#define UART01x_FR_CTS BIT(0)
#define UART01x_FR_TMSK (UART01x_FR_TXFF + UART01x_FR_BUSY)
/*
* Some bits of Flag Register on ZTE device have different position from
* standard ones.
*/
-#define ZX_UART01x_FR_BUSY 0x100
-#define ZX_UART01x_FR_DSR 0x008
-#define ZX_UART01x_FR_CTS 0x002
-#define ZX_UART011_FR_RI 0x001
-
-#define UART011_CR_CTSEN 0x8000 /* CTS hardware flow control */
-#define UART011_CR_RTSEN 0x4000 /* RTS hardware flow control */
-#define UART011_CR_OUT2 0x2000 /* OUT2 */
-#define UART011_CR_OUT1 0x1000 /* OUT1 */
-#define UART011_CR_RTS 0x0800 /* RTS */
-#define UART011_CR_DTR 0x0400 /* DTR */
-#define UART011_CR_RXE 0x0200 /* receive enable */
-#define UART011_CR_TXE 0x0100 /* transmit enable */
-#define UART011_CR_LBE 0x0080 /* loopback enable */
-#define UART010_CR_RTIE 0x0040
-#define UART010_CR_TIE 0x0020
-#define UART010_CR_RIE 0x0010
-#define UART010_CR_MSIE 0x0008
-#define ST_UART011_CR_OVSFACT 0x0008 /* Oversampling factor */
-#define UART01x_CR_IIRLP 0x0004 /* SIR low power mode */
-#define UART01x_CR_SIREN 0x0002 /* SIR enable */
-#define UART01x_CR_UARTEN 0x0001 /* UART enable */
-
-#define UART011_LCRH_SPS 0x80
+#define ZX_UART01x_FR_BUSY BIT(8)
+#define ZX_UART01x_FR_DSR BIT(3)
+#define ZX_UART01x_FR_CTS BIT(1)
+#define ZX_UART011_FR_RI BIT(0)
+
+#define UART011_CR_CTSEN BIT(15) /* CTS hardware flow control */
+#define UART011_CR_RTSEN BIT(14) /* RTS hardware flow control */
+#define UART011_CR_OUT2 BIT(13) /* OUT2 */
+#define UART011_CR_OUT1 BIT(12) /* OUT1 */
+#define UART011_CR_RTS BIT(11) /* RTS */
+#define UART011_CR_DTR BIT(10) /* DTR */
+#define UART011_CR_RXE BIT(9) /* receive enable */
+#define UART011_CR_TXE BIT(8) /* transmit enable */
+#define UART011_CR_LBE BIT(7) /* loopback enable */
+#define UART010_CR_RTIE BIT(6)
+#define UART010_CR_TIE BIT(5)
+#define UART010_CR_RIE BIT(4)
+#define UART010_CR_MSIE BIT(3)
+#define ST_UART011_CR_OVSFACT BIT(3) /* Oversampling factor */
+#define UART01x_CR_IIRLP BIT(2) /* SIR low power mode */
+#define UART01x_CR_SIREN BIT(1) /* SIR enable */
+#define UART01x_CR_UARTEN BIT(0) /* UART enable */
+
+#define UART011_LCRH_SPS BIT(7)
#define UART01x_LCRH_WLEN_8 0x60
#define UART01x_LCRH_WLEN_7 0x40
#define UART01x_LCRH_WLEN_6 0x20
#define UART01x_LCRH_WLEN_5 0x00
-#define UART01x_LCRH_FEN 0x10
-#define UART01x_LCRH_STP2 0x08
-#define UART01x_LCRH_EPS 0x04
-#define UART01x_LCRH_PEN 0x02
-#define UART01x_LCRH_BRK 0x01
-
-#define ST_UART011_DMAWM_RX_1 (0 << 3)
-#define ST_UART011_DMAWM_RX_2 (1 << 3)
-#define ST_UART011_DMAWM_RX_4 (2 << 3)
-#define ST_UART011_DMAWM_RX_8 (3 << 3)
-#define ST_UART011_DMAWM_RX_16 (4 << 3)
-#define ST_UART011_DMAWM_RX_32 (5 << 3)
-#define ST_UART011_DMAWM_RX_48 (6 << 3)
-#define ST_UART011_DMAWM_TX_1 0
-#define ST_UART011_DMAWM_TX_2 1
-#define ST_UART011_DMAWM_TX_4 2
-#define ST_UART011_DMAWM_TX_8 3
-#define ST_UART011_DMAWM_TX_16 4
-#define ST_UART011_DMAWM_TX_32 5
-#define ST_UART011_DMAWM_TX_48 6
-
-#define UART010_IIR_RTIS 0x08
-#define UART010_IIR_TIS 0x04
-#define UART010_IIR_RIS 0x02
-#define UART010_IIR_MIS 0x01
-
-#define UART011_IFLS_RX1_8 (0 << 3)
-#define UART011_IFLS_RX2_8 (1 << 3)
-#define UART011_IFLS_RX4_8 (2 << 3)
-#define UART011_IFLS_RX6_8 (3 << 3)
-#define UART011_IFLS_RX7_8 (4 << 3)
-#define UART011_IFLS_TX1_8 (0 << 0)
-#define UART011_IFLS_TX2_8 (1 << 0)
-#define UART011_IFLS_TX4_8 (2 << 0)
-#define UART011_IFLS_TX6_8 (3 << 0)
-#define UART011_IFLS_TX7_8 (4 << 0)
+#define UART01x_LCRH_FEN BIT(4)
+#define UART01x_LCRH_STP2 BIT(3)
+#define UART01x_LCRH_EPS BIT(2)
+#define UART01x_LCRH_PEN BIT(1)
+#define UART01x_LCRH_BRK BIT(0)
+
+#define ST_UART011_DMAWM_RX GENMASK(5, 3)
+#define ST_UART011_DMAWM_RX_1 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 0)
+#define ST_UART011_DMAWM_RX_2 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 1)
+#define ST_UART011_DMAWM_RX_4 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 2)
+#define ST_UART011_DMAWM_RX_8 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 3)
+#define ST_UART011_DMAWM_RX_16 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 4)
+#define ST_UART011_DMAWM_RX_32 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 5)
+#define ST_UART011_DMAWM_RX_48 FIELD_PREP_CONST(ST_UART011_DMAWM_RX, 6)
+#define ST_UART011_DMAWM_TX GENMASK(2, 0)
+#define ST_UART011_DMAWM_TX_1 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 0)
+#define ST_UART011_DMAWM_TX_2 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 1)
+#define ST_UART011_DMAWM_TX_4 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 2)
+#define ST_UART011_DMAWM_TX_8 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 3)
+#define ST_UART011_DMAWM_TX_16 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 4)
+#define ST_UART011_DMAWM_TX_32 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 5)
+#define ST_UART011_DMAWM_TX_48 FIELD_PREP_CONST(ST_UART011_DMAWM_TX, 6)
+
+#define UART010_IIR_RTIS BIT(3)
+#define UART010_IIR_TIS BIT(2)
+#define UART010_IIR_RIS BIT(1)
+#define UART010_IIR_MIS BIT(0)
+
+#define UART011_IFLS_RXIFLSEL GENMASK(5, 3)
+#define UART011_IFLS_RX1_8 FIELD_PREP_CONST(UART011_IFLS_RXIFLSEL, 0)
+#define UART011_IFLS_RX2_8 FIELD_PREP_CONST(UART011_IFLS_RXIFLSEL, 1)
+#define UART011_IFLS_RX4_8 FIELD_PREP_CONST(UART011_IFLS_RXIFLSEL, 2)
+#define UART011_IFLS_RX6_8 FIELD_PREP_CONST(UART011_IFLS_RXIFLSEL, 3)
+#define UART011_IFLS_RX7_8 FIELD_PREP_CONST(UART011_IFLS_RXIFLSEL, 4)
+#define UART011_IFLS_TXIFLSEL GENMASK(2, 0)
+#define UART011_IFLS_TX1_8 FIELD_PREP_CONST(UART011_IFLS_TXIFLSEL, 0)
+#define UART011_IFLS_TX2_8 FIELD_PREP_CONST(UART011_IFLS_TXIFLSEL, 1)
+#define UART011_IFLS_TX4_8 FIELD_PREP_CONST(UART011_IFLS_TXIFLSEL, 2)
+#define UART011_IFLS_TX6_8 FIELD_PREP_CONST(UART011_IFLS_TXIFLSEL, 3)
+#define UART011_IFLS_TX7_8 FIELD_PREP_CONST(UART011_IFLS_TXIFLSEL, 4)
/* special values for ST vendor with deeper fifo */
-#define UART011_IFLS_RX_HALF (5 << 3)
-#define UART011_IFLS_TX_HALF (5 << 0)
-
-#define UART011_OEIM (1 << 10) /* overrun error interrupt mask */
-#define UART011_BEIM (1 << 9) /* break error interrupt mask */
-#define UART011_PEIM (1 << 8) /* parity error interrupt mask */
-#define UART011_FEIM (1 << 7) /* framing error interrupt mask */
-#define UART011_RTIM (1 << 6) /* receive timeout interrupt mask */
-#define UART011_TXIM (1 << 5) /* transmit interrupt mask */
-#define UART011_RXIM (1 << 4) /* receive interrupt mask */
-#define UART011_DSRMIM (1 << 3) /* DSR interrupt mask */
-#define UART011_DCDMIM (1 << 2) /* DCD interrupt mask */
-#define UART011_CTSMIM (1 << 1) /* CTS interrupt mask */
-#define UART011_RIMIM (1 << 0) /* RI interrupt mask */
-
-#define UART011_OEIS (1 << 10) /* overrun error interrupt status */
-#define UART011_BEIS (1 << 9) /* break error interrupt status */
-#define UART011_PEIS (1 << 8) /* parity error interrupt status */
-#define UART011_FEIS (1 << 7) /* framing error interrupt status */
-#define UART011_RTIS (1 << 6) /* receive timeout interrupt status */
-#define UART011_TXIS (1 << 5) /* transmit interrupt status */
-#define UART011_RXIS (1 << 4) /* receive interrupt status */
-#define UART011_DSRMIS (1 << 3) /* DSR interrupt status */
-#define UART011_DCDMIS (1 << 2) /* DCD interrupt status */
-#define UART011_CTSMIS (1 << 1) /* CTS interrupt status */
-#define UART011_RIMIS (1 << 0) /* RI interrupt status */
-
-#define UART011_OEIC (1 << 10) /* overrun error interrupt clear */
-#define UART011_BEIC (1 << 9) /* break error interrupt clear */
-#define UART011_PEIC (1 << 8) /* parity error interrupt clear */
-#define UART011_FEIC (1 << 7) /* framing error interrupt clear */
-#define UART011_RTIC (1 << 6) /* receive timeout interrupt clear */
-#define UART011_TXIC (1 << 5) /* transmit interrupt clear */
-#define UART011_RXIC (1 << 4) /* receive interrupt clear */
-#define UART011_DSRMIC (1 << 3) /* DSR interrupt clear */
-#define UART011_DCDMIC (1 << 2) /* DCD interrupt clear */
-#define UART011_CTSMIC (1 << 1) /* CTS interrupt clear */
-#define UART011_RIMIC (1 << 0) /* RI interrupt clear */
-
-#define UART011_DMAONERR (1 << 2) /* disable dma on error */
-#define UART011_TXDMAE (1 << 1) /* enable transmit dma */
-#define UART011_RXDMAE (1 << 0) /* enable receive dma */
+#define UART011_IFLS_RX_HALF FIELD_PREP_CONST(UART011_IFLS_RXIFLSEL, 5)
+#define UART011_IFLS_TX_HALF FIELD_PREP_CONST(UART011_IFLS_TXIFLSEL, 5)
+
+#define UART011_OEIM BIT(10) /* overrun error interrupt mask */
+#define UART011_BEIM BIT(9) /* break error interrupt mask */
+#define UART011_PEIM BIT(8) /* parity error interrupt mask */
+#define UART011_FEIM BIT(7) /* framing error interrupt mask */
+#define UART011_RTIM BIT(6) /* receive timeout interrupt mask */
+#define UART011_TXIM BIT(5) /* transmit interrupt mask */
+#define UART011_RXIM BIT(4) /* receive interrupt mask */
+#define UART011_DSRMIM BIT(3) /* DSR interrupt mask */
+#define UART011_DCDMIM BIT(2) /* DCD interrupt mask */
+#define UART011_CTSMIM BIT(1) /* CTS interrupt mask */
+#define UART011_RIMIM BIT(0) /* RI interrupt mask */
+
+#define UART011_OEIS BIT(10) /* overrun error interrupt status */
+#define UART011_BEIS BIT(9) /* break error interrupt status */
+#define UART011_PEIS BIT(8) /* parity error interrupt status */
+#define UART011_FEIS BIT(7) /* framing error interrupt status */
+#define UART011_RTIS BIT(6) /* receive timeout interrupt status */
+#define UART011_TXIS BIT(5) /* transmit interrupt status */
+#define UART011_RXIS BIT(4) /* receive interrupt status */
+#define UART011_DSRMIS BIT(3) /* DSR interrupt status */
+#define UART011_DCDMIS BIT(2) /* DCD interrupt status */
+#define UART011_CTSMIS BIT(1) /* CTS interrupt status */
+#define UART011_RIMIS BIT(0) /* RI interrupt status */
+
+#define UART011_OEIC BIT(10) /* overrun error interrupt clear */
+#define UART011_BEIC BIT(9) /* break error interrupt clear */
+#define UART011_PEIC BIT(8) /* parity error interrupt clear */
+#define UART011_FEIC BIT(7) /* framing error interrupt clear */
+#define UART011_RTIC BIT(6) /* receive timeout interrupt clear */
+#define UART011_TXIC BIT(5) /* transmit interrupt clear */
+#define UART011_RXIC BIT(4) /* receive interrupt clear */
+#define UART011_DSRMIC BIT(3) /* DSR interrupt clear */
+#define UART011_DCDMIC BIT(2) /* DCD interrupt clear */
+#define UART011_CTSMIC BIT(1) /* CTS interrupt clear */
+#define UART011_RIMIC BIT(0) /* RI interrupt clear */
+
+#define UART011_DMAONERR BIT(2) /* disable dma on error */
+#define UART011_TXDMAE BIT(1) /* enable transmit dma */
+#define UART011_RXDMAE BIT(0) /* enable receive dma */
#define UART01x_RSR_ANY (UART01x_RSR_OE | UART01x_RSR_BE | UART01x_RSR_PE | UART01x_RSR_FE)
#define UART01x_FR_MODEM_ANY (UART01x_FR_DCD | UART01x_FR_DSR | UART01x_FR_CTS)
--
2.42.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/6] tty: serial: amba-pl011: cleanup driver
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 1/6] tty: serial: amba: cleanup whitespace Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 2/6] tty: serial: amba: Use linux/bits.h macros for constant declarations Théo Lebrun
@ 2023-11-23 13:42 ` Théo Lebrun
2023-11-23 19:35 ` Greg Kroah-Hartman
2023-11-23 13:42 ` [PATCH v4 4/6] tty: serial: amba-pl011: replace TIOCMBIT macros by static functions Théo Lebrun
` (3 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
Follow recommandations from:
$ ./scripts/checkpatch.pl --strict --file \
drivers/tty/serial/amba-pl011.c
It does NOT fix alerts relative to TIOCMBIT which will be dealt with in
another patch. Fixes following alerts:
CHECK: Alignment should match open parenthesis
CHECK: Blank lines aren't necessary after an open brace '{'
CHECK: Comparison to NULL could be written [...]
CHECK: Lines should not end with a '('
CHECK: Please don't use multiple blank lines
CHECK: Please use a blank line after function/struct/union/enum declarations
CHECK: Prefer using the BIT macro
CHECK: Unbalanced braces around else statement
CHECK: Unnecessary parentheses around [...]
CHECK: braces {} should be used on all arms of this statement
CHECK: spaces preferred around that '/' (ctx:VxV)
CHECK: spaces preferred around that '|' (ctx:VxV)
ERROR: do not initialise statics to false
WARNING: Comparisons should place the constant on the right side of the test
WARNING: Possible unnecessary 'out of memory' message
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
WARNING: quoted string split across lines
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/tty/serial/amba-pl011.c | 147 +++++++++++++++++++---------------------
1 file changed, 71 insertions(+), 76 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3dc9b0fcab1c..3beac358909f 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -50,8 +50,8 @@
#define AMBA_ISR_PASS_LIMIT 256
-#define UART_DR_ERROR (UART011_DR_OE|UART011_DR_BE|UART011_DR_PE|UART011_DR_FE)
-#define UART_DUMMY_DR_RX (1 << 16)
+#define UART_DR_ERROR (UART011_DR_OE | UART011_DR_BE | UART011_DR_PE | UART011_DR_FE)
+#define UART_DUMMY_DR_RX BIT(16)
enum {
REG_DR,
@@ -125,7 +125,7 @@ static unsigned int get_fifosize_arm(struct amba_device *dev)
static struct vendor_data vendor_arm = {
.reg_offset = pl011_std_offsets,
- .ifls = UART011_IFLS_RX4_8|UART011_IFLS_TX4_8,
+ .ifls = UART011_IFLS_RX4_8 | UART011_IFLS_TX4_8,
.fr_busy = UART01x_FR_BUSY,
.fr_dsr = UART01x_FR_DSR,
.fr_cts = UART01x_FR_CTS,
@@ -203,7 +203,7 @@ static unsigned int get_fifosize_st(struct amba_device *dev)
static struct vendor_data vendor_st = {
.reg_offset = pl011_st_offsets,
- .ifls = UART011_IFLS_RX_HALF|UART011_IFLS_TX_HALF,
+ .ifls = UART011_IFLS_RX_HALF | UART011_IFLS_TX_HALF,
.fr_busy = UART01x_FR_BUSY,
.fr_dsr = UART01x_FR_DSR,
.fr_cts = UART01x_FR_CTS,
@@ -275,13 +275,13 @@ struct uart_amba_port {
static unsigned int pl011_tx_empty(struct uart_port *port);
static unsigned int pl011_reg_to_offset(const struct uart_amba_port *uap,
- unsigned int reg)
+ unsigned int reg)
{
return uap->reg_offset[reg];
}
static unsigned int pl011_read(const struct uart_amba_port *uap,
- unsigned int reg)
+ unsigned int reg)
{
void __iomem *addr = uap->port.membase + pl011_reg_to_offset(uap, reg);
@@ -290,7 +290,7 @@ static unsigned int pl011_read(const struct uart_amba_port *uap,
}
static void pl011_write(unsigned int val, const struct uart_amba_port *uap,
- unsigned int reg)
+ unsigned int reg)
{
void __iomem *addr = uap->port.membase + pl011_reg_to_offset(uap, reg);
@@ -328,10 +328,12 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
uap->port.icount.brk++;
if (uart_handle_break(&uap->port))
continue;
- } else if (ch & UART011_DR_PE)
+ } else if (ch & UART011_DR_PE) {
uap->port.icount.parity++;
- else if (ch & UART011_DR_FE)
+ } else if (ch & UART011_DR_FE) {
uap->port.icount.frame++;
+ }
+
if (ch & UART011_DR_OE)
uap->port.icount.overrun++;
@@ -356,7 +358,6 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
return fifotaken;
}
-
/*
* All the DMA operation mode stuff goes inside this ifdef.
* This assumes that you have a generic DMA device interface,
@@ -367,18 +368,18 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap)
#define PL011_DMA_BUFFER_SIZE PAGE_SIZE
static int pl011_sgbuf_init(struct dma_chan *chan, struct pl011_sgbuf *sg,
- enum dma_data_direction dir)
+ enum dma_data_direction dir)
{
dma_addr_t dma_addr;
sg->buf = dma_alloc_coherent(chan->device->dev,
- PL011_DMA_BUFFER_SIZE, &dma_addr, GFP_KERNEL);
+ PL011_DMA_BUFFER_SIZE, &dma_addr, GFP_KERNEL);
if (!sg->buf)
return -ENOMEM;
sg_init_table(&sg->sg, 1);
sg_set_page(&sg->sg, phys_to_page(dma_addr),
- PL011_DMA_BUFFER_SIZE, offset_in_page(dma_addr));
+ PL011_DMA_BUFFER_SIZE, offset_in_page(dma_addr));
sg_dma_address(&sg->sg) = dma_addr;
sg_dma_len(&sg->sg) = PL011_DMA_BUFFER_SIZE;
@@ -386,12 +387,11 @@ static int pl011_sgbuf_init(struct dma_chan *chan, struct pl011_sgbuf *sg,
}
static void pl011_sgbuf_free(struct dma_chan *chan, struct pl011_sgbuf *sg,
- enum dma_data_direction dir)
+ enum dma_data_direction dir)
{
if (sg->buf) {
- dma_free_coherent(chan->device->dev,
- PL011_DMA_BUFFER_SIZE, sg->buf,
- sg_dma_address(&sg->sg));
+ dma_free_coherent(chan->device->dev, PL011_DMA_BUFFER_SIZE,
+ sg->buf, sg_dma_address(&sg->sg));
}
}
@@ -430,7 +430,7 @@ static void pl011_dma_probe(struct uart_amba_port *uap)
dma_cap_set(DMA_SLAVE, mask);
chan = dma_request_channel(mask, plat->dma_filter,
- plat->dma_tx_param);
+ plat->dma_tx_param);
if (!chan) {
dev_err(uap->port.dev, "no TX DMA channel!\n");
return;
@@ -471,12 +471,12 @@ static void pl011_dma_probe(struct uart_amba_port *uap)
* If the controller does, check for suitable residue processing
* otherwise assime all is well.
*/
- if (0 == dma_get_slave_caps(chan, &caps)) {
+ if (dma_get_slave_caps(chan, &caps) == 0) {
if (caps.residue_granularity ==
DMA_RESIDUE_GRANULARITY_DESCRIPTOR) {
dma_release_channel(chan);
dev_info(uap->port.dev,
- "RX DMA disabled - no residue processing\n");
+ "RX DMA disabled - no residue processing\n");
return;
}
}
@@ -505,18 +505,16 @@ static void pl011_dma_probe(struct uart_amba_port *uap)
else
uap->dmarx.poll_timeout = 3000;
} else if (!plat && dev->of_node) {
- uap->dmarx.auto_poll_rate = of_property_read_bool(
- dev->of_node, "auto-poll");
+ uap->dmarx.auto_poll_rate =
+ of_property_read_bool(dev->of_node, "auto-poll");
if (uap->dmarx.auto_poll_rate) {
u32 x;
- if (0 == of_property_read_u32(dev->of_node,
- "poll-rate-ms", &x))
+ if (of_property_read_u32(dev->of_node, "poll-rate-ms", &x) == 0)
uap->dmarx.poll_rate = x;
else
uap->dmarx.poll_rate = 100;
- if (0 == of_property_read_u32(dev->of_node,
- "poll-timeout-ms", &x))
+ if (of_property_read_u32(dev->of_node, "poll-timeout-ms", &x) == 0)
uap->dmarx.poll_timeout = x;
else
uap->dmarx.poll_timeout = 3000;
@@ -624,9 +622,9 @@ static int pl011_dma_tx_refill(struct uart_amba_port *uap)
if (count > PL011_DMA_BUFFER_SIZE)
count = PL011_DMA_BUFFER_SIZE;
- if (xmit->tail < xmit->head)
+ if (xmit->tail < xmit->head) {
memcpy(&dmatx->buf[0], &xmit->buf[xmit->tail], count);
- else {
+ } else {
size_t first = UART_XMIT_SIZE - xmit->tail;
size_t second;
@@ -648,7 +646,7 @@ static int pl011_dma_tx_refill(struct uart_amba_port *uap)
}
desc = dmaengine_prep_slave_sg(chan, &dmatx->sg, 1, DMA_MEM_TO_DEV,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
if (!desc) {
dma_unmap_sg(dma_dev->dev, &dmatx->sg, 1, DMA_TO_DEVICE);
uap->dmatx.queued = false;
@@ -759,8 +757,9 @@ static inline bool pl011_dma_tx_start(struct uart_amba_port *uap)
if (pl011_dma_tx_refill(uap) > 0) {
uap->im &= ~UART011_TXIM;
pl011_write(uap->im, uap, REG_IMSC);
- } else
+ } else {
ret = false;
+ }
} else if (!(uap->dmacr & UART011_TXDMAE)) {
uap->dmacr |= UART011_TXDMAE;
pl011_write(uap->dmacr, uap, REG_DMACR);
@@ -836,9 +835,8 @@ static int pl011_dma_rx_trigger_dma(struct uart_amba_port *uap)
/* Start the RX DMA job */
sgbuf = uap->dmarx.use_buf_b ?
&uap->dmarx.sgbuf_b : &uap->dmarx.sgbuf_a;
- desc = dmaengine_prep_slave_sg(rxchan, &sgbuf->sg, 1,
- DMA_DEV_TO_MEM,
- DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ desc = dmaengine_prep_slave_sg(rxchan, &sgbuf->sg, 1, DMA_DEV_TO_MEM,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
/*
* If the DMA engine is busy and cannot prepare a
* channel, no big deal, the driver will fall back
@@ -894,14 +892,13 @@ static void pl011_dma_rx_chars(struct uart_amba_port *uap,
/* Pick the remain data from the DMA */
if (pending) {
-
/*
* First take all chars in the DMA pipe, then look in the FIFO.
* Note that tty_insert_flip_buf() tries to take as many chars
* as it can.
*/
dma_count = tty_insert_flip_string(port, sgbuf->buf + dmataken,
- pending);
+ pending);
uap->port.icount.rx += dma_count;
if (dma_count < pending)
@@ -983,8 +980,8 @@ static void pl011_dma_rx_irq(struct uart_amba_port *uap)
/* Switch buffer & re-trigger DMA job */
dmarx->use_buf_b = !dmarx->use_buf_b;
if (pl011_dma_rx_trigger_dma(uap)) {
- dev_dbg(uap->port.dev, "could not retrigger RX DMA job "
- "fall back to interrupt mode\n");
+ dev_dbg(uap->port.dev,
+ "could not retrigger RX DMA job fall back to interrupt mode\n");
uap->im |= UART011_RXIM;
pl011_write(uap->im, uap, REG_IMSC);
}
@@ -1031,8 +1028,8 @@ static void pl011_dma_rx_callback(void *data)
* get some IRQ immediately from RX.
*/
if (ret) {
- dev_dbg(uap->port.dev, "could not retrigger RX DMA job "
- "fall back to interrupt mode\n");
+ dev_dbg(uap->port.dev,
+ "could not retrigger RX DMA job fall back to interrupt mode\n");
uap->im |= UART011_RXIM;
pl011_write(uap->im, uap, REG_IMSC);
}
@@ -1077,7 +1074,7 @@ static void pl011_dma_rx_poll(struct timer_list *t)
dmataken = sgbuf->sg.length - dmarx->last_residue;
size = dmarx->last_residue - state.residue;
dma_count = tty_insert_flip_string(port, sgbuf->buf + dmataken,
- size);
+ size);
if (dma_count == size)
dmarx->last_residue = state.residue;
dmarx->last_jiffies = jiffies;
@@ -1090,7 +1087,6 @@ static void pl011_dma_rx_poll(struct timer_list *t)
*/
if (jiffies_to_msecs(jiffies - dmarx->last_jiffies)
> uap->dmarx.poll_timeout) {
-
spin_lock_irqsave(&uap->port.lock, flags);
pl011_dma_rx_stop(uap);
uap->im |= UART011_RXIM;
@@ -1102,7 +1098,7 @@ static void pl011_dma_rx_poll(struct timer_list *t)
del_timer(&uap->dmarx.timer);
} else {
mod_timer(&uap->dmarx.timer,
- jiffies + msecs_to_jiffies(uap->dmarx.poll_rate));
+ jiffies + msecs_to_jiffies(uap->dmarx.poll_rate));
}
}
@@ -1118,7 +1114,6 @@ static void pl011_dma_startup(struct uart_amba_port *uap)
uap->dmatx.buf = kmalloc(PL011_DMA_BUFFER_SIZE, GFP_KERNEL | __GFP_DMA);
if (!uap->dmatx.buf) {
- dev_err(uap->port.dev, "no memory for DMA TX buffer\n");
uap->port.fifosize = uap->fifosize;
return;
}
@@ -1169,13 +1164,12 @@ static void pl011_dma_startup(struct uart_amba_port *uap)
if (uap->using_rx_dma) {
if (pl011_dma_rx_trigger_dma(uap))
- dev_dbg(uap->port.dev, "could not trigger initial "
- "RX DMA job, fall back to interrupt mode\n");
+ dev_dbg(uap->port.dev,
+ "could not trigger initial RX DMA job, fall back to interrupt mode\n");
if (uap->dmarx.poll_rate) {
timer_setup(&uap->dmarx.timer, pl011_dma_rx_poll, 0);
mod_timer(&uap->dmarx.timer,
- jiffies +
- msecs_to_jiffies(uap->dmarx.poll_rate));
+ jiffies + msecs_to_jiffies(uap->dmarx.poll_rate));
uap->dmarx.last_residue = PL011_DMA_BUFFER_SIZE;
uap->dmarx.last_jiffies = jiffies;
}
@@ -1363,8 +1357,8 @@ static void pl011_stop_rx(struct uart_port *port)
struct uart_amba_port *uap =
container_of(port, struct uart_amba_port, port);
- uap->im &= ~(UART011_RXIM|UART011_RTIM|UART011_FEIM|
- UART011_PEIM|UART011_BEIM|UART011_OEIM);
+ uap->im &= ~(UART011_RXIM | UART011_RTIM | UART011_FEIM |
+ UART011_PEIM | UART011_BEIM | UART011_OEIM);
pl011_write(uap->im, uap, REG_IMSC);
pl011_dma_rx_stop(uap);
@@ -1384,7 +1378,7 @@ static void pl011_enable_ms(struct uart_port *port)
struct uart_amba_port *uap =
container_of(port, struct uart_amba_port, port);
- uap->im |= UART011_RIMIM|UART011_CTSMIM|UART011_DCDMIM|UART011_DSRMIM;
+ uap->im |= UART011_RIMIM | UART011_CTSMIM | UART011_DCDMIM | UART011_DSRMIM;
pl011_write(uap->im, uap, REG_IMSC);
}
@@ -1402,8 +1396,8 @@ __acquires(&uap->port.lock)
*/
if (pl011_dma_rx_available(uap)) {
if (pl011_dma_rx_trigger_dma(uap)) {
- dev_dbg(uap->port.dev, "could not trigger RX DMA job "
- "fall back to interrupt mode again\n");
+ dev_dbg(uap->port.dev,
+ "could not trigger RX DMA job fall back to interrupt mode again\n");
uap->im |= UART011_RXIM;
pl011_write(uap->im, uap, REG_IMSC);
} else {
@@ -1413,8 +1407,7 @@ __acquires(&uap->port.lock)
uap->dmarx.last_jiffies = jiffies;
uap->dmarx.last_residue = PL011_DMA_BUFFER_SIZE;
mod_timer(&uap->dmarx.timer,
- jiffies +
- msecs_to_jiffies(uap->dmarx.poll_rate));
+ jiffies + msecs_to_jiffies(uap->dmarx.poll_rate));
}
#endif
}
@@ -1561,18 +1554,17 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
do {
check_apply_cts_event_workaround(uap);
- pl011_write(status & ~(UART011_TXIS|UART011_RTIS|
- UART011_RXIS),
+ pl011_write(status & ~(UART011_TXIS | UART011_RTIS | UART011_RXIS),
uap, REG_ICR);
- if (status & (UART011_RTIS|UART011_RXIS)) {
+ if (status & (UART011_RTIS | UART011_RXIS)) {
if (pl011_dma_rx_running(uap))
pl011_dma_rx_irq(uap);
else
pl011_rx_chars(uap);
}
- if (status & (UART011_DSRMIS|UART011_DCDMIS|
- UART011_CTSMIS|UART011_RIMIS))
+ if (status & (UART011_DSRMIS | UART011_DCDMIS |
+ UART011_CTSMIS | UART011_RIMIS))
pl011_modem_status(uap);
if (status & UART011_TXIS)
pl011_tx_chars(uap, true);
@@ -1712,7 +1704,7 @@ static int pl011_get_poll_char(struct uart_port *port)
}
static void pl011_put_poll_char(struct uart_port *port,
- unsigned char ch)
+ unsigned char ch)
{
struct uart_amba_port *uap =
container_of(port, struct uart_amba_port, port);
@@ -1914,13 +1906,13 @@ static int sbsa_uart_startup(struct uart_port *port)
}
static void pl011_shutdown_channel(struct uart_amba_port *uap,
- unsigned int lcrh)
+ unsigned int lcrh)
{
- unsigned long val;
+ unsigned long val;
- val = pl011_read(uap, lcrh);
- val &= ~(UART01x_LCRH_BRK | UART01x_LCRH_FEN);
- pl011_write(val, uap, lcrh);
+ val = pl011_read(uap, lcrh);
+ val &= ~(UART01x_LCRH_BRK | UART01x_LCRH_FEN);
+ pl011_write(val, uap, lcrh);
}
/*
@@ -2069,7 +2061,7 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
uap->dmarx.poll_rate = DIV_ROUND_UP(10000000, baud);
#endif
- if (baud > port->uartclk/16)
+ if (baud > port->uartclk / 16)
quot = DIV_ROUND_CLOSEST(port->uartclk * 8, baud);
else
quot = DIV_ROUND_CLOSEST(port->uartclk * 4, baud);
@@ -2151,9 +2143,9 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
* else we see data corruption.
*/
if (uap->vendor->oversampling) {
- if ((baud >= 3000000) && (baud < 3250000) && (quot > 1))
+ if (baud >= 3000000 && baud < 3250000 && quot > 1)
quot -= 1;
- else if ((baud > 3250000) && (quot > 2))
+ else if (baud > 3250000 && quot > 2)
quot -= 2;
}
/* Set baud rate */
@@ -2222,13 +2214,14 @@ static void pl011_config_port(struct uart_port *port, int flags)
static int pl011_verify_port(struct uart_port *port, struct serial_struct *ser)
{
int ret = 0;
+
if (ser->type != PORT_UNKNOWN && ser->type != PORT_AMBA)
ret = -EINVAL;
if (ser->irq < 0 || ser->irq >= nr_irqs)
ret = -EINVAL;
if (ser->baud_base < 9600)
ret = -EINVAL;
- if (port->mapbase != (unsigned long) ser->iomem_base)
+ if (port->mapbase != (unsigned long)ser->iomem_base)
ret = -EINVAL;
return ret;
}
@@ -2537,7 +2530,7 @@ static void qdf2400_e44_putc(struct uart_port *port, unsigned char c)
cpu_relax();
}
-static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned n)
+static void qdf2400_e44_early_write(struct console *con, const char *s, unsigned int n)
{
struct earlycon_device *dev = con->data;
@@ -2556,7 +2549,7 @@ static void pl011_putc(struct uart_port *port, unsigned char c)
cpu_relax();
}
-static void pl011_early_write(struct console *con, const char *s, unsigned n)
+static void pl011_early_write(struct console *con, const char *s, unsigned int n)
{
struct earlycon_device *dev = con->data;
@@ -2617,6 +2610,7 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
return 0;
}
+
OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
OF_EARLYCON_DECLARE(pl011, "arm,sbsa-uart", pl011_early_console_setup);
@@ -2640,6 +2634,7 @@ qdf2400_e44_early_console_setup(struct earlycon_device *device,
device->con->write = qdf2400_e44_early_write;
return 0;
}
+
EARLYCON_DECLARE(qdf2400_e44, qdf2400_e44_early_console_setup);
#else
@@ -2659,8 +2654,8 @@ static struct uart_driver amba_reg = {
static int pl011_probe_dt_alias(int index, struct device *dev)
{
struct device_node *np;
- static bool seen_dev_with_alias = false;
- static bool seen_dev_without_alias = false;
+ static bool seen_dev_with_alias;
+ static bool seen_dev_without_alias;
int ret = index;
if (!IS_ENABLED(CONFIG_OF))
@@ -2676,7 +2671,7 @@ static int pl011_probe_dt_alias(int index, struct device *dev)
ret = index;
} else {
seen_dev_with_alias = true;
- if (ret >= ARRAY_SIZE(amba_ports) || amba_ports[ret] != NULL) {
+ if (ret >= ARRAY_SIZE(amba_ports) || amba_ports[ret]) {
dev_warn(dev, "requested serial port %d not available.\n", ret);
ret = index;
}
@@ -2710,7 +2705,7 @@ static int pl011_find_free_port(void)
int i;
for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
- if (amba_ports[i] == NULL)
+ if (!amba_ports[i])
return i;
return -EBUSY;
@@ -3002,7 +2997,7 @@ static struct amba_driver pl011_driver = {
static int __init pl011_init(void)
{
- printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
+ pr_info("Serial: AMBA PL011 UART driver\n");
if (platform_driver_register(&arm_sbsa_uart_platform_driver))
pr_warn("could not register SBSA UART platform driver\n");
--
2.42.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 4/6] tty: serial: amba-pl011: replace TIOCMBIT macros by static functions
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
` (2 preceding siblings ...)
2023-11-23 13:42 ` [PATCH v4 3/6] tty: serial: amba-pl011: cleanup driver Théo Lebrun
@ 2023-11-23 13:42 ` Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 5/6] tty: serial: amba-pl011: unindent pl011_console_get_options function body Théo Lebrun
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
The driver uses two TIOCMBIT macros inside pl011_{get,set}_mctrl to
simplify the logic. Those look scary to checkpatch because they contain
ifs without do-while loops.
Avoid the macros by creating small equivalent static functions; that
lets the compiler do its type checking & avoids checkpatch errors.
For the second instance __assign_bit is not usable because it deals with
unsigned long pointers whereas we have an unsigned int in
pl011_set_mctrl.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/tty/serial/amba-pl011.c | 45 ++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 3beac358909f..cf9aefe2fd28 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -1594,6 +1594,12 @@ static unsigned int pl011_tx_empty(struct uart_port *port)
0 : TIOCSER_TEMT;
}
+static void pl011_maybe_set_bit(bool cond, unsigned int *ptr, unsigned int mask)
+{
+ if (cond)
+ *ptr |= mask;
+}
+
static unsigned int pl011_get_mctrl(struct uart_port *port)
{
struct uart_amba_port *uap =
@@ -1601,18 +1607,22 @@ static unsigned int pl011_get_mctrl(struct uart_port *port)
unsigned int result = 0;
unsigned int status = pl011_read(uap, REG_FR);
-#define TIOCMBIT(uartbit, tiocmbit) \
- if (status & uartbit) \
- result |= tiocmbit
+ pl011_maybe_set_bit(status & UART01x_FR_DCD, &result, TIOCM_CAR);
+ pl011_maybe_set_bit(status & uap->vendor->fr_dsr, &result, TIOCM_DSR);
+ pl011_maybe_set_bit(status & uap->vendor->fr_cts, &result, TIOCM_CTS);
+ pl011_maybe_set_bit(status & uap->vendor->fr_ri, &result, TIOCM_RNG);
- TIOCMBIT(UART01x_FR_DCD, TIOCM_CAR);
- TIOCMBIT(uap->vendor->fr_dsr, TIOCM_DSR);
- TIOCMBIT(uap->vendor->fr_cts, TIOCM_CTS);
- TIOCMBIT(uap->vendor->fr_ri, TIOCM_RNG);
-#undef TIOCMBIT
return result;
}
+static void pl011_assign_bit(bool cond, unsigned int *ptr, unsigned int mask)
+{
+ if (cond)
+ *ptr |= mask;
+ else
+ *ptr &= ~mask;
+}
+
static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl)
{
struct uart_amba_port *uap =
@@ -1621,23 +1631,16 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl)
cr = pl011_read(uap, REG_CR);
-#define TIOCMBIT(tiocmbit, uartbit) \
- if (mctrl & tiocmbit) \
- cr |= uartbit; \
- else \
- cr &= ~uartbit
-
- TIOCMBIT(TIOCM_RTS, UART011_CR_RTS);
- TIOCMBIT(TIOCM_DTR, UART011_CR_DTR);
- TIOCMBIT(TIOCM_OUT1, UART011_CR_OUT1);
- TIOCMBIT(TIOCM_OUT2, UART011_CR_OUT2);
- TIOCMBIT(TIOCM_LOOP, UART011_CR_LBE);
+ pl011_assign_bit(mctrl & TIOCM_RTS, &cr, UART011_CR_RTS);
+ pl011_assign_bit(mctrl & TIOCM_DTR, &cr, UART011_CR_DTR);
+ pl011_assign_bit(mctrl & TIOCM_OUT1, &cr, UART011_CR_OUT1);
+ pl011_assign_bit(mctrl & TIOCM_OUT2, &cr, UART011_CR_OUT2);
+ pl011_assign_bit(mctrl & TIOCM_LOOP, &cr, UART011_CR_LBE);
if (port->status & UPSTAT_AUTORTS) {
/* We need to disable auto-RTS if we want to turn RTS off */
- TIOCMBIT(TIOCM_RTS, UART011_CR_RTSEN);
+ pl011_assign_bit(mctrl & TIOCM_RTS, &cr, UART011_CR_RTSEN);
}
-#undef TIOCMBIT
pl011_write(cr, uap, REG_CR);
}
--
2.42.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 5/6] tty: serial: amba-pl011: unindent pl011_console_get_options function body
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
` (3 preceding siblings ...)
2023-11-23 13:42 ` [PATCH v4 4/6] tty: serial: amba-pl011: replace TIOCMBIT macros by static functions Théo Lebrun
@ 2023-11-23 13:42 ` Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 6/6] tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe Théo Lebrun
2023-11-25 7:22 ` [PATCH v4 0/6] Cleanup AMBA PL011 driver Greg Kroah-Hartman
6 siblings, 0 replies; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
The whole function body is encapsulated inside an if-condition. Reverse
the if logic and early return to remove one indentation level.
Also turn two nested ifs into a single one at the end of the function.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/tty/serial/amba-pl011.c | 43 ++++++++++++++++++++---------------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index cf9aefe2fd28..4185d6fd243b 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2369,35 +2369,34 @@ pl011_console_write(struct console *co, const char *s, unsigned int count)
static void pl011_console_get_options(struct uart_amba_port *uap, int *baud,
int *parity, int *bits)
{
- if (pl011_read(uap, REG_CR) & UART01x_CR_UARTEN) {
- unsigned int lcr_h, ibrd, fbrd;
+ unsigned int lcr_h, ibrd, fbrd;
- lcr_h = pl011_read(uap, REG_LCRH_TX);
+ if (!(pl011_read(uap, REG_CR) & UART01x_CR_UARTEN))
+ return;
- *parity = 'n';
- if (lcr_h & UART01x_LCRH_PEN) {
- if (lcr_h & UART01x_LCRH_EPS)
- *parity = 'e';
- else
- *parity = 'o';
- }
+ lcr_h = pl011_read(uap, REG_LCRH_TX);
- if ((lcr_h & 0x60) == UART01x_LCRH_WLEN_7)
- *bits = 7;
+ *parity = 'n';
+ if (lcr_h & UART01x_LCRH_PEN) {
+ if (lcr_h & UART01x_LCRH_EPS)
+ *parity = 'e';
else
- *bits = 8;
+ *parity = 'o';
+ }
- ibrd = pl011_read(uap, REG_IBRD);
- fbrd = pl011_read(uap, REG_FBRD);
+ if ((lcr_h & 0x60) == UART01x_LCRH_WLEN_7)
+ *bits = 7;
+ else
+ *bits = 8;
- *baud = uap->port.uartclk * 4 / (64 * ibrd + fbrd);
+ ibrd = pl011_read(uap, REG_IBRD);
+ fbrd = pl011_read(uap, REG_FBRD);
- if (uap->vendor->oversampling) {
- if (pl011_read(uap, REG_CR)
- & ST_UART011_CR_OVSFACT)
- *baud *= 2;
- }
- }
+ *baud = uap->port.uartclk * 4 / (64 * ibrd + fbrd);
+
+ if (uap->vendor->oversampling &&
+ (pl011_read(uap, REG_CR) & ST_UART011_CR_OVSFACT))
+ *baud *= 2;
}
static int pl011_console_setup(struct console *co, char *options)
--
2.42.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 6/6] tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
` (4 preceding siblings ...)
2023-11-23 13:42 ` [PATCH v4 5/6] tty: serial: amba-pl011: unindent pl011_console_get_options function body Théo Lebrun
@ 2023-11-23 13:42 ` Théo Lebrun
2023-11-23 14:35 ` Ilpo Järvinen
2023-11-25 7:22 ` [PATCH v4 0/6] Cleanup AMBA PL011 driver Greg Kroah-Hartman
6 siblings, 1 reply; 10+ messages in thread
From: Théo Lebrun @ 2023-11-23 13:42 UTC (permalink / raw)
To: Russell King, Greg Kroah-Hartman, Jiri Slaby
Cc: linux-kernel, linux-serial, Linus Walleij, Grégory Clement,
Alexandre Belloni, Thomas Petazzoni, Vladimir Kondratiev,
Tawfik Bayouk, Ilpo Järvinen, Théo Lebrun
On this platform, different vendor data is used. That requires a
compile-time check as we access (1) a global boolean & (2) our local
vendor data. Both symbols are accessible only when
CONFIG_ACPI_SPCR_TABLE is enabled.
Factor the vendor data overriding to a separate function that is empty
when CONFIG_ACPI_SPCR_TABLE is not defined.
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
drivers/tty/serial/amba-pl011.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 4185d6fd243b..f42f4a94c239 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2874,6 +2874,22 @@ static int pl011_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pl011_dev_pm_ops, pl011_suspend, pl011_resume);
+#ifdef CONFIG_ACPI_SPCR_TABLE
+static void qpdf2400_erratum44_workaround(struct device *dev,
+ struct uart_amba_port *uap)
+{
+ if (!qdf2400_e44_present)
+ return;
+
+ dev_info(dev, "working around QDF2400 SoC erratum 44\n");
+ uap->vendor = &vendor_qdt_qdf2400_e44;
+}
+#else
+static void qpdf2400_erratum44_workaround(struct device *dev,
+ struct uart_amba_port *uap)
+{ /* empty */ }
+#endif
+
static int sbsa_uart_probe(struct platform_device *pdev)
{
struct uart_amba_port *uap;
@@ -2909,13 +2925,8 @@ static int sbsa_uart_probe(struct platform_device *pdev)
return ret;
uap->port.irq = ret;
-#ifdef CONFIG_ACPI_SPCR_TABLE
- if (qdf2400_e44_present) {
- dev_info(&pdev->dev, "working around QDF2400 SoC erratum 44\n");
- uap->vendor = &vendor_qdt_qdf2400_e44;
- } else
-#endif
- uap->vendor = &vendor_sbsa;
+ uap->vendor = &vendor_sbsa;
+ qpdf2400_erratum44_workaround(&pdev->dev, uap);
uap->reg_offset = uap->vendor->reg_offset;
uap->fifosize = 32;
--
2.42.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 6/6] tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe
2023-11-23 13:42 ` [PATCH v4 6/6] tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe Théo Lebrun
@ 2023-11-23 14:35 ` Ilpo Järvinen
0 siblings, 0 replies; 10+ messages in thread
From: Ilpo Järvinen @ 2023-11-23 14:35 UTC (permalink / raw)
To: Théo Lebrun
Cc: Russell King, Greg Kroah-Hartman, Jiri Slaby, LKML, linux-serial,
Linus Walleij, Grégory Clement, Alexandre Belloni,
Thomas Petazzoni, Vladimir Kondratiev, Tawfik Bayouk
[-- Attachment #1: Type: text/plain, Size: 584 bytes --]
On Thu, 23 Nov 2023, Théo Lebrun wrote:
> On this platform, different vendor data is used. That requires a
> compile-time check as we access (1) a global boolean & (2) our local
> vendor data. Both symbols are accessible only when
> CONFIG_ACPI_SPCR_TABLE is enabled.
>
> Factor the vendor data overriding to a separate function that is empty
> when CONFIG_ACPI_SPCR_TABLE is not defined.
>
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/6] tty: serial: amba-pl011: cleanup driver
2023-11-23 13:42 ` [PATCH v4 3/6] tty: serial: amba-pl011: cleanup driver Théo Lebrun
@ 2023-11-23 19:35 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2023-11-23 19:35 UTC (permalink / raw)
To: Théo Lebrun
Cc: Russell King, Jiri Slaby, linux-kernel, linux-serial,
Linus Walleij, Grégory Clement, Alexandre Belloni,
Thomas Petazzoni, Vladimir Kondratiev, Tawfik Bayouk,
Ilpo Järvinen
On Thu, Nov 23, 2023 at 02:42:41PM +0100, Théo Lebrun wrote:
> Follow recommandations from:
>
> $ ./scripts/checkpatch.pl --strict --file \
> drivers/tty/serial/amba-pl011.c
>
> It does NOT fix alerts relative to TIOCMBIT which will be dealt with in
> another patch. Fixes following alerts:
>
> CHECK: Alignment should match open parenthesis
> CHECK: Blank lines aren't necessary after an open brace '{'
> CHECK: Comparison to NULL could be written [...]
> CHECK: Lines should not end with a '('
> CHECK: Please don't use multiple blank lines
> CHECK: Please use a blank line after function/struct/union/enum declarations
> CHECK: Prefer using the BIT macro
> CHECK: Unbalanced braces around else statement
> CHECK: Unnecessary parentheses around [...]
> CHECK: braces {} should be used on all arms of this statement
> CHECK: spaces preferred around that '/' (ctx:VxV)
> CHECK: spaces preferred around that '|' (ctx:VxV)
> ERROR: do not initialise statics to false
> WARNING: Comparisons should place the constant on the right side of the test
> WARNING: Possible unnecessary 'out of memory' message
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> WARNING: Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ...
> WARNING: quoted string split across lines
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> drivers/tty/serial/amba-pl011.c | 147 +++++++++++++++++++---------------------
> 1 file changed, 71 insertions(+), 76 deletions(-)
This patch does not apply.
And really, you should have broken it up into smaller pieces and most of
them would have applied :(
Please rebase and break it up and resend.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 0/6] Cleanup AMBA PL011 driver
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
` (5 preceding siblings ...)
2023-11-23 13:42 ` [PATCH v4 6/6] tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe Théo Lebrun
@ 2023-11-25 7:22 ` Greg Kroah-Hartman
6 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2023-11-25 7:22 UTC (permalink / raw)
To: Théo Lebrun
Cc: Russell King, Jiri Slaby, linux-kernel, linux-serial,
Linus Walleij, Grégory Clement, Alexandre Belloni,
Thomas Petazzoni, Vladimir Kondratiev, Tawfik Bayouk,
Ilpo Järvinen
On Thu, Nov 23, 2023 at 02:42:38PM +0100, Théo Lebrun wrote:
> Hi,
>
> While adding upstream support to a new platform (Mobileye EyeQ5[1]) that
> uses the AMBA PL011 driver, I took some time to look at the PL011
> driver and ended up with a few patches that cleanup parts of it. The
> line-diff is big mostly because of the checkpatch-fixing commits.
>
> The driver hadn't received any love for quite some time. See commit
> messages for more information.
>
> We can thank Ilpo Järvinen for the V3 & V4. V4 fixes an inversed if
> expression bug added in [PATCH V3 6/6].
This series causes build errors in linux-next so I'm dropping them from
my tree now.
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-11-25 7:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-23 13:42 [PATCH v4 0/6] Cleanup AMBA PL011 driver Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 1/6] tty: serial: amba: cleanup whitespace Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 2/6] tty: serial: amba: Use linux/bits.h macros for constant declarations Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 3/6] tty: serial: amba-pl011: cleanup driver Théo Lebrun
2023-11-23 19:35 ` Greg Kroah-Hartman
2023-11-23 13:42 ` [PATCH v4 4/6] tty: serial: amba-pl011: replace TIOCMBIT macros by static functions Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 5/6] tty: serial: amba-pl011: unindent pl011_console_get_options function body Théo Lebrun
2023-11-23 13:42 ` [PATCH v4 6/6] tty: serial: amba-pl011: factor QDF2400 SoC erratum 44 out of probe Théo Lebrun
2023-11-23 14:35 ` Ilpo Järvinen
2023-11-25 7:22 ` [PATCH v4 0/6] Cleanup AMBA PL011 driver Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).