Re: [RFC 08/10] hw/mos6522: Call mos6522_update_irq() when appropriate

[Mark Cave-Ayland]

On 24/08/2021 11:09, Finn Thain wrote:

> It necessary to call mos6522_update_irq() when the interrupt flags
> change and unnecessary when they haven't.
>
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
> ---
>   hw/misc/mos6522.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> index 0a241fe9f8..0dd3ccf945 100644
> --- a/hw/misc/mos6522.c
> +++ b/hw/misc/mos6522.c
> @@ -208,11 +208,13 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned 
> size)
>           s->timers[0].oneshot_fired = true;
>           mos6522_timer1_update(s, &s->timers[0], now);
>           s->ifr |= T1_INT;
> +        mos6522_update_irq(s);
>       }
>       if (now >= s->timers[1].next_irq_time) {
>           s->timers[1].oneshot_fired = true;
>           mos6522_timer2_update(s, &s->timers[1], now);
>           s->ifr |= T2_INT;
> +        mos6522_update_irq(s);
>       }

Again this seems to be in the block of code I'm not sure is correct, so my first 
instinct is to see if removing it helps first - although the patch logically seems 
correct.

>       switch (addr) {
>       case VIA_REG_B:
> @@ -237,7 +239,6 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned 
> size)
>           break;
>       case VIA_REG_T1CH:
>           val = get_counter(s, &s->timers[0]) >> 8;
> -        mos6522_update_irq(s);

As get_counter() simply generates the current counter value I'd say this part 
is correct.

>           break;
>       case VIA_REG_T1LL:
>           val = s->timers[0].latch & 0xff;


ATB,

Mark.