qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Peter Crosthwaite <peter.crosthwaite@petalogix.com>,
	patches@linaro.org, Alexander Graf <agraf@suse.de>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/2] device_tree: Add qemu_devtree_setprop_sized_cells() utility function
Date: Wed, 26 Jun 2013 09:38:50 +1000	[thread overview]
Message-ID: <20130625233850.GG25265@voom.fritz.box> (raw)
In-Reply-To: <CAFEAcA8tiuf3CuVOwP_mz-AUHV7cJY7ZYUdJML4tBxQi8OkLww@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]

On Mon, Jun 24, 2013 at 12:02:39PM +0100, Peter Maydell wrote:
> On 24 June 2013 11:56, Alexander Graf <agraf@suse.de> wrote:
> > On 24.06.2013, at 12:22, Peter Maydell wrote:
> >> We already have a qemu_devtree_setprop_cells() which sets a dtb
> >> property to an array of cells whose values are specified by varargs.
> >> However for the fairly common case of setting a property to a list
> >> of addresses or of address,size pairs the number of cells used by
> >> each element in the list depends on the parent's #address-cells
> >> and #size-cells properties. To make this easier we provide an analogous
> >> qemu_devtree_setprop_sized_cells() function which allows the number
> >> of cells used by each element to be specified.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >
> > This looks pretty complicated for something actually quite
> > simple: All you want is to pass in a number of 64bit values,
> > rather than 32bit ones, right?
> 
> Nope. If the device tree blob says #address-cells is 1
> and #size-cells is 1, then I want to pass in values to
> go in 32 bit cells. If it says #address-cells is 2 but
> #size-cells is still 1, then I want to pass in a value
> for a 64 bit cell then one for a 32 bit cell. If they're
> both 2 then I want to pass in values for two 64 bit
> cells.

Hmm.. the property certainly needs to be constructed that way.  But I
think Alex's point is that you could make the arguments all 64-bit,
and then truncate them in the generated property.

There's a bigger problem, though, that exists with both versions.  In
general people expect integer arguments like this not to care too much
about the exact integer type, because it will be promoted to the right
thing.  Except with varargs it won't.  So if you ever have a
notionally 64-bit address that happens to fit in a 32-bit variable and
you pass that it, this function will be broken.  And the worst of it
is, it'll work most of the time, until you happen to hit the wrong ABI
and parameter combination :(.

> It's pretty complicated because the device tree
> spec is pretty complicated (if it had just mandated
> 64 bit addr/size everywhere then this would be easy).

Actually, no, because #address-cells and #size-cells cover things
other than "memory-like" addresses.  e.g. #address-cells is 3 for PCI,
which wouldn't be covered by 64-bit addressing.

-- 
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

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

  reply	other threads:[~2013-06-25 23:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-24 10:22 [Qemu-devel] [PATCH 0/2] device_tree: add qemu_devtree_setprop_sized_cells() Peter Maydell
2013-06-24 10:22 ` [Qemu-devel] [PATCH 1/2] device_tree: Add qemu_devtree_setprop_sized_cells() utility function Peter Maydell
2013-06-24 10:56   ` Alexander Graf
2013-06-24 11:02     ` Peter Maydell
2013-06-25 23:38       ` David Gibson [this message]
2013-06-26  8:49         ` Peter Maydell
2013-06-26 10:31           ` Alexander Graf
2013-06-26 10:50             ` Peter Maydell
2013-06-26 11:42               ` Alexander Graf
2013-06-26 12:38               ` Peter Crosthwaite
2013-06-26 12:44                 ` Alexander Graf
2013-06-27  0:17                   ` David Gibson
2013-06-27  0:27                     ` Anthony Liguori
2013-06-26 13:13                 ` Peter Maydell
2013-06-26 13:31                 ` Peter Maydell
2013-06-27  0:15               ` David Gibson
2013-06-27  0:10           ` David Gibson
2013-06-24 10:22 ` [Qemu-devel] [PATCH 2/2] arm/boot: Use qemu_devtree_setprop_sized_cells() Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130625233850.GG25265@voom.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=patches@linaro.org \
    --cc=peter.crosthwaite@petalogix.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).