On 10/27/21 15:46, BALATON Zoltan wrote: > Turn the INTC_MODE defines into an enum (except the one which is a > flag) and clean up the function returning these to make it clearer by > removing nested ifs and superfluous parenthesis. > > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > --- > hw/intc/sh_intc.c | 43 +++++++++++++++++++------------------------ > 1 file changed, 19 insertions(+), 24 deletions(-) > > diff --git a/hw/intc/sh_intc.c b/hw/intc/sh_intc.c > index 0bd27aaf4f..18461ff554 100644 > --- a/hw/intc/sh_intc.c > +++ b/hw/intc/sh_intc.c > @@ -100,33 +100,27 @@ int sh_intc_get_pending_vector(struct intc_desc *desc, > int imask) > abort(); > } > > -#define INTC_MODE_NONE 0 > -#define INTC_MODE_DUAL_SET 1 > -#define INTC_MODE_DUAL_CLR 2 > -#define INTC_MODE_ENABLE_REG 3 > -#define INTC_MODE_MASK_REG 4 > -#define INTC_MODE_IS_PRIO 8 > - > -static unsigned int sh_intc_mode(unsigned long address, > - unsigned long set_reg, unsigned long > clr_reg) > +#define INTC_MODE_IS_PRIO 0x80
Why change 8 -> 0x80 without updating the definition usage? > +typedef enum { > + INTC_MODE_NONE, > + INTC_MODE_DUAL_SET, > + INTC_MODE_DUAL_CLR, > + INTC_MODE_ENABLE_REG, > + INTC_MODE_MASK_REG, If this is defined by the spec, better explicit the enum value, otherwise if someone add a misplaced field this would change the definitions. > +} SHIntCMode; > + > + > +static SHIntCMode sh_intc_mode(unsigned long address, unsigned long set_reg, > + unsigned long clr_reg) > { > - if ((address != A7ADDR(set_reg)) && > - (address != A7ADDR(clr_reg))) > + if (address != A7ADDR(set_reg) && address != A7ADDR(clr_reg)) { > return INTC_MODE_NONE; > - > - if (set_reg && clr_reg) { > - if (address == A7ADDR(set_reg)) { > - return INTC_MODE_DUAL_SET; > - } else { > - return INTC_MODE_DUAL_CLR; > - } > } > - > - if (set_reg) { > - return INTC_MODE_ENABLE_REG; > - } else { > - return INTC_MODE_MASK_REG; > + if (set_reg && clr_reg) { > + return address == A7ADDR(set_reg) ? > + INTC_MODE_DUAL_SET : INTC_MODE_DUAL_CLR; > } > + return set_reg ? INTC_MODE_ENABLE_REG : INTC_MODE_MASK_REG; > } > > static void sh_intc_locate(struct intc_desc *desc, > @@ -137,7 +131,8 @@ static void sh_intc_locate(struct intc_desc *desc, > unsigned int *width, > unsigned int *modep) > { > - unsigned int i, mode; > + SHIntCMode mode; > + unsigned int i; > > /* this is slow but works for now */ > >