[PATCH v5 0/4] serial: 8250_dw: Add support for UltraRISC DP1000 UART

[PATCH v5 0/4] serial: 8250_dw: Add support for UltraRISC DP1000 UART

[Jia Wang]

This patch series adds support for the UltraRISC DP1000 UART controller.

The series includes four patches. The first two are preparatory cleanups;
the last two add the DP1000 compatible and fixed CPR handling.

The patches have been tested on an UltraRISC DP1000 development board with
Linux v7.1-rc1, verifying basic UART functionality.

Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
Changes in v5:
- Rebased onto Linux v7.1-rc1.
- Patch 1:
  * Reorder and document the moved DesignWare register/bit definitions.
- Patch 2:
  * Add a FIFO size -> CPR FIFO_MODE helper and use it for RZ/N1.
- Patch 4:
  * Use the FIFO_MODE helper for DP1000.
- Link to v4: https://patch.msgid.link/20260424-ultrarisc-serial-v4-0-1765a0b4c4a0@ultrarisc.com

Changes in v4:
- Added two preparatory patches before the original series, shifting patch
  numbers (former 1/2 -> now 3/4).
- Patch 1:
  * Move all DesignWare UART register/field definitions into 8250_dwlib.h
    for shared use with 8250_dw.
- Patch 2:
  * Converted the Renesas RZ/N1 CPR magic value to use DW_UART_CPR_* macros
    and FIELD_PREP_CONST().
- Patch 4:
  * Converted the UltraRISC DP1000 CPR magic value to use
    DW_UART_CPR_* macros and FIELD_PREP_CONST() (value unchanged).
- Link to v3: https://patch.msgid.link/20260421-ultrarisc-serial-v3-0-3d7f09c2420e@ultrarisc.com

Changes in v3:
- Rebased on Linux v7.0-rc7.
- Patch 1:
   * Removed separate `items` entry for DP1000, merging it into the
     existing `enum` to comply with the schema.
   * Updated commit message to describe DP1000 UART hardware differences.
- Patch 2:
   * Drop the custom quirk for missing CPR register.
   * Switch to using DW_UART_QUIRK_CPR_VALUE to provide a fixed CPR value.
- Link to v2: https://patch.msgid.link/20260316-ultrarisc-serial-v2-0-6ab3e7fa891c@ultrarisc.com

Changes in v2:
- Rebased on Linux v7.0-rc4 (previously on v7.0-rc2).
- Reordered patch series: DT binding patch comes before driver changes.
- Updated commit message for DT binding patch.
- Link to v1: https://patch.msgid.link/20260316-ultrarisc-serial-v1-0-c464f3e933a5@ultrarisc.com

---
Jia Wang (4):
      serial: 8250_dwlib: move DesignWare register definitions to header
      serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions
      dt-bindings: serial: snps-dw-apb-uart: Add UltraRISC DP1000 UART
      serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART

 .../bindings/serial/snps-dw-apb-uart.yaml          |  1 +
 drivers/tty/serial/8250/8250_dw.c                  | 33 ++++++----
 drivers/tty/serial/8250/8250_dwlib.c               | 49 --------------
 drivers/tty/serial/8250/8250_dwlib.h               | 74 ++++++++++++++++++++++
 4 files changed, 96 insertions(+), 61 deletions(-)
---
base-commit: 3b3bea6d4b9c162f9e555905d96b8c1da67ecd5b
change-id: 20260309-ultrarisc-serial-64ff637edf26

Best regards,
--  
Jia Wang <wangjia@ultrarisc.com>

[PATCH v5 1/4] serial: 8250_dwlib: move DesignWare register definitions to header

[Jia Wang]

Move the DW_UART_* register offsets and CPR bit/field definitions from
8250_dwlib.c into 8250_dwlib.h so they can be shared by 8250_dw and
8250_dwlib users.

Add an include guard for 8250_dwlib.h.

Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
 drivers/tty/serial/8250/8250_dw.c    | 11 ------
 drivers/tty/serial/8250/8250_dwlib.c | 49 --------------------------
 drivers/tty/serial/8250/8250_dwlib.h | 67 ++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 60 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 94beadb4024d..467755bf0092 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -34,22 +34,11 @@
 
 #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 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 */
 
-/* DesignWare specific register fields */
-#define DW_UART_IIR_IID			GENMASK(3, 0)
-
-#define DW_UART_MCR_SIRE		BIT(6)
-
-#define DW_UART_USR_BUSY		BIT(0)
-
 /* Renesas specific register fields */
 #define RZN1_UART_xDMACR_DMA_EN		BIT(0)
 #define RZN1_UART_xDMACR_1_WORD_BURST	(0 << 1)
diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index b055d89cfb39..8859e66d2d71 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -13,55 +13,6 @@
 
 #include "8250_dwlib.h"
 
-/* Offsets for the DesignWare specific registers */
-#define DW_UART_TCR	0xac /* Transceiver Control Register (RS485) */
-#define DW_UART_DE_EN	0xb0 /* Driver Output Enable Register */
-#define DW_UART_RE_EN	0xb4 /* Receiver Output Enable Register */
-#define DW_UART_DLF	0xc0 /* Divisor Latch Fraction Register */
-#define DW_UART_RAR	0xc4 /* Receive Address Register */
-#define DW_UART_TAR	0xc8 /* Transmit Address Register */
-#define DW_UART_LCR_EXT	0xcc /* Line Extended Control Register */
-#define DW_UART_CPR	0xf4 /* Component Parameter Register */
-#define DW_UART_UCV	0xf8 /* UART Component Version */
-
-/* Receive / Transmit Address Register bits */
-#define DW_UART_ADDR_MASK		GENMASK(7, 0)
-
-/* Line Status Register bits */
-#define DW_UART_LSR_ADDR_RCVD		BIT(8)
-
-/* Transceiver Control Register bits */
-#define DW_UART_TCR_RS485_EN		BIT(0)
-#define DW_UART_TCR_RE_POL		BIT(1)
-#define DW_UART_TCR_DE_POL		BIT(2)
-#define DW_UART_TCR_XFER_MODE		GENMASK(4, 3)
-#define DW_UART_TCR_XFER_MODE_DE_DURING_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 0)
-#define DW_UART_TCR_XFER_MODE_SW_DE_OR_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 1)
-#define DW_UART_TCR_XFER_MODE_DE_OR_RE		FIELD_PREP(DW_UART_TCR_XFER_MODE, 2)
-
-/* Line Extended Control Register bits */
-#define DW_UART_LCR_EXT_DLS_E		BIT(0)
-#define DW_UART_LCR_EXT_ADDR_MATCH	BIT(1)
-#define DW_UART_LCR_EXT_SEND_ADDR	BIT(2)
-#define DW_UART_LCR_EXT_TRANSMIT_MODE	BIT(3)
-
-/* Component Parameter Register bits */
-#define DW_UART_CPR_ABP_DATA_WIDTH	GENMASK(1, 0)
-#define DW_UART_CPR_AFCE_MODE		BIT(4)
-#define DW_UART_CPR_THRE_MODE		BIT(5)
-#define DW_UART_CPR_SIR_MODE		BIT(6)
-#define DW_UART_CPR_SIR_LP_MODE		BIT(7)
-#define DW_UART_CPR_ADDITIONAL_FEATURES	BIT(8)
-#define DW_UART_CPR_FIFO_ACCESS		BIT(9)
-#define DW_UART_CPR_FIFO_STAT		BIT(10)
-#define DW_UART_CPR_SHADOW		BIT(11)
-#define DW_UART_CPR_ENCODED_PARMS	BIT(12)
-#define DW_UART_CPR_DMA_EXTRA		BIT(13)
-#define DW_UART_CPR_FIFO_MODE		GENMASK(23, 16)
-
-/* Helper for FIFO size calculation */
-#define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
-
 /*
  * divisor = div(I) + div(F)
  * "I" means integer, "F" means fractional
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 7dd2a8e7b780..2f26f9ecacbe 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -1,11 +1,76 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 /* Synopsys DesignWare 8250 library header file. */
 
+#ifndef _SERIAL_8250_DWLIB_H_
+#define _SERIAL_8250_DWLIB_H_
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
 #include <linux/io.h>
 #include <linux/types.h>
 
 #include "8250.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_TCR	0xac /* Transceiver Control Register (RS485) */
+#define DW_UART_DE_EN	0xb0 /* Driver Output Enable Register */
+#define DW_UART_RE_EN	0xb4 /* Receiver Output Enable Register */
+#define DW_UART_DLF	0xc0 /* Divisor Latch Fraction Register */
+#define DW_UART_RAR	0xc4 /* Receive Address Register */
+#define DW_UART_TAR	0xc8 /* Transmit Address Register */
+#define DW_UART_LCR_EXT	0xcc /* Line Extended Control Register */
+#define DW_UART_CPR	0xf4 /* Component Parameter Register */
+#define DW_UART_UCV	0xf8 /* UART Component Version */
+
+/* Interrupt ID Register bits */
+#define DW_UART_IIR_IID			GENMASK(3, 0)
+
+/* Modem Control Register bits */
+#define DW_UART_MCR_SIRE		BIT(6)
+
+/* Line Status Register bits */
+#define DW_UART_LSR_ADDR_RCVD		BIT(8)
+
+/* UART Status Register bits */
+#define DW_UART_USR_BUSY		BIT(0)
+
+/* Transceiver Control Register bits */
+#define DW_UART_TCR_RS485_EN		BIT(0)
+#define DW_UART_TCR_RE_POL		BIT(1)
+#define DW_UART_TCR_DE_POL		BIT(2)
+#define DW_UART_TCR_XFER_MODE		GENMASK(4, 3)
+#define DW_UART_TCR_XFER_MODE_DE_DURING_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 0)
+#define DW_UART_TCR_XFER_MODE_SW_DE_OR_RE	FIELD_PREP(DW_UART_TCR_XFER_MODE, 1)
+#define DW_UART_TCR_XFER_MODE_DE_OR_RE		FIELD_PREP(DW_UART_TCR_XFER_MODE, 2)
+
+/* Receive / Transmit Address Register bits */
+#define DW_UART_ADDR_MASK		GENMASK(7, 0)
+
+/* Line Extended Control Register bits */
+#define DW_UART_LCR_EXT_DLS_E		BIT(0)
+#define DW_UART_LCR_EXT_ADDR_MATCH	BIT(1)
+#define DW_UART_LCR_EXT_SEND_ADDR	BIT(2)
+#define DW_UART_LCR_EXT_TRANSMIT_MODE	BIT(3)
+
+/* Component Parameter Register bits */
+#define DW_UART_CPR_ABP_DATA_WIDTH	GENMASK(1, 0)
+#define DW_UART_CPR_AFCE_MODE		BIT(4)
+#define DW_UART_CPR_THRE_MODE		BIT(5)
+#define DW_UART_CPR_SIR_MODE		BIT(6)
+#define DW_UART_CPR_SIR_LP_MODE		BIT(7)
+#define DW_UART_CPR_ADDITIONAL_FEATURES	BIT(8)
+#define DW_UART_CPR_FIFO_ACCESS		BIT(9)
+#define DW_UART_CPR_FIFO_STAT		BIT(10)
+#define DW_UART_CPR_SHADOW		BIT(11)
+#define DW_UART_CPR_ENCODED_PARMS	BIT(12)
+#define DW_UART_CPR_DMA_EXTRA		BIT(13)
+#define DW_UART_CPR_FIFO_MODE		GENMASK(23, 16)
+
+/* Helper for FIFO size calculation */
+#define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
+
 struct dw8250_port_data {
 	/* Port properties */
 	int			line;
@@ -38,3 +103,5 @@ static inline void dw8250_writel_ext(struct uart_port *p, int offset, u32 reg)
 	else
 		writel(reg, p->membase + offset);
 }
+
+#endif /* _SERIAL_8250_DWLIB_H_ */

-- 
2.34.1

[PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Jia Wang]

Replace the magic CPR value for Renesas RZ/N1 with a composition using
DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().

Introduce a helper macro to convert a FIFO size (bytes) into the CPR
FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
bounds. Use it to replace the literal FIFO_MODE values in the RZN1.

Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
 drivers/tty/serial/8250/8250_dw.c    | 11 ++++++++++-
 drivers/tty/serial/8250/8250_dwlib.h |  7 +++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 467755bf0092..5cf3bb74b285 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -937,7 +937,16 @@ static const struct dw8250_platform_data dw8250_armada_38x_data = {
 
 static const struct dw8250_platform_data dw8250_renesas_rzn1_data = {
 	.usr_reg = DW_UART_USR,
-	.cpr_value = 0x00012f32,
+	.cpr_value = FIELD_PREP_CONST(DW_UART_CPR_ABP_DATA_WIDTH, 2) |
+		     DW_UART_CPR_AFCE_MODE |
+		     DW_UART_CPR_THRE_MODE |
+		     DW_UART_CPR_ADDITIONAL_FEATURES |
+		     DW_UART_CPR_FIFO_ACCESS |
+		     DW_UART_CPR_FIFO_STAT |
+		     DW_UART_CPR_SHADOW |
+		     DW_UART_CPR_DMA_EXTRA |
+		     FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE,
+				      DW_UART_CPR_FIFO_MODE_FROM_SIZE(16)),
 	.quirks = DW_UART_QUIRK_CPR_VALUE | DW_UART_QUIRK_IS_DMA_FC,
 };
 
diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
index 2f26f9ecacbe..c1f87cd42ecc 100644
--- a/drivers/tty/serial/8250/8250_dwlib.h
+++ b/drivers/tty/serial/8250/8250_dwlib.h
@@ -6,6 +6,8 @@
 
 #include <linux/bitfield.h>
 #include <linux/bits.h>
+#include <linux/build_bug.h>
+#include <linux/align.h>
 #include <linux/io.h>
 #include <linux/types.h>
 
@@ -70,6 +72,11 @@
 
 /* Helper for FIFO size calculation */
 #define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
+#define DW_UART_CPR_FIFO_MODE_MAX	0x80
+#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
+	(BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) +			\
+	 BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) +	\
+	 ((size) / 16))
 
 struct dw8250_port_data {
 	/* Port properties */

-- 
2.34.1

[PATCH v5 3/4] dt-bindings: serial: snps-dw-apb-uart: Add UltraRISC DP1000 UART

[Jia Wang]

UltraRISC DP1000 integrates a Synopsys DesignWare APB UART, but it does
not provide the standard CPR and UCV registers.

Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
index 685c1eceb782..49f51b002879 100644
--- a/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
+++ b/Documentation/devicetree/bindings/serial/snps-dw-apb-uart.yaml
@@ -78,6 +78,7 @@ properties:
               - starfive,jh7100-hsuart
               - starfive,jh7100-uart
               - starfive,jh7110-uart
+              - ultrarisc,dp1000-uart
           - const: snps,dw-apb-uart
       - const: snps,dw-apb-uart
 

-- 
2.34.1

[PATCH v5 4/4] serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART

[Jia Wang]

The UltraRISC DP1000 UART does not provide the standard CPR register used
by 8250_dw to discover port capabilities.

Provide a fixed CPR value for the DP1000-specific compatible so the
driver can configure the port correctly.

Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
---
 drivers/tty/serial/8250/8250_dw.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 5cf3bb74b285..6fa7c8440919 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -960,6 +960,16 @@ static const struct dw8250_platform_data dw8250_intc10ee = {
 	.quirks = DW_UART_QUIRK_IER_KICK,
 };
 
+static const struct dw8250_platform_data dw8250_ultrarisc_dp1000_data = {
+	.usr_reg = DW_UART_USR,
+	.cpr_value = FIELD_PREP_CONST(DW_UART_CPR_ABP_DATA_WIDTH, 2) |
+		     DW_UART_CPR_THRE_MODE |
+		     DW_UART_CPR_DMA_EXTRA |
+		     FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE,
+				      DW_UART_CPR_FIFO_MODE_FROM_SIZE(32)),
+	.quirks = DW_UART_QUIRK_CPR_VALUE,
+};
+
 static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "snps,dw-apb-uart", .data = &dw8250_dw_apb },
 	{ .compatible = "cavium,octeon-3860-uart", .data = &dw8250_octeon_3860_data },
@@ -967,6 +977,7 @@ static const struct of_device_id dw8250_of_match[] = {
 	{ .compatible = "renesas,rzn1-uart", .data = &dw8250_renesas_rzn1_data },
 	{ .compatible = "sophgo,sg2044-uart", .data = &dw8250_skip_set_rate_data },
 	{ .compatible = "starfive,jh7100-uart", .data = &dw8250_skip_set_rate_data },
+	{ .compatible = "ultrarisc,dp1000-uart", .data = &dw8250_ultrarisc_dp1000_data },
 	{ /* Sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, dw8250_of_match);

-- 
2.34.1

Re: [PATCH v5 1/4] serial: 8250_dwlib: move DesignWare register definitions to header

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 01:26:26PM +0800, Jia Wang wrote:
> Move the DW_UART_* register offsets and CPR bit/field definitions from
> 8250_dwlib.c into 8250_dwlib.h so they can be shared by 8250_dw and
> 8250_dwlib users.
> 
> Add an include guard for 8250_dwlib.h.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> Replace the magic CPR value for Renesas RZ/N1 with a composition using
> DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> 
> Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> bounds. Use it to replace the literal FIFO_MODE values in the RZN1.

A couple of nit-picks below. After addressing them you can add

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

>  #include <linux/bitfield.h>
>  #include <linux/bits.h>
> +#include <linux/build_bug.h>
> +#include <linux/align.h>

Preserve order, 'a' goes before 'b'.

>  #include <linux/io.h>
>  #include <linux/types.h>

...

>  /* Helper for FIFO size calculation */
>  #define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)

> +#define DW_UART_CPR_FIFO_MODE_MAX	0x80

You used decimal values elsewhere (id est 16), use upper limit in decimal
as well.

> +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> +	(BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) +			\
> +	 BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) +	\
> +	 ((size) / 16))

I don't see the need in having that maximum being defined separately (we don't
have that for 16, no need to have it for 128.

Since some ISA:s have one assembly instruction to get both / and % divisions,
it's better to use that instead of IS_ALIGNED(). Can you check code generation
for x86_64 / x86?

#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
	(BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))

Note, I dropped first division in order to show the upper limit in a plain
number since 16 is also FIFO size in bytes.

Also note, this evaluates (size) three times, which might be problematic,
but I think we can leave with that for now.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 4/4] serial: 8250_dw: Use a fixed CPR value for UltraRISC DP1000 UART

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 01:26:29PM +0800, Jia Wang wrote:
> The UltraRISC DP1000 UART does not provide the standard CPR register used
> by 8250_dw to discover port capabilities.
> 
> Provide a fixed CPR value for the DP1000-specific compatible so the
> driver can configure the port correctly.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Jia Wang]

On 2026-04-28 10:28 +0300, Andy Shevchenko wrote:
> On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> > Replace the magic CPR value for Renesas RZ/N1 with a composition using
> > DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> > 
> > Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> > FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> > bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
> 
> A couple of nit-picks below. After addressing them you can add
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>

Thanks for the review. I'll address them and add your Reviewed-by tag
in v6.
 
> ...
> 
> >  #include <linux/bitfield.h>
> >  #include <linux/bits.h>
> > +#include <linux/build_bug.h>
> > +#include <linux/align.h>
> 
> Preserve order, 'a' goes before 'b'.
> 

Will fix in v6.

> >  #include <linux/io.h>
> >  #include <linux/types.h>
> 
> ...
> 
> >  /* Helper for FIFO size calculation */
> >  #define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
> 
> > +#define DW_UART_CPR_FIFO_MODE_MAX	0x80
> 
> You used decimal values elsewhere (id est 16), use upper limit in decimal
> as well.
> 

This define will be removed in v6.

> > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> > +	(BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) +			\
> > +	 BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) +	\
> > +	 ((size) / 16))
> 
> I don't see the need in having that maximum being defined separately (we don't
> have that for 16, no need to have it for 128.
> 
> Since some ISA:s have one assembly instruction to get both / and % divisions,
> it's better to use that instead of IS_ALIGNED(). Can you check code generation
> for x86_64 / x86?
> 
> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> 	(BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
> 
> Note, I dropped first division in order to show the upper limit in a plain
> number since 16 is also FIFO size in bytes.
> 
> Also note, this evaluates (size) three times, which might be problematic,
> but I think we can leave with that for now.
>

Makes sense. I'll fold the upper limit into the macro and switch to
(size) > 2048 and (size) % 16 checks for v6.

In the driver, the macro is used only in static initializers, so the
constants are fully resolved at compile time.

I also checked code generation on x86_64/x86: both IS_ALIGNED() and % 16
produce identical instructions at -O2. I'll still switch to % 16 as you
suggest for clarity.

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
>

Best Regards,
Jia Wang 

Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Ilpo Järvinen]

On Tue, 28 Apr 2026, Andy Shevchenko wrote:

> On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> > Replace the magic CPR value for Renesas RZ/N1 with a composition using
> > DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> > 
> > Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> > FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> > bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
> 
> A couple of nit-picks below. After addressing them you can add
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> ...
> 
> >  #include <linux/bitfield.h>
> >  #include <linux/bits.h>
> > +#include <linux/build_bug.h>
> > +#include <linux/align.h>
> 
> Preserve order, 'a' goes before 'b'.
> 
> >  #include <linux/io.h>
> >  #include <linux/types.h>
> 
> ...
> 
> >  /* Helper for FIFO size calculation */
> >  #define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
> 
> > +#define DW_UART_CPR_FIFO_MODE_MAX	0x80
> 
> You used decimal values elsewhere (id est 16), use upper limit in decimal
> as well.
> 
> > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> > +	(BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) +			\
> > +	 BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) +	\
> > +	 ((size) / 16))
> 
> I don't see the need in having that maximum being defined separately (we don't
> have that for 16, no need to have it for 128.
> 
> Since some ISA:s have one assembly instruction to get both / and % divisions,
> it's better to use that instead of IS_ALIGNED(). Can you check code generation
> for x86_64 / x86?

Do those BUILD_BUGs even generate code, especially when they are expected 
to only appear in a struct initializer?

> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> 	(BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
> 
> Note, I dropped first division in order to show the upper limit in a plain
> number since 16 is also FIFO size in bytes.
> 
> Also note, this evaluates (size) three times, which might be problematic,
> but I think we can leave with that for now.

I'd put also FIELD_PREP_CONST() into the macro itself as I don't see much 
value for this macro outside of those .cpr_value initializations.

IMO, the entire macro would be cleaner looking as a truly multi-line 
construct. Can we use static_assert()s in struct field initialization 
(I'm not sure), something along these lines:

#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)			\
({								\
	typeof (size) __size = size;				\
								\
	static_assert(IS_ALIGNED((__size), 16));		\
	static_assert(__size <= DW_UART_CPR_FIFO_MODE_MAX);	\
								\
	FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE, __size / 16);	\
})

-- 
 i.

Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Jia Wang]

On 2026-04-28 11:41 +0300, Ilpo Järvinen wrote:
> On Tue, 28 Apr 2026, Andy Shevchenko wrote:
> 
> > On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:
> > > Replace the magic CPR value for Renesas RZ/N1 with a composition using
> > > DW_UART_CPR_* bit/field definitions and FIELD_PREP_CONST().
> > > 
> > > Introduce a helper macro to convert a FIFO size (bytes) into the CPR
> > > FIFO_MODE field value, with BUILD_BUG_ON_ZERO() checks for alignment and
> > > bounds. Use it to replace the literal FIFO_MODE values in the RZN1.
> > 
> > A couple of nit-picks below. After addressing them you can add
> > 
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > ...
> > 
> > >  #include <linux/bitfield.h>
> > >  #include <linux/bits.h>
> > > +#include <linux/build_bug.h>
> > > +#include <linux/align.h>
> > 
> > Preserve order, 'a' goes before 'b'.
> > 
> > >  #include <linux/io.h>
> > >  #include <linux/types.h>
> > 
> > ...
> > 
> > >  /* Helper for FIFO size calculation */
> > >  #define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
> > 
> > > +#define DW_UART_CPR_FIFO_MODE_MAX	0x80
> > 
> > You used decimal values elsewhere (id est 16), use upper limit in decimal
> > as well.
> > 
> > > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> > > +	(BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) +			\
> > > +	 BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) +	\
> > > +	 ((size) / 16))
> > 
> > I don't see the need in having that maximum being defined separately (we don't
> > have that for 16, no need to have it for 128.
> > 
> > Since some ISA:s have one assembly instruction to get both / and % divisions,
> > it's better to use that instead of IS_ALIGNED(). Can you check code generation
> > for x86_64 / x86?
> 
> Do those BUILD_BUGs even generate code, especially when they are expected 
> to only appear in a struct initializer?
> 
> > #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> > 	(BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
> > 
> > Note, I dropped first division in order to show the upper limit in a plain
> > number since 16 is also FIFO size in bytes.
> > 
> > Also note, this evaluates (size) three times, which might be problematic,
> > but I think we can leave with that for now.
> 
> I'd put also FIELD_PREP_CONST() into the macro itself as I don't see much 
> value for this macro outside of those .cpr_value initializations.
> 
> IMO, the entire macro would be cleaner looking as a truly multi-line 
> construct. Can we use static_assert()s in struct field initialization 
> (I'm not sure), something along these lines:
> 
> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)			\
> ({								\
> 	typeof (size) __size = size;				\
> 								\
> 	static_assert(IS_ALIGNED((__size), 16));		\
> 	static_assert(__size <= DW_UART_CPR_FIFO_MODE_MAX);	\
> 								\
> 	FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE, __size / 16);	\
> })
> 

Thanks. I tried that approach, but the statement-expression form does
not work in this case because the helper is used in static initializers.
So I'll keep it as a plain expression macro for now, and just rework it
into a cleaner multi-line form.

> -- 
>  i.
> 
>

Best Regards,
Jia Wang 

Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 11:41:27AM +0300, Ilpo Järvinen wrote:
> On Tue, 28 Apr 2026, Andy Shevchenko wrote:
> > On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:

...

> > >  /* Helper for FIFO size calculation */
> > >  #define DW_UART_CPR_FIFO_SIZE(a)	(FIELD_GET(DW_UART_CPR_FIFO_MODE, (a)) * 16)
> > 
> > > +#define DW_UART_CPR_FIFO_MODE_MAX	0x80
> > 
> > You used decimal values elsewhere (id est 16), use upper limit in decimal
> > as well.
> > 
> > > +#define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> > > +	(BUILD_BUG_ON_ZERO(!IS_ALIGNED((size), 16)) +			\
> > > +	 BUILD_BUG_ON_ZERO(((size) / 16) > DW_UART_CPR_FIFO_MODE_MAX) +	\
> > > +	 ((size) / 16))
> > 
> > I don't see the need in having that maximum being defined separately (we don't
> > have that for 16, no need to have it for 128.
> > 
> > Since some ISA:s have one assembly instruction to get both / and % divisions,
> > it's better to use that instead of IS_ALIGNED(). Can you check code generation
> > for x86_64 / x86?
> 
> Do those BUILD_BUGs even generate code, especially when they are expected 
> to only appear in a struct initializer?

Good question if this affects the code generation.

> > #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)				\
> > 	(BUILD_BUG_ON_ZERO((size) > 2048) + BUILD_BUG_ON_ZERO((size) % 16) + ((size) / 16))
> > 
> > Note, I dropped first division in order to show the upper limit in a plain
> > number since 16 is also FIFO size in bytes.
> > 
> > Also note, this evaluates (size) three times, which might be problematic,
> > but I think we can leave with that for now.
> 
> I'd put also FIELD_PREP_CONST() into the macro itself as I don't see much 
> value for this macro outside of those .cpr_value initializations.

Yep, and it would make it on par with the existing _FIFO_SIZE() that has
FIELD_GET() there.

> IMO, the entire macro would be cleaner looking as a truly multi-line 
> construct. Can we use static_assert()s in struct field initialization 
> (I'm not sure), something along these lines:

I believe one may put there static_assert():s.

> #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)			\
> ({								\
> 	typeof (size) __size = size;				\

Perhaps auto ?

> 								\
> 	static_assert(IS_ALIGNED((__size), 16));		\
> 	static_assert(__size <= DW_UART_CPR_FIFO_MODE_MAX);	\

But I still think the % and / paired are clearer (for reading and understanding)
even if they do not affect code generation. Also I think the plain number of the
maximum size is better for the same reasons we do not have it for 16.

> 								\
> 	FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE, __size / 16);	\
> })

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/4] serial: 8250_dw: build Renesas RZN1 CPR value from DW_UART_CPR_* definitions

[Andy Shevchenko]

On Tue, Apr 28, 2026 at 05:07:49PM +0800, Jia Wang wrote:
> On 2026-04-28 11:41 +0300, Ilpo Järvinen wrote:
> > On Tue, 28 Apr 2026, Andy Shevchenko wrote:
> > > On Tue, Apr 28, 2026 at 01:26:27PM +0800, Jia Wang wrote:

...

> > #define DW_UART_CPR_FIFO_MODE_FROM_SIZE(size)			\
> > ({								\
> > 	typeof (size) __size = size;				\
> > 								\
> > 	static_assert(IS_ALIGNED((__size), 16));		\
> > 	static_assert(__size <= DW_UART_CPR_FIFO_MODE_MAX);	\
> > 								\
> > 	FIELD_PREP_CONST(DW_UART_CPR_FIFO_MODE, __size / 16);	\
> > })
> 
> Thanks. I tried that approach, but the statement-expression form does
> not work in this case because the helper is used in static initializers.
> So I'll keep it as a plain expression macro for now, and just rework it
> into a cleaner multi-line form.

Still you can move FIELD_PREP_CONST() into it.

-- 
With Best Regards,
Andy Shevchenko