On 03/07/2012 09:32 PM, Peter Maydell wrote: > On 7 March 2012 17:49, Peter Maydell <peter.mayd...@linaro.org> wrote: > > git bisect blames this commit (5312bd8b3) for causing a Linux kernel > > on spitz to produce a bunch of pxa2xx_i2c warnings that weren't > > being emitted before: > > What seems to happen here is that we register a memory region > (this is for the second i2c device in hw/pxa2xx.c): > > memory_region_init_io(&s->iomem, &pxa2xx_i2c_ops, s, > "pxa2xx-i2x", s->region_size); > > where region_size is 0x100. We then map it at 0x40f00100 > (via sysbus_mmio_map). This used to result in our read and write > functions being called with offsets from the start of the page, > so in this case for the register at 0x90 into the device the > passed in addr would be 0x190. There is some hackery in pxa2xx_i2c_init > to work out what the offset is from the start of the region > when we map the device, we pass it in as a qdev 'offset' > property, and then read/write can fix things up to get the > actual register offset. > > With this commit read and write functions are now passed the actual > offset from the start of the device region, ie 0x90. So the hackery > ends up doing fixing up it doesn't need to do, and generates negative > offsets which cause the diagnostic messages. > > So it seems like the new behaviour is more like the right thing, > but was it an intentional change?
I don't recall whether it was intentional or not (i.e., whether I was aware I was changing behaviour or not), but it's certainly the desired behaviour. > Should we just drop the offset > hackery as a workaround for a now-fixed bug? Yes. I'll live the patch to you. > > Are we running into the "mapping devices at non-page-offsets isn't > supported" issue here? It wasn't supported? > <optimism>Is that now supported after this > patch series?</> I'm sure that there are some rough edges but I made quite an effort to cover corner cases. For example, a region that starts and ends and non-aligned offsets, but spans more than a page, ought to work. > (I think the other devices I know of which include workarounds > for being passed relative-to-page-base addresses handle it by > masking out the high bits of the address, eg arm11mpcore.c, > so they weren't broken by this commit.) I assumed this was due to the days where absolute addresses were passed, but yes. -- error compiling committee.c: too many arguments to function
