On Wed, Jan 6, 2016 at 5:32 AM, Peter Crosthwaite <crosthwaitepe...@gmail.com> wrote: > On Tue, Jan 5, 2016 at 10:07 PM, Andrew Baumann > <andrew.baum...@microsoft.com> wrote: >>> From: Alistair Francis [mailto:alistai...@gmail.com] >>> Sent: Tuesday, 5 January 2016 18:14 >>> On Thu, Dec 31, 2015 at 4:31 PM, Andrew Baumann >>> <andrew.baum...@microsoft.com> wrote: >>> > This device maintains all the non-CPU peripherals on bcm2835 (Pi1) >>> > which are also present on bcm2836 (Pi2). It also implements the >>> > private address spaces used for DMA and mailboxes. >> [...] >>> > + obj = object_property_get_link(OBJECT(dev), "ram", &err); >>> > + if (obj == NULL) { >>> > + error_setg(errp, "%s: required ram link not found: %s", >>> > + __func__, error_get_pretty(err)); >>> > + return; >>> > + } >>> >>> I only had a quick read of this patch, but this RAM linking looks fine >>> to me. Out of curiosity is there a reason you use >>> object_property_get_link() instead of object_property_add_link() in >>> the init? >> > > The const link system removes the need for the object to have storage > for the link pointer in state. This means you don't need the state > field or add_link(), but the only way to get the pointer for your own > use is to get_link() on yourself. This is slightly simpler but has the > disadvantage that you cannot unlink and then relink something else (I > think?).
Ok, that makes sense. Thanks, Alistair > > I don't have an opinion over which way is more correct so both are > fine for me but if the QOM people have a preferred style we should > probably make the two patches consistent. > > Regards, > Peter > >> I'm not sure I understand your question... it wouldn't work the other way. I >> allocate the ram and add the link using object_property_add_const_link() in >> hw/arm/raspi.c. This file needs to consume the ram to setup alias mappings, >> so it is using get_link(). (Note there's also level of indirection; raspi >> creates bcm2836, which does nothing but get the link set by its parent and >> add it to its bcm2835_peripherals child.) >> >> I suppose I could do it the other way around (allocate and set link in >> bcm2835_peripherals, based on a size passed from the board), but it seemed >> more logical to treat the RAM as created/owned of the board rather than the >> SoC. >> >> Cheers, >> Andrew
