On Tue, 3 Mar 2026 at 22:22, Bernhard Beschow <[email protected]> wrote: > > Rather than requiring users of TYPE_SERIAL to initialize the MMIO region > themselves, make it generic enough to be configured via properties. This > makes TYPE_SERIAL more self-contained and prepares it for being turned > into a SysBusDevice. > > Signed-off-by: Bernhard Beschow <[email protected]> > --- > include/hw/char/serial-mm.h | 3 -- > include/hw/char/serial.h | 4 +- > hw/char/diva-gsp.c | 5 --- > hw/char/serial-isa.c | 1 - > hw/char/serial-mm.c | 51 ------------------------- > hw/char/serial-pci-multi.c | 5 --- > hw/char/serial-pci.c | 1 - > hw/char/serial.c | 76 ++++++++++++++++++++++++++++++------- > 8 files changed, 66 insertions(+), 80 deletions(-)
This does leave TYPE_SERIAL_MM a bit of a do-nothing extra wrapper, as Balaton says. However I don't think it's being used by any versioned machine types, so we could maybe look after this at the possibility of dropping the wrapper (by having serial_mm_init() create a TYPE_SERIAL directly, and dropping TYPE_SERIAL_MM entirely). I don't think we strictly need to do that, though -- being able to clean up the reset handling here is worthwhile in itself. I do have one question: > + memory_region_init_io(&s->io, OBJECT(s), > + s->regshift ? &serial_mm_ops[s->endianness] > + : &serial_io_ops, > + s, "serial", 8 << s->regshift); This means that users of serial_mm_init() which pass 0 for regshift now get serial_io_ops rather than serial_mm_ops. That affects a handful of machines. I'm not sure if this makes a guest-visible difference (very possibly not), but for a "refactoring" type patch like this I'd rather err on the safe side and keep the same MemoryRegionOps if we can. This could be awkward, though, because we definitely have users of TYPE_SERIAL_MM which rely on the default regshift property value being 0. Maybe we can have the handful of places that want the serial_io_ops do an explicit setting of some property to request that? The other approach is to analyse the code and satisfy ourselves that serial_io_ops and serial_mm_ops with regshift == 0 really do have exactly identical behaviour, and note that in the commit message. thanks -- PMM
