diff mbox series

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

Message ID c886cdc2911f250c3e8e15ceb717124c7044b50b.1629799776.git.fthain@linux-m68k.org (mailing list archive)
State New, archived
Headers show
Series [RFC,01/10] hw/mos6522: Remove get_load_time() methods and functions | expand

Commit Message

Finn Thain Aug. 24, 2021, 10:09 a.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé Aug. 24, 2021, 10:22 a.m. UTC | #1 
On 8/24/21 12:09 PM, 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(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Mark Cave-Ayland Aug. 25, 2021, 8:26 a.m. UTC | #2 
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.
diff mbox series

Patch

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);
     }
     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);
         break;
     case VIA_REG_T1LL:
         val = s->timers[0].latch & 0xff;