Am 27.04.2013 09:12, schrieb Artyom Tarasenko: > PrEP and SPARC machines use slightly different variations of
PReP :) > a Mostek NVRAM chip. Since the SPARC variant is much closer > to a m48t08 type, the model can be used to differentiate between > the PIO and MMIO accesses. > > Signed-off-by: Artyom Tarasenko <atar4q...@gmail.com> > --- > hw/timer/m48t59.c | 38 +++++++++++++++++++++++++++++++++++--- > 1 files changed, 35 insertions(+), 3 deletions(-) > > diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c > index 5019e06..00ad417 100644 > --- a/hw/timer/m48t59.c > +++ b/hw/timer/m48t59.c > @@ -632,6 +632,33 @@ static const MemoryRegionOps m48t59_io_ops = { > .endianness = DEVICE_LITTLE_ENDIAN, > }; > > +static uint64_t nvram_read(void *opaque, hwaddr addr, unsigned size) > +{ > + M48t59State *NVRAM = opaque; Probably this is just copy&paste from old code, but please use lower_case_names for variables in new code. > + uint32_t retval; > + > + retval = m48t59_read(NVRAM, addr); > + return retval; > +} > + > +static void nvram_write(void *opaque, hwaddr addr, uint64_t value, > + unsigned size) Indentation one-off. > +{ > + M48t59State *NVRAM = opaque; > + > + m48t59_write(NVRAM, addr, value & 0xff); > +} > + > +static const MemoryRegionOps m48t59_mmio_ops = { > + .read = nvram_read, > + .write = nvram_write, > + .impl = { > + .min_access_size = 1, > + .max_access_size = 1, > + }, > + .endianness = DEVICE_LITTLE_ENDIAN, > +}; > + > /* Initialisation routine */ > M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base, > uint32_t io_base, uint16_t size, int model) > @@ -676,7 +703,11 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t > io_base, uint16_t size, > d = DO_UPCAST(M48t59ISAState, busdev, dev); > s = &d->state; > > - memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4); > + if (model == 59) { > + memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4); > + } else { > + memory_region_init_io(&d->io, &m48t59_mmio_ops, s, "m48t59", size); If you distinguish here, it may be a good idea to reflect that in the region's name. > + } > if (io_base != 0) { > isa_register_ioport(dev, &d->io, io_base); > } > @@ -700,8 +731,9 @@ static int m48t59_init_isa1(ISADevice *dev) > { > M48t59ISAState *d = DO_UPCAST(M48t59ISAState, busdev, dev); > M48t59State *s = &d->state; > - > - isa_init_irq(dev, &s->IRQ, 8); > + if (s->model == 59) { > + isa_init_irq(dev, &s->IRQ, 8); > + } > m48t59_init_common(s); > > return 0; Regarding your question of whether to move this: With my ISA realize series this function becomes a ..._realize function. isa_init_irq() relies on the device being on an ISA bus to retrieve the qemu_irq, so this cannot be done in instance_init, thus in DeviceClass::init/realize. The existing legacy m48t59_init_isa() function should probably rather shrink in size and one day possibly be inlined rather than growing with stuff that was encapsulated in initfn or realizefn. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg