Re: [PATCH 6/6] serial: 8250_dw: Ensure BUSY is deasserted

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

On Tue, Jan 27, 2026 at 03:35:27PM +0200, Ilpo Järvinen wrote:
> On Sat, 24 Jan 2026, Andy Shevchenko wrote:
> > On Fri, Jan 23, 2026 at 07:27:39PM +0200, Ilpo Järvinen wrote:

+Cc: printk people to check on printing from a serial driver routines.

...

> > > +	/* Prevent triggering interrupt from RBR filling */
> > > +	p->serial_out(p, UART_IER, 0);
> > 
> > Do we specifically use callbacks directly and not wrappers all over the change?
> 
> I guess it's just a habit, I suppose you meant using serial_port_in/out 
> instead. I can try to change those.

Not (only) me. Jiri updated this driver (and many others) to use callbacks.
That's why I added comments here and there about possible recursions.

...

> > > +	serial8250_fifo_wait_for_lsr_thre(up, p->fifosize);
> > > +	ndelay(p->frame_time);
> > 
> > Wouldn't be a problem on lowest baud rates (exempli gratia 110)?
> 
> Perhaps, but until somebody comes with an issue report related to 110, I'm 
> wondering if this really is worth trying to address. Any suggestion how is 
> welcome as well?

Polling work? Timer?

> > > +	retries = 4;	/* Arbitrary limit, 2 was always enough in tests */
> > > +	do {
> > > +		serial8250_clear_fifos(up);
> > > +		if (!(p->serial_in(p, usr_reg) & DW_UART_USR_BUSY))
> > > +			break;
> > > +		ndelay(p->frame_time);
> > > +	} while (--retries);
> > 
> > read_poll_timeout_atomic() ? I assume it can't be used due to small frame time?
> 
> Frame time is in nanoseconds yes. I did consider 
> read_poll_timeout_atomic() but it would have required nsec -> usec 
> conversion so I left this as it is.

Yeah with the same issue on low baud rates. So far I think we need to consider
9600 as commonly used by the old HW (which may be connected to a modern PC with
this new kernel running), so the frame time sounds like close to a millisecond.
And this can be met in real life.

Maybe put TODO/FIXME around these ndelay() calls?

> > > +	if (d->in_idle) {
> > 
> > > +		/*
> > > +		 * FIXME: this deadlocks if port->lock is already held
> > > +		 * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > > +		 */
> > 
> > Hmm... That FIXME should gone since we have non-blocking consoles, no?
> 
> No, lockdep still gets angry if printing is used while holding port's 
> lock.

Hmm... Let's ask PRINTK people about this. John, do we still have a gap
with nbcon? Or did I misunderstand the scope of its use?

> What would be possible though, is to mark the port's lock critical section 
> for print deferral (but it's outside the scope of this series). In case of 
> serial, it would be justified to use deferred printing (which is only 
> meant for special cases) because serial console and printing are related.
> 
> > > +		return;
> > > +	}

-- 
With Best Regards,
Andy Shevchenko