Re: [PATCH 3/8] q800: use GLUE IRQ numbers instead of IRQ level for GLUE IRQs

[Mark Cave-Ayland]

On 15/10/2021 07:31, Laurent Vivier wrote:

> Le 13/10/2021 à 23:21, Mark Cave-Ayland a écrit :
> > In order to allow dynamic routing of IRQs to different IRQ levels on the CPU
> > depending upon port B bit 6, use GLUE IRQ numbers and map them to the the
> > corresponding CPU IRQ level accordingly.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/m68k/q800.c | 32 ++++++++++++++++++++++++++++----
> >   1 file changed, 28 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> > index 15f3067811..81c335bf16 100644
> > --- a/hw/m68k/q800.c
> > +++ b/hw/m68k/q800.c
> > @@ -102,11 +102,34 @@ struct GLUEState {
> >       uint8_t ipr;
> >   };
> >   
> > +#define GLUE_IRQ_IN_VIA1       0
> > +#define GLUE_IRQ_IN_VIA2       1
> > +#define GLUE_IRQ_IN_SONIC      2
> > +#define GLUE_IRQ_IN_ESCC       3
> > +
> >   static void GLUE_set_irq(void *opaque, int irq, int level)
> >   {
> >       GLUEState *s = opaque;
> >       int i;
> >   
> > +    switch (irq) {
> > +    case GLUE_IRQ_IN_VIA1:
> > +        irq = 5;
> > +        break;
>
> Perhaps you can move this patch before patch 2 to help to understand why 
> GLUE_IRQ_IN_VIA1 (0) is
> mapped to irq 5 (before patch 2 it would be to 0).

I think it should stay in the existing order because patch 2 is really a bug fix: all 
of the other IRQs are statically wired in A/UX mode except for VIA1. Once this is 
fixed, this patch then abstracts the *input* IRQs away from the CPU level so they can 
be swizzled independently depending upon whether A/UX mode is selected.

> > +
> > +    case GLUE_IRQ_IN_VIA2:
> > +        irq = 1;
> > +        break;
> > +
> > +    case GLUE_IRQ_IN_SONIC:
> > +        irq = 2;
> > +        break;
> > +
> > +    case GLUE_IRQ_IN_ESCC:
> > +        irq = 3;
> > +        break;
> > +    }
> > +
> >       if (level) {
> >           s->ipr |= 1 << irq;
>
> perhaps you can rename here "irq" to "shift"?

At this point it is the CPU level IRQ so "irq" seems correct here. Are you thinking 
that using a different intermediate variable from the function parameter would help?


ATB,

Mark.