Re: [PATCH 01/11] mos6522: add defines for IFR bit flags

[Philippe Mathieu-Daudé]

On 5/2/22 11:51, Mark Cave-Ayland wrote:
> On 27/01/2022 23:16, BALATON Zoltan wrote:
>
> > On Thu, 27 Jan 2022, Mark Cave-Ayland wrote:
> > > These are intended to make it easier to see how the physical control 
> > > lines
> > > are wired for each instance.
> > >
> > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > > ---
> > > include/hw/misc/mos6522.h | 22 +++++++++++++++-------
> > > 1 file changed, 15 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
> > > index fc95d22b0f..12abd8b8d2 100644
> > > --- a/include/hw/misc/mos6522.h
> > > +++ b/include/hw/misc/mos6522.h
> > > @@ -41,13 +41,21 @@
> > > #define IER_SET            0x80    /* set bits in IER */
> > > #define IER_CLR            0       /* clear bits in IER */
> > >
> > > -#define CA2_INT            0x01
> > > -#define CA1_INT            0x02
> > > -#define SR_INT             0x04    /* Shift register full/empty */
> > > -#define CB2_INT            0x08
> > > -#define CB1_INT            0x10
> > > -#define T2_INT             0x20    /* Timer 2 interrupt */
> > > -#define T1_INT             0x40    /* Timer 1 interrupt */
> > > +#define CA2_INT_BIT        0
> > > +#define CA1_INT_BIT        1
> > > +#define SR_INT_BIT         2       /* Shift register full/empty */
> > > +#define CB2_INT_BIT        3
> > > +#define CB1_INT_BIT        4
> > > +#define T2_INT_BIT         5       /* Timer 2 interrupt */
> > > +#define T1_INT_BIT         6       /* Timer 1 interrupt */
> > > +
> > > +#define CA2_INT            (1 << CA2_INT_BIT)
> > > +#define CA1_INT            (1 << CA1_INT_BIT)
> > > +#define SR_INT             (1 << SR_INT_BIT)
> > > +#define CB2_INT            (1 << CB2_INT_BIT)
> > > +#define CB1_INT            (1 << CB1_INT_BIT)
> > > +#define T2_INT             (1 << T2_INT_BIT)
> > > +#define T1_INT             (1 << T1_INT_BIT)
> >
> > Maybe you could leave the #defines called XX_INT and then use 
> > BIT(XX_INT) instead of the second set of #defines which would provide 
> > same readability but with less #defines needed.
>
> I'm not so keen on removing the _INT defines since that would require 
> updating all users to use BIT() everywhere which I don't think gains us 
> much. I could certainly replace these definitions with BIT(FOO) instead 
> of (1 << FOO) if that helps readability though.

Do you mean simply doing this?

-#define T1_INT             0x40    /* Timer 1 interrupt */
+#define T1_INT             BIT(6)  /* Timer 1 interrupt */