From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41441) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UrzuW-0007Gh-Bq for qemu-devel@nongnu.org; Wed, 26 Jun 2013 20:18:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UrzuS-0005AK-2R for qemu-devel@nongnu.org; Wed, 26 Jun 2013 20:18:04 -0400 Received: from ozlabs.org ([203.10.76.45]:41732) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UrzuR-00059p-GO for qemu-devel@nongnu.org; Wed, 26 Jun 2013 20:18:00 -0400 Date: Thu, 27 Jun 2013 10:17:37 +1000 From: David Gibson Message-ID: <20130627001737.GC10614@voom.fritz.box> References: <1372069376-30640-1-git-send-email-peter.maydell@linaro.org> <1372069376-30640-2-git-send-email-peter.maydell@linaro.org> <3708A7FB-9BEF-494C-B5A2-B1A97C1F78C4@suse.de> <20130625233850.GG25265@voom.fritz.box> <51CAE221.2000607@suse.de> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="FsscpQKzF/jJk6ya" Content-Disposition: inline In-Reply-To: <51CAE221.2000607@suse.de> Subject: Re: [Qemu-devel] [PATCH 1/2] device_tree: Add qemu_devtree_setprop_sized_cells() utility function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Peter Maydell , Peter Crosthwaite , qemu-devel@nongnu.org, patches@linaro.org --FsscpQKzF/jJk6ya Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jun 26, 2013 at 02:44:17PM +0200, Alexander Graf wrote: > On 06/26/2013 02:38 PM, Peter Crosthwaite wrote: > >Hi, > > > >On Wed, Jun 26, 2013 at 8:50 PM, Peter Maydell= wrote: > >>On 26 June 2013 11:31, Alexander Graf wrote: > >>>I think it makes sense to make this API special-purpose for "reg". > >>>We currently have a generic "put any number of 32bit values into the > >>>property" function (qemu_devtree_setprop_cells). > >>Yes, but that doesn't work for things that aren't simple arrays > >>of 32 bit values, so I think that a generic way to deal > >>with those too would be useful. If you wanted to write a > >>"ranges" property you'd need this too, so it doesn't just > >>apply to "reg". > >> > >+1. And wouldn't an implementation of such a reg-specific function > >back onto Peter's new function quite nicely anyway? > > > >>I think we could avoid the "varargs doesn't promote" problem > >>by using a varargs-macro wrapper: > >> > >>#define qemu_devtree_setprop_sized_cells(fdt, node, prop, ...) \ > >> do { \ > >> uint64_t args[] =3D { __VA_ARGS__ }; \ > >> do_qemu_devtree_setprop_sized_cells(fdt, node, prop, \ > >> args, sizeof(args)); > >> } while (0) > >> > >Are statement expressions sanctioned? Or do we need to give up the > >nice caller accessible return codes? > > > >And can we factor out common functionality (mainly the error checking) > >with existing set_prop_cells to make the interfaces consistent? (we > >need to get rid of those aborts sooner or later) > > > >>which will promote everything (including the size arguments, > >>harmlessly) to uint64_t, and avoids having a varargs function. > >> > >>>Can't we also just add a qemu_devtree_setprop_reg() that walks > >>>the tree downwards in search for #address-cells and #size-cells > >>>and assembles the correct reg property from a list of 64bit > >>>arguments? > >I have a patch in my tree that generalises qemu_devtree_getprop* to > >allow you walk parents for properties (as per the #foo-cells > >semantic). I use it for interrupt cells however, which kinda suggests > >that this wish for parent traversal is more generic than just > >populating reg. I think that Peters patch, along with a parent search > >friendly property search will be enough to be able to do whatever you > >want in only a handful of lines. > > > >>Do we have an actual use for this? It seems pretty complicated. > >>I would expect that in practice there are two major use cases: > >> (a) create your own fdt from scratch (in which case you can > >> just make everything 64 bits and in any case will know > >> when creating nodes what the #address-cells etc are) > >> (b) modify an existing fdt, in which case you definitely don't > >> want to go poking around too deeply in the tree; anything > >> more than just "put an extra node in the root" is starting > >> to get pretty chancy. > >> > >Looking to the future, what about -device adding a periph then having > >qemu add it to the dtb before boot? >=20 > I've had a lengthy discussion about that with Anthony a while ago. > His take was that this is perfectly reasonable, as long as the > device tree generation code stays within the machine model. The > machine would just traverse the QOM hierachy and generate device > tree nodes for everything it knows. I also talked with Anthony about this. Although he's insistent on the fdt generation staying within the machine, I think it would make sense to have some shared helpers for this between the fdt platforms. Note that spapr already contains a half-arsed implementation of this. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --FsscpQKzF/jJk6ya Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) iEYEARECAAYFAlHLhKEACgkQaILKxv3ab8aAnACePFtdHpD9Q6+wQQmu6NNoqYiT 64kAnRVirqKKvhj+BsoZHqAXKS8hdYDf =7b95 -----END PGP SIGNATURE----- --FsscpQKzF/jJk6ya--