Re: [PATCH v2] serial: 8250_dw: preserve BUSY-safe fractional divisor programming

[Ilpo Järvinen]

On Sat, 24 Oct 2026, James Hilliard wrote:

> dw8250_probe() installs a divisor setter that makes the UART idle and
> checks DLAB before writing DLL and DLH. When dw8250_setup_port() detects
> fractional-divisor support, it replaces that setter with the library's
> callback. The replacement writes DLF and uses serial8250_do_set_divisor(),
> bypassing the divisor setter's idle sequence and DLAB readback.
> 
> If the LCR update fails to set DLAB, the subsequent divisor writes can
> reach TX and IER instead of DLL and DLH.
> 
> Expose the library's fractional-divisor calculation and DLF write as
> shared helpers. Install the BUSY-safe setter after hardware discovery and
> use the shared DLF helper to program the fractional field only after DLAB
> has been confirmed, alongside DLL and DLH. Select the shared calculator
> when fractional divisors are supported, preserving the core calculation
> for integer-only ports.
> 
> Fixes: a7b9ce39fbe4 ("serial: 8250_dw: Ensure BUSY is deasserted")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> ---
> Changes in v2:
> - Share the fractional-divisor calculation and DLF write through exported
>   dwlib helpers, and use them in the platform driver as requested by Andy.
> - Select the fractional calculator only when DLF is implemented, preserving
>   the existing core calculation for integer-only ports.
> - Link to v1: https://lore.kernel.org/r/20261023-submit-serial-8250-dw-divisor-v1-v1-1-f452d4bb1b1f@xxxxxxxxx
> 
> Previously included in the DMA cancellation series v4:
> https://lore.kernel.org/r/20261023-submit-serial-8250-dma-cancel-v1-v4-0-e759de1839de@xxxxxxxxx
> 
> Changes in DMA cancellation v4:
> - Keep DesignWare's BUSY-safe divisor setter after fractional-divisor
>   discovery, writing DLF and DLL/DLH only after confirming DLAB.
> - Link to v3: https://patch.msgid.link/20261023-submit-serial-8250-dma-cancel-v1-v3-0-d78f9e4c4932@xxxxxxxxx
> ---
>  drivers/tty/serial/8250/8250_dw.c    |  9 ++++++++-
>  drivers/tty/serial/8250/8250_dwlib.c | 15 +++++++++++----
>  drivers/tty/serial/8250/8250_dwlib.h |  2 ++
>  3 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index eca02e8bd6d1..d036a2fe258a 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -190,6 +190,7 @@ static int dw8250_idle_enter(struct uart_port *p)
>  static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
>  			       unsigned int quot, unsigned int quot_frac)
>  {
> +	struct dw8250_port_data *pd = p->private_data;
>  	struct uart_8250_port *up = up_to_u8250p(p);
>  	int ret;
>  
> @@ -201,6 +202,8 @@ static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
>  	if (!(serial_port_in(p, UART_LCR) & UART_LCR_DLAB))
>  		goto idle_failed;
>  
> +	if (pd->dlf_size)

Why does 8250_dw.c have to care about this detail? It feels 8250_dw is 
still crossing "layer" entirely unnecessarily here, can't dwlib's 
dw8250_set_divisor_dlf() just do nothing if dlf_size is not set? That is, 
do this in dw8250_set_divisor_dlf():

	if (!pd->dlf_size)
		return;

-- 
 i.

> +		dw8250_set_divisor_dlf(p, quot_frac);
>  	serial_dl_write(up, quot);
>  	serial_port_out(p, UART_LCR, up->lcr);
>  
> @@ -651,7 +654,6 @@ static int dw8250_probe(struct platform_device *pdev)
>  
>  	p->set_ldisc	= dw8250_set_ldisc;
>  	p->set_termios	= dw8250_set_termios;
> -	p->set_divisor	= dw8250_set_divisor;
>  
>  	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>  	if (!data)
> @@ -768,6 +770,11 @@ static int dw8250_probe(struct platform_device *pdev)
>  	if (!data->skip_autocfg)
>  		dw8250_setup_port(p);
>  
> +	/* Use the BUSY-safe sequence for both integer and fractional divisors. */
> +	if (data->data.dlf_size)
> +		p->get_divisor = dw8250_get_divisor_dlf;
> +	p->set_divisor = dw8250_set_divisor;
> +
>  	/* If we have a valid fifosize, try hooking up DMA */
>  	if (p->fifosize) {
>  		data->data.dma.rxconf.src_maxburst = p->fifosize / 4;
> diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
> index 9bb02a4ab11f..88562c8d5d1d 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.c
> +++ b/drivers/tty/serial/8250/8250_dwlib.c
> @@ -23,8 +23,8 @@
>   * we have: div(F) * (16 * baud) = rem
>   * so frac = 2^dlf_size * rem / (16 * baud) = (rem << dlf_size) / (16 * baud)
>   */
> -static unsigned int dw8250_get_divisor(struct uart_port *p, unsigned int baud,
> -				       unsigned int *frac)
> +unsigned int dw8250_get_divisor_dlf(struct uart_port *p, unsigned int baud,
> +				  unsigned int *frac)
>  {
>  	unsigned int quot, rem, base_baud = baud * 16;
>  	struct dw8250_port_data *d = p->private_data;
> @@ -35,11 +35,18 @@ static unsigned int dw8250_get_divisor(struct uart_port *p, unsigned int baud,
>  
>  	return quot;
>  }
> +EXPORT_SYMBOL_GPL(dw8250_get_divisor_dlf);
> +
> +void dw8250_set_divisor_dlf(struct uart_port *p, unsigned int quot_frac)
> +{
> +	dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
> +}
> +EXPORT_SYMBOL_GPL(dw8250_set_divisor_dlf);
>  
>  static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
>  			       unsigned int quot, unsigned int quot_frac)
>  {
> -	dw8250_writel_ext(p, DW_UART_DLF, quot_frac);
> +	dw8250_set_divisor_dlf(p, quot_frac);
>  	serial8250_do_set_divisor(p, baud, quot);
>  }
>  
> @@ -217,7 +224,7 @@ void dw8250_setup_port(struct uart_port *p)
>  
>  	if (reg) {
>  		pd->dlf_size = fls(reg);
> -		p->get_divisor = dw8250_get_divisor;
> +		p->get_divisor = dw8250_get_divisor_dlf;
>  		p->set_divisor = dw8250_set_divisor;
>  	}
>  
> diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
> index 39fcb1b323ad..7191cabfb086 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.h
> +++ b/drivers/tty/serial/8250/8250_dwlib.h
> @@ -97,6 +97,8 @@ struct dw8250_port_data {
>  	int		srbr;
>  };
>  
> +unsigned int dw8250_get_divisor_dlf(struct uart_port *p, unsigned int baud, unsigned int *frac);
> +void dw8250_set_divisor_dlf(struct uart_port *p, unsigned int quot_frac);
>  void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, const struct ktermios *old);
>  void dw8250_setup_port(struct uart_port *p);
>  
> 
> ---
> base-commit: 1184f87e2790e8af9be3116f608d2e86b4792d7d
> change-id: 20261023-submit-serial-8250-dw-divisor-v1-5f1689ed05a8
> 
> Best regards,
> --  
> James Hilliard <james.hilliard1@xxxxxxxxx>
>