Re: [PATCH v3 19/24] xen/8250-uart: add missing definitions

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.01.2025 02:58, Denis Mukhin via B4 Relay wrote:
> @@ -51,12 +54,19 @@
>  #define UART_IIR_THR      0x02    /*  - tx reg. empty     */
>  #define UART_IIR_MSI      0x00    /*  - MODEM status      */
>  #define UART_IIR_BSY      0x07    /*  - busy detect (DW) */
> +#define UART_IIR_FE       0xC0    /* FIFO enabled (2 bits) */

Please can you use lower case hex digits?

> @@ -64,17 +74,17 @@
>  
>  /*
>   * Note: The FIFO trigger levels are chip specific:
> - *   RX:76 = 00  01  10  11  TX:54 = 00  01  10  11
> - * PC16550D:  1   4   8  14          xx  xx  xx  xx
> - * TI16C550A:         1   4   8  14          xx  xx  xx  xx
> - * TI16C550C:         1   4   8  14          xx  xx  xx  xx
> - * ST16C550:  1   4   8  14          xx  xx  xx  xx
> - * ST16C650:  8  16  24  28          16   8  24  30  PORT_16650V2
> - * NS16C552:  1   4   8  14          xx  xx  xx  xx
> - * ST16C654:  8  16  56  60           8  16  32  56  PORT_16654
> - * TI16C750:  1  16  32  56          xx  xx  xx  xx  PORT_16750
> - * TI16C752:  8  16  56  60           8  16  32  56
> - * Tegra:     1   4   8  14          16   8   4   1  PORT_TEGRA
> + *  RX:76 = 00  01  10  11  TX:54 = 00  01  10  11
> + * PC16550D:     1   4   8  14      xx  xx  xx  xx
> + * TI16C550A:    1   4   8  14      xx  xx  xx  xx
> + * TI16C550C:    1   4   8  14      xx  xx  xx  xx
> + * ST16C550:     1   4   8  14      xx  xx  xx  xx
> + * ST16C650:     8  16  24  28      16   8  24  30  PORT_16650V2
> + * NS16C552:     1   4   8  14      xx  xx  xx  xx
> + * ST16C654:     8  16  56  60       8  16  32  56  PORT_16654
> + * TI16C750:     1  16  32  56      xx  xx  xx  xx  PORT_16750
> + * TI16C752:     8  16  56  60       8  16  32  56
> + * Tegra:        1   4   8  14      16   8   4   1  PORT_TEGRA
>   */

What is this change about? All I can see is that the table heading no
longer aligns with table contents.

> @@ -96,11 +106,33 @@
>  #define UART_LCR_CONF_MODE_B 0xBF            /* Configuration mode B */
>  
>  /* Modem Control Register */
> -#define UART_MCR_DTR      0x01    /* Data Terminal Ready  */
> -#define UART_MCR_RTS      0x02    /* Request to Send      */
> -#define UART_MCR_OUT2     0x08    /* OUT2: interrupt mask */
> -#define UART_MCR_LOOP     0x10    /* Enable loopback test mode */
> -#define UART_MCR_TCRTLR   0x40    /* Access TCR/TLR (TI16C752, EFR[4]=1) */
> +#define UART_MCR_DTR            BIT(0, U)   /* Data Terminal Ready  */
> +#define UART_MCR_RTS            BIT(1, U)   /* Request to Send      */
> +#define UART_MCR_OUT1           BIT(2, U)   /* OUT1: interrupt mask */
> +#define UART_MCR_OUT2           BIT(3, U)   /* OUT2: interrupt mask */
> +#define UART_MCR_LOOP           BIT(4, U)   /* Enable loopback test mode */
> +#define UART_MCR_RESERVED0      BIT(5, U)   /* Reserved #0 */
> +#define UART_MCR_RESERVED1      BIT(6, U)   /* Reserved #1 */
> +#define UART_MCR_TCRTLR         BIT(6, U)   /* Access TCR/TLR (TI16C752, 
> EFR[4]=1) */

I find it odd for a bit to be reserved and also not reserved.

> +#define UART_MCR_RESERVED2      BIT(7, U)   /* Reserved #2 */
> +#define UART_MCR_MASK \
> +    (UART_MCR_DTR | UART_MCR_RTS | \
> +     UART_MCR_OUT1 | UART_MCR_OUT2 | \
> +     UART_MCR_LOOP)

Yet then not including UART_MCR_TCRTLR?

> @@ -111,6 +143,7 @@
>  #define UART_LSR_THRE     0x20    /* Xmit hold reg empty  */
>  #define UART_LSR_TEMT     0x40    /* Xmitter empty        */
>  #define UART_LSR_ERR      0x80    /* Error                */
> +#define UART_LSR_MASK     (UART_LSR_OE | UART_LSR_BI)

On what basis were these two bits chose and the others left out?

Jan