| Message ID | 20190404221238.12468-5-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/mips/r4k: Refactor the Super I/O chipset | expand |
On 05/04/2019 00.12, Philippe Mathieu-Daudé wrote: > ISA Super I/O are already modeled by the ISASuperIODevice abstract > device. > Since this board uses a generic ISA Super I/O chipset, refactor it > as the TYPE_R4K_SUPERIO device, child of ISASuperIODevice. Good idea! > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > hw/mips/mips_r4k.c | 61 ++++++++++++++++++++++++++++++++++------------ > 1 file changed, 45 insertions(+), 16 deletions(-) > > diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c > index 93dbf76bb49..b51a9523b43 100644 > --- a/hw/mips/mips_r4k.c > +++ b/hw/mips/mips_r4k.c > @@ -18,6 +18,7 @@ > #include "hw/i386/pc.h" > #include "hw/char/serial.h" > #include "hw/isa/isa.h" > +#include "hw/isa/superio.h" > #include "net/net.h" > #include "hw/net/ne2000-isa.h" > #include "sysemu/sysemu.h" > @@ -29,7 +30,6 @@ > #include "hw/loader.h" > #include "elf.h" > #include "hw/timer/mc146818rtc.h" > -#include "hw/input/i8042.h" > #include "hw/timer/i8254.h" > #include "exec/address-spaces.h" > #include "sysemu/qtest.h" > @@ -37,10 +37,6 @@ > > #define MAX_IDE_BUS 2 > > -static const int ide_iobase[2] = { 0x1f0, 0x170 }; > -static const int ide_iobase2[2] = { 0x3f6, 0x376 }; > -static const int ide_irq[2] = { 14, 15 }; > - > static ISADevice *pit; /* PIT i8254 */ > > /* i8254 PIT is attached to the IRQ0 at PIC i8259 */ > @@ -73,6 +69,49 @@ static const MemoryRegionOps mips_qemu_ops = { > .endianness = DEVICE_NATIVE_ENDIAN, > }; > > +#define TYPE_R4K_SUPERIO "r4k-superio" > + > +static uint16_t get_ide_iobase(ISASuperIODevice *sio, uint8_t index) > +{ > + static const uint16_t ide_iobase[] = { 0x1f0, 0x3f6, 0x170, 0x376 }; > + > + return ide_iobase[index]; > +} > + > +static unsigned int get_ide_irq(ISASuperIODevice *sio, uint8_t index) > +{ > + return index < MAX_IDE_DEVS ? 14 : 15; > +} > + > +static void r4k_superio_class_init(ObjectClass *klass, void *data) > +{ > + ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass); > + > + sc->serial.count = MAX_ISA_SERIAL_PORTS; > + sc->parallel.count = 0; > + sc->floppy.count = 0; > + sc->ide = (ISASuperIOFuncs){ > + .count = MAX_IDE_BUS * MAX_IDE_DEVS, > + .get_iobase = get_ide_iobase, > + .get_irq = get_ide_irq, > + }; > +} > + > +static const TypeInfo r4k_superio_type_info = { > + .name = TYPE_R4K_SUPERIO, > + .parent = TYPE_ISA_SUPERIO, > + .instance_size = sizeof(ISASuperIODevice), > + .class_size = sizeof(ISASuperIOClass), > + .class_init = r4k_superio_class_init, > +}; > + > +static void r4k_superio_register_types(void) > +{ > + type_register_static(&r4k_superio_type_info); > +} > + > +type_init(r4k_superio_register_types) > + > typedef struct ResetData { > MIPSCPU *cpu; > uint64_t vector; > @@ -179,10 +218,8 @@ void mips_r4k_init(MachineState *machine) > MIPSCPU *cpu; > CPUMIPSState *env; > ResetData *reset_info; > - int i; > qemu_irq *i8259; > ISABus *isa_bus; > - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; > DriveInfo *dinfo; > int be; > > @@ -274,20 +311,12 @@ void mips_r4k_init(MachineState *machine) > > pit = i8254_pit_init(isa_bus, 0x40, 0, NULL); > > - serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS); > - > isa_vga_init(isa_bus); > > if (nd_table[0].used) > isa_ne2000_init(isa_bus, 0x300, 9, &nd_table[0]); > > - ide_drive_get(hd, ARRAY_SIZE(hd)); > - for(i = 0; i < MAX_IDE_BUS; i++) > - isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i], > - hd[MAX_IDE_DEVS * i], > - hd[MAX_IDE_DEVS * i + 1]); > - > - isa_create_simple(isa_bus, TYPE_I8042); > + isa_create_simple(isa_bus, TYPE_R4K_SUPERIO); What about the ide_drive_get() and the ide_create_drive() that is done by isa_ide_init() internally? As far as I can see, the superio code does not do this job for you? So don't you have to do that manually here after creating the R4K_SUPERIO device? Thomas
On 4/5/19 6:51 AM, Thomas Huth wrote: > On 05/04/2019 00.12, Philippe Mathieu-Daudé wrote: >> ISA Super I/O are already modeled by the ISASuperIODevice abstract >> device. >> Since this board uses a generic ISA Super I/O chipset, refactor it >> as the TYPE_R4K_SUPERIO device, child of ISASuperIODevice. > > Good idea! > >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> hw/mips/mips_r4k.c | 61 ++++++++++++++++++++++++++++++++++------------ >> 1 file changed, 45 insertions(+), 16 deletions(-) >> >> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c >> index 93dbf76bb49..b51a9523b43 100644 >> --- a/hw/mips/mips_r4k.c >> +++ b/hw/mips/mips_r4k.c >> @@ -18,6 +18,7 @@ >> #include "hw/i386/pc.h" >> #include "hw/char/serial.h" >> #include "hw/isa/isa.h" >> +#include "hw/isa/superio.h" >> #include "net/net.h" >> #include "hw/net/ne2000-isa.h" >> #include "sysemu/sysemu.h" >> @@ -29,7 +30,6 @@ >> #include "hw/loader.h" >> #include "elf.h" >> #include "hw/timer/mc146818rtc.h" >> -#include "hw/input/i8042.h" >> #include "hw/timer/i8254.h" >> #include "exec/address-spaces.h" >> #include "sysemu/qtest.h" >> @@ -37,10 +37,6 @@ >> >> #define MAX_IDE_BUS 2 >> >> -static const int ide_iobase[2] = { 0x1f0, 0x170 }; >> -static const int ide_iobase2[2] = { 0x3f6, 0x376 }; >> -static const int ide_irq[2] = { 14, 15 }; >> - >> static ISADevice *pit; /* PIT i8254 */ >> >> /* i8254 PIT is attached to the IRQ0 at PIC i8259 */ >> @@ -73,6 +69,49 @@ static const MemoryRegionOps mips_qemu_ops = { >> .endianness = DEVICE_NATIVE_ENDIAN, >> }; >> >> +#define TYPE_R4K_SUPERIO "r4k-superio" >> + >> +static uint16_t get_ide_iobase(ISASuperIODevice *sio, uint8_t index) >> +{ >> + static const uint16_t ide_iobase[] = { 0x1f0, 0x3f6, 0x170, 0x376 }; >> + >> + return ide_iobase[index]; >> +} >> + >> +static unsigned int get_ide_irq(ISASuperIODevice *sio, uint8_t index) >> +{ >> + return index < MAX_IDE_DEVS ? 14 : 15; >> +} >> + >> +static void r4k_superio_class_init(ObjectClass *klass, void *data) >> +{ >> + ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass); >> + >> + sc->serial.count = MAX_ISA_SERIAL_PORTS; >> + sc->parallel.count = 0; >> + sc->floppy.count = 0; >> + sc->ide = (ISASuperIOFuncs){ >> + .count = MAX_IDE_BUS * MAX_IDE_DEVS, >> + .get_iobase = get_ide_iobase, >> + .get_irq = get_ide_irq, >> + }; >> +} >> + >> +static const TypeInfo r4k_superio_type_info = { >> + .name = TYPE_R4K_SUPERIO, >> + .parent = TYPE_ISA_SUPERIO, >> + .instance_size = sizeof(ISASuperIODevice), >> + .class_size = sizeof(ISASuperIOClass), >> + .class_init = r4k_superio_class_init, >> +}; >> + >> +static void r4k_superio_register_types(void) >> +{ >> + type_register_static(&r4k_superio_type_info); >> +} >> + >> +type_init(r4k_superio_register_types) >> + >> typedef struct ResetData { >> MIPSCPU *cpu; >> uint64_t vector; >> @@ -179,10 +218,8 @@ void mips_r4k_init(MachineState *machine) >> MIPSCPU *cpu; >> CPUMIPSState *env; >> ResetData *reset_info; >> - int i; >> qemu_irq *i8259; >> ISABus *isa_bus; >> - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; >> DriveInfo *dinfo; >> int be; >> >> @@ -274,20 +311,12 @@ void mips_r4k_init(MachineState *machine) >> >> pit = i8254_pit_init(isa_bus, 0x40, 0, NULL); >> >> - serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS); >> - >> isa_vga_init(isa_bus); >> >> if (nd_table[0].used) >> isa_ne2000_init(isa_bus, 0x300, 9, &nd_table[0]); >> >> - ide_drive_get(hd, ARRAY_SIZE(hd)); >> - for(i = 0; i < MAX_IDE_BUS; i++) >> - isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i], >> - hd[MAX_IDE_DEVS * i], >> - hd[MAX_IDE_DEVS * i + 1]); >> - >> - isa_create_simple(isa_bus, TYPE_I8042); >> + isa_create_simple(isa_bus, TYPE_R4K_SUPERIO); > > What about the ide_drive_get() and the ide_create_drive() that is done > by isa_ide_init() internally? As far as I can see, the superio code does > not do this job for you? So don't you have to do that manually here > after creating the R4K_SUPERIO device? Oops... I was scared I did the same mistake with the Floppy controller in 7313b1f28be but hopefully not. However you made me notice I never considered the case MAX_FD>2. I'll send a mail asking if this case is still used. Thanks! Phil.
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c index 93dbf76bb49..b51a9523b43 100644 --- a/hw/mips/mips_r4k.c +++ b/hw/mips/mips_r4k.c @@ -18,6 +18,7 @@ #include "hw/i386/pc.h" #include "hw/char/serial.h" #include "hw/isa/isa.h" +#include "hw/isa/superio.h" #include "net/net.h" #include "hw/net/ne2000-isa.h" #include "sysemu/sysemu.h" @@ -29,7 +30,6 @@ #include "hw/loader.h" #include "elf.h" #include "hw/timer/mc146818rtc.h" -#include "hw/input/i8042.h" #include "hw/timer/i8254.h" #include "exec/address-spaces.h" #include "sysemu/qtest.h" @@ -37,10 +37,6 @@ #define MAX_IDE_BUS 2 -static const int ide_iobase[2] = { 0x1f0, 0x170 }; -static const int ide_iobase2[2] = { 0x3f6, 0x376 }; -static const int ide_irq[2] = { 14, 15 }; - static ISADevice *pit; /* PIT i8254 */ /* i8254 PIT is attached to the IRQ0 at PIC i8259 */ @@ -73,6 +69,49 @@ static const MemoryRegionOps mips_qemu_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +#define TYPE_R4K_SUPERIO "r4k-superio" + +static uint16_t get_ide_iobase(ISASuperIODevice *sio, uint8_t index) +{ + static const uint16_t ide_iobase[] = { 0x1f0, 0x3f6, 0x170, 0x376 }; + + return ide_iobase[index]; +} + +static unsigned int get_ide_irq(ISASuperIODevice *sio, uint8_t index) +{ + return index < MAX_IDE_DEVS ? 14 : 15; +} + +static void r4k_superio_class_init(ObjectClass *klass, void *data) +{ + ISASuperIOClass *sc = ISA_SUPERIO_CLASS(klass); + + sc->serial.count = MAX_ISA_SERIAL_PORTS; + sc->parallel.count = 0; + sc->floppy.count = 0; + sc->ide = (ISASuperIOFuncs){ + .count = MAX_IDE_BUS * MAX_IDE_DEVS, + .get_iobase = get_ide_iobase, + .get_irq = get_ide_irq, + }; +} + +static const TypeInfo r4k_superio_type_info = { + .name = TYPE_R4K_SUPERIO, + .parent = TYPE_ISA_SUPERIO, + .instance_size = sizeof(ISASuperIODevice), + .class_size = sizeof(ISASuperIOClass), + .class_init = r4k_superio_class_init, +}; + +static void r4k_superio_register_types(void) +{ + type_register_static(&r4k_superio_type_info); +} + +type_init(r4k_superio_register_types) + typedef struct ResetData { MIPSCPU *cpu; uint64_t vector; @@ -179,10 +218,8 @@ void mips_r4k_init(MachineState *machine) MIPSCPU *cpu; CPUMIPSState *env; ResetData *reset_info; - int i; qemu_irq *i8259; ISABus *isa_bus; - DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; DriveInfo *dinfo; int be; @@ -274,20 +311,12 @@ void mips_r4k_init(MachineState *machine) pit = i8254_pit_init(isa_bus, 0x40, 0, NULL); - serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS); - isa_vga_init(isa_bus); if (nd_table[0].used) isa_ne2000_init(isa_bus, 0x300, 9, &nd_table[0]); - ide_drive_get(hd, ARRAY_SIZE(hd)); - for(i = 0; i < MAX_IDE_BUS; i++) - isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i], - hd[MAX_IDE_DEVS * i], - hd[MAX_IDE_DEVS * i + 1]); - - isa_create_simple(isa_bus, TYPE_I8042); + isa_create_simple(isa_bus, TYPE_R4K_SUPERIO); } static void mips_machine_init(MachineClass *mc)
ISA Super I/O are already modeled by the ISASuperIODevice abstract device. Since this board uses a generic ISA Super I/O chipset, refactor it as the TYPE_R4K_SUPERIO device, child of ISASuperIODevice. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/mips/mips_r4k.c | 61 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 16 deletions(-)