diff mbox series

[v6,16/30] hw/intc/sh_intc: Turn some defines into an enum

Message ID 4adf4e1ac9d2e728e5a536c69e310d77f0c4455a.1635541329.git.balaton@eik.bme.hu
State New
Headers show
Series More SH4 clean ups (including code style series) | expand

Commit Message

BALATON Zoltan Oct. 29, 2021, 9:02 p.m. UTC
Turn the INTC_MODE defines into an enum and clean up the function
returning these to make it clearer by removing nested ifs and
superfluous parenthesis. The one remaining #define is a flag which is
moved further apart by changing its value from 8 to 0x80 to leave some
spare bits as this is or-ed with the enum value at some places.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/intc/sh_intc.c | 43 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 30, 2021, 10:42 a.m. UTC | #1
On 10/29/21 23:02, BALATON Zoltan wrote:
> Turn the INTC_MODE defines into an enum and clean up the function
> returning these to make it clearer by removing nested ifs and
> superfluous parenthesis. The one remaining #define is a flag which is
> moved further apart by changing its value from 8 to 0x80 to leave some
> spare bits as this is or-ed with the enum value at some places.
> 
> Signed-off-by: BALATON Zoltan <balaton@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 ac47b7f905..16d94458d4 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
> +typedef enum {
> +    INTC_MODE_NONE,
> +    INTC_MODE_DUAL_SET,
> +    INTC_MODE_DUAL_CLR,
> +    INTC_MODE_ENABLE_REG,
> +    INTC_MODE_MASK_REG,
> +} SHIntCMode;

This doesn't seem clearer, because sh_intc_write() does:

    switch (mode) {
    case INTC_MODE_ENABLE_REG | INTC_MODE_IS_PRIO:
        break;
    case INTC_MODE_DUAL_SET:
        value |= *valuep;
        break;
    case INTC_MODE_DUAL_CLR:
        value = *valuep & ~value;
        break;
    default:
        g_assert_not_reached();
    }

Anyway I expect this whole file to be rewritten by Yoshinori,
so I don't mind much.
diff mbox series

Patch

diff --git a/hw/intc/sh_intc.c b/hw/intc/sh_intc.c
index ac47b7f905..16d94458d4 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
+typedef enum {
+    INTC_MODE_NONE,
+    INTC_MODE_DUAL_SET,
+    INTC_MODE_DUAL_CLR,
+    INTC_MODE_ENABLE_REG,
+    INTC_MODE_MASK_REG,
+} 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 */