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

[Jia Wang <wangjia@ultrarisc.com>]

On 2026-04-21 15:18 +0300, Ilpo Järvinen wrote:
> On Tue, 21 Apr 2026, 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.
> > 
> > Signed-off-by: Jia Wang <wangjia@ultrarisc.com>
> > ---
> >  drivers/tty/serial/8250/8250_dw.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index 94beadb4024d..ca6dbdf75918 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -962,6 +962,12 @@ 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 = 0x00022022,
> 
> Hi,
> 
> Please construct the cpr_value by ORing DW_UART_CPR_* defines together.
> For fields, FIELD_PREP_CONST() may be useful.
> 
> In order to be able to use the DW_UART_CPR_* defines, they need to be 
> moved into 8250_dwlib.h (I'd move all DW_UART register defines in a 
> preparatory patch).
> 
> I know the existing Renesas' .cpr_value doesn't follow this convention but 
> that could be converted as well (in another patch, or leave the Renesas 
> entry conversion to me if you don't want to do that).
>

Thanks for the review.

Just to confirm: since you mentioned that the preparatory patch moving
the DW_UART register defines is already in place, I don't need to move
them again, correct?

I will update my patch to use the DW_UART_CPR_* macros and
FIELD_PREP_CONST() accordingly, and I’m happy to add a separate patch in
the next revision to convert the Renesas .cpr_value as well.
 
> > +	.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 },
> > @@ -969,6 +975,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);
> > 
> > 
> 
> -- 
>  i.
> 
> 

Regards,
Jia