Re: [RFC 03/10] hw/mos6522: Remove redundant mos6522_timer1_update() calls

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

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

> Reads and writes to the TL and TC registers have no immediate effect on
> a running timer, with the exception of a write to TCH. Hence these
> mos6522_timer_update() calls are not needed.
> 
> Signed-off-by: Finn Thain <fthain@linux-m68k.org>

Perhaps better to flip this description around i.e. mention that the low bytes are 
written to a latch and then the full 16-bit value is transferred to the latch/counter 
when the high byte is written?

Otherwise I think this looks okay.

> ---
>   hw/misc/mos6522.c | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
> index ff246b5437..1d4a56077e 100644
> --- a/hw/misc/mos6522.c
> +++ b/hw/misc/mos6522.c
> @@ -234,7 +234,6 @@ uint64_t mos6522_read(void *opaque, hwaddr addr, unsigned size)
>           val = s->timers[0].latch & 0xff;
>           break;
>       case VIA_REG_T1LH:
> -        /* XXX: check this */
>           val = (s->timers[0].latch >> 8) & 0xff;
>           break;
>       case VIA_REG_T2CL:
> @@ -303,8 +302,6 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>           break;
>       case VIA_REG_T1CL:
>           s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> -        mos6522_timer1_update(s, &s->timers[0],
> -                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>           break;
>       case VIA_REG_T1CH:
>           s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
> @@ -313,14 +310,10 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>           break;
>       case VIA_REG_T1LL:
>           s->timers[0].latch = (s->timers[0].latch & 0xff00) | val;
> -        mos6522_timer1_update(s, &s->timers[0],
> -                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>           break;
>       case VIA_REG_T1LH:
>           s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
>           s->ifr &= ~T1_INT;
> -        mos6522_timer1_update(s, &s->timers[0],
> -                              qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
>           break;
>       case VIA_REG_T2CL:
>           s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
> 


ATB,

Mark.