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
