On 11/26/12 16:18, Andreas Färber wrote: > 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.
Setting .valid matches previous behavior (only byte handlers registered). I'm not fully sure what the defaults for .valid are in case only .impl is specified. I usually either set .valid only or explicitly specify both if I want the memory api split dword writes into bytes for me. > 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 }? As far I know the whole (outer) struct is zero-initialized. cheers, Gerd