On Thu, Feb 20, 2025 at 12:16:03PM +0100, Jiri Slaby (SUSE) wrote:
> There are serial_in/out() helpers to be used instead of direct
> p->serial_in/out(). Use them in various 8250 drivers.
Okay, I have checked all calls, and I think to avoid possible issues
we need to add a few comments here and there, see below.
But first of all, can we actually use serial_port_in()/serial_port_out()
in the cases where we have already a pointer to uart_port?
...
Here we should add a comment like
/*
* This function is being called as part of the uart_port::serial_out()
* routine. Hence it must not call serial_out() on itself against
* the modified registers here, i.e. LCR.
*/
> static void dw8250_force_idle(struct uart_port *p)
> {
> struct uart_8250_port *up = up_to_u8250p(p);
> unsigned int lsr;
Here we should add a comment like
/*
* The following call currently performs serial_out()
* against the FCR register. Because it differs to LCR
* there will be no dead loop, but if it ever gets
* modified, we might need a new custom version that
* avoids infinite recursion.
*/
> serial8250_clear_and_reinit_fifos(up);
...
On top of this function we need to add the same comment as on top of
dw8250_force_idle() above.
> static void dw8250_check_lcr(struct uart_port *p, int offset, int value)
...
The rest is legit as we shouldn't use those in the ->serial_in()/->serial_out().
--
With Best Regards,
Andy Shevchenko
