Re: [PATCH v4 17/37] mips: inline serial_init()
On Wednesday, November 20, 2019, Marc-André Lureau < marcandre.lur...@redhat.com> wrote: > The function is specific to mipssim, let's inline it. > > Signed-off-by: Marc-André Lureau > --- > hw/char/serial.c | 16 > hw/mips/mips_mipssim.c | 15 --- > include/hw/char/serial.h | 2 -- > 3 files changed, 12 insertions(+), 21 deletions(-) > > Reviewed-by: Aleksandar Markovic > diff --git a/hw/char/serial.c b/hw/char/serial.c > index 164146ede8..23f0b02516 100644 > --- a/hw/char/serial.c > +++ b/hw/char/serial.c > @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { > .class_init = serial_io_class_init, > }; > > -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, > - Chardev *chr, MemoryRegion *system_io) > -{ > -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); > - > -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); > -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); > -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); > -qdev_init_nofail(DEVICE(sio)); > - > -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); > -memory_region_add_subregion(system_io, base, >serial.io); > - > -return sio; > -} > - > static Property serial_properties[] = { > DEFINE_PROP_CHR("chardev", SerialState, chr), > DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), > diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c > index 282bbecb24..bfafa4d7e9 100644 > --- a/hw/mips/mips_mipssim.c > +++ b/hw/mips/mips_mipssim.c > @@ -40,6 +40,7 @@ > #include "hw/loader.h" > #include "elf.h" > #include "hw/sysbus.h" > +#include "hw/qdev-properties.h" > #include "exec/address-spaces.h" > #include "qemu/error-report.h" > #include "sysemu/qtest.h" > @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) > * A single 16450 sits at offset 0x3f8. It is attached to > * MIPS CPU INT2, which is interrupt 4. > */ > -if (serial_hd(0)) > -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), > -get_system_io()); > +if (serial_hd(0)) { > +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); > + > +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); > +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); > +qdev_set_legacy_instance_id(dev, 0x3f8, 2); > +qdev_init_nofail(dev); > +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); > +memory_region_add_subregion(get_system_io(), 0x3f8, > +_IO(dev)->serial.io); > +} > > if (nd_table[0].used) > /* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */ > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h > index d356ba838c..535fa23a2b 100644 > --- a/include/hw/char/serial.h > +++ b/include/hw/char/serial.h > @@ -108,8 +108,6 @@ void serial_set_frequency(SerialState *s, uint32_t > frequency); > #define TYPE_SERIAL_IO "serial-io" > #define SERIAL_IO(s) OBJECT_CHECK(SerialIO, (s), TYPE_SERIAL_IO) > > -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, > - Chardev *chr, MemoryRegion *system_io); > SerialMM *serial_mm_init(MemoryRegion *address_space, > hwaddr base, int regshift, > qemu_irq irq, int baudbase, > -- > 2.24.0 > > >
Re: [PATCH v4 17/37] mips: inline serial_init()
Hi On Mon, Nov 25, 2019 at 5:33 PM Aleksandar Markovic wrote: > > > > On Monday, November 25, 2019, Marc-André Lureau > wrote: >> >> Hi >> >> On Mon, Nov 25, 2019 at 2:12 PM Aleksandar Markovic >> wrote: >> > >> > >> > >> > On Wednesday, November 20, 2019, Marc-André Lureau >> > wrote: >> >> >> >> The function is specific to mipssim, let's inline it. >> >> >> >> Signed-off-by: Marc-André Lureau >> >> --- >> >> hw/char/serial.c | 16 >> >> hw/mips/mips_mipssim.c | 15 --- >> >> include/hw/char/serial.h | 2 -- >> >> 3 files changed, 12 insertions(+), 21 deletions(-) >> >> >> >> diff --git a/hw/char/serial.c b/hw/char/serial.c >> >> index 164146ede8..23f0b02516 100644 >> >> --- a/hw/char/serial.c >> >> +++ b/hw/char/serial.c >> >> @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { >> >> .class_init = serial_io_class_init, >> >> }; >> >> >> >> -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, >> >> - Chardev *chr, MemoryRegion *system_io) >> >> -{ >> >> -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); >> >> - >> >> -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); >> >> -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); >> >> -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); >> >> -qdev_init_nofail(DEVICE(sio)); >> >> - >> >> -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); >> >> -memory_region_add_subregion(system_io, base, >serial.io); >> >> - >> >> -return sio; >> >> -} >> >> - >> >> static Property serial_properties[] = { >> >> DEFINE_PROP_CHR("chardev", SerialState, chr), >> >> DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), >> >> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c >> >> index 282bbecb24..bfafa4d7e9 100644 >> >> --- a/hw/mips/mips_mipssim.c >> >> +++ b/hw/mips/mips_mipssim.c >> >> @@ -40,6 +40,7 @@ >> >> #include "hw/loader.h" >> >> #include "elf.h" >> >> #include "hw/sysbus.h" >> >> +#include "hw/qdev-properties.h" >> >> #include "exec/address-spaces.h" >> >> #include "qemu/error-report.h" >> >> #include "sysemu/qtest.h" >> >> @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) >> >> * A single 16450 sits at offset 0x3f8. It is attached to >> >> * MIPS CPU INT2, which is interrupt 4. >> >> */ >> >> -if (serial_hd(0)) a >> >> -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), >> >> -get_system_io()); >> >> +if (serial_hd(0)) { >> >> +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); >> >> + >> >> +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); >> >> +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); >> >> +qdev_set_legacy_instance_id(dev, 0x3f8, 2); >> >> +qdev_init_nofail(dev); >> >> +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); >> >> +memory_region_add_subregion(get_system_io(), 0x3f8, >> >> +_IO(dev)->serial.io); >> >> +} >> > >> > >> > >> > Please explain why the code in the deleted function and the new function >> > are not identical. Why is the new code better? >> >> What differences do you spot? >> >> serial_init() looks very generic, but it is not, and this is >> confusing. QDev users should call qdev_create(). >> >> If there is a single user, it is better to inline the code where it is >> being used, or move the function there. >> > > Marc-Andre, you misunderstood me: I am not complaining about inlining at all, > I would do the same if I were you. > > I just see you expressed the same thing from former function in a slightly > different way in the inlined version, and they look functionally equivalent > to me. But I suppose the new way is better, and I want to know why. Is it a > more standard way, or something even more? Please understand that I don't > come from device emulation background - I want to learn a little bit about > device-related QEMU customs and conventions, that is all. :) > > For example, > > in old version: > > >> -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, > >> - Chardev *chr, MemoryRegion *system_io) > >> -{ > >> -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); > >> - > >> -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); > >> -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); > >> -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); > >> -qdev_init_nofail(DEVICE(sio)); > >> - > >> -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); > >> -memory_region_add_subregion(system_io, base, >serial.io); > >> - > >> -return sio; > >> -} > > in new version: > > >> +if (serial_hd(0)) { > >> +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); > >> + > >> +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); > >> +qdev_prop_set_chr(dev, "chardev", serial_hd(0));
Re: [PATCH v4 17/37] mips: inline serial_init()
On Monday, November 25, 2019, Marc-André Lureau wrote: > Hi > > On Mon, Nov 25, 2019 at 2:12 PM Aleksandar Markovic > wrote: > > > > > > > > On Wednesday, November 20, 2019, Marc-André Lureau < > marcandre.lur...@redhat.com> wrote: > >> > >> The function is specific to mipssim, let's inline it. > >> > >> Signed-off-by: Marc-André Lureau > >> --- > >> hw/char/serial.c | 16 > >> hw/mips/mips_mipssim.c | 15 --- > >> include/hw/char/serial.h | 2 -- > >> 3 files changed, 12 insertions(+), 21 deletions(-) > >> > >> diff --git a/hw/char/serial.c b/hw/char/serial.c > >> index 164146ede8..23f0b02516 100644 > >> --- a/hw/char/serial.c > >> +++ b/hw/char/serial.c > >> @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { > >> .class_init = serial_io_class_init, > >> }; > >> > >> -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, > >> - Chardev *chr, MemoryRegion *system_io) > >> -{ > >> -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); > >> - > >> -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); > >> -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); > >> -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); > >> -qdev_init_nofail(DEVICE(sio)); > >> - > >> -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); > >> -memory_region_add_subregion(system_io, base, >serial.io); > >> - > >> -return sio; > >> -} > >> - > >> static Property serial_properties[] = { > >> DEFINE_PROP_CHR("chardev", SerialState, chr), > >> DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), > >> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c > >> index 282bbecb24..bfafa4d7e9 100644 > >> --- a/hw/mips/mips_mipssim.c > >> +++ b/hw/mips/mips_mipssim.c > >> @@ -40,6 +40,7 @@ > >> #include "hw/loader.h" > >> #include "elf.h" > >> #include "hw/sysbus.h" > >> +#include "hw/qdev-properties.h" > >> #include "exec/address-spaces.h" > >> #include "qemu/error-report.h" > >> #include "sysemu/qtest.h" > >> @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) > >> * A single 16450 sits at offset 0x3f8. It is attached to > >> * MIPS CPU INT2, which is interrupt 4. > >> */ > >> -if (serial_hd(0)) a > >> -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), > >> -get_system_io()); > >> +if (serial_hd(0)) { > >> +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); > >> + > >> +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); > >> +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); > >> +qdev_set_legacy_instance_id(dev, 0x3f8, 2); > >> +qdev_init_nofail(dev); > >> +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); > >> +memory_region_add_subregion(get_system_io(), 0x3f8, > >> +_IO(dev)->serial.io); > >> +} > > > > > > > > Please explain why the code in the deleted function and the new function > are not identical. Why is the new code better? > > What differences do you spot? > > serial_init() looks very generic, but it is not, and this is > confusing. QDev users should call qdev_create(). > > If there is a single user, it is better to inline the code where it is > being used, or move the function there. > > Marc-Andre, you misunderstood me: I am not complaining about inlining at all, I would do the same if I were you. I just see you expressed the same thing from former function in a slightly different way in the inlined version, and they look functionally equivalent to me. But I suppose the new way is better, and I want to know why. Is it a more standard way, or something even more? Please understand that I don't come from device emulation background - I want to learn a little bit about device-related QEMU customs and conventions, that is all. :) For example, in old version: >> -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, >> - Chardev *chr, MemoryRegion *system_io) >> -{ >> -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); >> - >> -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); >> -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); >> -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); >> -qdev_init_nofail(DEVICE(sio)); >> - >> -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); >> -memory_region_add_subregion(system_io, base, >serial.io); >> - >> -return sio; >> -} in new version: >> +if (serial_hd(0)) { >> +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); >> + >> +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); >> +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); <-- you didn't use here DEVICE(), while it is used in the previous line, and also in the old version, and similar questions >> +qdev_set_legacy_instance_id(dev, 0x3f8, 2); >> +qdev_init_nofail(dev);
Re: [PATCH v4 17/37] mips: inline serial_init()
Hi On Mon, Nov 25, 2019 at 2:12 PM Aleksandar Markovic wrote: > > > > On Wednesday, November 20, 2019, Marc-André Lureau > wrote: >> >> The function is specific to mipssim, let's inline it. >> >> Signed-off-by: Marc-André Lureau >> --- >> hw/char/serial.c | 16 >> hw/mips/mips_mipssim.c | 15 --- >> include/hw/char/serial.h | 2 -- >> 3 files changed, 12 insertions(+), 21 deletions(-) >> >> diff --git a/hw/char/serial.c b/hw/char/serial.c >> index 164146ede8..23f0b02516 100644 >> --- a/hw/char/serial.c >> +++ b/hw/char/serial.c >> @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { >> .class_init = serial_io_class_init, >> }; >> >> -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, >> - Chardev *chr, MemoryRegion *system_io) >> -{ >> -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); >> - >> -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); >> -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); >> -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); >> -qdev_init_nofail(DEVICE(sio)); >> - >> -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); >> -memory_region_add_subregion(system_io, base, >serial.io); >> - >> -return sio; >> -} >> - >> static Property serial_properties[] = { >> DEFINE_PROP_CHR("chardev", SerialState, chr), >> DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), >> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c >> index 282bbecb24..bfafa4d7e9 100644 >> --- a/hw/mips/mips_mipssim.c >> +++ b/hw/mips/mips_mipssim.c >> @@ -40,6 +40,7 @@ >> #include "hw/loader.h" >> #include "elf.h" >> #include "hw/sysbus.h" >> +#include "hw/qdev-properties.h" >> #include "exec/address-spaces.h" >> #include "qemu/error-report.h" >> #include "sysemu/qtest.h" >> @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) >> * A single 16450 sits at offset 0x3f8. It is attached to >> * MIPS CPU INT2, which is interrupt 4. >> */ >> -if (serial_hd(0)) a >> -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), >> -get_system_io()); >> +if (serial_hd(0)) { >> +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); >> + >> +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); >> +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); >> +qdev_set_legacy_instance_id(dev, 0x3f8, 2); >> +qdev_init_nofail(dev); >> +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); >> +memory_region_add_subregion(get_system_io(), 0x3f8, >> +_IO(dev)->serial.io); >> +} > > > > Please explain why the code in the deleted function and the new function are > not identical. Why is the new code better? What differences do you spot? serial_init() looks very generic, but it is not, and this is confusing. QDev users should call qdev_create(). If there is a single user, it is better to inline the code where it is being used, or move the function there.
Re: [PATCH v4 17/37] mips: inline serial_init()
On Wednesday, November 20, 2019, Marc-André Lureau < marcandre.lur...@redhat.com> wrote: > The function is specific to mipssim, let's inline it. > > Signed-off-by: Marc-André Lureau > --- > hw/char/serial.c | 16 > hw/mips/mips_mipssim.c | 15 --- > include/hw/char/serial.h | 2 -- > 3 files changed, 12 insertions(+), 21 deletions(-) > > diff --git a/hw/char/serial.c b/hw/char/serial.c > index 164146ede8..23f0b02516 100644 > --- a/hw/char/serial.c > +++ b/hw/char/serial.c > @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { > .class_init = serial_io_class_init, > }; > > -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, > - Chardev *chr, MemoryRegion *system_io) > -{ > -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); > - > -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); > -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); > -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); > -qdev_init_nofail(DEVICE(sio)); > - > -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); > -memory_region_add_subregion(system_io, base, >serial.io); > - > -return sio; > -} > - > static Property serial_properties[] = { > DEFINE_PROP_CHR("chardev", SerialState, chr), > DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), > diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c > index 282bbecb24..bfafa4d7e9 100644 > --- a/hw/mips/mips_mipssim.c > +++ b/hw/mips/mips_mipssim.c > @@ -40,6 +40,7 @@ > #include "hw/loader.h" > #include "elf.h" > #include "hw/sysbus.h" > +#include "hw/qdev-properties.h" > #include "exec/address-spaces.h" > #include "qemu/error-report.h" > #include "sysemu/qtest.h" > @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) > * A single 16450 sits at offset 0x3f8. It is attached to > * MIPS CPU INT2, which is interrupt 4. > */ > -if (serial_hd(0)) > -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), > -get_system_io()); > +if (serial_hd(0)) { > +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); > + > +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); > +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); > +qdev_set_legacy_instance_id(dev, 0x3f8, 2); > +qdev_init_nofail(dev); > +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); > +memory_region_add_subregion(get_system_io(), 0x3f8, > +_IO(dev)->serial.io); > +} Please explain why the code in the deleted function and the new function are not identical. Why is the new code better? Thanks, Aleksandar > > if (nd_table[0].used) > /* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */ > diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h > index d356ba838c..535fa23a2b 100644 > --- a/include/hw/char/serial.h > +++ b/include/hw/char/serial.h > @@ -108,8 +108,6 @@ void serial_set_frequency(SerialState *s, uint32_t > frequency); > #define TYPE_SERIAL_IO "serial-io" > #define SERIAL_IO(s) OBJECT_CHECK(SerialIO, (s), TYPE_SERIAL_IO) > > -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, > - Chardev *chr, MemoryRegion *system_io); > SerialMM *serial_mm_init(MemoryRegion *address_space, > hwaddr base, int regshift, > qemu_irq irq, int baudbase, > -- > 2.24.0 > > >
Re: [PATCH v4 17/37] mips: inline serial_init()
On 11/20/19 4:24 PM, Marc-André Lureau wrote: The function is specific to mipssim, let's inline it. Signed-off-by: Marc-André Lureau --- hw/char/serial.c | 16 hw/mips/mips_mipssim.c | 15 --- include/hw/char/serial.h | 2 -- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/hw/char/serial.c b/hw/char/serial.c index 164146ede8..23f0b02516 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { .class_init = serial_io_class_init, }; -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, - Chardev *chr, MemoryRegion *system_io) -{ -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); - -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); -qdev_init_nofail(DEVICE(sio)); - -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); -memory_region_add_subregion(system_io, base, >serial.io); - -return sio; -} - static Property serial_properties[] = { DEFINE_PROP_CHR("chardev", SerialState, chr), DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c index 282bbecb24..bfafa4d7e9 100644 --- a/hw/mips/mips_mipssim.c +++ b/hw/mips/mips_mipssim.c @@ -40,6 +40,7 @@ #include "hw/loader.h" #include "elf.h" #include "hw/sysbus.h" +#include "hw/qdev-properties.h" #include "exec/address-spaces.h" #include "qemu/error-report.h" #include "sysemu/qtest.h" @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) * A single 16450 sits at offset 0x3f8. It is attached to * MIPS CPU INT2, which is interrupt 4. */ -if (serial_hd(0)) -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), -get_system_io()); +if (serial_hd(0)) { +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); + +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); +qdev_set_legacy_instance_id(dev, 0x3f8, 2); +qdev_init_nofail(dev); +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); +memory_region_add_subregion(get_system_io(), 0x3f8, +_IO(dev)->serial.io); +} if (nd_table[0].used) /* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */ diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h index d356ba838c..535fa23a2b 100644 --- a/include/hw/char/serial.h +++ b/include/hw/char/serial.h @@ -108,8 +108,6 @@ void serial_set_frequency(SerialState *s, uint32_t frequency); #define TYPE_SERIAL_IO "serial-io" #define SERIAL_IO(s) OBJECT_CHECK(SerialIO, (s), TYPE_SERIAL_IO) -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, - Chardev *chr, MemoryRegion *system_io); SerialMM *serial_mm_init(MemoryRegion *address_space, hwaddr base, int regshift, qemu_irq irq, int baudbase, Reviewed-by: Philippe Mathieu-Daudé
[PATCH v4 17/37] mips: inline serial_init()
The function is specific to mipssim, let's inline it. Signed-off-by: Marc-André Lureau --- hw/char/serial.c | 16 hw/mips/mips_mipssim.c | 15 --- include/hw/char/serial.h | 2 -- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/hw/char/serial.c b/hw/char/serial.c index 164146ede8..23f0b02516 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -1023,22 +1023,6 @@ static const TypeInfo serial_io_info = { .class_init = serial_io_class_init, }; -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, - Chardev *chr, MemoryRegion *system_io) -{ -SerialIO *sio = SERIAL_IO(qdev_create(NULL, TYPE_SERIAL_IO)); - -qdev_prop_set_uint32(DEVICE(sio), "baudbase", baudbase); -qdev_prop_set_chr(DEVICE(sio), "chardev", chr); -qdev_set_legacy_instance_id(DEVICE(sio), base, 2); -qdev_init_nofail(DEVICE(sio)); - -sysbus_connect_irq(SYS_BUS_DEVICE(sio), 0, irq); -memory_region_add_subregion(system_io, base, >serial.io); - -return sio; -} - static Property serial_properties[] = { DEFINE_PROP_CHR("chardev", SerialState, chr), DEFINE_PROP_UINT32("baudbase", SerialState, baudbase, 115200), diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c index 282bbecb24..bfafa4d7e9 100644 --- a/hw/mips/mips_mipssim.c +++ b/hw/mips/mips_mipssim.c @@ -40,6 +40,7 @@ #include "hw/loader.h" #include "elf.h" #include "hw/sysbus.h" +#include "hw/qdev-properties.h" #include "exec/address-spaces.h" #include "qemu/error-report.h" #include "sysemu/qtest.h" @@ -219,9 +220,17 @@ mips_mipssim_init(MachineState *machine) * A single 16450 sits at offset 0x3f8. It is attached to * MIPS CPU INT2, which is interrupt 4. */ -if (serial_hd(0)) -serial_init(0x3f8, env->irq[4], 115200, serial_hd(0), -get_system_io()); +if (serial_hd(0)) { +DeviceState *dev = qdev_create(NULL, TYPE_SERIAL_IO); + +qdev_prop_set_uint32(DEVICE(dev), "baudbase", 115200); +qdev_prop_set_chr(dev, "chardev", serial_hd(0)); +qdev_set_legacy_instance_id(dev, 0x3f8, 2); +qdev_init_nofail(dev); +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, env->irq[4]); +memory_region_add_subregion(get_system_io(), 0x3f8, +_IO(dev)->serial.io); +} if (nd_table[0].used) /* MIPSnet uses the MIPS CPU INT0, which is interrupt 2. */ diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h index d356ba838c..535fa23a2b 100644 --- a/include/hw/char/serial.h +++ b/include/hw/char/serial.h @@ -108,8 +108,6 @@ void serial_set_frequency(SerialState *s, uint32_t frequency); #define TYPE_SERIAL_IO "serial-io" #define SERIAL_IO(s) OBJECT_CHECK(SerialIO, (s), TYPE_SERIAL_IO) -SerialIO *serial_init(int base, qemu_irq irq, int baudbase, - Chardev *chr, MemoryRegion *system_io); SerialMM *serial_mm_init(MemoryRegion *address_space, hwaddr base, int regshift, qemu_irq irq, int baudbase, -- 2.24.0