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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Fri, Apr 24, 2026 at 01:39:28PM +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.

...

> -/* 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)
> -

One nit-pick, though, these three need the similar comments in the header and a
blank line between them.

...

> +/* 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 */
> +
> +/* 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)

Like this one, the IIR. MCR, and USR bits should be commented. Also preserve
the sorting by the register offset, so the groups of bits follow the above list
of registers, where USR bits, for example, should go before TCR.

> +/* 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)

> +/* 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)

I.o.w. uncouple these as per above.

-- 
With Best Regards,
Andy Shevchenko