Skip to contentSkip to conversation

serial: 8250_dw: Clean up register macros and error handling

PATCH In review Artem Shimko 2 participants 3 messages Apr 24 – May 5

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 serial: 8250_dw: Clean up register macros and error handling

Align register offset definitions for DW_UART_USR, DW_UART_DMASA, OCTEON_UART_USR, RZN1_UART_TDMACR and RZN1_UART_RDMACR to improve readability. Replace raw shift with FIELD_PREP() and GENMASK() for RZN1_UART_xDMACR burst field definitions — this documents that the field occupies bits [2:1] and prevents accidental overflow when new burst values are added.

Simplify ENXIO handling in dw8250_probe(): instead of explicitly zeroing 'err' and then checking it, use a single conditional that allows -ENXIO (no interrupt, fall back to polling) while treating any other error as fatal. No functional change intended.

---
Hi,

This small cleanup patch addresses a few minor style and robustness issues I noticed while working with the driver.

Thank you.

--
Regards,
Artem

 drivers/tty/serial/8250/8250_dw.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)
drivers/tty/serial/8250/8250_dw.c +10 -11
index 94beadb4024d..3e7b79154d9d 100644
@@ -35,13 +35,13 @@
 #include "8250_dwlib.h"
 
 /* Offsets for the DesignWare specific registers */
-#define DW_UART_USR	0x1f /* UART Status Register */
-#define DW_UART_DMASA	0xa8 /* DMA Software Ack */
+#define DW_UART_USR			0x1f /* UART Status Register */
+#define DW_UART_DMASA			0xa8 /* DMA Software Ack */
 
-#define OCTEON_UART_USR	0x27 /* UART Status Register */
+#define OCTEON_UART_USR			0x27 /* UART Status Register */
 
-#define RZN1_UART_TDMACR 0x10c /* DMA Control Register Transmit Mode */
-#define RZN1_UART_RDMACR 0x110 /* DMA Control Register Receive Mode */
+#define RZN1_UART_TDMACR		0x10c /* DMA Control Register Transmit Mode */
+#define RZN1_UART_RDMACR		0x110 /* DMA Control Register Receive Mode */
 
 /* DesignWare specific register fields */
 #define DW_UART_IIR_IID			GENMASK(3, 0)
@@ -52,9 +52,10 @@
 
 /* Renesas specific register fields */
 #define RZN1_UART_xDMACR_DMA_EN		BIT(0)
-#define RZN1_UART_xDMACR_1_WORD_BURST	(0 << 1)
-#define RZN1_UART_xDMACR_4_WORD_BURST	(1 << 1)
-#define RZN1_UART_xDMACR_8_WORD_BURST	(2 << 1)
+#define RZN1_UART_xDMACR_BURST_MASK	GENMASK(2, 1)
+#define RZN1_UART_xDMACR_1_WORD_BURST	FIELD_PREP(RZN1_UART_xDMACR_BURST_MASK, 0)
+#define RZN1_UART_xDMACR_4_WORD_BURST	FIELD_PREP(RZN1_UART_xDMACR_BURST_MASK, 1)
+#define RZN1_UART_xDMACR_8_WORD_BURST	FIELD_PREP(RZN1_UART_xDMACR_BURST_MASK, 2)
 #define RZN1_UART_xDMACR_BLK_SZ(x)	((x) << 3)
 
 /* Quirks */
@@ -729,9 +730,7 @@ static int dw8250_probe(struct platform_device *pdev)
 
 	err = uart_read_port_properties(p);
 	/* no interrupt -> fall back to polling */
-	if (err == -ENXIO)
-		err = 0;
-	if (err)
+	if (err && err != -ENXIO)
 		return err;
 
 	switch (p->iotype) {
Signature · 1 line
2.43.0
#1 lore raw
Quoted 11 lines · Artem Shimko

Align register offset definitions for DW_UART_USR, DW_UART_DMASA, OCTEON_UART_USR, RZN1_UART_TDMACR and RZN1_UART_RDMACR to improve readability. Replace raw shift with FIELD_PREP() and GENMASK() for RZN1_UART_xDMACR burst field definitions — this documents that the field occupies bits [2:1] and prevents accidental overflow when new burst values are added.

Simplify ENXIO handling in dw8250_probe(): instead of explicitly zeroing 'err' and then checking it, use a single conditional that allows -ENXIO (no interrupt, fall back to polling) while treating any other error as fatal. No functional change intended.

There is real patch series in-flight which we want to apply.

This one may wait till that happen. You may also help reviewing that in a way that this patch won't be needed at all (or be minimal).

https://lore.kernel.org/linux-serial/[email protected]/

Signature · 2 lines
With Best Regards,
Andy Shevchenko