Hi Dick,
On 2019-12-20, Dick Hollenbeck <dick@xxxxxxxxxxx> wrote:
> I've included a partial patch against Greg KH's tree below just to
> convey the design concepts that I introduce below in 4), 5) and 6).
>
> Hopefully this can draw a bridge between mainline, RT and where Greg
> is going with his tree, and make everyone happy.
After spending time working with and completing your patch, I realized
that the 8250 atomic console implementation does not need to be so
complex. Only serial8250_console_write_atomic() needs to track if it
needs to re-enable IER. Everything else can be as in mainline.
I will post a patch that reverts the unneeded complexity, which will
solve your problem.
Creating a macro for clearing IER is probably a good idea anyway to
eliminate all the capabilities-checking redundancy. But I'll leave that
as an excercise for mainline.
Your patch helped me to realize the insanity in what I was doing. Thank
you.
John Ogness
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index 33ad9d6de..e035e91ac 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -130,12 +130,31 @@ static inline void serial_dl_write(struct uart_8250_port *up, int value)
> up->dl_write(up, value);
> }
>
> +/* All write access to IER comes through here, for instrumentation sake. */
> +static inline unsigned char serial8250_set_IER(struct uart_8250_port *up, unsigned char ier)
> +{
> +#ifdef CONFIG_NMI_CONSOLE_CORNER_CASE
> + return __serial9250_set_IER(p, ier)
> +#else
> + unsigned char prior = up->ier;
> +
> + up->ier = ier; /* struct's ier field is always current with hardware */
> + serial_out(up, UART_IER, ier);
> + return prior;
> +#endif
> +}
> +
> +static inline unsigned char serial8250_clear_IER(struct uart_8250_port *up)
> +{
> + return serial8250_set_IER(up, up->capabilities & UART_CAP_UUE ? UART_IER_UUE : 0);
> +}
> +
> static inline bool serial8250_set_THRI(struct uart_8250_port *up)
> {
> if (up->ier & UART_IER_THRI)
> return false;
> - up->ier |= UART_IER_THRI;
> - serial_out(up, UART_IER, up->ier);
> +
> + serial8250_set_IER(up, up->ier | UART_IER_THRI);
> return true;
> }
>
> @@ -143,8 +162,7 @@ static inline bool serial8250_clear_THRI(struct uart_8250_port *up)
> {
> if (!(up->ier & UART_IER_THRI))
> return false;
> - up->ier &= ~UART_IER_THRI;
> - serial_out(up, UART_IER, up->ier);
> + serial8250_set_IER(up, up->ier & ~UART_IER_THRI);
> return true;
> }
>
> @@ -343,3 +361,7 @@ static inline int serial_index(struct uart_port *port)
> {
> return port->minor - 64;
> }
> +
> +#ifdef CONFIG_NMI_CONSOLE_CORNER_CASE
> +unsigned char __serial8250_set_IER(struct uart_8250_port *up, unsigned char ier);
> +#endif
