From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43120) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGoCq-0007AE-MB for qemu-devel@nongnu.org; Wed, 06 Jan 2016 08:32:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGoCk-0004S1-C0 for qemu-devel@nongnu.org; Wed, 06 Jan 2016 08:32:52 -0500 MIME-Version: 1.0 In-Reply-To: References: <1451608294-16432-1-git-send-email-Andrew.Baumann@microsoft.com> <1451608294-16432-5-git-send-email-Andrew.Baumann@microsoft.com> Date: Wed, 6 Jan 2016 05:32:45 -0800 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 4/7] bcm2835_peripherals: add rollup device for bcm2835 peripherals List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Baumann , =?UTF-8?Q?Andreas_F=C3=A4rber?= Cc: Peter Maydell , =?UTF-8?Q?Gr=C3=A9gory_ESTRADE?= , Stefan Weil , Peter Crosthwaite , "qemu-devel@nongnu.org Developers" , qemu-arm , Paolo Bonzini , Alistair Francis On Tue, Jan 5, 2016 at 10:07 PM, Andrew Baumann wrote: >> From: Alistair Francis [mailto:alistair23@gmail.com] >> Sent: Tuesday, 5 January 2016 18:14 >> On Thu, Dec 31, 2015 at 4:31 PM, Andrew Baumann >> 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 =3D object_property_get_link(OBJECT(dev), "ram", &err); >> > + if (obj =3D=3D 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 mapp= ings, so it is using get_link(). (Note there's also level of indirection; r= aspi 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 bc= m2835_peripherals, based on a size passed from the board), but it seemed mo= re logical to treat the RAM as created/owned of the board rather than the S= oC. > > Cheers, > Andrew