From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60676) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Td0Rr-0004T7-3f for qemu-devel@nongnu.org; Mon, 26 Nov 2012 10:18:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Td0Rm-0008JV-SG for qemu-devel@nongnu.org; Mon, 26 Nov 2012 10:18:15 -0500 Received: from cantor2.suse.de ([195.135.220.15]:50459 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Td0Rm-0008JP-II for qemu-devel@nongnu.org; Mon, 26 Nov 2012 10:18:10 -0500 Message-ID: <50B3882D.7000700@suse.de> Date: Mon, 26 Nov 2012 16:18:05 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1353685711-24573-1-git-send-email-kraxel@redhat.com> <1353685711-24573-16-git-send-email-kraxel@redhat.com> In-Reply-To: <1353685711-24573-16-git-send-email-kraxel@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 15/20] acpi: switch smbus to memory api List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Julien Grall , qemu-devel@nongnu.org Am 23.11.2012 16:48, schrieb Gerd Hoffmann: > diff --git a/hw/pm_smbus.c b/hw/pm_smbus.c > index 5d6046d..ea1380c 100644 > --- a/hw/pm_smbus.c > +++ b/hw/pm_smbus.c [...] > @@ -170,7 +170,16 @@ uint32_t smb_ioport_readb(void *opaque, uint32_t a= ddr) > return val; > } > =20 > +static const MemoryRegionOps pm_smbus_ops =3D { > + .read =3D smb_ioport_readb, > + .write =3D smb_ioport_writeb, > + .valid.min_access_size =3D 1, > + .valid.max_access_size =3D 1, > + .endianness =3D DEVICE_LITTLE_ENDIAN, > +}; I notice that in comparison to Julien's patch, you are setting .valid here where he used .impl. Also a generic C question: When using C99-style struct initializers as for the MemoryRegionOps, I understand that the fields not explicitly assigned are zero-initialized. Does that also apply to .foo.bar =3D baz notation or would it be advisable to use nested .foo =3D { .bar =3D baz }= ? > + > void pm_smbus_init(DeviceState *parent, PMSMBus *smb) > { > smb->smbus =3D i2c_init_bus(parent, "i2c"); > + memory_region_init_io(&smb->io, &pm_smbus_ops, smb, "pm-smbus", 64= ); > } > diff --git a/hw/pm_smbus.h b/hw/pm_smbus.h > index 4750a40..e3069bf 100644 > --- a/hw/pm_smbus.h > +++ b/hw/pm_smbus.h > @@ -3,6 +3,7 @@ > =20 > typedef struct PMSMBus { > i2c_bus *smbus; > + MemoryRegion io; > =20 > uint8_t smb_stat; > uint8_t smb_ctl; With a view to further QOM'ifying these devices, please keep the parent field separated, i.e. i2c_bus *smbus; + MemoryRegion io; + uint8_t ... (Once macros and types are correctly used - as you do for ICH9 below - smbus would also be renamed to parent_obj for clarity, cf. QOM PHB/IDE series, and optionally excluded from gtk-doc documentation.) Regards, Andreas > diff --git a/hw/smbus_ich9.c b/hw/smbus_ich9.c > index 6940583..54e7e12 100644 > --- a/hw/smbus_ich9.c > +++ b/hw/smbus_ich9.c [...] > @@ -54,42 +53,23 @@ static const VMStateDescription vmstate_ich9_smbus = =3D { > } > }; > =20 > -static void ich9_smb_ioport_writeb(void *opaque, hwaddr addr, > - uint64_t val, unsigned size) > +static void ich9_smbus_write_config(PCIDevice *d, uint32_t address, > + uint32_t val, int len) > { > - ICH9SMBState *s =3D opaque; > - uint8_t hostc =3D s->dev.config[ICH9_SMB_HOSTC]; > + ICH9SMBState *s =3D ICH9_SMB_DEVICE(d); [snip] --=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