Re: [PATCH v2 28/35] xen/8250-uart: add missing definitions

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Dec 05, 2024 at 08:41:58PM -0800, Denis Mukhin via B4 Relay wrote:
> From: Denis Mukhin <dmukhin@ford.com>
> 
> Added missing definitions needed for NS8250 UART emulator.
> 
> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> ---
>  xen/include/xen/8250-uart.h | 81 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 60 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
> index d13352940c13c50bac17d4cdf2f3bf584380776a..4a69b2b225140efda287bdf96fa0caa4aa70074f 100644
> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -32,16 +32,22 @@
>  #define UART_MCR          0x04    /* Modem control        */
>  #define UART_LSR          0x05    /* line status          */
>  #define UART_MSR          0x06    /* Modem status         */
> +#define UART_SCR          0x07    /* Scratch pad          */
>  #define UART_USR          0x1f    /* Status register (DW) */
>  #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
>  #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
>  #define UART_XR_EFR       0x09    /* Enhanced function register (Exar) */
>  
> +/* ns8250 emulator: range of emulated registers [0..UART_MAX-1] */
> +#define UART_MAX          (UART_SCR + 1)
> +
>  /* Interrupt Enable Register */
>  #define UART_IER_ERDAI    0x01    /* rx data recv'd       */
>  #define UART_IER_ETHREI   0x02    /* tx reg. empty        */
>  #define UART_IER_ELSI     0x04    /* rx line status       */
>  #define UART_IER_EMSI     0x08    /* MODEM status         */
> +#define UART_IER_MASK \
> +    (UART_IER_ERDAI | UART_IER_ETHREI | UART_IER_ELSI | UART_IER_EMSI)
>  
>  /* Interrupt Identification Register */
>  #define UART_IIR_NOINT    0x01    /* no interrupt pending */
> @@ -51,12 +57,21 @@
>  #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_FE0      BIT(6, U) /* FIFO enable #0 */
> +#define UART_IIR_FE1      BIT(7, U) /* FIFO enable #1 */
> +#define UART_IIR_FE_MASK  (UART_IIR_FE0 | UART_IIR_FE1)
>  
>  /* FIFO Control Register */
> -#define UART_FCR_ENABLE   0x01    /* enable FIFO          */
> -#define UART_FCR_CLRX     0x02    /* clear Rx FIFO        */
> -#define UART_FCR_CLTX     0x04    /* clear Tx FIFO        */
> -#define UART_FCR_DMA      0x10    /* enter DMA mode       */
> +#define UART_FCR_ENABLE     BIT(0, U)   /* enable FIFO          */
> +#define UART_FCR_CLRX       BIT(1, U)   /* clear Rx FIFO        */
> +#define UART_FCR_CLTX       BIT(2, U)   /* clear Tx FIFO        */
> +#define UART_FCR_DMA        BIT(3, U)   /* enter DMA mode       */
> +#define UART_FCR_RESERVED0  BIT(4, U)   /* reserved; always 0   */
> +#define UART_FCR_RESERVED1  BIT(5, U)   /* reserved; always 0   */

We don't usually define reserved bits I think, as I assume those won't
be used by the code?

Maybe a reserved mask for the hole register, to ensure the access to
the register doesn't attempt to set reserved bits?  But even then the
best we can do when emulating is print a warning message.

Thanks, Roger.