From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50152) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UW6rZ-0003nF-8k for qemu-devel@nongnu.org; Sat, 27 Apr 2013 11:16:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UW6rW-0007E7-Mr for qemu-devel@nongnu.org; Sat, 27 Apr 2013 11:16:33 -0400 Received: from cantor2.suse.de ([195.135.220.15]:35505 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UW6rW-0007E0-Dd for qemu-devel@nongnu.org; Sat, 27 Apr 2013 11:16:30 -0400 Message-ID: <517BEBCB.4050005@suse.de> Date: Sat, 27 Apr 2013 17:16:27 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/2] m48t59: use mmio for the m48t08 model of the m48t59_isa card List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Artyom Tarasenko Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org 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. >=20 > Signed-off-by: Artyom Tarasenko > --- > hw/timer/m48t59.c | 38 +++++++++++++++++++++++++++++++++++--- > 1 files changed, 35 insertions(+), 3 deletions(-) >=20 > 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 =3D { > .endianness =3D DEVICE_LITTLE_ENDIAN, > }; > =20 > +static uint64_t nvram_read(void *opaque, hwaddr addr, unsigned size) > +{ > + M48t59State *NVRAM =3D 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 =3D m48t59_read(NVRAM, addr); > + return retval; > +} > + > +static void nvram_write(void *opaque, hwaddr addr, uint64_t value, > + unsigned size) Indentation one-off. > +{ > + M48t59State *NVRAM =3D opaque; > + > + m48t59_write(NVRAM, addr, value & 0xff); > +} > + > +static const MemoryRegionOps m48t59_mmio_ops =3D { > + .read =3D nvram_read, > + .write =3D nvram_write, > + .impl =3D { > + .min_access_size =3D 1, > + .max_access_size =3D 1, > + }, > + .endianness =3D 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 =3D DO_UPCAST(M48t59ISAState, busdev, dev); > s =3D &d->state; > =20 > - memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4); > + if (model =3D=3D 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", s= ize); If you distinguish here, it may be a good idea to reflect that in the region's name. > + } > if (io_base !=3D 0) { > isa_register_ioport(dev, &d->io, io_base); > } > @@ -700,8 +731,9 @@ static int m48t59_init_isa1(ISADevice *dev) > { > M48t59ISAState *d =3D DO_UPCAST(M48t59ISAState, busdev, dev); > M48t59State *s =3D &d->state; > - > - isa_init_irq(dev, &s->IRQ, 8); > + if (s->model =3D=3D 59) { > + isa_init_irq(dev, &s->IRQ, 8); > + } > m48t59_init_common(s); > =20 > 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 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg