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

[Vladimir Prus]

On Sunday 08 February 2009 21:57:25 address@hidden wrote:
> Hi, Vladimir.
> 
> I managed to make the code working.
> r2d boot, SCI and CF working, and /proc/interrupts increases.
> 
> Essential points modified are below.
> 1. sh_intc_init() caller/callee mismatch.
> 2. nobody calls sh_intc_set_irl_priorities()
> 3. INTC_MODE_DUAL_SET/CLR swapped
> I'm not sure 3 in your patch is on purpose or not.
> 
> Attached patch is a diff against rev#6563.
> This is yours + my small fixes, which are..
> - Above three
> - sh_intc_set_irl_priorities() has switched to sh_intc_init_irl_priorities().
> - sh_intc_set_irl()'s enable hack removed.
> - indent,tab/space,brace changed (to what looks like code around)
> - reduce INTC_A7() usage
> - some others.

Hi Yoshii,
I have replaced my original patch with your patch in my patch set, and adjusted
sh7785 emulation for your changes. It works, but only after I re-do the
INTC_MODE_DUAL_SET/CLR swap form my original patch. I think that's about right.
Suppose you set a bit in SET register, the interrupt then should be masked out.
Here's the relevant code:

         case INTC_MODE_DUAL_SET: value = *valuep & ~value; break;

This clears the relevant bit in 'value', and then:

        sh_intc_toggle_mask(desc, enum_ids[k], value & mask, priority, 0);

this passes '0' for 'enable' parameter to sh_intc_toggle_mask, like it should.

However, unless I miss something, it seems like the value read from a register
is actually inverted. When reading from a set register we should get 1 for each
masked register, and we seem to get 1 for each enabled register. If you agree
with this analysis, I can adjust the read function.

But probably, it's best if your combined patch is checked in -- as I've said
I get a working sh4a emulation based on your patch, and it's problematic
to keep such big patches outside the official tree.

- Volodya