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/[email protected]/T/
Have a nice day,
Théo Lebrun
Signed-off-by: Théo Lebrun <[email protected]>
---
Changes in v4:
- Fix reverse if logic bug in [PATCH V3 6/6].
- Link to v3: https://lore.kernel.org/r/[email protected]
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/[email protected]
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/[email protected]
---
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
Signed-off-by: Théo Lebrun <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Théo Lebrun <[email protected]>
Reviewed-by: Ilpo Järvinen <[email protected]>
--
i.
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 <[email protected]>
> Reviewed-by: Ilpo J?rvinen <[email protected]>
> Signed-off-by: Th?o Lebrun <[email protected]>
> ---
> 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
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