On Sat, Sep 06, 2025 at 05:57:02PM +0300, Mykola Kvach wrote: > Hi Denis, > > On Sat, Sep 6, 2025 at 2:27 AM <dmuk...@xen.org> wrote: [..] > > /* 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 */ > > +#define UART_FCR_RTB0 BIT(6, U) /* receiver trigger bit #0 */ > > +#define UART_FCR_RTB1 BIT(7, U) /* receiver trigger bit #1 */ > > +#define UART_FCR_TRG_MASK (UART_FCR_RTB0 | UART_FCR_RTB1) > > Thanks for the patch. I noticed that in this changeset some bit > definitions (e.g. UART_FCR_*) were rewritten using the BIT(n, U) > macro, while others (e.g. UART_IER_* and rest of UART_FCR_*) are > still left as plain hex values (0x01, 0x02, etc.), even though they > are also powers of two. > > Could you clarify the reasoning behind this choice? From a reader’s > perspective it looks inconsistent. Would it make sense to either: > > - update all of them to use BIT() for consistency, or > - keep the existing style unchanged in this patch and move a full > conversion to BIT() into a separate cleanup patch? > > This would make the codebase easier to follow.
I find BIT() notation is more readable than raw bitmasks. But I agree that definitions should be consistently updated. Will update to the raw masks instead of BIT() in v7.
