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 addr) > return val; > } > > +static const MemoryRegionOps pm_smbus_ops = { > + .read = smb_ioport_readb, > + .write = smb_ioport_writeb, > + .valid.min_access_size = 1, > + .valid.max_access_size = 1, > + .endianness = 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 = baz notation or would it be advisable to use nested .foo = { .bar = baz }? > + > void pm_smbus_init(DeviceState *parent, PMSMBus *smb) > { > smb->smbus = 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 @@ > > typedef struct PMSMBus { > i2c_bus *smbus; > + MemoryRegion io; > > 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 = { > } > }; > > -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 = opaque; > - uint8_t hostc = s->dev.config[ICH9_SMB_HOSTC]; > + ICH9SMBState *s = ICH9_SMB_DEVICE(d); [snip] -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg