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

[Laurent Vivier]

Le 17/10/2021 à 11:40, Mark Cave-Ayland a écrit :
> 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).
>>
>>> +
>>> +    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"?
> 
> Were you happy to leave this as irq? Another alternative may be to use the 
> BIT() macro as suggested
> by Zoltan.

I have no problem to keep this like that.

Thanks,
Laurent