On Mon, Apr 30, 2012 at 14:33, Mark Cave-Ayland
> On 30/04/12 15:03, Peter Maydell wrote:
>>> Therefore I can't change it to my (modified) sbus_mmio_map() function
>>> because it would break other non-SPARC platforms, and AIUI there is
>>> in the memory API that allows me to move a subregion to a different
>>> MemoryRegion parent, even if I can get a reference to it with
>>> sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
>>> misunderstood something?
>> Init functions like esp_init should be purely convenience functions
>> which create, set properties on, and map the sysbus/qdev device. If
>> they make use of private knowledge about the internals of the device
>> then this is wrong and should be fixed. For esp_init() it looks like
>> the handling of dma_memory_read, dma_memory_write, dma_opaque, it_shift
>> and dma_enabled are wrong.
>> If you fix that then you can just ignore the convenience function,
>> and create, configure and map the device as appropriate for you.
> Right I think I'm starting to understand this now - in which case it becomes
> a matter of just copying a handful of lines within sun4m which is more
> In your view, would a suitable fix be to change dma_memory_read,
> dma_memory_write, dma_opaque, it_shift and dma_enabled to be qdev properties
> and modify esp_init() to return the qdev reference so they can be set by the
There's an ongoing work to introduce IOMMUs by changing how DMA work
and this could simplify the DMA part. There's no clean way to use
function pointers in qdev.
For it_shift, a qdev or QOM property should be OK.
The signal dma_enabled should be eventually replaced by a Pin.