On 28/10/2024 16:22, Thomas Huth wrote:
> Am Wed, 23 Oct 2024 09:58:26 +0100
> schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
>
>> The SCSI 4020 logic refers to the offset of the SCSI CSRs within the NeXTCube
>> address space. Due to the previously overlapping memory regions, there were
>> duplicate MMIO accessors in the next.scr memory region for these registers but
>> now this has been resolved. This allows us to move the more complex prototype
>> logic into the next-scsi MMIO accessors.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/m68k/next-cube.c | 139 ++++++++++++++++++++------------------------
>> 1 file changed, 62 insertions(+), 77 deletions(-)
>>
>> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
>> index 32466a425f..22da777006 100644
>> --- a/hw/m68k/next-cube.c
>> +++ b/hw/m68k/next-cube.c
>> @@ -365,8 +365,6 @@ static const MemoryRegionOps next_mmio_ops = {
>>
>> static uint64_t next_scr_readfn(void *opaque, hwaddr addr, unsigned size)
>> {
>> - NeXTPC *s = NEXT_PC(opaque);
>> - NeXTSCSI *ns = NEXT_SCSI(&s->next_scsi);
>> uint64_t val;
>>
>> switch (addr) {
>> @@ -375,16 +373,6 @@ static uint64_t next_scr_readfn(void *opaque, hwaddr addr, unsigned size)
>> val = 0x40 | 0x04 | 0x2 | 0x1;
>> break;
>>
>> - case 0x14020:
>> - DPRINTF("SCSI 4020 STATUS READ %X\n", ns->scsi_csr_1);
>> - val = ns->scsi_csr_1;
>> - break;
>> -
>> - case 0x14021:
>> - DPRINTF("SCSI 4021 STATUS READ %X\n", ns->scsi_csr_2);
>> - val = 0x40;
>
> Where is that hard-coded 0x40 gone now? Please mention this in the commit
> description, otherwise this looks like a mistake?
Heh I guess the part about the duplicate MMIO accessors was a little bit too cryptic?
I'll have a think as to how to improve the commit message for v2.
ATB,
Mark.
>> - break;
>> -
>> /*
>> * These 4 registers are the hardware timer, not sure which register
>> * is the latch instead of data, but no problems so far.
>> @@ -413,9 +401,6 @@ static uint64_t next_scr_readfn(void *opaque, hwaddr addr, unsigned size)
>> static void next_scr_writefn(void *opaque, hwaddr addr, uint64_t val,
>> unsigned size)
>> {
>> - NeXTPC *s = NEXT_PC(opaque);
>> - NeXTSCSI *ns = NEXT_SCSI(&s->next_scsi);
>> -
>> switch (addr) {
>> case 0x14108:
>> DPRINTF("FDCSR Write: %"PRIx64 "\n", val);
>> @@ -424,68 +409,6 @@ static void next_scr_writefn(void *opaque, hwaddr addr, uint64_t val,
>> }
>> break;
>>
>> - case 0x14020: /* SCSI Control Register */
>> - if (val & SCSICSR_FIFOFL) {
>> - DPRINTF("SCSICSR FIFO Flush\n");
>> - /* will have to add another irq to the esp if this is needed */
>> - /* esp_puflush_fifo(esp_g); */
>> - }
>> -
>> - if (val & SCSICSR_ENABLE) {
>> - DPRINTF("SCSICSR Enable\n");
>> - /*
>> - * qemu_irq_raise(s->scsi_dma);
>> - * s->scsi_csr_1 = 0xc0;
>> - * s->scsi_csr_1 |= 0x1;
>> - * qemu_irq_pulse(s->scsi_dma);
>> - */
>> - }
>> - /*
>> - * else
>> - * s->scsi_csr_1 &= ~SCSICSR_ENABLE;
>> - */
>> -
>> - if (val & SCSICSR_RESET) {
>> - DPRINTF("SCSICSR Reset\n");
>> - /* I think this should set DMADIR. CPUDMA and INTMASK to 0 */
>> - qemu_irq_raise(s->scsi_reset);
>> - ns->scsi_csr_1 &= ~(SCSICSR_INTMASK | 0x80 | 0x1);
>> - qemu_irq_lower(s->scsi_reset);
>> - }
>> - if (val & SCSICSR_DMADIR) {
>> - DPRINTF("SCSICSR DMAdir\n");
>> - }
>> - if (val & SCSICSR_CPUDMA) {
>> - DPRINTF("SCSICSR CPUDMA\n");
>> - /* qemu_irq_raise(s->scsi_dma); */
>> - s->int_status |= 0x4000000;
>> - } else {
>> - /* fprintf(stderr,"SCSICSR CPUDMA disabled\n"); */
>> - s->int_status &= ~(0x4000000);
>> - /* qemu_irq_lower(s->scsi_dma); */
>> - }
>> - if (val & SCSICSR_INTMASK) {
>> - DPRINTF("SCSICSR INTMASK\n");
>> - /*
>> - * int_mask &= ~0x1000;
>> - * s->scsi_csr_1 |= val;
>> - * s->scsi_csr_1 &= ~SCSICSR_INTMASK;
>> - * if (s->scsi_queued) {
>> - * s->scsi_queued = 0;
>> - * next_irq(s, NEXT_SCSI_I, level);
>> - * }
>> - */
>> - } else {
>> - /* int_mask |= 0x1000; */
>> - }
>> - if (val & 0x80) {
>> - /* int_mask |= 0x1000; */
>> - /* s->scsi_csr_1 |= 0x80; */
>> - }
>> - DPRINTF("SCSICSR Write: %"PRIx64 "\n", val);
>> - /* s->scsi_csr_1 = val; */
>> - break;
>> -
>> /* Hardware timer latch - not implemented yet */
>> case 0x1a000:
>> default:
>> @@ -846,13 +769,73 @@ static void next_scsi_csr_write(void *opaque, hwaddr addr, uint64_t val,
>> unsigned size)
>> {
>> NeXTSCSI *s = NEXT_SCSI(opaque);
>> + NeXTPC *pc = NEXT_PC(container_of(s, NeXTPC, next_scsi));
>>
>> switch (addr) {
>> case 0:
>> + if (val & SCSICSR_FIFOFL) {
>> + DPRINTF("SCSICSR FIFO Flush\n");
>> + /* will have to add another irq to the esp if this is needed */
>> + /* esp_puflush_fifo(esp_g); */
>> + }
>> +
>> + if (val & SCSICSR_ENABLE) {
>> + DPRINTF("SCSICSR Enable\n");
>> + /*
>> + * qemu_irq_raise(s->scsi_dma);
>> + * s->scsi_csr_1 = 0xc0;
>> + * s->scsi_csr_1 |= 0x1;
>> + * qemu_irq_pulse(s->scsi_dma);
>> + */
>> + }
>> + /*
>> + * else
>> + * s->scsi_csr_1 &= ~SCSICSR_ENABLE;
>> + */
>> +
>> + if (val & SCSICSR_RESET) {
>> + DPRINTF("SCSICSR Reset\n");
>> + /* I think this should set DMADIR. CPUDMA and INTMASK to 0 */
>> + qemu_irq_raise(pc->scsi_reset);
>> + s->scsi_csr_1 &= ~(SCSICSR_INTMASK | 0x80 | 0x1);
>> + qemu_irq_lower(pc->scsi_reset);
>> + }
>> + if (val & SCSICSR_DMADIR) {
>> + DPRINTF("SCSICSR DMAdir\n");
>> + }
>> + if (val & SCSICSR_CPUDMA) {
>> + DPRINTF("SCSICSR CPUDMA\n");
>> + /* qemu_irq_raise(s->scsi_dma); */
>> + pc->int_status |= 0x4000000;
>> + } else {
>> + /* fprintf(stderr,"SCSICSR CPUDMA disabled\n"); */
>> + pc->int_status &= ~(0x4000000);
>> + /* qemu_irq_lower(s->scsi_dma); */
>> + }
>> + if (val & SCSICSR_INTMASK) {
>> + DPRINTF("SCSICSR INTMASK\n");
>> + /*
>> + * int_mask &= ~0x1000;
>> + * s->scsi_csr_1 |= val;
>> + * s->scsi_csr_1 &= ~SCSICSR_INTMASK;
>> + * if (s->scsi_queued) {
>> + * s->scsi_queued = 0;
>> + * next_irq(s, NEXT_SCSI_I, level);
>> + * }
>> + */
>> + } else {
>> + /* int_mask |= 0x1000; */
>> + }
>> + if (val & 0x80) {
>> + /* int_mask |= 0x1000; */
>> + /* s->scsi_csr_1 |= 0x80; */
>> + }
>> + DPRINTF("SCSICSR1 Write: %"PRIx64 "\n", val);
>> s->scsi_csr_1 = val;
>> break;
>>
>> case 1:
>> + DPRINTF("SCSICSR2 Write: %"PRIx64 "\n", val);
>> s->scsi_csr_2 = val;
>> break;
>>
>> @@ -868,10 +851,12 @@ static uint64_t next_scsi_csr_read(void *opaque, hwaddr addr, unsigned size)
>>
>> switch (addr) {
>> case 0:
>> + DPRINTF("SCSI 4020 STATUS READ %X\n", s->scsi_csr_1);
>> val = s->scsi_csr_1;
>> break;
>>
>> case 1:
>> + DPRINTF("SCSI 4021 STATUS READ %X\n", s->scsi_csr_2);
>> val = s->scsi_csr_2;
>> break;
>>