On Mon, Apr 30, 2012 at 14:33, Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> wrote: > 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 >>> nothing >>> 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 > bearable. > > 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 > caller?
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. > > > ATB, > > Mark. >
