On 19 February 2013 23:54, Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote: > On Tue, Feb 19, 2013 at 6:19 AM, Andreas Färber <afaer...@suse.de> wrote: >> Am 18.02.2013 19:49, schrieb Peter Maydell: >>> You need to add a DeviceState* for the scu. >> >> No, not a DeviceState*, an A9SCUState. With object_initialize() and >> qdev_set_parent_bus(NULL) instead of qdev_create() to be exact and some >> child<A9SCUState> property for ownership transfer. 2/7 and commit >> message say why. > > Hi Andreas, what you are proposing is a major change pattern to pretty > much the entire tree - every device that is part of a container object > needs to inlined (thus creating public headers). Despite the fact that > I disagree with that approach, this change is way out of scope of this > series and changing just the SCU to be like this will make it > inconsistent with its peer devices GIC and MPTimer. This is cut and > paste re-organisation of existing code that is groundwork for what you > are talking about and the patch still stands in its own right in that > this scheme is better than what we have today. > > So I'd like to take a crawl before we walk approach to this patch. For > the next revision i'm going to do it Peters way and ask that we sort > out the big questions about QOM containers and inline-structs for > MPCore in another patch series. Then we can fix GIC and MPTimer at the > same time and everything is consistent. Too often when contributors > submit patches some minor issue get tangled in large out of scope > discussions about QOM that relate to the entire tree. The whole series > then ends up bitrotting on list or living out of tree forever.
I agree with all of this. I would like us to actually thrash out the "how do we do child devices in the QOM way" and write up the answers (rather than repeating the same skirmishes in every patch review thread). But we shouldn't derail every patch series until we have figured that out... For the moment this patch series is a clear improvement and (once the minor review issues are fixed) I'm happy to commit it to arm-devs.next. -- PMM