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?). 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