Re: [PATCH v2 4/7] serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling

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

On Wed, Jan 28, 2026 at 12:52:58PM +0200, Ilpo Järvinen wrote:
> dw8250_handle_irq() takes port's lock multiple times with no good
> reason to release it in between and calls serial8250_handle_irq()
> that also takes port's lock.
> 
> Take port's lock only once in dw8250_handle_irq() and use
> serial8250_handle_irq_locked() to avoid releasing port's lock in
> between.
> 
> As IIR_NO_INT check in serial8250_handle_irq() was outside of port's
> lock, it has to be done already in dw8250_handle_irq().
> 
> DW UART can, in addition to IIR_NO_INT, report BUSY_DETECT (0x7) which
> collided with the IIR_NO_INT (0x1) check in serial8250_handle_irq()
> (because & is used instead of ==) meaning that no other work is done by
> serial8250_handle_irq() during an BUSY_DETECT interrupt.
> 
> This allows reorganizing code in dw8250_handle_irq() to do both
> IIR_NO_INT and BUSY_DETECT handling right at the start simplifying
> the logic.

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

...

>  	if (!up->dma && rx_timeout) {
>  		status = serial_lsr_in(up);
>  
>  		if (!(status & (UART_LSR_DR | UART_LSR_BI)))
>  			serial_port_in(p, UART_RX);
>  	}
>  
>  	/* Manually stop the Rx DMA transfer when acting as flow controller */
>  	if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma && up->dma->rx_running && rx_timeout) {
>  		status = serial_lsr_in(up);
>  
>  		if (status & (UART_LSR_DR | UART_LSR_BI)) {
>  			dw8250_writel_ext(p, RZN1_UART_RDMACR, 0);
			...
>  		}
>  	}

Looks like now (perhaps in a separate change) this may be refactored even more.

	if (rx_timeout) {
		status = serial_lsr_in(up);

// Although not sure about moving the above read out from the specific conditions.

		if (up->dma && (status & (UART_LSR_DR | UART_LSR_BI))) {
			/* Manually stop the Rx DMA transfer when acting as flow controller */
			if (quirks & DW_UART_QUIRK_IS_DMA_FC && up->dma->rx_running) {
				dw8250_writel_ext(p, RZN1_UART_RDMACR, 0);
				dw8250_writel_ext(p, DW_UART_DMASA, 1);
		} else if (!up->dma && !(status & (UART_LSR_DR | UART_LSR_BI)))
			/* ... */
			serial_port_in(p, UART_RX);
		}
	}

-- 
With Best Regards,
Andy Shevchenko