Re: [Qemu-devel] SH: Improve the interrupt controller

[takasi-y@ops.dti.ne.jp]

Hi,

> On the CPU, INT2MSKR is initialized to all zeros, not masking anything. In QEMU,
Please read HW manual. According to a SH7785 HW manual(REJ09B0261-0100),
Initial value of four mask registers are {ff000000,ff000000,00000000,ffffffff}.
There's no general rule for initial value of registers.
But default = 0 not seems to be a bad idea.
Attached patch enables to supply initial value, otherwise 0.

Tested for little endian r2d on PC with source below
svn head: svn://svn.sv.gnu.org/qemu/trunk@6838
+ --append patch : Sun, 8 Mar 2009 03:00:17 +0900 (JST)
+ intc old patch : Mon, 9 Feb 2009 03:57:25 +0900 (JST)
+ intc last patch : Wed, 18 Feb 2009 03:32:15 +0900 (JST)
+ This patch.

BTW, our(mostly "my", sorry) slow conversation seems to upset some people.
I don't care writing code without your source code, because my reference
is always a HW manual. But, it is true that this style of development is
not efficient nor interesting.

How about committing your patches even if your target doesn't work?
I do care regression, and adding code not based on HW manual.
But your target is new one, it is not a regression ;)

/yoshii
---
Initial value for registers support.
More fix about priority 4<->5 bit conversion.

Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>

diff --git a/hw/sh7750.c b/hw/sh7750.c
index 57a5d7d..7c9bae8 100644
--- a/hw/sh7750.c
+++ b/hw/sh7750.c
@@ -505,7 +505,7 @@ static struct intc_prio_reg prio_registers[] = {
 	{ 0xffd00004, 0, 16, 4, /* IPRA */ { TMU0, TMU1, TMU2, RTC } },
 	{ 0xffd00008, 0, 16, 4, /* IPRB */ { WDT, REF, SCI1, 0 } },
 	{ 0xffd0000c, 0, 16, 4, /* IPRC */ { GPIOI, DMAC, SCIF, HUDI } },
-	{ 0xffd00010, 0, 16, 4, /* IPRD */ { IRL0, IRL1, IRL2, IRL3 } },
+	{ 0xffd00010, 0, 16, 4, /* IPRD */ { IRL0, IRL1, IRL2, IRL3 }, 0xda74 },
 	{ 0xfe080000, 0, 32, 4, /* INTPRI00 */ { 0, 0, 0, 0,
 						 TMU4, TMU3,
 						 PCIC1, PCIC0_PCISERR } },
diff --git a/hw/sh_intc.c b/hw/sh_intc.c
index 7cd1fad..a67d148 100644
--- a/hw/sh_intc.c
+++ b/hw/sh_intc.c
@@ -105,6 +105,9 @@ int sh_intc_get_pending_vector(struct intc_desc *desc, int *priority)
 	}
     }
 
+    /* Priority in intc is 5 bits, whereas processor knows only 4 bits. */
+    highest_priority >>= 1;
+
     if (found != -1  && highest_priority > *priority) {
 	struct intc_source *source = desc->sources + found;
 
@@ -115,8 +118,7 @@ int sh_intc_get_pending_vector(struct intc_desc *desc, int *priority)
 	if (desc->icr0 & ICR0_LVLMODE)
 	    sh_intc_toggle_source(source, 0, -1);
 
-	/* Priority in intc is 5 bits, whereas processor knows only 4 bits. */
-	*priority = highest_priority >> 1;
+	*priority = highest_priority;
 
 	return source->vect;
     }
@@ -365,8 +367,10 @@ static void sh_intc_register_source(struct intc_desc *desc,
 	    struct intc_mask_reg *mr = desc->mask_regs + i;
 
 	    for (k = 0; k < ARRAY_SIZE(mr->enum_ids); k++) {
+		int shift = mr->reg_width - (k+1);
 		if (mr->enum_ids[k] == source) {
 		    s->enable_max++;
+		    s->enable_count += (mr->value >> shift) & 1;
 		    break;
 		}
 	    }
@@ -378,8 +382,12 @@ static void sh_intc_register_source(struct intc_desc *desc,
 	    struct intc_prio_reg *pr = desc->prio_regs + i;
 
 	    for (k = 0; k < ARRAY_SIZE(pr->enum_ids); k++) {
+		int shift = pr->reg_width - pr->field_width * (k+1);
+		int mask = (1 << pr->field_width) - 1;
 		if (pr->enum_ids[k] == source) {
+		    s->priority = (pr->value >> shift) & mask;
 		    s->enable_max++;
+		    s->enable_count += s->priority?1:0;
 		    break;
 		}
 	    }