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? Should we just drop the offset hackery as a workaround for a now-fixed bug? Are we running into the "mapping devices at non-page-offsets isn't supported" issue here? <optimism>Is that now supported after this patch series?</> (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.) -- PMM