Re: [RFC 05/10] hw/mos6522: Don't clear T1 interrupt flag on latch write

[Laurent Vivier <laurent@vivier.eu>]

Le 26/08/2021 à 07:21, Finn Thain a écrit :
> On Wed, 25 Aug 2021, Mark Cave-Ayland wrote:
> 
>> On 24/08/2021 11:09, Finn Thain wrote:
>>
>>> The Synertek datasheet says, "A write to T1L-H loads an 8-bit count value
>>> into the latch. A read of T1L-H transfers the contents of the latch to
>>> the data bus. Neither operation has an affect [sic] on the interrupt
>>> flag."
>>>
>>> Signed-off-by: Finn Thain <fthain@linux-m68k.org>
>>> ---
>>>   hw/misc/mos6522.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c
>>> index c0d6bee4cc..ffff8991f4 100644
>>> --- a/hw/misc/mos6522.c
>>> +++ b/hw/misc/mos6522.c
>>> @@ -313,7 +313,6 @@ void mos6522_write(void *opaque, hwaddr addr, uint64_t
>>> val, unsigned size)
>>>           break;
>>>       case VIA_REG_T1LH:
>>>           s->timers[0].latch = (s->timers[0].latch & 0xff) | (val << 8);
>>> -        s->ifr &= ~T1_INT;
>>>           break;
>>>       case VIA_REG_T2CL:
>>>           s->timers[1].latch = (s->timers[1].latch & 0xff00) | val;
>>
>> Hmmm. The reference document I used for QEMU's 6522 device is at
>> http://archive.6502.org/datasheets/mos_6522_preliminary_nov_1977.pdf and
>> according to page 6 and the section "Writing the Timer 1 Registers" writing to
>> the high byte of the latch does indeed clear the T1 interrupt flag.
>>
>> Side note: there is reference in Gary Davidian's excellent CHM video that
>> 6522s obtained from different manufacturers had different behaviours, and
>> there are also web pages mentioning that 6522s integrated as part of other
>> silicon e.g. IOSB/CUDA also had their own bugs... :/
>>
> 
> The MOS document you've cited is much older than the Synertek and Rockwell 
> devices. The datasheets for the Synertek and Rockwell parts disagree with 
> MOS about T1LH behaviour. Apple certainly used SY6522 devices in my Mac II 
> and I'd assumed Apple would have used compatible logic cores in the custom 
> ICs found in later models. But I don't really trust assumptions and 
> datasheets so I wrote the Linux patch below and ran it on my Quadra 630.
> 
> diff --git a/arch/m68k/mac/via.c b/arch/m68k/mac/via.c
> index 3d11d6219cdd..ed41f6ae2bf2 100644
> --- a/arch/m68k/mac/via.c
> +++ b/arch/m68k/mac/via.c
> @@ -634,3 +634,27 @@ static u64 mac_read_clk(struct clocksource *cs)
>  
>  	return ticks;
>  }
> +
> +static int baz(void)
> +{
> +	u8 a, b, c;
> +
> +	local_irq_disable();
> +
> +	while (!(via1[vIFR] & VIA_TIMER_1_INT))
> +		continue;
> +	a = via1[vIFR] & VIA_TIMER_1_INT;
> +	via1[vT1LH] = via1[vT1LH];
> +	b = via1[vIFR] & VIA_TIMER_1_INT;
> +	via1[vT1LL] = via1[vT1LL];
> +	c = via1[vIFR] & VIA_TIMER_1_INT;
> +
> +	printk("a == %2x\n", a);
> +	printk("b == %2x\n", b);
> +	printk("c == %2x\n", c);
> +
> +	local_irq_enable();
> +
> +	return 0;
> +}
> +late_initcall(baz);
> 
> Based on the Synertek datasheet* one would expect to see b equal to a but 
> I got this result instead:
> 
> [   10.450000] a == 40
> [   10.450000] b ==  0
> [   10.450000] c ==  0
> 
> This amounts to a MOS design flaw and I doubt that this result from my 
> Quadra 630 would apply to other Mac models. So it would be great to see 
> the output from a Quadra 800. But until then, let's disregard this patch.
> 
> * http://archive.6502.org/datasheets/synertek_sy6522.pdf
> 

Tested on my Quadra 800:

[    4.730000] a == 40
[    4.730000] b ==  0
[    4.730000] c ==  0

Laurent