[Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Alexander Graf]

Today we have two separate places where we keep information which device
is where:

  - hw/ppce500_mpc8544ds.c to instantiate all devices
  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices

Every time we split crucial information, things can go terribly wrong. If
you update one file, but not the other, you can screw things up without
realizing it quickly.

The redundancy is also unnecessary, because QEMU already knows all the
information at which addresses its devices live. So we can generate the
device tree from the same variables - and even have the device tree adjust
if something changes in there.

The one functionality we lose with this approach is the ability to manually
patch the device tree to contain additional devices. To still be able to do
so easily, we introduce a new option -machine dumpdtb=<file> that creates a
dtb output file which can be used with -machine dtb=<file> later. In between
these 2 executions of QEMU, the dtb can be modified however much you like.

A lot of bits in this patch set are still hardcoded. We also don't accomodate
for dynamic creation of device tree nodes when -device is used. This requires
a bit more QOM'ification for us to be able to loop through all devices, so we
can dynamically create the device tree nodes for them. The basic concept should
still hold as is though.


Alex

v1 -> v2:

  - rename cell64 -> u64
  - don't treat memory as single u64
  - remove commit id from patch description
  - NEW: PPC: e500: Use new MPIC dt format
         PPC: e500: Use new SOC dt format
         PPC: e500: Define addresses as always 64bit
         PPC: e500: Extend address/size of / to 64bit
         dt: Add global option to set phandle start offset
         PPC: e500: Refactor serial dt generation

Alexander Graf (31):
  dt: allow add_subnode to create root subnodes
  dt: add helpers for 2, 3 and 4 cell adds
  dt: add helper for phandle references
  dt: temporarily disable subtree creation failure check
  dt: add helper for phandle enumeration
  dt: add helper for empty dt creation
  dt: add helper for phandle allocation
  dt: add helper for 64bit cell adds
  PPC: e500: require libfdt
  PPC: e500: dt: create memory node dynamically
  PPC: e500: dt: create /cpus node dynamically
  PPC: e500: dt: create /hypervisor node dynamically
  PPC: e500: dt: create / node dynamically
  PPC: e500: dt: create /chosen node dynamically
  PPC: e500: dt: create /soc8544 node dynamically
  PPC: e500: dt: create serial nodes dynamically
  PPC: e500: dt: create mpic node dynamically
  PPC: e500: dt: create global-utils node dynamically
  PPC: e500: dt: create pci node dynamically
  PPC: e500: dt: start with empty device tree
  dt: Add -machine dumpdtb option to dump the current dtb
  PPC: e500: dt: use 64bit cell helper
  PPC: e500: dt: use target_phys_addr_t for ramsize
  PPC: e500: enable manual loading of dtb blob
  Revert "dt: temporarily disable subtree creation failure check"
  PPC: e500: Use new MPIC dt format
  PPC: e500: Use new SOC dt format
  PPC: e500: Define addresses as always 64bit
  PPC: e500: Extend address/size of / to 64bit
  dt: Add global option to set phandle start offset
  PPC: e500: Refactor serial dt generation

 Makefile               |    1 -
 Makefile.target        |    2 +-
 device_tree.c          |  134 +++++++++++++++++++++++++++-
 device_tree.h          |   16 +++
 hw/ppce500_mpc8544ds.c |  237 +++++++++++++++++++++++++++++++++++++++++------
 pc-bios/mpc8544ds.dtb  |  Bin 2028 -> 0 bytes
 pc-bios/mpc8544ds.dts  |  119 ------------------------
 qemu-config.c          |    8 ++
 roms/openbios          |    2 +-
 9 files changed, 365 insertions(+), 154 deletions(-)
 delete mode 100644 pc-bios/mpc8544ds.dtb
 delete mode 100644 pc-bios/mpc8544ds.dts

[Qemu-devel] [PATCH 01/31] dt: allow add_subnode to create root subnodes

[Alexander Graf]

Our subnode creation helper can't handle creation of root subnodes,
like "/memory". Fix this by allowing the parent node to be an empty
string, indicating the root node.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 86a694c..94a239e 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -151,6 +151,7 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
     char *dupname = g_strdup(name);
     char *basename = strrchr(dupname, '/');
     int retval;
+    int parent = 0;
 
     if (!basename) {
         g_free(dupname);
@@ -160,7 +161,11 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
     basename[0] = '\0';
     basename++;
 
-    retval = fdt_add_subnode(fdt, findnode_nofail(fdt, dupname), basename);
+    if (dupname[0]) {
+        parent = findnode_nofail(fdt, dupname);
+    }
+
+    retval = fdt_add_subnode(fdt, parent, basename);
     if (retval < 0) {
         fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
                 fdt_strerror(retval));
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 01/31] dt: allow add_subnode to create root subnodes

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> Our subnode creation helper can't handle creation of root subnodes,
> like "/memory". Fix this by allowing the parent node to be an empty
> string, indicating the root node.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@petalogix.com>

> ---
>  device_tree.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 86a694c..94a239e 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -151,6 +151,7 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
>      char *dupname = g_strdup(name);
>      char *basename = strrchr(dupname, '/');
>      int retval;
> +    int parent = 0;
>  
>      if (!basename) {
>          g_free(dupname);
> @@ -160,7 +161,11 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
>      basename[0] = '\0';
>      basename++;
>  
> -    retval = fdt_add_subnode(fdt, findnode_nofail(fdt, dupname), basename);
> +    if (dupname[0]) {
> +        parent = findnode_nofail(fdt, dupname);
> +    }
> +
> +    retval = fdt_add_subnode(fdt, parent, basename);
>      if (retval < 0) {
>          fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
>                  fdt_strerror(retval));

[Qemu-devel] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[Alexander Graf]

We have device tree helpers that allow us to create single cell (u32)
wide properties. However, when creating properties that contain an array of
cells, we need to jump through hoops, manually passing in an array with
converted endianness.

To ease the pain of this a bit, create helpers for the most common array
sizes, namely 2, 3 and 4 cells wide properties.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |   30 ++++++++++++++++++++++++++++++
 device_tree.h |    9 +++++++++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 94a239e..b1dff4f 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
     return r;
 }
 
+int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
+                               const char *property, uint32_t val,
+                               uint32_t val2)
+{
+    uint32_t tmp[] = { cpu_to_be32(val),
+                       cpu_to_be32(val2) };
+    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
+}
+
+int qemu_devtree_setprop_cell3(void *fdt, const char *node_path,
+                               const char *property, uint32_t val,
+                               uint32_t val2, uint32_t val3)
+{
+    uint32_t tmp[] = { cpu_to_be32(val),
+                       cpu_to_be32(val2),
+                       cpu_to_be32(val3) };
+    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
+}
+
+int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
+                               const char *property, uint32_t val,
+                               uint32_t val2, uint32_t val3, uint32_t val4)
+{
+    uint32_t tmp[] = { cpu_to_be32(val),
+                       cpu_to_be32(val2),
+                       cpu_to_be32(val3),
+                       cpu_to_be32(val4) };
+    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
+}
+
 int qemu_devtree_setprop_string(void *fdt, const char *node_path,
                                 const char *property, const char *string)
 {
diff --git a/device_tree.h b/device_tree.h
index 4378685..9db7f86 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -20,6 +20,15 @@ int qemu_devtree_setprop(void *fdt, const char *node_path,
                          const char *property, void *val_array, int size);
 int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
                               const char *property, uint32_t val);
+int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
+                               const char *property, uint32_t val,
+                               uint32_t val2);
+int qemu_devtree_setprop_cell3(void *fdt, const char *node_path,
+                               const char *property, uint32_t val,
+                               uint32_t val2, uint32_t val3);
+int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
+                               const char *property, uint32_t val,
+                               uint32_t val2, uint32_t val3, uint32_t val4);
 int qemu_devtree_setprop_string(void *fdt, const char *node_path,
                                 const char *property, const char *string);
 int qemu_devtree_nop_node(void *fdt, const char *node_path);
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> We have device tree helpers that allow us to create single cell (u32)
> wide properties. However, when creating properties that contain an array of
> cells, we need to jump through hoops, manually passing in an array with
> converted endianness.
> 
> To ease the pain of this a bit, create helpers for the most common array
> sizes, namely 2, 3 and 4 cells wide properties.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  device_tree.c |   30 ++++++++++++++++++++++++++++++
>  device_tree.h |    9 +++++++++
>  2 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 94a239e..b1dff4f 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
>      return r;
>  }
>  
> +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
> +                               const char *property, uint32_t val,
> +                               uint32_t val2)
> +{
> +    uint32_t tmp[] = { cpu_to_be32(val),
> +                       cpu_to_be32(val2) };
> +    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
> +}
> +
> +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path,
> +                               const char *property, uint32_t val,
> +                               uint32_t val2, uint32_t val3)
> +{
> +    uint32_t tmp[] = { cpu_to_be32(val),
> +                       cpu_to_be32(val2),
> +                       cpu_to_be32(val3) };
> +    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
> +}
> +
> +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
> +                               const char *property, uint32_t val,
> +                               uint32_t val2, uint32_t val3, uint32_t val4)
> +{
> +    uint32_t tmp[] = { cpu_to_be32(val),
> +                       cpu_to_be32(val2),
> +                       cpu_to_be32(val3),
> +                       cpu_to_be32(val4) };
> +    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
> +}
> +

Cant this be generalised to the n case rather than having functional
replication for 2/3/4 word props?

+int qemu_devtree_setprop_celln(void *fdt, const char *node_path,
+                               const char *property, uint32_t *vals, int n)


>  int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>                                  const char *property, const char *string)
>  {
> diff --git a/device_tree.h b/device_tree.h
> index 4378685..9db7f86 100644
> --- a/device_tree.h
> +++ b/device_tree.h
> @@ -20,6 +20,15 @@ int qemu_devtree_setprop(void *fdt, const char *node_path,
>                           const char *property, void *val_array, int size);
>  int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
>                                const char *property, uint32_t val);
> +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
> +                               const char *property, uint32_t val,
> +                               uint32_t val2);
> +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path,
> +                               const char *property, uint32_t val,
> +                               uint32_t val2, uint32_t val3);
> +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
> +                               const char *property, uint32_t val,
> +                               uint32_t val2, uint32_t val3, uint32_t val4);
>  int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>                                  const char *property, const char *string);
>  int qemu_devtree_nop_node(void *fdt, const char *node_path);

Re: [Qemu-devel] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[Alexander Graf]

On 06/06/2012 07:01 AM, Peter Crosthwaite wrote:
> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>> We have device tree helpers that allow us to create single cell (u32)
>> wide properties. However, when creating properties that contain an array of
>> cells, we need to jump through hoops, manually passing in an array with
>> converted endianness.
>>
>> To ease the pain of this a bit, create helpers for the most common array
>> sizes, namely 2, 3 and 4 cells wide properties.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
>> ---
>>   device_tree.c |   30 ++++++++++++++++++++++++++++++
>>   device_tree.h |    9 +++++++++
>>   2 files changed, 39 insertions(+), 0 deletions(-)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index 94a239e..b1dff4f 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
>>       return r;
>>   }
>>
>> +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
>> +                               const char *property, uint32_t val,
>> +                               uint32_t val2)
>> +{
>> +    uint32_t tmp[] = { cpu_to_be32(val),
>> +                       cpu_to_be32(val2) };
>> +    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
>> +}
>> +
>> +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path,
>> +                               const char *property, uint32_t val,
>> +                               uint32_t val2, uint32_t val3)
>> +{
>> +    uint32_t tmp[] = { cpu_to_be32(val),
>> +                       cpu_to_be32(val2),
>> +                       cpu_to_be32(val3) };
>> +    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
>> +}
>> +
>> +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
>> +                               const char *property, uint32_t val,
>> +                               uint32_t val2, uint32_t val3, uint32_t val4)
>> +{
>> +    uint32_t tmp[] = { cpu_to_be32(val),
>> +                       cpu_to_be32(val2),
>> +                       cpu_to_be32(val3),
>> +                       cpu_to_be32(val4) };
>> +    return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp));
>> +}
>> +
> Cant this be generalised to the n case rather than having functional
> replication for 2/3/4 word props?
>
> +int qemu_devtree_setprop_celln(void *fdt, const char *node_path,
> +                               const char *property, uint32_t *vals, int n)

You mean internally? Yeah, probably. Externally? The point of these 
helpers is to make the code look less cluttered. We can already pass in 
an array just fine, but C is quite annoying about generating those on 
the fly, while it's easy to pass in ints as parameters :)


Alex

Re: [Qemu-devel] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[Scott Wood]

On 06/06/2012 10:55 AM, Alexander Graf wrote:
> On 06/06/2012 07:01 AM, Peter Crosthwaite wrote:
>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>> We have device tree helpers that allow us to create single cell (u32)
>>> wide properties. However, when creating properties that contain an
>>> array of
>>> cells, we need to jump through hoops, manually passing in an array with
>>> converted endianness.
>>>
>>> To ease the pain of this a bit, create helpers for the most common array
>>> sizes, namely 2, 3 and 4 cells wide properties.
>>>
>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>> ---
>>>   device_tree.c |   30 ++++++++++++++++++++++++++++++
>>>   device_tree.h |    9 +++++++++
>>>   2 files changed, 39 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/device_tree.c b/device_tree.c
>>> index 94a239e..b1dff4f 100644
>>> --- a/device_tree.c
>>> +++ b/device_tree.c
>>> @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const
>>> char *node_path,
>>>       return r;
>>>   }
>>>
>>> +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
>>> +                               const char *property, uint32_t val,
>>> +                               uint32_t val2)
>>> +{
>>> +    uint32_t tmp[] = { cpu_to_be32(val),
>>> +                       cpu_to_be32(val2) };
>>> +    return qemu_devtree_setprop(fdt, node_path, property, tmp,
>>> sizeof(tmp));
>>> +}

>From the subject line I was expecting these to add multi-cell numbers
together (e.g. for ranges parsing).

>>> +
>>> +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path,
>>> +                               const char *property, uint32_t val,
>>> +                               uint32_t val2, uint32_t val3)
>>> +{
>>> +    uint32_t tmp[] = { cpu_to_be32(val),
>>> +                       cpu_to_be32(val2),
>>> +                       cpu_to_be32(val3) };
>>> +    return qemu_devtree_setprop(fdt, node_path, property, tmp,
>>> sizeof(tmp));
>>> +}
>>> +
>>> +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
>>> +                               const char *property, uint32_t val,
>>> +                               uint32_t val2, uint32_t val3,
>>> uint32_t val4)
>>> +{
>>> +    uint32_t tmp[] = { cpu_to_be32(val),
>>> +                       cpu_to_be32(val2),
>>> +                       cpu_to_be32(val3),
>>> +                       cpu_to_be32(val4) };
>>> +    return qemu_devtree_setprop(fdt, node_path, property, tmp,
>>> sizeof(tmp));
>>> +}
>>> +
>> Cant this be generalised to the n case rather than having functional
>> replication for 2/3/4 word props?
>>
>> +int qemu_devtree_setprop_celln(void *fdt, const char *node_path,
>> +                               const char *property, uint32_t *vals,
>> int n)
> 
> You mean internally? Yeah, probably. Externally? The point of these
> helpers is to make the code look less cluttered. We can already pass in
> an array just fine, but C is quite annoying about generating those on
> the fly, while it's easy to pass in ints as parameters :)

Varargs?

-Scott

Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[David Gibson]

[snip]
> > You mean internally? Yeah, probably. Externally? The point of these
> > helpers is to make the code look less cluttered. We can already pass in
> > an array just fine, but C is quite annoying about generating those on
> > the fly, while it's easy to pass in ints as parameters :)
> 
> Varargs?

Ugly and risky with standard C varargs (because an explicit length
would be needed).  Could be done neatly with gcc macro varargs.

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[Alexander Graf]

On 07.06.2012, at 01:45, David Gibson wrote:

> [snip]
>>> You mean internally? Yeah, probably. Externally? The point of these
>>> helpers is to make the code look less cluttered. We can already pass in
>>> an array just fine, but C is quite annoying about generating those on
>>> the fly, while it's easy to pass in ints as parameters :)
>> 
>> Varargs?
> 
> Ugly and risky with standard C varargs (because an explicit length
> would be needed).  Could be done neatly with gcc macro varargs.

Could a combination of both like this work?

#include <stdio.h>
#include <stdarg.h>

#define __VA_NARG__(...) \
        (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1)
#define __VA_NARG_(...) \
        __VA_ARG_N(__VA_ARGS__)
#define __VA_ARG_N( \
         _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \
        _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \
        _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \
        _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \
        _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \
        _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \
        _61,_62,_63,N,...) N
#define __RSEQ_N() \
        63, 62, 61, 60,                         \
        59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \
        49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \
        39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \
        29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \
        19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \
         9,  8,  7,  6,  5,  4,  3,  2,  1,  0

#define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__)

int print_elems(void *fdt, unsigned int count, ...)
{
    int i;
    va_list argp;

    va_start(argp, count);

    for (i = 0; i < count; i++)
        printf("%d -> %#x\n", i, va_arg(argp, int));

    va_end(argp);

    return 0;
}

int main(int argc, char **argv)
{
    PRINT_ELEMS(NULL, 1, 2, 3, 4);
    return 0;
}

Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[David Gibson]

On Thu, Jun 07, 2012 at 01:27:56PM +0200, Alexander Graf wrote:
> 
> On 07.06.2012, at 01:45, David Gibson wrote:
> 
> > [snip]
> >>> You mean internally? Yeah, probably. Externally? The point of these
> >>> helpers is to make the code look less cluttered. We can already pass in
> >>> an array just fine, but C is quite annoying about generating those on
> >>> the fly, while it's easy to pass in ints as parameters :)
> >> 
> >> Varargs?
> > 
> > Ugly and risky with standard C varargs (because an explicit length
> > would be needed).  Could be done neatly with gcc macro varargs.
> 
> Could a combination of both like this work?
> 
> #include <stdio.h>
> #include <stdarg.h>
> 
> #define __VA_NARG__(...) \
>         (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1)
> #define __VA_NARG_(...) \
>         __VA_ARG_N(__VA_ARGS__)
> #define __VA_ARG_N( \
>          _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \
>         _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \
>         _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \
>         _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \
>         _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \
>         _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \
>         _61,_62,_63,N,...) N
> #define __RSEQ_N() \
>         63, 62, 61, 60,                         \
>         59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \
>         49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \
>         39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \
>         29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \
>         19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \
>          9,  8,  7,  6,  5,  4,  3,  2,  1,  0
> 
> #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__)

Um.. that might work, but it's ludicrously complicated.  If we're
prepared to use the gcc statement expression extension and we're just
going to abort on errors like findnode_nofail, it can be done much
more easily using c99 variadic macros:

	#define setprop_ints(fdt, path, prop, ...) \
		do { \
			uint32_t tmp[] = {__VA_ARGS__}; \

			if (fdt_setprop(findnode_nofail(fdt, path), prop, \
			           tmp, sizeof(tmp)) != 0) { \
				  /* error message */ \
				  abort(); \
			} \
		} while (0)

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[Alexander Graf]

On 07.06.2012, at 14:13, David Gibson wrote:

> On Thu, Jun 07, 2012 at 01:27:56PM +0200, Alexander Graf wrote:
>> 
>> On 07.06.2012, at 01:45, David Gibson wrote:
>> 
>>> [snip]
>>>>> You mean internally? Yeah, probably. Externally? The point of these
>>>>> helpers is to make the code look less cluttered. We can already pass in
>>>>> an array just fine, but C is quite annoying about generating those on
>>>>> the fly, while it's easy to pass in ints as parameters :)
>>>> 
>>>> Varargs?
>>> 
>>> Ugly and risky with standard C varargs (because an explicit length
>>> would be needed).  Could be done neatly with gcc macro varargs.
>> 
>> Could a combination of both like this work?
>> 
>> #include <stdio.h>
>> #include <stdarg.h>
>> 
>> #define __VA_NARG__(...) \
>>        (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1)
>> #define __VA_NARG_(...) \
>>        __VA_ARG_N(__VA_ARGS__)
>> #define __VA_ARG_N( \
>>         _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \
>>        _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \
>>        _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \
>>        _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \
>>        _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \
>>        _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \
>>        _61,_62,_63,N,...) N
>> #define __RSEQ_N() \
>>        63, 62, 61, 60,                         \
>>        59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \
>>        49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \
>>        39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \
>>        29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \
>>        19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \
>>         9,  8,  7,  6,  5,  4,  3,  2,  1,  0
>> 
>> #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__)
> 
> Um.. that might work, but it's ludicrously complicated.  If we're
> prepared to use the gcc statement expression extension and we're just
> going to abort on errors like findnode_nofail, it can be done much
> more easily using c99 variadic macros:
> 
> 	#define setprop_ints(fdt, path, prop, ...) \
> 		do { \
> 			uint32_t tmp[] = {__VA_ARGS__}; \
> 
> 			if (fdt_setprop(findnode_nofail(fdt, path), prop, \
> 			           tmp, sizeof(tmp)) != 0) { \
> 				  /* error message */ \
> 				  abort(); \
> 			} \
> 		} while (0)

Hrm. But here we'd be overloading the name space, no? If anyone passes in tmp[3] as parameter to setprop_ints, it would conflict with the internal variable tmp, right?


Alex

Re: [Qemu-devel] [Qemu-ppc] [PATCH 02/31] dt: add helpers for 2, 3 and 4 cell adds

[David Gibson]

On Fri, Jun 08, 2012 at 03:00:56PM +0200, Alexander Graf wrote:
> 
> On 07.06.2012, at 14:13, David Gibson wrote:
> 
> > On Thu, Jun 07, 2012 at 01:27:56PM +0200, Alexander Graf wrote:
> >> 
> >> On 07.06.2012, at 01:45, David Gibson wrote:
> >> 
> >>> [snip]
> >>>>> You mean internally? Yeah, probably. Externally? The point of these
> >>>>> helpers is to make the code look less cluttered. We can already pass in
> >>>>> an array just fine, but C is quite annoying about generating those on
> >>>>> the fly, while it's easy to pass in ints as parameters :)
> >>>> 
> >>>> Varargs?
> >>> 
> >>> Ugly and risky with standard C varargs (because an explicit length
> >>> would be needed).  Could be done neatly with gcc macro varargs.
> >> 
> >> Could a combination of both like this work?
> >> 
> >> #include <stdio.h>
> >> #include <stdarg.h>
> >> 
> >> #define __VA_NARG__(...) \
> >>        (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1)
> >> #define __VA_NARG_(...) \
> >>        __VA_ARG_N(__VA_ARGS__)
> >> #define __VA_ARG_N( \
> >>         _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \
> >>        _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \
> >>        _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \
> >>        _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \
> >>        _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \
> >>        _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \
> >>        _61,_62,_63,N,...) N
> >> #define __RSEQ_N() \
> >>        63, 62, 61, 60,                         \
> >>        59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \
> >>        49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \
> >>        39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \
> >>        29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \
> >>        19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \
> >>         9,  8,  7,  6,  5,  4,  3,  2,  1,  0
> >> 
> >> #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__)
> > 
> > Um.. that might work, but it's ludicrously complicated.  If we're
> > prepared to use the gcc statement expression extension and we're just
> > going to abort on errors like findnode_nofail, it can be done much
> > more easily using c99 variadic macros:
> > 
> > 	#define setprop_ints(fdt, path, prop, ...) \
> > 		do { \
> > 			uint32_t tmp[] = {__VA_ARGS__}; \
> > 
> > 			if (fdt_setprop(findnode_nofail(fdt, path), prop, \
> > 			           tmp, sizeof(tmp)) != 0) { \
> > 				  /* error message */ \
> > 				  abort(); \
> > 			} \
> > 		} while (0)
> 
> Hrm. But here we'd be overloading the name space, no? If anyone
> passes in tmp[3] as parameter to setprop_ints, it would conflict
> with the internal variable tmp, right?

Standard macro problem.  So, call it _tmp, whatever.

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

[Qemu-devel] [PATCH 03/31] dt: add helper for phandle references

[Alexander Graf]

Phandles are the fancy device tree name for "pointer to another node".
To create a phandle property, we most likely want to reference to the
node we're pointing to by its path. So create a helper that allows
us to do so.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    7 +++++++
 device_tree.h |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index b1dff4f..8e9262c 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -162,6 +162,13 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
     return r;
 }
 
+int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
+                                 const char *property, const char *string)
+{
+    uint32_t phandle = fdt_get_phandle(fdt, findnode_nofail(fdt, string));
+    return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
+}
+
 int qemu_devtree_nop_node(void *fdt, const char *node_path)
 {
     int r;
diff --git a/device_tree.h b/device_tree.h
index 9db7f86..2e87c58 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -31,6 +31,8 @@ int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
                                uint32_t val2, uint32_t val3, uint32_t val4);
 int qemu_devtree_setprop_string(void *fdt, const char *node_path,
                                 const char *property, const char *string);
+int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
+                                 const char *property, const char *string);
 int qemu_devtree_nop_node(void *fdt, const char *node_path);
 int qemu_devtree_add_subnode(void *fdt, const char *name);
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 03/31] dt: add helper for phandle references

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> Phandles are the fancy device tree name for "pointer to another node".
> To create a phandle property, we most likely want to reference to the
> node we're pointing to by its path. So create a helper that allows
> us to do so.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  device_tree.c |    7 +++++++
>  device_tree.h |    2 ++
>  2 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index b1dff4f..8e9262c 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -162,6 +162,13 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>      return r;
>  }
>  
> +int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
> +                                 const char *property, const char *string)

"string" is a bit of a misnomer for this argument, considering its a
node_path. Maybe "target", or "target_node_path"

> +{
> +    uint32_t phandle = fdt_get_phandle(fdt, findnode_nofail(fdt, string));
> +    return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
> +}
> +
>  int qemu_devtree_nop_node(void *fdt, const char *node_path)
>  {
>      int r;
> diff --git a/device_tree.h b/device_tree.h
> index 9db7f86..2e87c58 100644
> --- a/device_tree.h
> +++ b/device_tree.h
> @@ -31,6 +31,8 @@ int qemu_devtree_setprop_cell4(void *fdt, const char *node_path,
>                                 uint32_t val2, uint32_t val3, uint32_t val4);
>  int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>                                  const char *property, const char *string);
> +int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
> +                                 const char *property, const char *string);
>  int qemu_devtree_nop_node(void *fdt, const char *node_path);
>  int qemu_devtree_add_subnode(void *fdt, const char *name);
>  

[Qemu-devel] [PATCH 04/31] dt: temporarily disable subtree creation failure check

[Alexander Graf]

Usually we want to know when creating a subtree fails. However, while
introducing this patch set we have to modify the device tree and some
times have the code to create a subtree in both the binary tree and
the dynamically created tree.

So ignore failures about this for now and enable them once we got rid
of the binary device tree.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 8e9262c..6cbc5af 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -203,11 +203,13 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
     }
 
     retval = fdt_add_subnode(fdt, parent, basename);
+#if 0
     if (retval < 0) {
         fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
                 fdt_strerror(retval));
         exit(1);
     }
+#endif
 
     g_free(dupname);
     return retval;
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 04/31] dt: temporarily disable subtree creation failure check

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> Usually we want to know when creating a subtree fails. However, while
> introducing this patch set we have to modify the device tree and some
> times have the code to create a subtree in both the binary tree and
> the dynamically created tree.
> 
> So ignore failures about this for now and enable them once we got rid
> of the binary device tree.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@petalogix.com>

> ---
>  device_tree.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 8e9262c..6cbc5af 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -203,11 +203,13 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
>      }
>  
>      retval = fdt_add_subnode(fdt, parent, basename);
> +#if 0
>      if (retval < 0) {
>          fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
>                  fdt_strerror(retval));
>          exit(1);
>      }
> +#endif

Doesnt this illustrate a failure in this functions return path in the
first place?? Should this check be removed altogether and an error code
returned to the caller? That way callers (like you platform under
construction) can choose to ignore/act-on the error as appropriate.

>  
>      g_free(dupname);
>      return retval;

Re: [Qemu-devel] [PATCH 04/31] dt: temporarily disable subtree creation failure check

[Alexander Graf]

On 06/06/2012 07:32 AM, Peter Crosthwaite wrote:
> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>> Usually we want to know when creating a subtree fails. However, while
>> introducing this patch set we have to modify the device tree and some
>> times have the code to create a subtree in both the binary tree and
>> the dynamically created tree.
>>
>> So ignore failures about this for now and enable them once we got rid
>> of the binary device tree.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
> Reviewed-by: Peter Crosthwaite<peter.crosthwaite@petalogix.com>
>
>> ---
>>   device_tree.c |    2 ++
>>   1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index 8e9262c..6cbc5af 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -203,11 +203,13 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
>>       }
>>
>>       retval = fdt_add_subnode(fdt, parent, basename);
>> +#if 0
>>       if (retval<  0) {
>>           fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
>>                   fdt_strerror(retval));
>>           exit(1);
>>       }
>> +#endif
> Doesnt this illustrate a failure in this functions return path in the
> first place?? Should this check be removed altogether and an error code
> returned to the caller? That way callers (like you platform under
> construction) can choose to ignore/act-on the error as appropriate.

That would mean that we'd have to check every single fdt helper call for 
its return value. At the end of the day, we'd have an insane amount of 
code just for checking for potential errors, making the actual 
functional code unreadable.

So no, the one thing that these helpers buy you vs calling libfdt 
manually is exactly the exit(1) :).


Alex

[Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Alexander Graf]

This patch adds a helper to search for a node's phandle by its path. This
is especially useful when the phandle is part of an array, not just a single
cell in which case qemu_devtree_setprop_phandle would be the easy choice.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |   16 +++++++++++++++-
 device_tree.h |    1 +
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 6cbc5af..6745d17 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
     return r;
 }
 
+uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
+{
+    uint32_t r;
+
+    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
+    if (r <= 0) {
+        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n", __func__,
+                path, fdt_strerror(r));
+        exit(1);
+    }
+
+    return r;
+}
+
 int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
                                  const char *property, const char *string)
 {
-    uint32_t phandle = fdt_get_phandle(fdt, findnode_nofail(fdt, string));
+    uint32_t phandle = qemu_devtree_get_phandle(fdt, string);
     return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
 }
 
diff --git a/device_tree.h b/device_tree.h
index 2e87c58..376287a 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -33,6 +33,7 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
                                 const char *property, const char *string);
 int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
                                  const char *property, const char *string);
+uint32_t qemu_devtree_get_phandle(void *fdt, const char *path);
 int qemu_devtree_nop_node(void *fdt, const char *node_path);
 int qemu_devtree_add_subnode(void *fdt, const char *name);
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> This patch adds a helper to search for a node's phandle by its path. This
> is especially useful when the phandle is part of an array, not just a single
> cell in which case qemu_devtree_setprop_phandle would be the easy choice.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  device_tree.c |   16 +++++++++++++++-
>  device_tree.h |    1 +
>  2 files changed, 16 insertions(+), 1 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 6cbc5af..6745d17 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>      return r;
>  }
>  
> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
> +{
> +    uint32_t r;
> +
> +    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
> +    if (r <= 0) {
> +        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n", __func__,
> +                path, fdt_strerror(r));
> +        exit(1);

Is it really this functions job to terminate qemu on fail?  There may be
scenarios where a node does not have a phandle where the client can
handle that. Perhaps return -1 on error and the client has to check?

> +    }
> +
> +    return r;
> +}
> +
>  int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
>                                   const char *property, const char *string)
>  {
> -    uint32_t phandle = fdt_get_phandle(fdt, findnode_nofail(fdt, string));
> +    uint32_t phandle = qemu_devtree_get_phandle(fdt, string);
>      return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
>  }
>  
> diff --git a/device_tree.h b/device_tree.h
> index 2e87c58..376287a 100644
> --- a/device_tree.h
> +++ b/device_tree.h
> @@ -33,6 +33,7 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>                                  const char *property, const char *string);
>  int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
>                                   const char *property, const char *string);
> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path);
>  int qemu_devtree_nop_node(void *fdt, const char *node_path);
>  int qemu_devtree_add_subnode(void *fdt, const char *name);
>  

Re: [Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Alexander Graf]

On 06/06/2012 07:11 AM, Peter Crosthwaite wrote:
> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>> This patch adds a helper to search for a node's phandle by its path. This
>> is especially useful when the phandle is part of an array, not just a single
>> cell in which case qemu_devtree_setprop_phandle would be the easy choice.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
>> ---
>>   device_tree.c |   16 +++++++++++++++-
>>   device_tree.h |    1 +
>>   2 files changed, 16 insertions(+), 1 deletions(-)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index 6cbc5af..6745d17 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>>       return r;
>>   }
>>
>> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
>> +{
>> +    uint32_t r;
>> +
>> +    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
>> +    if (r<= 0) {
>> +        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n", __func__,
>> +                path, fdt_strerror(r));
>> +        exit(1);
> Is it really this functions job to terminate qemu on fail?  There may be
> scenarios where a node does not have a phandle where the client can
> handle that. Perhaps return -1 on error and the client has to check?

If it can, what's the point in not calling libfdt directly then?


Alex

Re: [Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Peter Crosthwaite]

On Thu, Jun 7, 2012 at 1:58 AM, Alexander Graf <agraf@suse.de> wrote:
> On 06/06/2012 07:11 AM, Peter Crosthwaite wrote:
>>
>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>>
>>> This patch adds a helper to search for a node's phandle by its path. This
>>> is especially useful when the phandle is part of an array, not just a
>>> single
>>> cell in which case qemu_devtree_setprop_phandle would be the easy choice.
>>>
>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>> ---
>>>  device_tree.c |   16 +++++++++++++++-
>>>  device_tree.h |    1 +
>>>  2 files changed, 16 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/device_tree.c b/device_tree.c
>>> index 6cbc5af..6745d17 100644
>>> --- a/device_tree.c
>>> +++ b/device_tree.c
>>> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const
>>> char *node_path,
>>>      return r;
>>>  }
>>>
>>> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
>>> +{
>>> +    uint32_t r;
>>> +
>>> +    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
>>> +    if (r<= 0) {
>>> +        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n",
>>> __func__,
>>> +                path, fdt_strerror(r));
>>> +        exit(1);
>>
>> Is it really this functions job to terminate qemu on fail?  There may be
>> scenarios where a node does not have a phandle where the client can
>> handle that. Perhaps return -1 on error and the client has to check?
>
>
> If it can, what's the point in not calling libfdt directly then?
>

Its a very good question. If the point of this function is to fail of
error though, perhaps it should have the _nofail suffix for clarity?

>
> Alex
>

Re: [Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Alexander Graf]

On 07.06.2012, at 02:28, Peter Crosthwaite wrote:

> On Thu, Jun 7, 2012 at 1:58 AM, Alexander Graf <agraf@suse.de> wrote:
>> On 06/06/2012 07:11 AM, Peter Crosthwaite wrote:
>>> 
>>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>>> 
>>>> This patch adds a helper to search for a node's phandle by its path. This
>>>> is especially useful when the phandle is part of an array, not just a
>>>> single
>>>> cell in which case qemu_devtree_setprop_phandle would be the easy choice.
>>>> 
>>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>>> ---
>>>>  device_tree.c |   16 +++++++++++++++-
>>>>  device_tree.h |    1 +
>>>>  2 files changed, 16 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/device_tree.c b/device_tree.c
>>>> index 6cbc5af..6745d17 100644
>>>> --- a/device_tree.c
>>>> +++ b/device_tree.c
>>>> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const
>>>> char *node_path,
>>>>      return r;
>>>>  }
>>>> 
>>>> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
>>>> +{
>>>> +    uint32_t r;
>>>> +
>>>> +    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
>>>> +    if (r<= 0) {
>>>> +        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n",
>>>> __func__,
>>>> +                path, fdt_strerror(r));
>>>> +        exit(1);
>>> 
>>> Is it really this functions job to terminate qemu on fail?  There may be
>>> scenarios where a node does not have a phandle where the client can
>>> handle that. Perhaps return -1 on error and the client has to check?
>> 
>> 
>> If it can, what's the point in not calling libfdt directly then?
>> 
> 
> Its a very good question. If the point of this function is to fail of
> error though, perhaps it should have the _nofail suffix for clarity?

If we do a global s/qemu_devtree_/qdt/g throughout the code base, I'd be open to add _nofail to all function names at the end :). Otherwise we'll get into even more trouble of staying within 80 characters per line...


Alex

Re: [Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Peter Crosthwaite]

On Fri, Jun 8, 2012 at 10:46 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 07.06.2012, at 02:28, Peter Crosthwaite wrote:
>
>> On Thu, Jun 7, 2012 at 1:58 AM, Alexander Graf <agraf@suse.de> wrote:
>>> On 06/06/2012 07:11 AM, Peter Crosthwaite wrote:
>>>>
>>>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>>>>
>>>>> This patch adds a helper to search for a node's phandle by its path. This
>>>>> is especially useful when the phandle is part of an array, not just a
>>>>> single
>>>>> cell in which case qemu_devtree_setprop_phandle would be the easy choice.
>>>>>
>>>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>>>> ---
>>>>>  device_tree.c |   16 +++++++++++++++-
>>>>>  device_tree.h |    1 +
>>>>>  2 files changed, 16 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/device_tree.c b/device_tree.c
>>>>> index 6cbc5af..6745d17 100644
>>>>> --- a/device_tree.c
>>>>> +++ b/device_tree.c
>>>>> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const
>>>>> char *node_path,
>>>>>      return r;
>>>>>  }
>>>>>
>>>>> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
>>>>> +{
>>>>> +    uint32_t r;
>>>>> +
>>>>> +    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
>>>>> +    if (r<= 0) {
>>>>> +        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n",
>>>>> __func__,
>>>>> +                path, fdt_strerror(r));
>>>>> +        exit(1);
>>>>
>>>> Is it really this functions job to terminate qemu on fail?  There may be
>>>> scenarios where a node does not have a phandle where the client can
>>>> handle that. Perhaps return -1 on error and the client has to check?
>>>
>>>
>>> If it can, what's the point in not calling libfdt directly then?
>>>
>>
>> Its a very good question. If the point of this function is to fail of
>> error though, perhaps it should have the _nofail suffix for clarity?
>
> If we do a global s/qemu_devtree_/qdt/g throughout the code base, I'd be open to add _nofail to all function names at the end :). Otherwise we'll get into even more trouble of staying within 80 characters per line...
>

Since the majority of those functions are wrappers around "fdt_" API
calls, perhaps it should be:

s/qemu_devtree_/qemu_fdt_/g

buys you 4 chars, which should minimise the incidence of 80 char
violations when adding _nofail suffixes. Do we have a large number of
lines already between 78-80 chars?

Regards,
Peter

>
> Alex
>

Re: [Qemu-devel] [PATCH 05/31] dt: add helper for phandle enumeration

[Alexander Graf]

On 09.06.2012, at 03:02, Peter Crosthwaite wrote:

> On Fri, Jun 8, 2012 at 10:46 PM, Alexander Graf <agraf@suse.de> wrote:
>> 
>> On 07.06.2012, at 02:28, Peter Crosthwaite wrote:
>> 
>>> On Thu, Jun 7, 2012 at 1:58 AM, Alexander Graf <agraf@suse.de> wrote:
>>>> On 06/06/2012 07:11 AM, Peter Crosthwaite wrote:
>>>>> 
>>>>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>>>>> 
>>>>>> This patch adds a helper to search for a node's phandle by its path. This
>>>>>> is especially useful when the phandle is part of an array, not just a
>>>>>> single
>>>>>> cell in which case qemu_devtree_setprop_phandle would be the easy choice.
>>>>>> 
>>>>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>>>>> ---
>>>>>>  device_tree.c |   16 +++++++++++++++-
>>>>>>  device_tree.h |    1 +
>>>>>>  2 files changed, 16 insertions(+), 1 deletions(-)
>>>>>> 
>>>>>> diff --git a/device_tree.c b/device_tree.c
>>>>>> index 6cbc5af..6745d17 100644
>>>>>> --- a/device_tree.c
>>>>>> +++ b/device_tree.c
>>>>>> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const
>>>>>> char *node_path,
>>>>>>      return r;
>>>>>>  }
>>>>>> 
>>>>>> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
>>>>>> +{
>>>>>> +    uint32_t r;
>>>>>> +
>>>>>> +    r = fdt_get_phandle(fdt, findnode_nofail(fdt, path));
>>>>>> +    if (r<= 0) {
>>>>>> +        fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n",
>>>>>> __func__,
>>>>>> +                path, fdt_strerror(r));
>>>>>> +        exit(1);
>>>>> 
>>>>> Is it really this functions job to terminate qemu on fail?  There may be
>>>>> scenarios where a node does not have a phandle where the client can
>>>>> handle that. Perhaps return -1 on error and the client has to check?
>>>> 
>>>> 
>>>> If it can, what's the point in not calling libfdt directly then?
>>>> 
>>> 
>>> Its a very good question. If the point of this function is to fail of
>>> error though, perhaps it should have the _nofail suffix for clarity?
>> 
>> If we do a global s/qemu_devtree_/qdt/g throughout the code base, I'd be open to add _nofail to all function names at the end :). Otherwise we'll get into even more trouble of staying within 80 characters per line...
>> 
> 
> Since the majority of those functions are wrappers around "fdt_" API
> calls, perhaps it should be:
> 
> s/qemu_devtree_/qemu_fdt_/g
> 
> buys you 4 chars, which should minimise the incidence of 80 char
> violations when adding _nofail suffixes. Do we have a large number of
> lines already between 78-80 chars?

Hrm. Let's keep this in mind for a later cleanup series. It certainly is out of scope of this patch set :).


Alex

[Qemu-devel] [PATCH 06/31] dt: add helper for empty dt creation

[Alexander Graf]

We want to get rid of the concept of loading an external device tree and instead
generate our own. However, to do this we need to also create a device tree
template programatically.

This patch adds a helper to create an empty device tree in memory.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |   37 +++++++++++++++++++++++++++++++++++++
 device_tree.h |    1 +
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 6745d17..d4f1f0a 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -25,6 +25,43 @@
 
 #include <libfdt.h>
 
+#define FDT_MAX_SIZE  0x10000
+
+void *create_device_tree(int *sizep)
+{
+    void *fdt;
+    int ret;
+
+    *sizep = FDT_MAX_SIZE;
+    fdt = g_malloc0(FDT_MAX_SIZE);
+    ret = fdt_create(fdt, FDT_MAX_SIZE);
+    if (ret < 0) {
+        goto fail;
+    }
+    ret = fdt_begin_node(fdt, "");
+    if (ret < 0) {
+        goto fail;
+    }
+    ret = fdt_end_node(fdt);
+    if (ret < 0) {
+        goto fail;
+    }
+    ret = fdt_finish(fdt);
+    if (ret < 0) {
+        goto fail;
+    }
+    ret = fdt_open_into(fdt, fdt, *sizep);
+    if (ret) {
+        fprintf(stderr, "Unable to copy device tree in memory\n");
+        exit(1);
+    }
+
+    return fdt;
+fail:
+    fprintf(stderr, "%s Couldn't create dt: %s\n", __func__, fdt_strerror(ret));
+    exit(1);
+}
+
 void *load_device_tree(const char *filename_path, int *sizep)
 {
     int dt_size;
diff --git a/device_tree.h b/device_tree.h
index 376287a..5464dc7 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -14,6 +14,7 @@
 #ifndef __DEVICE_TREE_H__
 #define __DEVICE_TREE_H__
 
+void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 
 int qemu_devtree_setprop(void *fdt, const char *node_path,
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 06/31] dt: add helper for empty dt creation

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> We want to get rid of the concept of loading an external device tree and instead
> generate our own. However, to do this we need to also create a device tree
> template programatically.
> 
> This patch adds a helper to create an empty device tree in memory.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@petalogix.com>

> ---
>  device_tree.c |   37 +++++++++++++++++++++++++++++++++++++
>  device_tree.h |    1 +
>  2 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 6745d17..d4f1f0a 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -25,6 +25,43 @@
>  
>  #include <libfdt.h>
>  
> +#define FDT_MAX_SIZE  0x10000
> +
> +void *create_device_tree(int *sizep)
> +{
> +    void *fdt;
> +    int ret;
> +
> +    *sizep = FDT_MAX_SIZE;
> +    fdt = g_malloc0(FDT_MAX_SIZE);
> +    ret = fdt_create(fdt, FDT_MAX_SIZE);
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    ret = fdt_begin_node(fdt, "");
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    ret = fdt_end_node(fdt);
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    ret = fdt_finish(fdt);
> +    if (ret < 0) {
> +        goto fail;
> +    }
> +    ret = fdt_open_into(fdt, fdt, *sizep);
> +    if (ret) {
> +        fprintf(stderr, "Unable to copy device tree in memory\n");
> +        exit(1);
> +    }
> +
> +    return fdt;
> +fail:
> +    fprintf(stderr, "%s Couldn't create dt: %s\n", __func__, fdt_strerror(ret));
> +    exit(1);
> +}
> +
>  void *load_device_tree(const char *filename_path, int *sizep)
>  {
>      int dt_size;
> diff --git a/device_tree.h b/device_tree.h
> index 376287a..5464dc7 100644
> --- a/device_tree.h
> +++ b/device_tree.h
> @@ -14,6 +14,7 @@
>  #ifndef __DEVICE_TREE_H__
>  #define __DEVICE_TREE_H__
>  
> +void *create_device_tree(int *sizep);
>  void *load_device_tree(const char *filename_path, int *sizep);
>  
>  int qemu_devtree_setprop(void *fdt, const char *node_path,

[Qemu-devel] [PATCH 07/31] dt: add helper for phandle allocation

[Alexander Graf]

Phandle references work by having 2 pieces:

  - a "phandle" 1-cell property in the device tree node
  - a reference to the same value in a property we want to point
    to the other node

To generate the 1-cell property, we need an allocation mechanism that
gives us a unique number space. This patch adds an allocator for these
properties.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    7 +++++++
 device_tree.h |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index d4f1f0a..317bdd0 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -220,6 +220,13 @@ int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
     return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
 }
 
+uint32_t qemu_devtree_alloc_phandle(void *fdt)
+{
+    static int phandle = 0x8000;
+
+    return phandle++;
+}
+
 int qemu_devtree_nop_node(void *fdt, const char *node_path)
 {
     int r;
diff --git a/device_tree.h b/device_tree.h
index 5464dc7..f37a4da 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -35,6 +35,7 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
 int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
                                  const char *property, const char *string);
 uint32_t qemu_devtree_get_phandle(void *fdt, const char *path);
+uint32_t qemu_devtree_alloc_phandle(void *fdt);
 int qemu_devtree_nop_node(void *fdt, const char *node_path);
 int qemu_devtree_add_subnode(void *fdt, const char *name);
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 07/31] dt: add helper for phandle allocation

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> Phandle references work by having 2 pieces:
> 
>   - a "phandle" 1-cell property in the device tree node
>   - a reference to the same value in a property we want to point
>     to the other node
> 
> To generate the 1-cell property, we need an allocation mechanism that
> gives us a unique number space. This patch adds an allocator for these
> properties.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  device_tree.c |    7 +++++++
>  device_tree.h |    1 +
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index d4f1f0a..317bdd0 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -220,6 +220,13 @@ int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
>      return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
>  }
>  
> +uint32_t qemu_devtree_alloc_phandle(void *fdt)
> +{
> +    static int phandle = 0x8000;

can easily double check for duplicates. Would also allow you to start
from 1 rather than magic number 0x8000?

while (fdt_node_offset_by_phandle(fdt, phandle) != FDT_ERR_NOTFOUND)
    phandle++;

> +
> +    return phandle++;
> +}
> +
>  int qemu_devtree_nop_node(void *fdt, const char *node_path)
>  {
>      int r;
> diff --git a/device_tree.h b/device_tree.h
> index 5464dc7..f37a4da 100644
> --- a/device_tree.h
> +++ b/device_tree.h
> @@ -35,6 +35,7 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
>  int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
>                                   const char *property, const char *string);
>  uint32_t qemu_devtree_get_phandle(void *fdt, const char *path);
> +uint32_t qemu_devtree_alloc_phandle(void *fdt);
>  int qemu_devtree_nop_node(void *fdt, const char *node_path);
>  int qemu_devtree_add_subnode(void *fdt, const char *name);
>  

Re: [Qemu-devel] [PATCH 07/31] dt: add helper for phandle allocation

[Alexander Graf]

On 06/06/2012 07:18 AM, Peter Crosthwaite wrote:
> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>> Phandle references work by having 2 pieces:
>>
>>    - a "phandle" 1-cell property in the device tree node
>>    - a reference to the same value in a property we want to point
>>      to the other node
>>
>> To generate the 1-cell property, we need an allocation mechanism that
>> gives us a unique number space. This patch adds an allocator for these
>> properties.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
>> ---
>>   device_tree.c |    7 +++++++
>>   device_tree.h |    1 +
>>   2 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/device_tree.c b/device_tree.c
>> index d4f1f0a..317bdd0 100644
>> --- a/device_tree.c
>> +++ b/device_tree.c
>> @@ -220,6 +220,13 @@ int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
>>       return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
>>   }
>>
>> +uint32_t qemu_devtree_alloc_phandle(void *fdt)
>> +{
>> +    static int phandle = 0x8000;
> can easily double check for duplicates. Would also allow you to start
> from 1 rather than magic number 0x8000?
>
> while (fdt_node_offset_by_phandle(fdt, phandle) != FDT_ERR_NOTFOUND)
>      phandle++;

That's a nice one. It'd even allow us to drop the external phandle 
starting id patch.

Scott, would this work for you?


Alex

Re: [Qemu-devel] [PATCH 07/31] dt: add helper for phandle allocation

[Scott Wood]

On 06/06/2012 11:00 AM, Alexander Graf wrote:
> On 06/06/2012 07:18 AM, Peter Crosthwaite wrote:
>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>> Phandle references work by having 2 pieces:
>>>
>>>    - a "phandle" 1-cell property in the device tree node
>>>    - a reference to the same value in a property we want to point
>>>      to the other node
>>>
>>> To generate the 1-cell property, we need an allocation mechanism that
>>> gives us a unique number space. This patch adds an allocator for these
>>> properties.
>>>
>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>> ---
>>>   device_tree.c |    7 +++++++
>>>   device_tree.h |    1 +
>>>   2 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/device_tree.c b/device_tree.c
>>> index d4f1f0a..317bdd0 100644
>>> --- a/device_tree.c
>>> +++ b/device_tree.c
>>> @@ -220,6 +220,13 @@ int qemu_devtree_setprop_phandle(void *fdt,
>>> const char *node_path,
>>>       return qemu_devtree_setprop_cell(fdt, node_path, property,
>>> phandle);
>>>   }
>>>
>>> +uint32_t qemu_devtree_alloc_phandle(void *fdt)
>>> +{
>>> +    static int phandle = 0x8000;
>> can easily double check for duplicates. Would also allow you to start
>> from 1 rather than magic number 0x8000?

You can't check for duplicates, because the tree fragments you'll be
conflicting with haven't been added to the tree yet.  That's done by a
tool operating on the tree output by the first pass of qemu, and is fed
back into the second pass of qemu.

-Scott

Re: [Qemu-devel] [PATCH 07/31] dt: add helper for phandle allocation

[Peter Crosthwaite]

On Thu, Jun 7, 2012 at 2:55 AM, Scott Wood <scottwood@freescale.com> wrote:
> On 06/06/2012 11:00 AM, Alexander Graf wrote:
>> On 06/06/2012 07:18 AM, Peter Crosthwaite wrote:
>>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>>> Phandle references work by having 2 pieces:
>>>>
>>>>    - a "phandle" 1-cell property in the device tree node
>>>>    - a reference to the same value in a property we want to point
>>>>      to the other node
>>>>
>>>> To generate the 1-cell property, we need an allocation mechanism that
>>>> gives us a unique number space. This patch adds an allocator for these
>>>> properties.
>>>>
>>>> Signed-off-by: Alexander Graf<agraf@suse.de>
>>>> ---
>>>>   device_tree.c |    7 +++++++
>>>>   device_tree.h |    1 +
>>>>   2 files changed, 8 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/device_tree.c b/device_tree.c
>>>> index d4f1f0a..317bdd0 100644
>>>> --- a/device_tree.c
>>>> +++ b/device_tree.c
>>>> @@ -220,6 +220,13 @@ int qemu_devtree_setprop_phandle(void *fdt,
>>>> const char *node_path,
>>>>       return qemu_devtree_setprop_cell(fdt, node_path, property,
>>>> phandle);
>>>>   }
>>>>
>>>> +uint32_t qemu_devtree_alloc_phandle(void *fdt)
>>>> +{
>>>> +    static int phandle = 0x8000;
>>> can easily double check for duplicates. Would also allow you to start
>>> from 1 rather than magic number 0x8000?
>
> You can't check for duplicates, because the tree fragments you'll be
> conflicting with haven't been added to the tree yet.  That's done by a
> tool operating on the tree output by the first pass of qemu, and is fed
> back into the second pass of qemu.
>

Im not sure what you mean by "fragments [which] havent been added to
the tree yet"? Im thinking here of the use model where you use the DTB
API to modify and existing DTB (probably pc-bios/foo.dtb). Yes, for
Alex's series im not sure it wins anything as you are starting a DTB
from scratch (which will have no pre-existing phandles), but it guards
against a potentially very obscure and hard to find bug in some other
potential uses of this API.

Regards,
Peter

> -Scott
>

Re: [Qemu-devel] [PATCH 07/31] dt: add helper for phandle allocation

[Scott Wood]

On 06/06/2012 07:15 PM, Peter Crosthwaite wrote:
> On Thu, Jun 7, 2012 at 2:55 AM, Scott Wood <scottwood@freescale.com> wrote:
>> On 06/06/2012 11:00 AM, Alexander Graf wrote:
>>> On 06/06/2012 07:18 AM, Peter Crosthwaite wrote:
>>>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
>>>>> +uint32_t qemu_devtree_alloc_phandle(void *fdt)
>>>>> +{
>>>>> +    static int phandle = 0x8000;
>>>> can easily double check for duplicates. Would also allow you to start
>>>> from 1 rather than magic number 0x8000?
>>
>> You can't check for duplicates, because the tree fragments you'll be
>> conflicting with haven't been added to the tree yet.  That's done by a
>> tool operating on the tree output by the first pass of qemu, and is fed
>> back into the second pass of qemu.
>>
> 
> Im not sure what you mean by "fragments [which] havent been added to
> the tree yet"?

The use case I have in mind is running QEMU once in dumpdtb mode,
merging in fragments from the host device tree for directly assigned
devices, and passing the result back to a second invocation of QEMU.

For the first pass of QEMU, the managing tool would look at the host
device tree to determine a suitable phandle range to pass to QEMU.

> Im thinking here of the use model where you use the DTB
> API to modify and existing DTB (probably pc-bios/foo.dtb). Yes, for
> Alex's series im not sure it wins anything as you are starting a DTB
> from scratch (which will have no pre-existing phandles), but it guards
> against a potentially very obscure and hard to find bug in some other
> potential uses of this API.

I'm not sure the API misuse it would protect against is particularly
likely -- you'd have to have some phandle creators ignore the API, and
others use the API, and the misbehaving phandle generator has to do its
thing before the conflicting API-compliant phandle generation.  Doing a
phandle search is a fairly heavy operation, which may not be the biggest
concern here, but still it shouldn't be called trivially.

It would be different if QEMU were adding nodes to an existing tree,
rather than creating a tree from scratch.

In any case, it doesn't eliminate the need for passing in a starting point.

-Scott

[Qemu-devel] [PATCH 08/31] dt: add helper for 64bit cell adds

[Alexander Graf]

Some times in the device tree, we find an array of 2 u32 cells that
really are a single u64 value. This patch adds a helper to make the
creation of these easy.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - rename cell64 -> u64
---
 device_tree.c |    7 +++++++
 device_tree.h |    2 ++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 317bdd0..644d766 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -154,6 +154,13 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
     return r;
 }
 
+int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
+                             const char *property, uint64_t val)
+{
+    val = cpu_to_be64(val);
+    return qemu_devtree_setprop(fdt, node_path, property, &val, sizeof(val));
+}
+
 int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
                                const char *property, uint32_t val,
                                uint32_t val2)
diff --git a/device_tree.h b/device_tree.h
index f37a4da..39dd259 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -21,6 +21,8 @@ int qemu_devtree_setprop(void *fdt, const char *node_path,
                          const char *property, void *val_array, int size);
 int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
                               const char *property, uint32_t val);
+int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
+                             const char *property, uint64_t val);
 int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
                                const char *property, uint32_t val,
                                uint32_t val2);
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 08/31] dt: add helper for 64bit cell adds

[Peter Crosthwaite]

On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote:
> Some times in the device tree, we find an array of 2 u32 cells that
> really are a single u64 value. This patch adds a helper to make the
> creation of these easy.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@petalogix.com>

> ---
> 
> v1 -> v2:
> 
>   - rename cell64 -> u64
> ---
>  device_tree.c |    7 +++++++
>  device_tree.h |    2 ++
>  2 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/device_tree.c b/device_tree.c
> index 317bdd0..644d766 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -154,6 +154,13 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
>      return r;
>  }
>  
> +int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
> +                             const char *property, uint64_t val)
> +{
> +    val = cpu_to_be64(val);
> +    return qemu_devtree_setprop(fdt, node_path, property, &val, sizeof(val));
> +}
> +
>  int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
>                                 const char *property, uint32_t val,
>                                 uint32_t val2)
> diff --git a/device_tree.h b/device_tree.h
> index f37a4da..39dd259 100644
> --- a/device_tree.h
> +++ b/device_tree.h
> @@ -21,6 +21,8 @@ int qemu_devtree_setprop(void *fdt, const char *node_path,
>                           const char *property, void *val_array, int size);
>  int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
>                                const char *property, uint32_t val);
> +int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
> +                             const char *property, uint64_t val);
>  int qemu_devtree_setprop_cell2(void *fdt, const char *node_path,
>                                 const char *property, uint32_t val,
>                                 uint32_t val2);

[Qemu-devel] [PATCH 09/31] PPC: e500: require libfdt

[Alexander Graf]

Now that we're moving all of the device tree generation from an external
pre-execution generated blob to runtime generation using libfdt, we absolutely
must have libfdt around.

This requirement was there before already, as the only way to not require libfdt
with e500 was to not use -kernel, which was the only way to boot the mpc8544ds
machine. This patch only manifests said requirement in the build system.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 Makefile.target        |    2 +-
 hw/ppce500_mpc8544ds.c |    5 -----
 2 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 1582904..19a2990 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -248,7 +248,7 @@ obj-ppc-$(CONFIG_PSERIES) += spapr_pci.o device-hotplug.o pci-hotplug.o
 obj-ppc-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
 obj-ppc-y += ppc440_bamboo.o
 # PowerPC E500 boards
-obj-ppc-y += ppce500_mpc8544ds.o mpc8544_guts.o ppce500_spin.o
+obj-ppc-$(CONFIG_FDT) += ppce500_mpc8544ds.o mpc8544_guts.o ppce500_spin.o
 # PowerPC 440 Xilinx ML507 reference board.
 obj-ppc-y += virtex_ml507.o
 obj-ppc-$(CONFIG_KVM) += kvm_ppc.o
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 42a63aa..f162cd3 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -68,7 +68,6 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                     const char *kernel_cmdline)
 {
     int ret = -1;
-#ifdef CONFIG_FDT
     uint32_t mem_reg_property[] = {0, cpu_to_be32(ramsize)};
     char *filename;
     int fdt_size;
@@ -173,7 +172,6 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     ret = fdt_size;
 
 out:
-#endif
 
     return ret;
 }
@@ -386,9 +384,6 @@ static void mpc8544ds_init(ram_addr_t ram_size,
         struct boot_info *boot_info;
         int dt_size;
 
-#ifndef CONFIG_FDT
-        cpu_abort(env, "Compiled without FDT support - can't load kernel\n");
-#endif
         dt_base = (loadaddr + kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK;
         dt_size = mpc8544_load_device_tree(env, dt_base, ram_size, initrd_base,
                                            initrd_size, kernel_cmdline);
-- 
1.6.0.2

[Qemu-devel] [PATCH 10/31] PPC: e500: dt: create memory node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    8 ++++----
 pc-bios/mpc8544ds.dtb  |  Bin 2028 -> 1972 bytes
 pc-bios/mpc8544ds.dts  |    5 -----
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index f162cd3..650c910 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -88,10 +88,10 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     }
 
     /* Manipulate device tree in memory. */
-    ret = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
-                               sizeof(mem_reg_property));
-    if (ret < 0)
-        fprintf(stderr, "couldn't set /memory/reg\n");
+    qemu_devtree_add_subnode(fdt, "/memory");
+    qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
+    qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
+                         sizeof(mem_reg_property));
 
     if (initrd_size) {
         ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index c6d302153c7407d5d0127be29b0c35f80e47f8fb..db9fb701f246e058bca4c2fe9546c9f2493a57b1 100644
GIT binary patch
delta 104
zcmaFEzlC4m0`I@K3=HgB7#J8V7#P@QOcW4jOxUQQ!8o~tF`dzO@&`t7#*oPzOxY|U
z3=FQ5xtSd&?_lx=2{E?=$qCHM8ACQ(uxw`psb#GO3gxhE;!Mm-Pc3FBN==`v&VCdC
D8)_c+

delta 159
zcmdnO|At@S0`I@K3=HgV7#J8V7#P?tOcW4joUu_ugHb0pH8;Pg5-85VzzoFfK<tPn
z#sL)b1!9KDE{w&Jfgsic5Fr2}z`DRHCSPF8X7rtG!!#L4USMhmk`c_y8GSdgY-eN)
coV<gz8Yp7Iwuv(}ouMc(FFmz*@_V+U0H2g3&j0`b

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 7eb3160..f46e9ed 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -27,11 +27,6 @@
 		#size-cells = <0>;
 	};
 
-	memory {
-		device_type = "memory";
-		reg = <0x0 0x0>;	// Filled by U-Boot
-	};
-
 	soc8544@e0000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
1.6.0.2

[Qemu-devel] [PATCH 11/31] PPC: e500: dt: create /cpus node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    5 +++++
 pc-bios/mpc8544ds.dtb  |  Bin 1972 -> 1924 bytes
 pc-bios/mpc8544ds.dts  |    5 -----
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 650c910..106251e 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -125,6 +125,11 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                              hypercall, sizeof(hypercall));
     }
 
+    /* Create CPU nodes */
+    qemu_devtree_add_subnode(fdt, "/cpus");
+    qemu_devtree_setprop_cell(fdt, "/cpus", "#address-cells", 1);
+    qemu_devtree_setprop_cell(fdt, "/cpus", "#size-cells", 0);
+
     /* We need to generate the cpu nodes in reverse order, so Linux can pick
        the first node as boot node and be happy */
     for (i = smp_cpus - 1; i >= 0; i--) {
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index db9fb701f246e058bca4c2fe9546c9f2493a57b1..a85b93c1e6e66c318c3f0c1910abae78f4b78f5e 100644
GIT binary patch
delta 34
qcmdnO-@-3&f%o5A1_t&P1_lNT1_ri_i2~w`1`{=YYz|;dVFLiG$q8rx

delta 43
zcmZqS-@-3&f%o5A1_t&m3=9kw3=C{DCJKl%CQQ`$!IE51T0Gf+QF*gCV=fy28+Z(E

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index f46e9ed..1fcb865 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -22,11 +22,6 @@
 		pci0 = &pci0;
 	};
 
-	cpus {
-		#address-cells = <1>;
-		#size-cells = <0>;
-	};
-
 	soc8544@e0000000 {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
1.6.0.2

[Qemu-devel] [PATCH 12/31] PPC: e500: dt: create /hypervisor node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    1 +
 pc-bios/mpc8544ds.dtb  |  Bin 1924 -> 1904 bytes
 pc-bios/mpc8544ds.dts  |    3 ---
 3 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 106251e..6ad2897 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -118,6 +118,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
         tb_freq = kvmppc_get_tbfreq();
 
         /* indicate KVM hypercall interface */
+        qemu_devtree_add_subnode(fdt, "/hypervisor");
         qemu_devtree_setprop_string(fdt, "/hypervisor", "compatible",
                                     "linux,kvm");
         kvmppc_get_hypercall(env, hypercall, sizeof(hypercall));
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index a85b93c1e6e66c318c3f0c1910abae78f4b78f5e..8194aa2e6f292fb34023feb596aa19448b8af0d0 100644
GIT binary patch
delta 35
rcmZqS|G+13f%o5A1_t&51_lNT1_ri}i2~w`G8;A2*)}J$DKY{8vYZHv

delta 47
zcmeys*TOGwf%o5A1_t&P1_lNT1_ri_i2~w`1{*ch*|-@qDhpDJ$})@di#EHlX)yu-
DKg10i

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 1fcb865..2ca7c54 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -103,7 +103,4 @@
 	chosen {
 		linux,stdout-path = "/soc8544@e0000000/serial@4500";
 	};
-
-	hypervisor {
-	};
 };
-- 
1.6.0.2

[Qemu-devel] [PATCH 13/31] PPC: e500: dt: create / node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    8 ++++++++
 pc-bios/mpc8544ds.dtb  |  Bin 1904 -> 1810 bytes
 pc-bios/mpc8544ds.dts  |    5 -----
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 6ad2897..39b221d 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -76,6 +76,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     uint32_t clock_freq = 400000000;
     uint32_t tb_freq = 400000000;
     int i;
+    char compatible[] = "MPC8544DS\0MPC85xxDS";
+    char model[] = "MPC8544DS";
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
     if (!filename) {
@@ -88,6 +90,12 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     }
 
     /* Manipulate device tree in memory. */
+    qemu_devtree_setprop_string(fdt, "/", "model", model);
+    qemu_devtree_setprop(fdt, "/", "compatible", compatible,
+                         sizeof(compatible));
+    qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 1);
+    qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 1);
+
     qemu_devtree_add_subnode(fdt, "/memory");
     qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
     qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index 8194aa2e6f292fb34023feb596aa19448b8af0d0..25d92f681dec184530af63e2d2cea61cb4cccd04 100644
GIT binary patch
literal 1810
zcmb7EyKdA#6rCkONyI||2}FgEk^-Svaira7i!HW+bSd}&;&>;!qn)*l$3_V>REUy}
ze?a^K5)yPYv~)`kJs<>}GvjgWMJaNn>$&fHJacD0U-|Q0h?VO?h`taPe`CE1z6M?g
zgE}{1|LEk_w^M1INUO+5Lv;y!o5Hq9<9@H(9m>$rwvoAt^sw6tLk672uAUvc+l;-6
zob~N2R<>pzWo;R80ZV7GopV_{%aCs{226a^Hy88}`7l}kC9DIZW|@}3VQGKM+AqVt
z$DlbsZg*I36}&&kr!x8;53PyV+JEl-2bG`t3OICe*6QmH60@`0>)Ai`wtXS)Bgk&Q
zuQjz<4nOfc1K$I4Z+y%P$V_tkRbi@j*vA}HG1SkA=|PoR_d7SHOvS@4rv;Tj#6Wrt
z_V{>?B(J}P?Elf8gFRiIu#4f$4B|<iZKat^0(7u0o4*DzY@6%4@8E0B%ZEz0dpFkU
zVWfkyP+ApA8aLy%Fmdtj433Q6nq~G11mm)BQ{*r?oJ8h^h&iNBVAi)i{vY!i+W}%o
z%;OX`dW8Y<ypOvOHgv{*5#k5^o_NNrGY5YY;w6@v`2Y31Ech%&_?e5CR$gVZC$Z=w
zCSMopE2uSjdzIhv`F>S?%jf%5F)iOhzj_b-U0^QHgZJeYm^HlZ7i7|3Fl`}tj{&6j
z6;r+gCU{R@z2J<@C64LR&*2-aUgzvG!t0xmeSgMt*6AbLnVE~{5ZA$OM&e0oWJ1-(
z;N9&kpZ%8B?=E|g*W7y(3b*bE%t|OWqR}Xq#ssm{+K3IKp2|ud$tNn7kBXB_ia4ER
zQK1gC6nT`4@%ra-Ebv?gN4b1l$|OD!tPrSVB#%X`(|Fo&sics3US_x$wHRbkE|a2R
zh|{FVQ>|q#HcrDFFs+jEcq^Mk$p{#D-6oL<a#O^4qrxo8f>~syZIi`mVGCEr6r;2(
I0HBzNe~HoS>i_@%

literal 1904
zcmb7FyKWOf6de=fknm8DC@2t$lm;QM;z)Lsg2EP$E)o(c=+I_&Vy`mZ-R!K9Lq$3Y
z3VwhepyUgXsOaDeFdsle1r2b{JT|+IQshd<bKm#Q%-!+V>wkX=v78Gbc7$l}VZ8_3
z1l9mh>kjn58EQGr64q7nyH6kP^n1NW&#Zy^TR{6%Z@AgadeD9uU@hkI^172-p-Mt6
zHAG{(i?y?<vxr3ovTJEm%OVNYpx5tq>3QngJKuM^?t23<4>t2CF*$dT)}#@au(TG-
zSyR>PdA<9+3=z)|%2$E5PA0jM!T!{2%jmB`zYB;tf9$E#{|c}-$B$F`oX?Gly)}34
z?FY_Ic^Md5Kcn?|+o|5#?)S}3y$*N(7*6I|eXb)DW&3A8C%}DxXXAa|f@hFDFV~Yh
zVeiA9{2gDgrzbA7s(0a@@+2DcI4#HoVo#*^fbWm@Nb>SW@P3cx9?Yb2TCERfrX7uo
zyp!cJ4N$?DO#BMiVN+e@{S2Ew2WI=?WOH^SmF<Trn3jo3%0y~A<M6OG*<<@atIi}h
zpFl90mUfD)-S)}O>=1D;q;CN8n;K`wyy~q!H=M#+t!{ugs4CxKO{VX6A%4^DiFeHU
zbKsW{Z?IIv{a5p{U^6|!&RF;~;+)Rh#G;L7>GNWJ2eBfrckvCIujk?$Heb*AG;9m?
zs&nz*0%Lg}C%|ohIh>B|82X1_>bk%#9Y~)X$gcyHH~1fbA**C8`#4(qGx&hj%bZ<$
zczGSXuTOu@5p9H-nYnlkb`7vI5{)x0Q;Nn1?`~`L{I|4vcUgm2nPjn4BFZLtSSI^Q
zij6Ri3#oMwNu{*d(8(+5c>6YWg<ZD<=Bg`V`7(**i}E;^q6pIw{sRg*5_mL&gF+rn
zWf~m|<_eNDmNOBlEIO>2lC+dXG0pLLRBVukg-pwuBFfUT$dr<W*f@2wEvh7)&N}%x
pOk?!Vk8T|5pwMMh)G|!MJeY*~u&FYAnzn^aqsus(mS~_Hi~kmQ^d$fQ

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 2ca7c54..1eac8ef 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -11,11 +11,6 @@
 
 /dts-v1/;
 / {
-	model = "MPC8544DS";
-	compatible = "MPC8544DS", "MPC85xxDS";
-	#address-cells = <1>;
-	#size-cells = <1>;
-
 	aliases {
 		serial0 = &serial0;
 		serial1 = &serial1;
-- 
1.6.0.2

[Qemu-devel] [PATCH 14/31] PPC: e500: dt: create /chosen node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 39b221d..63515bb 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -101,6 +101,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
                          sizeof(mem_reg_property));
 
+    qemu_devtree_add_subnode(fdt, "/chosen");
     if (initrd_size) {
         ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
                                         initrd_base);
-- 
1.6.0.2

[Qemu-devel] [PATCH 15/31] PPC: e500: dt: create /soc8544 node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   17 +++++++++++++++++
 pc-bios/mpc8544ds.dts  |    9 ---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 63515bb..5ff2d24 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -43,6 +43,8 @@
 #define RAM_SIZES_ALIGN            (64UL << 20)
 
 #define MPC8544_CCSRBAR_BASE       0xE0000000
+#define MPC8544_CCSRBAR_REGSIZE    0x00001000
+#define MPC8544_CCSRBAR_SIZE       0x00100000
 #define MPC8544_MPIC_REGS_BASE     (MPC8544_CCSRBAR_BASE + 0x40000)
 #define MPC8544_SERIAL0_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4500)
 #define MPC8544_SERIAL1_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4600)
@@ -78,6 +80,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     int i;
     char compatible[] = "MPC8544DS\0MPC85xxDS";
     char model[] = "MPC8544DS";
+    char soc[128];
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
     if (!filename) {
@@ -179,6 +182,20 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
         }
     }
 
+    /* XXX These should go into their respective devices' code */
+    sprintf(soc, "/soc8544@%x", MPC8544_CCSRBAR_BASE);
+    qemu_devtree_add_subnode(fdt, soc);
+    qemu_devtree_setprop_string(fdt, soc, "device_type", "soc");
+    qemu_devtree_setprop_string(fdt, soc, "compatible", "simple-bus");
+    qemu_devtree_setprop_cell(fdt, soc, "#address-cells", 1);
+    qemu_devtree_setprop_cell(fdt, soc, "#size-cells", 1);
+    qemu_devtree_setprop_cell3(fdt, soc, "ranges", 0x0, MPC8544_CCSRBAR_BASE,
+                               MPC8544_CCSRBAR_SIZE);
+    qemu_devtree_setprop_cell2(fdt, soc, "reg", MPC8544_CCSRBAR_BASE,
+                               MPC8544_CCSRBAR_REGSIZE);
+    /* XXX should contain a reasonable value */
+    qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
+
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     if (ret < 0) {
         goto out;
diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 1eac8ef..01b53ba 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -18,15 +18,6 @@
 	};
 
 	soc8544@e0000000 {
-		#address-cells = <1>;
-		#size-cells = <1>;
-		device_type = "soc";
-		compatible = "simple-bus";
-
-		ranges = <0x0 0xe0000000 0x100000>;
-		reg = <0xe0000000 0x1000>;	// CCSRBAR 1M
-		bus-frequency = <0>;		// Filled out by uboot.
-
 		serial0: serial@4500 {
 			cell-index = <0>;
 			device_type = "serial";
-- 
1.6.0.2

[Qemu-devel] [PATCH 16/31] PPC: e500: dt: create serial nodes dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   35 +++++++++++++++++++++++++++++++++++
 pc-bios/mpc8544ds.dts  |   26 --------------------------
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 5ff2d24..493ad6e 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -81,6 +81,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     char compatible[] = "MPC8544DS\0MPC85xxDS";
     char model[] = "MPC8544DS";
     char soc[128];
+    char ser0[128];
+    char ser1[128];
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
     if (!filename) {
@@ -182,6 +184,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
         }
     }
 
+    qemu_devtree_add_subnode(fdt, "/aliases");
     /* XXX These should go into their respective devices' code */
     sprintf(soc, "/soc8544@%x", MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, soc);
@@ -196,6 +199,38 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     /* XXX should contain a reasonable value */
     qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
 
+    /*
+     * We have to generate ser1 first, because Linux takes the first
+     * device it finds in the dt as serial output device. And we generate
+     * devices in reverse order to the dt.
+     */
+    sprintf(ser1, "%s/serial@%x", soc,
+            MPC8544_SERIAL1_REGS_BASE - MPC8544_CCSRBAR_BASE);
+    qemu_devtree_add_subnode(fdt, ser1);
+    qemu_devtree_setprop_string(fdt, ser1, "device_type", "serial");
+    qemu_devtree_setprop_string(fdt, ser1, "compatible", "ns16550");
+    qemu_devtree_setprop_cell2(fdt, ser1, "reg", MPC8544_SERIAL1_REGS_BASE -
+                               MPC8544_CCSRBAR_BASE, 0x100);
+    qemu_devtree_setprop_cell(fdt, ser1, "cell-index", 1);
+    qemu_devtree_setprop_cell(fdt, ser1, "clock-frequency", 0);
+    qemu_devtree_setprop_cell2(fdt, ser1, "interrupts", 42, 2);
+    qemu_devtree_setprop_phandle(fdt, ser1, "interrupt-parent", mpic);
+    qemu_devtree_setprop_string(fdt, "/aliases", "serial1", ser1);
+
+    sprintf(ser0, "%s/serial@%x", soc,
+            MPC8544_SERIAL0_REGS_BASE - MPC8544_CCSRBAR_BASE);
+    qemu_devtree_add_subnode(fdt, ser0);
+    qemu_devtree_setprop_string(fdt, ser0, "device_type", "serial");
+    qemu_devtree_setprop_string(fdt, ser0, "compatible", "ns16550");
+    qemu_devtree_setprop_cell2(fdt, ser0, "reg", MPC8544_SERIAL0_REGS_BASE -
+                               MPC8544_CCSRBAR_BASE, 0x100);
+    qemu_devtree_setprop_cell(fdt, ser0, "cell-index", 0);
+    qemu_devtree_setprop_cell(fdt, ser0, "clock-frequency", 0);
+    qemu_devtree_setprop_cell2(fdt, ser0, "interrupts", 42, 2);
+    qemu_devtree_setprop_phandle(fdt, ser0, "interrupt-parent", mpic);
+    qemu_devtree_setprop_string(fdt, "/aliases", "serial0", ser0);
+    qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser0);
+
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     if (ret < 0) {
         goto out;
diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 01b53ba..e536ab1 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -12,32 +12,10 @@
 /dts-v1/;
 / {
 	aliases {
-		serial0 = &serial0;
-		serial1 = &serial1;
 		pci0 = &pci0;
 	};
 
 	soc8544@e0000000 {
-		serial0: serial@4500 {
-			cell-index = <0>;
-			device_type = "serial";
-			compatible = "ns16550";
-			reg = <0x4500 0x100>;
-			clock-frequency = <0>;
-			interrupts = <42 2>;
-			interrupt-parent = <&mpic>;
-		};
-
-		serial1: serial@4600 {
-			cell-index = <1>;
-			device_type = "serial";
-			compatible = "ns16550";
-			reg = <0x4600 0x100>;
-			clock-frequency = <0>;
-			interrupts = <42 2>;
-			interrupt-parent = <&mpic>;
-		};
-
 		mpic: pic@40000 {
 			interrupt-controller;
 			#address-cells = <0>;
@@ -85,8 +63,4 @@
 		#address-cells = <3>;
 		reg = <0xe0008000 0x1000>;
 	};
-
-	chosen {
-		linux,stdout-path = "/soc8544@e0000000/serial@4500";
-	};
 };
-- 
1.6.0.2

[Qemu-devel] [PATCH 17/31] PPC: e500: dt: create mpic node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 493ad6e..3d073dd 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -83,6 +83,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     char soc[128];
     char ser0[128];
     char ser1[128];
+    char mpic[128];
+    uint32_t mpic_ph;
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
     if (!filename) {
@@ -199,6 +201,20 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     /* XXX should contain a reasonable value */
     qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
 
+    sprintf(mpic, "%s/pic@%x", soc,
+            MPC8544_MPIC_REGS_BASE - MPC8544_CCSRBAR_BASE);
+    qemu_devtree_add_subnode(fdt, mpic);
+    qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
+    qemu_devtree_setprop_string(fdt, mpic, "compatible", "chrp,open-pic");
+    qemu_devtree_setprop_cell2(fdt, mpic, "reg", MPC8544_MPIC_REGS_BASE -
+                               MPC8544_CCSRBAR_BASE, 0x40000);
+    qemu_devtree_setprop_cell(fdt, mpic, "#address-cells", 0);
+    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
+    mpic_ph = qemu_devtree_alloc_phandle(fdt);
+    qemu_devtree_setprop_cell(fdt, mpic, "phandle", mpic_ph);
+    qemu_devtree_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
+    qemu_devtree_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
+
     /*
      * We have to generate ser1 first, because Linux takes the first
      * device it finds in the dt as serial output device. And we generate
-- 
1.6.0.2

[Qemu-devel] [PATCH 18/31] PPC: e500: dt: create global-utils node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    9 +++++++++
 pc-bios/mpc8544ds.dts  |    6 ------
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 3d073dd..3f6c6e3 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -85,6 +85,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     char ser1[128];
     char mpic[128];
     uint32_t mpic_ph;
+    char gutil[128];
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
     if (!filename) {
@@ -247,6 +248,14 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_string(fdt, "/aliases", "serial0", ser0);
     qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser0);
 
+    sprintf(gutil, "%s/global-utilities@%x", soc,
+            MPC8544_UTIL_BASE - MPC8544_CCSRBAR_BASE);
+    qemu_devtree_add_subnode(fdt, gutil);
+    qemu_devtree_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
+    qemu_devtree_setprop_cell2(fdt, gutil, "reg", MPC8544_UTIL_BASE -
+                               MPC8544_CCSRBAR_BASE, 0x1000);
+    qemu_devtree_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
+
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     if (ret < 0) {
         goto out;
diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index e536ab1..4c7bd75 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -24,12 +24,6 @@
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
 		};
-
-                global-utilities@e0000 {        //global utilities block
-                        compatible = "fsl,mpc8544-guts";
-                        reg = <0xe0000 0x1000>;
-                        fsl,has-rstcr;
-                };
 	};
 
 	pci0: pci@e0008000 {
-- 
1.6.0.2

[Qemu-devel] [PATCH 19/31] PPC: e500: dt: create pci node dynamically

[Alexander Graf]

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 pc-bios/mpc8544ds.dtb  |  Bin 1810 -> 72 bytes
 pc-bios/mpc8544ds.dts  |   46 --------------------------------------------
 3 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 3f6c6e3..d9a3d50 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -62,6 +62,27 @@ struct boot_info
     uint32_t entry;
 };
 
+static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
+{
+    int i;
+    const uint32_t tmp[] = {
+                             /* IDSEL 0x11 J17 Slot 1 */
+                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
+                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
+                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
+                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
+
+                             /* IDSEL 0x12 J16 Slot 2 */
+                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
+                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
+                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
+                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
+                           };
+    for (i = 0; i < (7 * 8); i++) {
+        pci_map[i] = cpu_to_be32(tmp[i]);
+    }
+}
+
 static int mpc8544_load_device_tree(CPUPPCState *env,
                                     target_phys_addr_t addr,
                                     uint32_t ramsize,
@@ -86,6 +107,11 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     char mpic[128];
     uint32_t mpic_ph;
     char gutil[128];
+    char pci[128];
+    uint32_t pci_map[7 * 8];
+    uint32_t pci_ranges[12] = { 0x2000000, 0x0, 0xc0000000, 0xc0000000, 0x0,
+                                0x20000000, 0x1000000, 0x0, 0x0, 0xe1000000,
+                                0x0, 0x10000 };
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
     if (!filename) {
@@ -256,6 +282,30 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                MPC8544_CCSRBAR_BASE, 0x1000);
     qemu_devtree_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
 
+    sprintf(pci, "/pci@%x", MPC8544_PCI_REGS_BASE);
+    qemu_devtree_add_subnode(fdt, pci);
+    qemu_devtree_setprop_cell(fdt, pci, "cell-index", 0);
+    qemu_devtree_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
+    qemu_devtree_setprop_string(fdt, pci, "device_type", "pci");
+    qemu_devtree_setprop_cell4(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
+                               0x0, 0x7);
+    pci_map_create(fdt, pci_map, qemu_devtree_get_phandle(fdt, mpic));
+    qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, sizeof(pci_map));
+    qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
+    qemu_devtree_setprop_cell2(fdt, pci, "interrupts", 24, 2);
+    qemu_devtree_setprop_cell2(fdt, pci, "bus-range", 0, 255);
+    for (i = 0; i < 12; i++) {
+        pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
+    }
+    qemu_devtree_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
+    qemu_devtree_setprop_cell2(fdt, pci, "reg", MPC8544_PCI_REGS_BASE,
+                               0x1000);
+    qemu_devtree_setprop_cell(fdt, pci, "clock-frequency", 66666666);
+    qemu_devtree_setprop_cell(fdt, pci, "#interrupt-cells", 1);
+    qemu_devtree_setprop_cell(fdt, pci, "#size-cells", 2);
+    qemu_devtree_setprop_cell(fdt, pci, "#address-cells", 3);
+    qemu_devtree_setprop_string(fdt, "/aliases", "pci0", pci);
+
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     if (ret < 0) {
         goto out;
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index 25d92f681dec184530af63e2d2cea61cb4cccd04..90ef5c00243b04f4aa3f812b89d5b37c63be09f2 100644
GIT binary patch
literal 72
mcmcb>`|m9S1A_+;TR>?IAT0>Q0zeD{$ZVJxBb31eq&Wct_yhI;

literal 1810
zcmb7EyKdA#6rCkONyI||2}FgEk^-Svaira7i!HW+bSd}&;&>;!qn)*l$3_V>REUy}
ze?a^K5)yPYv~)`kJs<>}GvjgWMJaNn>$&fHJacD0U-|Q0h?VO?h`taPe`CE1z6M?g
zgE}{1|LEk_w^M1INUO+5Lv;y!o5Hq9<9@H(9m>$rwvoAt^sw6tLk672uAUvc+l;-6
zob~N2R<>pzWo;R80ZV7GopV_{%aCs{226a^Hy88}`7l}kC9DIZW|@}3VQGKM+AqVt
z$DlbsZg*I36}&&kr!x8;53PyV+JEl-2bG`t3OICe*6QmH60@`0>)Ai`wtXS)Bgk&Q
zuQjz<4nOfc1K$I4Z+y%P$V_tkRbi@j*vA}HG1SkA=|PoR_d7SHOvS@4rv;Tj#6Wrt
z_V{>?B(J}P?Elf8gFRiIu#4f$4B|<iZKat^0(7u0o4*DzY@6%4@8E0B%ZEz0dpFkU
zVWfkyP+ApA8aLy%Fmdtj433Q6nq~G11mm)BQ{*r?oJ8h^h&iNBVAi)i{vY!i+W}%o
z%;OX`dW8Y<ypOvOHgv{*5#k5^o_NNrGY5YY;w6@v`2Y31Ech%&_?e5CR$gVZC$Z=w
zCSMopE2uSjdzIhv`F>S?%jf%5F)iOhzj_b-U0^QHgZJeYm^HlZ7i7|3Fl`}tj{&6j
z6;r+gCU{R@z2J<@C64LR&*2-aUgzvG!t0xmeSgMt*6AbLnVE~{5ZA$OM&e0oWJ1-(
z;N9&kpZ%8B?=E|g*W7y(3b*bE%t|OWqR}Xq#ssm{+K3IKp2|ud$tNn7kBXB_ia4ER
zQK1gC6nT`4@%ra-Ebv?gN4b1l$|OD!tPrSVB#%X`(|Fo&sics3US_x$wHRbkE|a2R
zh|{FVQ>|q#HcrDFFs+jEcq^Mk$p{#D-6oL<a#O^4qrxo8f>~syZIi`mVGCEr6r;2(
I0HBzNe~HoS>i_@%

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 4c7bd75..16aba2b 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -11,50 +11,4 @@
 
 /dts-v1/;
 / {
-	aliases {
-		pci0 = &pci0;
-	};
-
-	soc8544@e0000000 {
-		mpic: pic@40000 {
-			interrupt-controller;
-			#address-cells = <0>;
-			#interrupt-cells = <2>;
-			reg = <0x40000 0x40000>;
-			compatible = "chrp,open-pic";
-			device_type = "open-pic";
-		};
-	};
-
-	pci0: pci@e0008000 {
-		cell-index = <0>;
-		compatible = "fsl,mpc8540-pci";
-		device_type = "pci";
-		interrupt-map-mask = <0xf800 0x0 0x0 0x7>;
-		interrupt-map = <
-
-			/* IDSEL 0x11 J17 Slot 1 */
-			0x8800 0x0 0x0 0x1 &mpic 0x2 0x1
-			0x8800 0x0 0x0 0x2 &mpic 0x3 0x1
-			0x8800 0x0 0x0 0x3 &mpic 0x4 0x1
-			0x8800 0x0 0x0 0x4 &mpic 0x1 0x1
-
-			/* IDSEL 0x12 J16 Slot 2 */
-
-			0x9000 0x0 0x0 0x1 &mpic 0x3 0x1
-			0x9000 0x0 0x0 0x2 &mpic 0x4 0x1
-			0x9000 0x0 0x0 0x3 &mpic 0x2 0x1
-			0x9000 0x0 0x0 0x4 &mpic 0x1 0x1>;
-
-		interrupt-parent = <&mpic>;
-		interrupts = <24 2>;
-		bus-range = <0 255>;
-		ranges = <0x2000000 0x0 0xc0000000 0xc0000000 0x0 0x20000000
-			  0x1000000 0x0 0x0 0xe1000000 0x0 0x10000>;
-		clock-frequency = <66666666>;
-		#interrupt-cells = <1>;
-		#size-cells = <2>;
-		#address-cells = <3>;
-		reg = <0xe0008000 0x1000>;
-	};
 };
-- 
1.6.0.2

[Qemu-devel] [PATCH 20/31] PPC: e500: dt: start with empty device tree

[Alexander Graf]

Now that all of the device tree bits are generated during runtime, we
can get rid of the device tree blob and instead start from scratch with
an empty device tree.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 Makefile               |    1 -
 hw/ppce500_mpc8544ds.c |    8 +-------
 pc-bios/mpc8544ds.dtb  |  Bin 72 -> 0 bytes
 pc-bios/mpc8544ds.dts  |   14 --------------
 4 files changed, 1 insertions(+), 22 deletions(-)
 delete mode 100644 pc-bios/mpc8544ds.dtb
 delete mode 100644 pc-bios/mpc8544ds.dts

diff --git a/Makefile b/Makefile
index 9b7a85e..6be0c94 100644
--- a/Makefile
+++ b/Makefile
@@ -259,7 +259,6 @@ pxe-e1000.rom pxe-eepro100.rom pxe-ne2k_pci.rom \
 pxe-pcnet.rom pxe-rtl8139.rom pxe-virtio.rom \
 qemu-icon.bmp \
 bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
-mpc8544ds.dtb \
 multiboot.bin linuxboot.bin kvmvapic.bin \
 s390-zipl.rom \
 spapr-rtas.bin slof.bin \
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index d9a3d50..30683e7 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -92,7 +92,6 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 {
     int ret = -1;
     uint32_t mem_reg_property[] = {0, cpu_to_be32(ramsize)};
-    char *filename;
     int fdt_size;
     void *fdt;
     uint8_t hypercall[16];
@@ -113,12 +112,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                 0x20000000, 0x1000000, 0x0, 0x0, 0xe1000000,
                                 0x0, 0x10000 };
 
-    filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
-    if (!filename) {
-        goto out;
-    }
-    fdt = load_device_tree(filename, &fdt_size);
-    g_free(filename);
+    fdt = create_device_tree(&fdt_size);
     if (fdt == NULL) {
         goto out;
     }
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
deleted file mode 100644
index 90ef5c00243b04f4aa3f812b89d5b37c63be09f2..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 72
mcmcb>`|m9S1A_+;TR>?IAT0>Q0zeD{$ZVJxBb31eq&Wct_yhI;

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
deleted file mode 100644
index 16aba2b..0000000
--- a/pc-bios/mpc8544ds.dts
+++ /dev/null
@@ -1,14 +0,0 @@
-/*
- * MPC8544 DS Device Tree Source
- *
- * Copyright 2007, 2008 Freescale Semiconductor Inc.
- *
- * This program is free software; you can redistribute  it and/or modify it
- * under  the terms of  the GNU General  Public License as published by the
- * Free Software Foundation;  either version 2 of the  License, or (at your
- * option) any later version.
- */
-
-/dts-v1/;
-/ {
-};
-- 
1.6.0.2

[Qemu-devel] [PATCH 21/31] dt: Add -machine dumpdtb option to dump the current dtb

[Alexander Graf]

Now that we are dynamically creating the dtb, it's really useful to
be able to dump the created blob for debugging.

This patch implements a -machine dumpdtb=<file> option for e500 that
dumps the dtb exactly in the form the guest would get it to disk. It
can then be analyzed by dtc to get information about the guest
configuration.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   18 ++++++++++++++++++
 qemu-config.c          |    4 ++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 30683e7..2069891 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -111,6 +111,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     uint32_t pci_ranges[12] = { 0x2000000, 0x0, 0xc0000000, 0xc0000000, 0x0,
                                 0x20000000, 0x1000000, 0x0, 0x0, 0xe1000000,
                                 0x0, 0x10000 };
+    QemuOpts *machine_opts;
+    const char *dumpdtb = NULL;
 
     fdt = create_device_tree(&fdt_size);
     if (fdt == NULL) {
@@ -300,6 +302,22 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_cell(fdt, pci, "#address-cells", 3);
     qemu_devtree_setprop_string(fdt, "/aliases", "pci0", pci);
 
+    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
+    if (machine_opts) {
+        dumpdtb = qemu_opt_get(machine_opts, "dumpdtb");
+    }
+    if (dumpdtb) {
+        /* Dump the dtb to a file and quit */
+        FILE *f = fopen(dumpdtb, "wb");
+        size_t len;
+        len = fwrite(fdt, fdt_size, 1, f);
+        fclose(f);
+        if (len != fdt_size) {
+            exit(1);
+        }
+        exit(0);
+    }
+
     ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
     if (ret < 0) {
         goto out;
diff --git a/qemu-config.c b/qemu-config.c
index be84a03..ef4714e 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -582,6 +582,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "dtb",
             .type = QEMU_OPT_STRING,
             .help = "Linux kernel device tree file",
+        }, {
+            .name = "dumpdtb",
+            .type = QEMU_OPT_STRING,
+            .help = "Dump current dtb to a file and quit",
         },
         { /* End of list */ }
     },
-- 
1.6.0.2

[Qemu-devel] [PATCH 22/31] PPC: e500: dt: use 64bit cell helper

[Alexander Graf]

We have a nice 64bit helper to ease the device tree generation and
make the code more readable when creating 64bit 2-cell parameters.
Use it when generating the device tree.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - rename cell64 -> u64
  - don't treat memory as single u64
---
 hw/ppce500_mpc8544ds.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 2069891..f856944 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -174,7 +174,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
        the first node as boot node and be happy */
     for (i = smp_cpus - 1; i >= 0; i--) {
         char cpu_name[128];
-        uint64_t cpu_release_addr = cpu_to_be64(MPC8544_SPIN_BASE + (i * 0x20));
+        uint64_t cpu_release_addr = MPC8544_SPIN_BASE + (i * 0x20);
 
         for (env = first_cpu; env != NULL; env = env->next_cpu) {
             if (env->cpu_index == i) {
@@ -202,8 +202,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
         if (env->cpu_index) {
             qemu_devtree_setprop_string(fdt, cpu_name, "status", "disabled");
             qemu_devtree_setprop_string(fdt, cpu_name, "enable-method", "spin-table");
-            qemu_devtree_setprop(fdt, cpu_name, "cpu-release-addr",
-                                 &cpu_release_addr, sizeof(cpu_release_addr));
+            qemu_devtree_setprop_u64(fdt, cpu_name, "cpu-release-addr",
+                                     cpu_release_addr);
         } else {
             qemu_devtree_setprop_string(fdt, cpu_name, "status", "okay");
         }
-- 
1.6.0.2

[Qemu-devel] [PATCH 23/31] PPC: e500: dt: use target_phys_addr_t for ramsize

[Alexander Graf]

We're passing the ram size as uint32_t, capping it to 32 bits atm.
Change to target_phys_addr_t (uint64_t) to make sure we have all
the bits.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index f856944..aa9bbd5 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -85,7 +85,7 @@ static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
 
 static int mpc8544_load_device_tree(CPUPPCState *env,
                                     target_phys_addr_t addr,
-                                    uint32_t ramsize,
+                                    target_phys_addr_t ramsize,
                                     target_phys_addr_t initrd_base,
                                     target_phys_addr_t initrd_size,
                                     const char *kernel_cmdline)
-- 
1.6.0.2

[Qemu-devel] [PATCH 24/31] PPC: e500: enable manual loading of dtb blob

[Alexander Graf]

We want to be able to override the automatically created device tree
by using the -dtb option. Implement this for the mpc8544ds machine.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index aa9bbd5..bad114c 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -113,6 +113,27 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                 0x0, 0x10000 };
     QemuOpts *machine_opts;
     const char *dumpdtb = NULL;
+    const char *dtb_file = NULL;
+
+    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
+    if (machine_opts) {
+        dumpdtb = qemu_opt_get(machine_opts, "dumpdtb");
+        dtb_file = qemu_opt_get(machine_opts, "dtb");
+    }
+
+    if (dtb_file) {
+        char *filename;
+        filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_file);
+        if (!filename) {
+            goto out;
+        }
+
+        fdt = load_device_tree(filename, &fdt_size);
+        if (!fdt) {
+            goto out;
+        }
+        goto done;
+    }
 
     fdt = create_device_tree(&fdt_size);
     if (fdt == NULL) {
@@ -302,10 +323,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_cell(fdt, pci, "#address-cells", 3);
     qemu_devtree_setprop_string(fdt, "/aliases", "pci0", pci);
 
-    machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
-    if (machine_opts) {
-        dumpdtb = qemu_opt_get(machine_opts, "dumpdtb");
-    }
+done:
     if (dumpdtb) {
         /* Dump the dtb to a file and quit */
         FILE *f = fopen(dumpdtb, "wb");
-- 
1.6.0.2

[Qemu-devel] [PATCH 25/31] Revert "dt: temporarily disable subtree creation failure check"

[Alexander Graf]

This reverts commit "dt: temporarily disable subtree creation
failure check" which was meant as a temporary solution to keep
external and dynamic device tree construction intact.

Now that we switched to fully dynamic dt construction, it's no
longer necessary.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - remove commit id from patch description
---
 device_tree.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 644d766..a2039e5 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -268,13 +268,11 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
     }
 
     retval = fdt_add_subnode(fdt, parent, basename);
-#if 0
     if (retval < 0) {
         fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
                 fdt_strerror(retval));
         exit(1);
     }
-#endif
 
     g_free(dupname);
     return retval;
-- 
1.6.0.2

[Qemu-devel] [PATCH 26/31] PPC: e500: Use new MPIC dt format

[Alexander Graf]

Due to popular demand, we're updating the way we generate the MPIC
node and interrupt lines based on what the current state of art is.

Requested-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   33 ++++++++++++++++++---------------
 roms/openbios          |    2 +-
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index bad114c..35b470a 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -67,18 +67,18 @@ static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
     int i;
     const uint32_t tmp[] = {
                              /* IDSEL 0x11 J17 Slot 1 */
-                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
-                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
-                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
-                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
+                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1, 0x0, 0x0,
+                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1, 0x0, 0x0,
+                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1, 0x0, 0x0,
+                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1, 0x0, 0x0,
 
                              /* IDSEL 0x12 J16 Slot 2 */
-                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
-                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
-                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
-                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
+                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1, 0x0, 0x0,
+                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1, 0x0, 0x0,
+                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1, 0x0, 0x0,
+                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1, 0x0, 0x0,
                            };
-    for (i = 0; i < (7 * 8); i++) {
+    for (i = 0; i < ARRAY_SIZE(tmp); i++) {
         pci_map[i] = cpu_to_be32(tmp[i]);
     }
 }
@@ -107,7 +107,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     uint32_t mpic_ph;
     char gutil[128];
     char pci[128];
-    uint32_t pci_map[7 * 8];
+    uint32_t pci_map[9 * 8];
     uint32_t pci_ranges[12] = { 0x2000000, 0x0, 0xc0000000, 0xc0000000, 0x0,
                                 0x20000000, 0x1000000, 0x0, 0x0, 0xe1000000,
                                 0x0, 0x10000 };
@@ -249,15 +249,18 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
             MPC8544_MPIC_REGS_BASE - MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, mpic);
     qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
-    qemu_devtree_setprop_string(fdt, mpic, "compatible", "chrp,open-pic");
+    qemu_devtree_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
     qemu_devtree_setprop_cell2(fdt, mpic, "reg", MPC8544_MPIC_REGS_BASE -
                                MPC8544_CCSRBAR_BASE, 0x40000);
     qemu_devtree_setprop_cell(fdt, mpic, "#address-cells", 0);
-    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
+    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 4);
     mpic_ph = qemu_devtree_alloc_phandle(fdt);
     qemu_devtree_setprop_cell(fdt, mpic, "phandle", mpic_ph);
     qemu_devtree_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
     qemu_devtree_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
+    qemu_devtree_setprop(fdt, mpic, "big-endian", NULL, 0);
+    qemu_devtree_setprop(fdt, mpic, "single-cpu-affinity", NULL, 0);
+    qemu_devtree_setprop_cell(fdt, mpic, "last-interrupt-source", 255);
 
     /*
      * We have to generate ser1 first, because Linux takes the first
@@ -273,7 +276,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                MPC8544_CCSRBAR_BASE, 0x100);
     qemu_devtree_setprop_cell(fdt, ser1, "cell-index", 1);
     qemu_devtree_setprop_cell(fdt, ser1, "clock-frequency", 0);
-    qemu_devtree_setprop_cell2(fdt, ser1, "interrupts", 42, 2);
+    qemu_devtree_setprop_cell4(fdt, ser1, "interrupts", 42, 2, 0, 0);
     qemu_devtree_setprop_phandle(fdt, ser1, "interrupt-parent", mpic);
     qemu_devtree_setprop_string(fdt, "/aliases", "serial1", ser1);
 
@@ -286,7 +289,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                MPC8544_CCSRBAR_BASE, 0x100);
     qemu_devtree_setprop_cell(fdt, ser0, "cell-index", 0);
     qemu_devtree_setprop_cell(fdt, ser0, "clock-frequency", 0);
-    qemu_devtree_setprop_cell2(fdt, ser0, "interrupts", 42, 2);
+    qemu_devtree_setprop_cell4(fdt, ser0, "interrupts", 42, 2, 0, 0);
     qemu_devtree_setprop_phandle(fdt, ser0, "interrupt-parent", mpic);
     qemu_devtree_setprop_string(fdt, "/aliases", "serial0", ser0);
     qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser0);
@@ -309,7 +312,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     pci_map_create(fdt, pci_map, qemu_devtree_get_phandle(fdt, mpic));
     qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, sizeof(pci_map));
     qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
-    qemu_devtree_setprop_cell2(fdt, pci, "interrupts", 24, 2);
+    qemu_devtree_setprop_cell4(fdt, pci, "interrupts", 24, 2, 0, 0);
     qemu_devtree_setprop_cell2(fdt, pci, "bus-range", 0, 255);
     for (i = 0; i < 12; i++) {
         pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
diff --git a/roms/openbios b/roms/openbios
index d1d2787..ff61d97 160000
--- a/roms/openbios
+++ b/roms/openbios
@@ -1 +1 @@
-Subproject commit d1d2787f87167edf487a60e61b9168514d5a7434
+Subproject commit ff61d973e5a4a68b29e485b3f88e6a2d1d96cf45
-- 
1.6.0.2

Re: [Qemu-devel] [Qemu-ppc] [PATCH 26/31] PPC: e500: Use new MPIC dt format

[Blue Swirl]

On Tue, Jun 5, 2012 at 11:53 PM, Alexander Graf <agraf@suse.de> wrote:
> Due to popular demand, we're updating the way we generate the MPIC
> node and interrupt lines based on what the current state of art is.
>
> Requested-by: Scott Wood <scottwood@freescale.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/ppce500_mpc8544ds.c |   33 ++++++++++++++++++---------------
>  roms/openbios          |    2 +-
>  2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
> index bad114c..35b470a 100644
> --- a/hw/ppce500_mpc8544ds.c
> +++ b/hw/ppce500_mpc8544ds.c
> @@ -67,18 +67,18 @@ static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
>     int i;
>     const uint32_t tmp[] = {
>                              /* IDSEL 0x11 J17 Slot 1 */
> -                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
> -                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
> -                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
> -                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
> +                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1, 0x0, 0x0,
> +                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1, 0x0, 0x0,
> +                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1, 0x0, 0x0,
> +                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1, 0x0, 0x0,
>
>                              /* IDSEL 0x12 J16 Slot 2 */
> -                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
> -                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
> -                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
> -                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
> +                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1, 0x0, 0x0,
> +                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1, 0x0, 0x0,
> +                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1, 0x0, 0x0,
> +                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1, 0x0, 0x0,
>                            };
> -    for (i = 0; i < (7 * 8); i++) {
> +    for (i = 0; i < ARRAY_SIZE(tmp); i++) {
>         pci_map[i] = cpu_to_be32(tmp[i]);
>     }
>  }
> @@ -107,7 +107,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>     uint32_t mpic_ph;
>     char gutil[128];
>     char pci[128];
> -    uint32_t pci_map[7 * 8];
> +    uint32_t pci_map[9 * 8];
>     uint32_t pci_ranges[12] = { 0x2000000, 0x0, 0xc0000000, 0xc0000000, 0x0,
>                                 0x20000000, 0x1000000, 0x0, 0x0, 0xe1000000,
>                                 0x0, 0x10000 };
> @@ -249,15 +249,18 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>             MPC8544_MPIC_REGS_BASE - MPC8544_CCSRBAR_BASE);
>     qemu_devtree_add_subnode(fdt, mpic);
>     qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
> -    qemu_devtree_setprop_string(fdt, mpic, "compatible", "chrp,open-pic");
> +    qemu_devtree_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
>     qemu_devtree_setprop_cell2(fdt, mpic, "reg", MPC8544_MPIC_REGS_BASE -
>                                MPC8544_CCSRBAR_BASE, 0x40000);
>     qemu_devtree_setprop_cell(fdt, mpic, "#address-cells", 0);
> -    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
> +    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 4);
>     mpic_ph = qemu_devtree_alloc_phandle(fdt);
>     qemu_devtree_setprop_cell(fdt, mpic, "phandle", mpic_ph);
>     qemu_devtree_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
>     qemu_devtree_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
> +    qemu_devtree_setprop(fdt, mpic, "big-endian", NULL, 0);
> +    qemu_devtree_setprop(fdt, mpic, "single-cpu-affinity", NULL, 0);
> +    qemu_devtree_setprop_cell(fdt, mpic, "last-interrupt-source", 255);
>
>     /*
>      * We have to generate ser1 first, because Linux takes the first
> @@ -273,7 +276,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>                                MPC8544_CCSRBAR_BASE, 0x100);
>     qemu_devtree_setprop_cell(fdt, ser1, "cell-index", 1);
>     qemu_devtree_setprop_cell(fdt, ser1, "clock-frequency", 0);
> -    qemu_devtree_setprop_cell2(fdt, ser1, "interrupts", 42, 2);
> +    qemu_devtree_setprop_cell4(fdt, ser1, "interrupts", 42, 2, 0, 0);
>     qemu_devtree_setprop_phandle(fdt, ser1, "interrupt-parent", mpic);
>     qemu_devtree_setprop_string(fdt, "/aliases", "serial1", ser1);
>
> @@ -286,7 +289,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>                                MPC8544_CCSRBAR_BASE, 0x100);
>     qemu_devtree_setprop_cell(fdt, ser0, "cell-index", 0);
>     qemu_devtree_setprop_cell(fdt, ser0, "clock-frequency", 0);
> -    qemu_devtree_setprop_cell2(fdt, ser0, "interrupts", 42, 2);
> +    qemu_devtree_setprop_cell4(fdt, ser0, "interrupts", 42, 2, 0, 0);
>     qemu_devtree_setprop_phandle(fdt, ser0, "interrupt-parent", mpic);
>     qemu_devtree_setprop_string(fdt, "/aliases", "serial0", ser0);
>     qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser0);
> @@ -309,7 +312,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>     pci_map_create(fdt, pci_map, qemu_devtree_get_phandle(fdt, mpic));
>     qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, sizeof(pci_map));
>     qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
> -    qemu_devtree_setprop_cell2(fdt, pci, "interrupts", 24, 2);
> +    qemu_devtree_setprop_cell4(fdt, pci, "interrupts", 24, 2, 0, 0);
>     qemu_devtree_setprop_cell2(fdt, pci, "bus-range", 0, 255);
>     for (i = 0; i < 12; i++) {
>         pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
> diff --git a/roms/openbios b/roms/openbios
> index d1d2787..ff61d97 160000
> --- a/roms/openbios
> +++ b/roms/openbios
> @@ -1 +1 @@
> -Subproject commit d1d2787f87167edf487a60e61b9168514d5a7434
> +Subproject commit ff61d973e5a4a68b29e485b3f88e6a2d1d96cf45

This does not look intentional.

> --
> 1.6.0.2
>
>

[Qemu-devel] [PATCH 27/31] PPC: e500: Use new SOC dt format

[Alexander Graf]

Due to popular demand, let's clean up the soc node a bit and use
more recent dt notions.

Requested-by: Scott Wood <scottwood@freescale.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 35b470a..18a2328 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -43,7 +43,6 @@
 #define RAM_SIZES_ALIGN            (64UL << 20)
 
 #define MPC8544_CCSRBAR_BASE       0xE0000000
-#define MPC8544_CCSRBAR_REGSIZE    0x00001000
 #define MPC8544_CCSRBAR_SIZE       0x00100000
 #define MPC8544_MPIC_REGS_BASE     (MPC8544_CCSRBAR_BASE + 0x40000)
 #define MPC8544_SERIAL0_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4500)
@@ -99,6 +98,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     uint32_t tb_freq = 400000000;
     int i;
     char compatible[] = "MPC8544DS\0MPC85xxDS";
+    char compatible_sb[] = "fsl,mpc8544-immr\0simple-bus";
     char model[] = "MPC8544DS";
     char soc[128];
     char ser0[128];
@@ -232,16 +232,15 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 
     qemu_devtree_add_subnode(fdt, "/aliases");
     /* XXX These should go into their respective devices' code */
-    sprintf(soc, "/soc8544@%x", MPC8544_CCSRBAR_BASE);
+    sprintf(soc, "/soc@%x", MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, soc);
     qemu_devtree_setprop_string(fdt, soc, "device_type", "soc");
-    qemu_devtree_setprop_string(fdt, soc, "compatible", "simple-bus");
+    qemu_devtree_setprop(fdt, soc, "compatible", compatible_sb,
+                         sizeof(compatible_sb));
     qemu_devtree_setprop_cell(fdt, soc, "#address-cells", 1);
     qemu_devtree_setprop_cell(fdt, soc, "#size-cells", 1);
     qemu_devtree_setprop_cell3(fdt, soc, "ranges", 0x0, MPC8544_CCSRBAR_BASE,
                                MPC8544_CCSRBAR_SIZE);
-    qemu_devtree_setprop_cell2(fdt, soc, "reg", MPC8544_CCSRBAR_BASE,
-                               MPC8544_CCSRBAR_REGSIZE);
     /* XXX should contain a reasonable value */
     qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
 
-- 
1.6.0.2

[Qemu-devel] [PATCH 28/31] PPC: e500: Define addresses as always 64bit

[Alexander Graf]

Every time we use an address constant, it needs to potentially fit into
a 64bit physical address space. So let's define things accordingly.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 18a2328..2815340 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -42,17 +42,17 @@
 
 #define RAM_SIZES_ALIGN            (64UL << 20)
 
-#define MPC8544_CCSRBAR_BASE       0xE0000000
-#define MPC8544_CCSRBAR_SIZE       0x00100000
-#define MPC8544_MPIC_REGS_BASE     (MPC8544_CCSRBAR_BASE + 0x40000)
-#define MPC8544_SERIAL0_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4500)
-#define MPC8544_SERIAL1_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4600)
-#define MPC8544_PCI_REGS_BASE      (MPC8544_CCSRBAR_BASE + 0x8000)
-#define MPC8544_PCI_REGS_SIZE      0x1000
-#define MPC8544_PCI_IO             0xE1000000
-#define MPC8544_PCI_IOLEN          0x10000
-#define MPC8544_UTIL_BASE          (MPC8544_CCSRBAR_BASE + 0xe0000)
-#define MPC8544_SPIN_BASE          0xEF000000
+#define MPC8544_CCSRBAR_BASE       0xE0000000ULL
+#define MPC8544_CCSRBAR_SIZE       0x00100000ULL
+#define MPC8544_MPIC_REGS_BASE     (MPC8544_CCSRBAR_BASE + 0x40000ULL)
+#define MPC8544_SERIAL0_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4500ULL)
+#define MPC8544_SERIAL1_REGS_BASE  (MPC8544_CCSRBAR_BASE + 0x4600ULL)
+#define MPC8544_PCI_REGS_BASE      (MPC8544_CCSRBAR_BASE + 0x8000ULL)
+#define MPC8544_PCI_REGS_SIZE      0x1000ULL
+#define MPC8544_PCI_IO             0xE1000000ULL
+#define MPC8544_PCI_IOLEN          0x10000ULL
+#define MPC8544_UTIL_BASE          (MPC8544_CCSRBAR_BASE + 0xe0000ULL)
+#define MPC8544_SPIN_BASE          0xEF000000ULL
 
 struct boot_info
 {
@@ -232,7 +232,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 
     qemu_devtree_add_subnode(fdt, "/aliases");
     /* XXX These should go into their respective devices' code */
-    sprintf(soc, "/soc@%x", MPC8544_CCSRBAR_BASE);
+    sprintf(soc, "/soc@%llx", MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, soc);
     qemu_devtree_setprop_string(fdt, soc, "device_type", "soc");
     qemu_devtree_setprop(fdt, soc, "compatible", compatible_sb,
@@ -244,7 +244,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     /* XXX should contain a reasonable value */
     qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
 
-    sprintf(mpic, "%s/pic@%x", soc,
+    sprintf(mpic, "%s/pic@%llx", soc,
             MPC8544_MPIC_REGS_BASE - MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, mpic);
     qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
@@ -266,7 +266,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
      * device it finds in the dt as serial output device. And we generate
      * devices in reverse order to the dt.
      */
-    sprintf(ser1, "%s/serial@%x", soc,
+    sprintf(ser1, "%s/serial@%llx", soc,
             MPC8544_SERIAL1_REGS_BASE - MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, ser1);
     qemu_devtree_setprop_string(fdt, ser1, "device_type", "serial");
@@ -279,7 +279,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_phandle(fdt, ser1, "interrupt-parent", mpic);
     qemu_devtree_setprop_string(fdt, "/aliases", "serial1", ser1);
 
-    sprintf(ser0, "%s/serial@%x", soc,
+    sprintf(ser0, "%s/serial@%llx", soc,
             MPC8544_SERIAL0_REGS_BASE - MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, ser0);
     qemu_devtree_setprop_string(fdt, ser0, "device_type", "serial");
@@ -293,7 +293,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_string(fdt, "/aliases", "serial0", ser0);
     qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser0);
 
-    sprintf(gutil, "%s/global-utilities@%x", soc,
+    sprintf(gutil, "%s/global-utilities@%llx", soc,
             MPC8544_UTIL_BASE - MPC8544_CCSRBAR_BASE);
     qemu_devtree_add_subnode(fdt, gutil);
     qemu_devtree_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
@@ -301,7 +301,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                MPC8544_CCSRBAR_BASE, 0x1000);
     qemu_devtree_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
 
-    sprintf(pci, "/pci@%x", MPC8544_PCI_REGS_BASE);
+    sprintf(pci, "/pci@%llx", MPC8544_PCI_REGS_BASE);
     qemu_devtree_add_subnode(fdt, pci);
     qemu_devtree_setprop_cell(fdt, pci, "cell-index", 0);
     qemu_devtree_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
-- 
1.6.0.2

[Qemu-devel] [PATCH 29/31] PPC: e500: Extend address/size of / to 64bit

[Alexander Graf]

We want to be able to support >= 4GB of RAM. To do so, we need to be able
to tell the guest OS how much RAM it has.

However, that information today is capped to 32bit. So let's extend the
offset and size fields to 64bit, so we can fit in big addresses and even
one day - if we wish to do so - map devices above 32bit.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 2815340..6c01093 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -90,7 +90,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                                     const char *kernel_cmdline)
 {
     int ret = -1;
-    uint32_t mem_reg_property[] = {0, cpu_to_be32(ramsize)};
+    uint64_t mem_reg_property[] = { 0, cpu_to_be64(ramsize) };
     int fdt_size;
     void *fdt;
     uint8_t hypercall[16];
@@ -108,9 +108,16 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     char gutil[128];
     char pci[128];
     uint32_t pci_map[9 * 8];
-    uint32_t pci_ranges[12] = { 0x2000000, 0x0, 0xc0000000, 0xc0000000, 0x0,
-                                0x20000000, 0x1000000, 0x0, 0x0, 0xe1000000,
-                                0x0, 0x10000 };
+    uint32_t pci_ranges[14] =
+        {
+            0x2000000, 0x0, 0xc0000000,
+            0x0, 0xc0000000,
+            0x0, 0x20000000,
+
+            0x1000000, 0x0, 0x0,
+            0x0, 0xe1000000,
+            0x0, 0x10000,
+        };
     QemuOpts *machine_opts;
     const char *dumpdtb = NULL;
     const char *dtb_file = NULL;
@@ -144,8 +151,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_string(fdt, "/", "model", model);
     qemu_devtree_setprop(fdt, "/", "compatible", compatible,
                          sizeof(compatible));
-    qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 1);
-    qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 1);
+    qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 2);
+    qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 2);
 
     qemu_devtree_add_subnode(fdt, "/memory");
     qemu_devtree_setprop_string(fdt, "/memory", "device_type", "memory");
@@ -239,7 +246,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
                          sizeof(compatible_sb));
     qemu_devtree_setprop_cell(fdt, soc, "#address-cells", 1);
     qemu_devtree_setprop_cell(fdt, soc, "#size-cells", 1);
-    qemu_devtree_setprop_cell3(fdt, soc, "ranges", 0x0, MPC8544_CCSRBAR_BASE,
+    qemu_devtree_setprop_cell4(fdt, soc, "ranges", 0x0,
+                               MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
                                MPC8544_CCSRBAR_SIZE);
     /* XXX should contain a reasonable value */
     qemu_devtree_setprop_cell(fdt, soc, "bus-frequency", 0);
@@ -313,12 +321,12 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
     qemu_devtree_setprop_cell4(fdt, pci, "interrupts", 24, 2, 0, 0);
     qemu_devtree_setprop_cell2(fdt, pci, "bus-range", 0, 255);
-    for (i = 0; i < 12; i++) {
+    for (i = 0; i < 14; i++) {
         pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
     }
     qemu_devtree_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
-    qemu_devtree_setprop_cell2(fdt, pci, "reg", MPC8544_PCI_REGS_BASE,
-                               0x1000);
+    qemu_devtree_setprop_cell4(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
+                               MPC8544_PCI_REGS_BASE, 0, 0x1000);
     qemu_devtree_setprop_cell(fdt, pci, "clock-frequency", 66666666);
     qemu_devtree_setprop_cell(fdt, pci, "#interrupt-cells", 1);
     qemu_devtree_setprop_cell(fdt, pci, "#size-cells", 2);
-- 
1.6.0.2

[Qemu-devel] [PATCH 30/31] dt: Add global option to set phandle start offset

[Alexander Graf]

If anyone outside of QEMU wants to mess with a QEMU generated device tree,
he needs to know which range phandles are valid in. So let's expose a
machine option that an external program can use to set the start allocate
id for phandles in QEMU.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |   27 ++++++++++++++++++++++++++-
 qemu-config.c |    4 ++++
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index a2039e5..5bbe3cd 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -22,6 +22,7 @@
 #include "qemu-common.h"
 #include "device_tree.h"
 #include "hw/loader.h"
+#include "sysemu.h"
 
 #include <libfdt.h>
 
@@ -229,7 +230,31 @@ int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
 
 uint32_t qemu_devtree_alloc_phandle(void *fdt)
 {
-    static int phandle = 0x8000;
+    static int phandle = 0x0;
+
+    /*
+     * We need to find out if the user gave us special instruction at
+     * which phandle id to start allocting phandles.
+     */
+    if (!phandle) {
+        QemuOpts *machine_opts;
+        machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
+        if (machine_opts) {
+            const char *phandle_start;
+            phandle_start = qemu_opt_get(machine_opts, "phandle_start");
+            if (phandle_start) {
+                phandle = strtoul(phandle_start, NULL, 0);
+            }
+        }
+    }
+
+    if (!phandle) {
+        /*
+         * None of invalid phandle given on the command line, so fall back to
+         * default starting point.
+         */
+        phandle = 0x8000;
+    }
 
     return phandle++;
 }
diff --git a/qemu-config.c b/qemu-config.c
index ef4714e..a96b247 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -586,6 +586,10 @@ static QemuOptsList qemu_machine_opts = {
             .name = "dumpdtb",
             .type = QEMU_OPT_STRING,
             .help = "Dump current dtb to a file and quit",
+        }, {
+            .name = "phandle_start",
+            .type = QEMU_OPT_STRING,
+            .help = "The first phandle ID we may generate dynamically",
         },
         { /* End of list */ }
     },
-- 
1.6.0.2

[Qemu-devel] [PATCH 31/31] PPC: e500: Refactor serial dt generation

[Alexander Graf]

When generating serial port device tree nodes, we duplicate quite a bit
of code, because there are 2 of them in the mpc8544ds board we emulate.

Shove the generating code into a function, so we duplicate less code.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppce500_mpc8544ds.c |   54 +++++++++++++++++++++++------------------------
 1 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 6c01093..3c5a483 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -82,6 +82,28 @@ static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
     }
 }
 
+static void dt_serial_create(void *fdt, unsigned long long offset,
+                             const char *soc, const char *mpic,
+                             const char *alias, int idx, bool defcon)
+{
+    char ser[128];
+
+    sprintf(ser, "%s/serial@%llx", soc, offset);
+    qemu_devtree_add_subnode(fdt, ser);
+    qemu_devtree_setprop_string(fdt, ser, "device_type", "serial");
+    qemu_devtree_setprop_string(fdt, ser, "compatible", "ns16550");
+    qemu_devtree_setprop_cell2(fdt, ser, "reg", offset, 0x100);
+    qemu_devtree_setprop_cell(fdt, ser, "cell-index", idx);
+    qemu_devtree_setprop_cell(fdt, ser, "clock-frequency", 0);
+    qemu_devtree_setprop_cell4(fdt, ser, "interrupts", 42, 2, 0, 0);
+    qemu_devtree_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
+    qemu_devtree_setprop_string(fdt, "/aliases", alias, ser);
+
+    if (defcon) {
+        qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser);
+    }
+}
+
 static int mpc8544_load_device_tree(CPUPPCState *env,
                                     target_phys_addr_t addr,
                                     target_phys_addr_t ramsize,
@@ -101,8 +123,6 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
     char compatible_sb[] = "fsl,mpc8544-immr\0simple-bus";
     char model[] = "MPC8544DS";
     char soc[128];
-    char ser0[128];
-    char ser1[128];
     char mpic[128];
     uint32_t mpic_ph;
     char gutil[128];
@@ -274,32 +294,10 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
      * device it finds in the dt as serial output device. And we generate
      * devices in reverse order to the dt.
      */
-    sprintf(ser1, "%s/serial@%llx", soc,
-            MPC8544_SERIAL1_REGS_BASE - MPC8544_CCSRBAR_BASE);
-    qemu_devtree_add_subnode(fdt, ser1);
-    qemu_devtree_setprop_string(fdt, ser1, "device_type", "serial");
-    qemu_devtree_setprop_string(fdt, ser1, "compatible", "ns16550");
-    qemu_devtree_setprop_cell2(fdt, ser1, "reg", MPC8544_SERIAL1_REGS_BASE -
-                               MPC8544_CCSRBAR_BASE, 0x100);
-    qemu_devtree_setprop_cell(fdt, ser1, "cell-index", 1);
-    qemu_devtree_setprop_cell(fdt, ser1, "clock-frequency", 0);
-    qemu_devtree_setprop_cell4(fdt, ser1, "interrupts", 42, 2, 0, 0);
-    qemu_devtree_setprop_phandle(fdt, ser1, "interrupt-parent", mpic);
-    qemu_devtree_setprop_string(fdt, "/aliases", "serial1", ser1);
-
-    sprintf(ser0, "%s/serial@%llx", soc,
-            MPC8544_SERIAL0_REGS_BASE - MPC8544_CCSRBAR_BASE);
-    qemu_devtree_add_subnode(fdt, ser0);
-    qemu_devtree_setprop_string(fdt, ser0, "device_type", "serial");
-    qemu_devtree_setprop_string(fdt, ser0, "compatible", "ns16550");
-    qemu_devtree_setprop_cell2(fdt, ser0, "reg", MPC8544_SERIAL0_REGS_BASE -
-                               MPC8544_CCSRBAR_BASE, 0x100);
-    qemu_devtree_setprop_cell(fdt, ser0, "cell-index", 0);
-    qemu_devtree_setprop_cell(fdt, ser0, "clock-frequency", 0);
-    qemu_devtree_setprop_cell4(fdt, ser0, "interrupts", 42, 2, 0, 0);
-    qemu_devtree_setprop_phandle(fdt, ser0, "interrupt-parent", mpic);
-    qemu_devtree_setprop_string(fdt, "/aliases", "serial0", ser0);
-    qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser0);
+    dt_serial_create(fdt, MPC8544_SERIAL1_REGS_BASE - MPC8544_CCSRBAR_BASE,
+                     soc, mpic, "serial1", 1, false);
+    dt_serial_create(fdt, MPC8544_SERIAL0_REGS_BASE - MPC8544_CCSRBAR_BASE,
+                     soc, mpic, "serial0", 0, true);
 
     sprintf(gutil, "%s/global-utilities@%llx", soc,
             MPC8544_UTIL_BASE - MPC8544_CCSRBAR_BASE);
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Blue Swirl]

On Tue, Jun 5, 2012 at 11:52 PM, Alexander Graf <agraf@suse.de> wrote:
> Today we have two separate places where we keep information which device
> is where:
>
>  - hw/ppce500_mpc8544ds.c to instantiate all devices
>  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices
>
> Every time we split crucial information, things can go terribly wrong. If
> you update one file, but not the other, you can screw things up without
> realizing it quickly.
>
> The redundancy is also unnecessary, because QEMU already knows all the
> information at which addresses its devices live. So we can generate the
> device tree from the same variables - and even have the device tree adjust
> if something changes in there.
>
> The one functionality we lose with this approach is the ability to manually
> patch the device tree to contain additional devices. To still be able to do
> so easily, we introduce a new option -machine dumpdtb=<file> that creates a
> dtb output file which can be used with -machine dtb=<file> later. In between
> these 2 executions of QEMU, the dtb can be modified however much you like.
>
> A lot of bits in this patch set are still hardcoded. We also don't accomodate
> for dynamic creation of device tree nodes when -device is used. This requires
> a bit more QOM'ification for us to be able to loop through all devices, so we
> can dynamically create the device tree nodes for them. The basic concept should
> still hold as is though.
>
>
> Alex

Please use snprintf() instead of sprintf().

>
> v1 -> v2:
>
>  - rename cell64 -> u64
>  - don't treat memory as single u64
>  - remove commit id from patch description
>  - NEW: PPC: e500: Use new MPIC dt format
>         PPC: e500: Use new SOC dt format
>         PPC: e500: Define addresses as always 64bit
>         PPC: e500: Extend address/size of / to 64bit
>         dt: Add global option to set phandle start offset
>         PPC: e500: Refactor serial dt generation
>
> Alexander Graf (31):
>  dt: allow add_subnode to create root subnodes
>  dt: add helpers for 2, 3 and 4 cell adds
>  dt: add helper for phandle references
>  dt: temporarily disable subtree creation failure check
>  dt: add helper for phandle enumeration
>  dt: add helper for empty dt creation
>  dt: add helper for phandle allocation
>  dt: add helper for 64bit cell adds
>  PPC: e500: require libfdt
>  PPC: e500: dt: create memory node dynamically
>  PPC: e500: dt: create /cpus node dynamically
>  PPC: e500: dt: create /hypervisor node dynamically
>  PPC: e500: dt: create / node dynamically
>  PPC: e500: dt: create /chosen node dynamically
>  PPC: e500: dt: create /soc8544 node dynamically
>  PPC: e500: dt: create serial nodes dynamically
>  PPC: e500: dt: create mpic node dynamically
>  PPC: e500: dt: create global-utils node dynamically
>  PPC: e500: dt: create pci node dynamically
>  PPC: e500: dt: start with empty device tree
>  dt: Add -machine dumpdtb option to dump the current dtb
>  PPC: e500: dt: use 64bit cell helper
>  PPC: e500: dt: use target_phys_addr_t for ramsize
>  PPC: e500: enable manual loading of dtb blob
>  Revert "dt: temporarily disable subtree creation failure check"
>  PPC: e500: Use new MPIC dt format
>  PPC: e500: Use new SOC dt format
>  PPC: e500: Define addresses as always 64bit
>  PPC: e500: Extend address/size of / to 64bit
>  dt: Add global option to set phandle start offset
>  PPC: e500: Refactor serial dt generation
>
>  Makefile               |    1 -
>  Makefile.target        |    2 +-
>  device_tree.c          |  134 +++++++++++++++++++++++++++-
>  device_tree.h          |   16 +++
>  hw/ppce500_mpc8544ds.c |  237 +++++++++++++++++++++++++++++++++++++++++------
>  pc-bios/mpc8544ds.dtb  |  Bin 2028 -> 0 bytes
>  pc-bios/mpc8544ds.dts  |  119 ------------------------
>  qemu-config.c          |    8 ++
>  roms/openbios          |    2 +-
>  9 files changed, 365 insertions(+), 154 deletions(-)
>  delete mode 100644 pc-bios/mpc8544ds.dtb
>  delete mode 100644 pc-bios/mpc8544ds.dts
>
>

Re: [Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Alexander Graf]

On 07.06.2012, at 23:09, Blue Swirl wrote:

> On Tue, Jun 5, 2012 at 11:52 PM, Alexander Graf <agraf@suse.de> wrote:
>> Today we have two separate places where we keep information which device
>> is where:
>> 
>>  - hw/ppce500_mpc8544ds.c to instantiate all devices
>>  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices
>> 
>> Every time we split crucial information, things can go terribly wrong. If
>> you update one file, but not the other, you can screw things up without
>> realizing it quickly.
>> 
>> The redundancy is also unnecessary, because QEMU already knows all the
>> information at which addresses its devices live. So we can generate the
>> device tree from the same variables - and even have the device tree adjust
>> if something changes in there.
>> 
>> The one functionality we lose with this approach is the ability to manually
>> patch the device tree to contain additional devices. To still be able to do
>> so easily, we introduce a new option -machine dumpdtb=<file> that creates a
>> dtb output file which can be used with -machine dtb=<file> later. In between
>> these 2 executions of QEMU, the dtb can be modified however much you like.
>> 
>> A lot of bits in this patch set are still hardcoded. We also don't accomodate
>> for dynamic creation of device tree nodes when -device is used. This requires
>> a bit more QOM'ification for us to be able to loop through all devices, so we
>> can dynamically create the device tree nodes for them. The basic concept should
>> still hold as is though.
>> 
>> 
>> Alex
> 
> Please use snprintf() instead of sprintf().

Oh how much I would love to be able to just call asprintf() and call it a day :).


Alex

Re: [Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Blue Swirl]

On Tue, Jun 19, 2012 at 12:54 PM, Alexander Graf <agraf@suse.de> wrote:
>
> On 07.06.2012, at 23:09, Blue Swirl wrote:
>
>> On Tue, Jun 5, 2012 at 11:52 PM, Alexander Graf <agraf@suse.de> wrote:
>>> Today we have two separate places where we keep information which device
>>> is where:
>>>
>>>  - hw/ppce500_mpc8544ds.c to instantiate all devices
>>>  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices
>>>
>>> Every time we split crucial information, things can go terribly wrong. If
>>> you update one file, but not the other, you can screw things up without
>>> realizing it quickly.
>>>
>>> The redundancy is also unnecessary, because QEMU already knows all the
>>> information at which addresses its devices live. So we can generate the
>>> device tree from the same variables - and even have the device tree adjust
>>> if something changes in there.
>>>
>>> The one functionality we lose with this approach is the ability to manually
>>> patch the device tree to contain additional devices. To still be able to do
>>> so easily, we introduce a new option -machine dumpdtb=<file> that creates a
>>> dtb output file which can be used with -machine dtb=<file> later. In between
>>> these 2 executions of QEMU, the dtb can be modified however much you like.
>>>
>>> A lot of bits in this patch set are still hardcoded. We also don't accomodate
>>> for dynamic creation of device tree nodes when -device is used. This requires
>>> a bit more QOM'ification for us to be able to loop through all devices, so we
>>> can dynamically create the device tree nodes for them. The basic concept should
>>> still hold as is though.
>>>
>>>
>>> Alex
>>
>> Please use snprintf() instead of sprintf().
>
> Oh how much I would love to be able to just call asprintf() and call it a day :).

There's g_vasprintf(), which could be used to implement qemu_asprintf().

By the way, what is your plan about PPC patch queue, for example AREG0
patches? There may be some need for rebasing already.

>
>
> Alex
>

Re: [Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Alexander Graf]

On 19.06.2012, at 20:39, Blue Swirl wrote:

> On Tue, Jun 19, 2012 at 12:54 PM, Alexander Graf <agraf@suse.de> wrote:
>> 
>> On 07.06.2012, at 23:09, Blue Swirl wrote:
>> 
>>> On Tue, Jun 5, 2012 at 11:52 PM, Alexander Graf <agraf@suse.de> wrote:
>>>> Today we have two separate places where we keep information which device
>>>> is where:
>>>> 
>>>>  - hw/ppce500_mpc8544ds.c to instantiate all devices
>>>>  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices
>>>> 
>>>> Every time we split crucial information, things can go terribly wrong. If
>>>> you update one file, but not the other, you can screw things up without
>>>> realizing it quickly.
>>>> 
>>>> The redundancy is also unnecessary, because QEMU already knows all the
>>>> information at which addresses its devices live. So we can generate the
>>>> device tree from the same variables - and even have the device tree adjust
>>>> if something changes in there.
>>>> 
>>>> The one functionality we lose with this approach is the ability to manually
>>>> patch the device tree to contain additional devices. To still be able to do
>>>> so easily, we introduce a new option -machine dumpdtb=<file> that creates a
>>>> dtb output file which can be used with -machine dtb=<file> later. In between
>>>> these 2 executions of QEMU, the dtb can be modified however much you like.
>>>> 
>>>> A lot of bits in this patch set are still hardcoded. We also don't accomodate
>>>> for dynamic creation of device tree nodes when -device is used. This requires
>>>> a bit more QOM'ification for us to be able to loop through all devices, so we
>>>> can dynamically create the device tree nodes for them. The basic concept should
>>>> still hold as is though.
>>>> 
>>>> 
>>>> Alex
>>> 
>>> Please use snprintf() instead of sprintf().
>> 
>> Oh how much I would love to be able to just call asprintf() and call it a day :).
> 
> There's g_vasprintf(), which could be used to implement qemu_asprintf().

Oh? Certainly something to look at. Having all those statically sized arrays on the stack makes me nervous.

> By the way, what is your plan about PPC patch queue, for example AREG0
> patches? There may be some need for rebasing already.

I already did rebase them, no worries :). I'll send out a PULL request soon.


Alex

[Qemu-devel] [PATCH 00/31] PPC: mpc8544ds: Create device tree dynamically

[Alexander Graf]

Today we have two separate places where we keep information which device
is where:

  - hw/ppce500_mpc8544ds.c to instantiate all devices
  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices

Every time we split crucial information, things can go terribly wrong. If
you update one file, but not the other, you can screw things up without
realizing it quickly.

The redundancy is also unnecessary, because QEMU already knows all the
information at which addresses its devices live. So we can generate the
device tree from the same variables - and even have the device tree adjust
if something changes in there.

The one functionality we lose with this approach is the ability to manually
patch the device tree to contain additional devices. To still be able to do
so easily, we introduce a new option -machine dumpdtb=<file> that creates a
dtb output file which can be used with -machine dtb=<file> later. In between
these 2 executions of QEMU, the dtb can be modified however much you like.

A lot of bits in this patch set are still hardcoded. We also don't accomodate
for dynamic creation of device tree nodes when -device is used. This requires
a bit more QOM'ification for us to be able to loop through all devices, so we
can dynamically create the device tree nodes for them. The basic concept should
still hold as is though.


Alex

v1 -> v2:

  - rename cell64 -> u64
  - don't treat memory as single u64
  - remove commit id from patch description
  - NEW: PPC: e500: Use new MPIC dt format
         PPC: e500: Use new SOC dt format
         PPC: e500: Define addresses as always 64bit
         PPC: e500: Extend address/size of / to 64bit
         dt: Add global option to set phandle start offset
         PPC: e500: Refactor serial dt generation

v2 -> v3:

  - [phandle helper] rename "string" to target_node_path
  - [phandle helper] add correct header includes
  - use snprintf
  - create and use new multi-cell setting api

Alexander Graf (31):
  dt: allow add_subnode to create root subnodes
  dt: add helpers for multi-cell adds
  dt: add helper for phandle references
  dt: temporarily disable subtree creation failure check
  dt: add helper for phandle enumeration
  dt: add helper for empty dt creation
  dt: add helper for phandle allocation
  dt: add helper for 64bit cell adds
  PPC: e500: require libfdt
  PPC: e500: dt: create memory node dynamically
  PPC: e500: dt: create /cpus node dynamically
  PPC: e500: dt: create /hypervisor node dynamically
  PPC: e500: dt: create / node dynamically
  PPC: e500: dt: create /chosen node dynamically
  PPC: e500: dt: create /soc8544 node dynamically
  PPC: e500: dt: create serial nodes dynamically
  PPC: e500: dt: create mpic node dynamically
  PPC: e500: dt: create global-utils node dynamically
  PPC: e500: dt: create pci node dynamically
  PPC: e500: dt: start with empty device tree
  dt: Add -machine dumpdtb option to dump the current dtb
  PPC: e500: dt: use 64bit cell helper
  PPC: e500: dt: use target_phys_addr_t for ramsize
  PPC: e500: enable manual loading of dtb blob
  Revert "dt: temporarily disable subtree creation failure check"
  PPC: e500: Use new MPIC dt format
  PPC: e500: Use new SOC dt format
  PPC: e500: Define addresses as always 64bit
  PPC: e500: Extend address/size of / to 64bit
  dt: Add global option to set phandle start offset
  PPC: e500: Refactor serial dt generation

 Makefile               |    1 -
 device_tree.c          |  106 +++++++++++++++++++++-
 device_tree.h          |   20 ++++
 hw/ppc/Makefile.objs   |    2 +-
 hw/ppce500_mpc8544ds.c |  237 +++++++++++++++++++++++++++++++++++++++++------
 pc-bios/mpc8544ds.dtb  |  Bin 2028 -> 0 bytes
 pc-bios/mpc8544ds.dts  |  119 ------------------------
 qemu-config.c          |    8 ++
 8 files changed, 340 insertions(+), 153 deletions(-)
 delete mode 100644 pc-bios/mpc8544ds.dtb
 delete mode 100644 pc-bios/mpc8544ds.dts

[Qemu-devel] [PATCH 04/31] dt: temporarily disable subtree creation failure check

[Alexander Graf]

Usually we want to know when creating a subtree fails. However, while
introducing this patch set we have to modify the device tree and some
times have the code to create a subtree in both the binary tree and
the dynamically created tree.

So ignore failures about this for now and enable them once we got rid
of the binary device tree.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 device_tree.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index 2905f9a..967c97a 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -174,11 +174,13 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
     }
 
     retval = fdt_add_subnode(fdt, parent, basename);
+#if 0
     if (retval < 0) {
         fprintf(stderr, "FDT: Failed to create subnode %s: %s\n", name,
                 fdt_strerror(retval));
         exit(1);
     }
+#endif
 
     g_free(dupname);
     return retval;
-- 
1.6.0.2