| Message ID | 20220127205405.23499-5-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | mos6522: switch to gpios, add control line edge-triggering and extra debugging | expand |
On Thu, 27 Jan 2022 at 21:01, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > > For historical reasons each mos6522 instance implements its own setting and > update of the IFR flag bits using methods exposed by MOS6522DeviceClass. As > of today this is no longer required, and it is now possible to implement > the mos6522 IRQs as standard qdev gpios. > > Switch over to use qdev gpios for the mos6522 device and update all instances > accordingly. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/misc/mac_via.c | 56 +++++++-------------------------------- > hw/misc/macio/cuda.c | 5 ++-- > hw/misc/macio/pmu.c | 4 +-- > hw/misc/mos6522.c | 15 +++++++++++ > include/hw/misc/mac_via.h | 6 +---- > include/hw/misc/mos6522.h | 2 ++ > 6 files changed, 32 insertions(+), 56 deletions(-) > -static void via2_nubus_irq_request(void *opaque, int irq, int level) > +static void via2_nubus_irq_request(void *opaque, int n, int level) > { > MOS6522Q800VIA2State *v2s = opaque; > MOS6522State *s = MOS6522(v2s); > - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); > + qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA2_IRQ_NUBUS_BIT); > > if (level) { > /* Port A nubus IRQ inputs are active LOW */ > - s->a &= ~(1 << irq); > - s->ifr |= 1 << VIA2_IRQ_NUBUS_BIT; > + s->a &= ~(1 << n); > } else { > - s->a |= (1 << irq); > - s->ifr &= ~(1 << VIA2_IRQ_NUBUS_BIT); > + s->a |= (1 << n); > } > > - mdc->update_irq(s); > + qemu_set_irq(irq, level); > } It feels a bit inconsistent here that we're still reaching into the MOS6522State to set s->a, but I guess this is still better than what we had before. > -#define VIA1_IRQ_NB 8 > - > #define VIA1_IRQ_ONE_SECOND (1 << VIA1_IRQ_ONE_SECOND_BIT) > #define VIA1_IRQ_60HZ (1 << VIA1_IRQ_60HZ_BIT) > #define VIA1_IRQ_ADB_READY (1 << VIA1_IRQ_ADB_READY_BIT) > @@ -42,7 +40,7 @@ struct MOS6522Q800VIA1State { > > MemoryRegion via_mem; > > - qemu_irq irqs[VIA1_IRQ_NB]; > + qemu_irq irqs[VIA_NUM_INTS]; This irqs[] array appears to be entirely unused. You could delete it as a separate patch before this one. > qemu_irq auxmode_irq; > uint8_t last_b; > > @@ -85,8 +83,6 @@ struct MOS6522Q800VIA1State { > #define VIA2_IRQ_SCSI_BIT CB2_INT_BIT > #define VIA2_IRQ_ASC_BIT CB1_INT_BIT > > -#define VIA2_IRQ_NB 8 > - > #define VIA2_IRQ_SCSI_DATA (1 << VIA2_IRQ_SCSI_DATA_BIT) > #define VIA2_IRQ_NUBUS (1 << VIA2_IRQ_NUBUS_BIT) > #define VIA2_IRQ_UNUSED (1 << VIA2_IRQ_SCSI_BIT) > diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h > index 12abd8b8d2..ced8a670bf 100644 > --- a/include/hw/misc/mos6522.h > +++ b/include/hw/misc/mos6522.h > @@ -57,6 +57,8 @@ > #define T2_INT (1 << T2_INT_BIT) > #define T1_INT (1 << T1_INT_BIT) > > +#define VIA_NUM_INTS 5 Were we not using 5,6,7 previously ? Anyway, Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 07/02/2022 19:29, Peter Maydell wrote: > On Thu, 27 Jan 2022 at 21:01, Mark Cave-Ayland > <mark.cave-ayland@ilande.co.uk> wrote: >> >> For historical reasons each mos6522 instance implements its own setting and >> update of the IFR flag bits using methods exposed by MOS6522DeviceClass. As >> of today this is no longer required, and it is now possible to implement >> the mos6522 IRQs as standard qdev gpios. >> >> Switch over to use qdev gpios for the mos6522 device and update all instances >> accordingly. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/misc/mac_via.c | 56 +++++++-------------------------------- >> hw/misc/macio/cuda.c | 5 ++-- >> hw/misc/macio/pmu.c | 4 +-- >> hw/misc/mos6522.c | 15 +++++++++++ >> include/hw/misc/mac_via.h | 6 +---- >> include/hw/misc/mos6522.h | 2 ++ >> 6 files changed, 32 insertions(+), 56 deletions(-) > > >> -static void via2_nubus_irq_request(void *opaque, int irq, int level) >> +static void via2_nubus_irq_request(void *opaque, int n, int level) >> { >> MOS6522Q800VIA2State *v2s = opaque; >> MOS6522State *s = MOS6522(v2s); >> - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); >> + qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA2_IRQ_NUBUS_BIT); >> >> if (level) { >> /* Port A nubus IRQ inputs are active LOW */ >> - s->a &= ~(1 << irq); >> - s->ifr |= 1 << VIA2_IRQ_NUBUS_BIT; >> + s->a &= ~(1 << n); >> } else { >> - s->a |= (1 << irq); >> - s->ifr &= ~(1 << VIA2_IRQ_NUBUS_BIT); >> + s->a |= (1 << n); >> } >> >> - mdc->update_irq(s); >> + qemu_set_irq(irq, level); >> } > > It feels a bit inconsistent here that we're still reaching into > the MOS6522State to set s->a, but I guess this is still > better than what we had before. Yeah it's a little bit messy. On the Mac the Nubus IRQs are ORd to produce the VIA interrupt line, but the individual Nubus IRQ lines are wired to the VIA port A to allow the state of each line to be determined. The VIA port IO lines are bi-directional, so rather than try and implement this with GPIOs it's easiest to have a separate VIA2 Nubus GPIOs that also handle the port IO lines as above. >> -#define VIA1_IRQ_NB 8 >> - >> #define VIA1_IRQ_ONE_SECOND (1 << VIA1_IRQ_ONE_SECOND_BIT) >> #define VIA1_IRQ_60HZ (1 << VIA1_IRQ_60HZ_BIT) >> #define VIA1_IRQ_ADB_READY (1 << VIA1_IRQ_ADB_READY_BIT) >> @@ -42,7 +40,7 @@ struct MOS6522Q800VIA1State { >> >> MemoryRegion via_mem; >> >> - qemu_irq irqs[VIA1_IRQ_NB]; >> + qemu_irq irqs[VIA_NUM_INTS]; > > This irqs[] array appears to be entirely unused. You could > delete it as a separate patch before this one. Ah yes, before this patch each VIA had its own GPIOs which used the methods in MOS6522DeviceClass to manipulate the IFR bit. Since the Q800 VIAs have the MOS6522 device as a parent class, the GPIOs are inherited from that and so this array should actually be removed as part of this patch. >> qemu_irq auxmode_irq; >> uint8_t last_b; >> >> @@ -85,8 +83,6 @@ struct MOS6522Q800VIA1State { >> #define VIA2_IRQ_SCSI_BIT CB2_INT_BIT >> #define VIA2_IRQ_ASC_BIT CB1_INT_BIT >> >> -#define VIA2_IRQ_NB 8 >> - >> #define VIA2_IRQ_SCSI_DATA (1 << VIA2_IRQ_SCSI_DATA_BIT) >> #define VIA2_IRQ_NUBUS (1 << VIA2_IRQ_NUBUS_BIT) >> #define VIA2_IRQ_UNUSED (1 << VIA2_IRQ_SCSI_BIT) >> diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h >> index 12abd8b8d2..ced8a670bf 100644 >> --- a/include/hw/misc/mos6522.h >> +++ b/include/hw/misc/mos6522.h >> @@ -57,6 +57,8 @@ >> #define T2_INT (1 << T2_INT_BIT) >> #define T1_INT (1 << T1_INT_BIT) >> >> +#define VIA_NUM_INTS 5 > > Were we not using 5,6,7 previously ? 5 and 6 are for the in-built timers, and 7 is a set/clear flag so they are not externally accessible. > Anyway, > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > thanks > -- PMM ATB, Mark.
diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c index b378e6b305..0756374f1b 100644 --- a/hw/misc/mac_via.c +++ b/hw/misc/mac_via.c @@ -325,10 +325,9 @@ static void via1_sixty_hz(void *opaque) { MOS6522Q800VIA1State *v1s = opaque; MOS6522State *s = MOS6522(v1s); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); + qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA1_IRQ_60HZ_BIT); - s->ifr |= VIA1_IRQ_60HZ; - mdc->update_irq(s); + qemu_set_irq(irq, 1); via1_sixty_hz_update(v1s); } @@ -337,44 +336,13 @@ static void via1_one_second(void *opaque) { MOS6522Q800VIA1State *v1s = opaque; MOS6522State *s = MOS6522(v1s); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); + qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA1_IRQ_ONE_SECOND_BIT); - s->ifr |= VIA1_IRQ_ONE_SECOND; - mdc->update_irq(s); + qemu_set_irq(irq, 1); via1_one_second_update(v1s); } -static void via1_irq_request(void *opaque, int irq, int level) -{ - MOS6522Q800VIA1State *v1s = opaque; - MOS6522State *s = MOS6522(v1s); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); - - if (level) { - s->ifr |= 1 << irq; - } else { - s->ifr &= ~(1 << irq); - } - - mdc->update_irq(s); -} - -static void via2_irq_request(void *opaque, int irq, int level) -{ - MOS6522Q800VIA2State *v2s = opaque; - MOS6522State *s = MOS6522(v2s); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); - - if (level) { - s->ifr |= 1 << irq; - } else { - s->ifr &= ~(1 << irq); - } - - mdc->update_irq(s); -} - static void pram_update(MOS6522Q800VIA1State *v1s) { @@ -1061,8 +1029,6 @@ static void mos6522_q800_via1_init(Object *obj) qbus_init((BusState *)&v1s->adb_bus, sizeof(v1s->adb_bus), TYPE_ADB_BUS, DEVICE(v1s), "adb.0"); - qdev_init_gpio_in(DEVICE(obj), via1_irq_request, VIA1_IRQ_NB); - /* A/UX mode */ qdev_init_gpio_out(DEVICE(obj), &v1s->auxmode_irq, 1); } @@ -1150,22 +1116,20 @@ static void mos6522_q800_via2_reset(DeviceState *dev) ms->a = 0x7f; } -static void via2_nubus_irq_request(void *opaque, int irq, int level) +static void via2_nubus_irq_request(void *opaque, int n, int level) { MOS6522Q800VIA2State *v2s = opaque; MOS6522State *s = MOS6522(v2s); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); + qemu_irq irq = qdev_get_gpio_in(DEVICE(s), VIA2_IRQ_NUBUS_BIT); if (level) { /* Port A nubus IRQ inputs are active LOW */ - s->a &= ~(1 << irq); - s->ifr |= 1 << VIA2_IRQ_NUBUS_BIT; + s->a &= ~(1 << n); } else { - s->a |= (1 << irq); - s->ifr &= ~(1 << VIA2_IRQ_NUBUS_BIT); + s->a |= (1 << n); } - mdc->update_irq(s); + qemu_set_irq(irq, level); } static void mos6522_q800_via2_init(Object *obj) @@ -1177,8 +1141,6 @@ static void mos6522_q800_via2_init(Object *obj) "via2", VIA_SIZE); sysbus_init_mmio(sbd, &v2s->via_mem); - qdev_init_gpio_in(DEVICE(obj), via2_irq_request, VIA2_IRQ_NB); - qdev_init_gpio_in_named(DEVICE(obj), via2_nubus_irq_request, "nubus-irq", VIA2_NUBUS_IRQ_NB); } diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c index e917a6a095..f76d9227d3 100644 --- a/hw/misc/macio/cuda.c +++ b/hw/misc/macio/cuda.c @@ -25,6 +25,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" +#include "hw/irq.h" #include "hw/ppc/mac.h" #include "hw/qdev-properties.h" #include "migration/vmstate.h" @@ -96,9 +97,9 @@ static void cuda_set_sr_int(void *opaque) CUDAState *s = opaque; MOS6522CUDAState *mcs = &s->mos6522_cuda; MOS6522State *ms = MOS6522(mcs); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(ms); + qemu_irq irq = qdev_get_gpio_in(DEVICE(ms), SR_INT_BIT); - mdc->set_sr_int(ms); + qemu_set_irq(irq, 1); } static void cuda_delay_set_sr_int(CUDAState *s) diff --git a/hw/misc/macio/pmu.c b/hw/misc/macio/pmu.c index eb39c64694..6e80fe1cfa 100644 --- a/hw/misc/macio/pmu.c +++ b/hw/misc/macio/pmu.c @@ -75,9 +75,9 @@ static void via_set_sr_int(void *opaque) PMUState *s = opaque; MOS6522PMUState *mps = MOS6522_PMU(&s->mos6522_pmu); MOS6522State *ms = MOS6522(mps); - MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(ms); + qemu_irq irq = qdev_get_gpio_in(DEVICE(ms), SR_INT_BIT); - mdc->set_sr_int(ms); + qemu_set_irq(irq, 1); } static void pmu_update_extirq(PMUState *s) diff --git a/hw/misc/mos6522.c b/hw/misc/mos6522.c index 1c57332b40..6be6853dc2 100644 --- a/hw/misc/mos6522.c +++ b/hw/misc/mos6522.c @@ -52,6 +52,19 @@ static void mos6522_update_irq(MOS6522State *s) } } +static void mos6522_set_irq(void *opaque, int n, int level) +{ + MOS6522State *s = MOS6522(opaque); + + if (level) { + s->ifr |= 1 << n; + } else { + s->ifr &= ~(1 << n); + } + + mos6522_update_irq(s); +} + static uint64_t get_counter_value(MOS6522State *s, MOS6522Timer *ti) { MOS6522DeviceClass *mdc = MOS6522_GET_CLASS(s); @@ -488,6 +501,8 @@ static void mos6522_init(Object *obj) s->timers[0].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer1, s); s->timers[1].timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, mos6522_timer2, s); + + qdev_init_gpio_in(DEVICE(obj), mos6522_set_irq, VIA_NUM_INTS); } static void mos6522_finalize(Object *obj) diff --git a/include/hw/misc/mac_via.h b/include/hw/misc/mac_via.h index 2df1ab01b6..bffeba38ee 100644 --- a/include/hw/misc/mac_via.h +++ b/include/hw/misc/mac_via.h @@ -24,8 +24,6 @@ #define VIA1_IRQ_ADB_DATA_BIT CB2_INT_BIT #define VIA1_IRQ_ADB_CLOCK_BIT CB1_INT_BIT -#define VIA1_IRQ_NB 8 - #define VIA1_IRQ_ONE_SECOND (1 << VIA1_IRQ_ONE_SECOND_BIT) #define VIA1_IRQ_60HZ (1 << VIA1_IRQ_60HZ_BIT) #define VIA1_IRQ_ADB_READY (1 << VIA1_IRQ_ADB_READY_BIT) @@ -42,7 +40,7 @@ struct MOS6522Q800VIA1State { MemoryRegion via_mem; - qemu_irq irqs[VIA1_IRQ_NB]; + qemu_irq irqs[VIA_NUM_INTS]; qemu_irq auxmode_irq; uint8_t last_b; @@ -85,8 +83,6 @@ struct MOS6522Q800VIA1State { #define VIA2_IRQ_SCSI_BIT CB2_INT_BIT #define VIA2_IRQ_ASC_BIT CB1_INT_BIT -#define VIA2_IRQ_NB 8 - #define VIA2_IRQ_SCSI_DATA (1 << VIA2_IRQ_SCSI_DATA_BIT) #define VIA2_IRQ_NUBUS (1 << VIA2_IRQ_NUBUS_BIT) #define VIA2_IRQ_UNUSED (1 << VIA2_IRQ_SCSI_BIT) diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h index 12abd8b8d2..ced8a670bf 100644 --- a/include/hw/misc/mos6522.h +++ b/include/hw/misc/mos6522.h @@ -57,6 +57,8 @@ #define T2_INT (1 << T2_INT_BIT) #define T1_INT (1 << T1_INT_BIT) +#define VIA_NUM_INTS 5 + /* Bits in ACR */ #define T1MODE 0xc0 /* Timer 1 mode */ #define T1MODE_CONT 0x40 /* continuous interrupts */
For historical reasons each mos6522 instance implements its own setting and update of the IFR flag bits using methods exposed by MOS6522DeviceClass. As of today this is no longer required, and it is now possible to implement the mos6522 IRQs as standard qdev gpios. Switch over to use qdev gpios for the mos6522 device and update all instances accordingly. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/misc/mac_via.c | 56 +++++++-------------------------------- hw/misc/macio/cuda.c | 5 ++-- hw/misc/macio/pmu.c | 4 +-- hw/misc/mos6522.c | 15 +++++++++++ include/hw/misc/mac_via.h | 6 +---- include/hw/misc/mos6522.h | 2 ++ 6 files changed, 32 insertions(+), 56 deletions(-)