* [Qemu-devel] [RFC PATCH v1 0/3] Device tree cleanups
@ 2013-07-12 4:27 peter.crosthwaite
2013-07-12 4:28 ` [Qemu-devel] [RFC PATCH v1 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: peter.crosthwaite @ 2013-07-12 4:27 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, agraf, david
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Fix the name stem of the devicetree API (P1 - s/qemu_devtree/qemu_fdt)
and start the error reporting cleanup (P3). Trivial patch P2 fixing an
arugment name along the way.
Looking for stylistic review comments on patch 3, before I go and apply
the change to the entire API.
Peter Crosthwaite (3):
device_tree: s/qemu_devtree/qemu_fdt globally
device_tree: qemu_fdt_setprop: Rename val_array arg
device_tree: qemu_fdt_setprop: Fixup error reporting
device_tree.c | 75 +++++++++------
hw/arm/boot.c | 25 ++---
hw/microblaze/boot.c | 4 +-
hw/ppc/e500.c | 217 ++++++++++++++++++++++---------------------
hw/ppc/e500plat.c | 6 +-
hw/ppc/mpc8544ds.c | 6 +-
hw/ppc/ppc440_bamboo.c | 28 +++---
hw/ppc/spapr_rtas.c | 16 ++--
hw/ppc/virtex_ml507.c | 2 +-
include/sysemu/device_tree.h | 133 +++++++++++++++++++++-----
10 files changed, 310 insertions(+), 202 deletions(-)
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [RFC PATCH v1 1/3] device_tree: s/qemu_devtree/qemu_fdt globally
2013-07-12 4:27 [Qemu-devel] [RFC PATCH v1 0/3] Device tree cleanups peter.crosthwaite
@ 2013-07-12 4:28 ` peter.crosthwaite
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
2 siblings, 0 replies; 7+ messages in thread
From: peter.crosthwaite @ 2013-07-12 4:28 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, agraf, david
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
The qemu_devtree API is a wrapper around the fdt_ set of APIs.
Rename accordingly.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
device_tree.c | 48 +++++-----
hw/arm/boot.c | 22 ++---
hw/microblaze/boot.c | 4 +-
hw/ppc/e500.c | 215 ++++++++++++++++++++++---------------------
hw/ppc/e500plat.c | 6 +-
hw/ppc/mpc8544ds.c | 6 +-
hw/ppc/ppc440_bamboo.c | 24 ++---
hw/ppc/spapr_rtas.c | 16 ++--
hw/ppc/virtex_ml507.c | 2 +-
include/sysemu/device_tree.h | 46 ++++-----
10 files changed, 195 insertions(+), 194 deletions(-)
diff --git a/device_tree.c b/device_tree.c
index 69be9da..9df78c2 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -126,8 +126,8 @@ static int findnode_nofail(void *fdt, const char *node_path)
return offset;
}
-int qemu_devtree_setprop(void *fdt, const char *node_path,
- const char *property, const void *val_array, int size)
+int qemu_fdt_setprop(void *fdt, const char *node_path,
+ const char *property, const void *val_array, int size)
{
int r;
@@ -141,8 +141,8 @@ int qemu_devtree_setprop(void *fdt, const char *node_path,
return r;
}
-int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
- const char *property, uint32_t val)
+int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
+ const char *property, uint32_t val)
{
int r;
@@ -156,15 +156,15 @@ 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)
+int qemu_fdt_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));
+ return qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val));
}
-int qemu_devtree_setprop_string(void *fdt, const char *node_path,
- const char *property, const char *string)
+int qemu_fdt_setprop_string(void *fdt, const char *node_path,
+ const char *property, const char *string)
{
int r;
@@ -178,8 +178,8 @@ int qemu_devtree_setprop_string(void *fdt, const char *node_path,
return r;
}
-const void *qemu_devtree_getprop(void *fdt, const char *node_path,
- const char *property, int *lenp)
+const void *qemu_fdt_getprop(void *fdt, const char *node_path,
+ const char *property, int *lenp)
{
int len;
const void *r;
@@ -195,11 +195,11 @@ const void *qemu_devtree_getprop(void *fdt, const char *node_path,
return r;
}
-uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path,
- const char *property)
+uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
+ const char *property)
{
int len;
- const uint32_t *p = qemu_devtree_getprop(fdt, node_path, property, &len);
+ const uint32_t *p = qemu_fdt_getprop(fdt, node_path, property, &len);
if (len != 4) {
fprintf(stderr, "%s: %s/%s not 4 bytes long (not a cell?)\n",
__func__, node_path, property);
@@ -208,7 +208,7 @@ uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path,
return be32_to_cpu(*p);
}
-uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
+uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
{
uint32_t r;
@@ -222,15 +222,15 @@ uint32_t qemu_devtree_get_phandle(void *fdt, const char *path)
return r;
}
-int qemu_devtree_setprop_phandle(void *fdt, const char *node_path,
- const char *property,
- const char *target_node_path)
+int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
+ const char *property,
+ const char *target_node_path)
{
- uint32_t phandle = qemu_devtree_get_phandle(fdt, target_node_path);
- return qemu_devtree_setprop_cell(fdt, node_path, property, phandle);
+ uint32_t phandle = qemu_fdt_get_phandle(fdt, target_node_path);
+ return qemu_fdt_setprop_cell(fdt, node_path, property, phandle);
}
-uint32_t qemu_devtree_alloc_phandle(void *fdt)
+uint32_t qemu_fdt_alloc_phandle(void *fdt)
{
static int phandle = 0x0;
@@ -261,7 +261,7 @@ uint32_t qemu_devtree_alloc_phandle(void *fdt)
return phandle++;
}
-int qemu_devtree_nop_node(void *fdt, const char *node_path)
+int qemu_fdt_nop_node(void *fdt, const char *node_path)
{
int r;
@@ -275,7 +275,7 @@ int qemu_devtree_nop_node(void *fdt, const char *node_path)
return r;
}
-int qemu_devtree_add_subnode(void *fdt, const char *name)
+int qemu_fdt_add_subnode(void *fdt, const char *name)
{
char *dupname = g_strdup(name);
char *basename = strrchr(dupname, '/');
@@ -305,7 +305,7 @@ int qemu_devtree_add_subnode(void *fdt, const char *name)
return retval;
}
-void qemu_devtree_dumpdtb(void *fdt, int size)
+void qemu_fdt_dumpdtb(void *fdt, int size)
{
QemuOpts *machine_opts;
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7c0090f..2768b2b 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -248,8 +248,8 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
}
g_free(filename);
- acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
- scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
+ acells = qemu_fdt_getprop_cell(fdt, "/", "#address-cells");
+ scells = qemu_fdt_getprop_cell(fdt, "/", "#size-cells");
if (acells == 0 || scells == 0) {
fprintf(stderr, "dtb file invalid (#address-cells or #size-cells 0)\n");
goto fail;
@@ -276,16 +276,16 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
goto fail;
}
- rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
- mem_reg_propsize * sizeof(uint32_t));
+ rc = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+ mem_reg_propsize * sizeof(uint32_t));
if (rc < 0) {
fprintf(stderr, "couldn't set /memory/reg\n");
goto fail;
}
if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
- rc = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
- binfo->kernel_cmdline);
+ rc = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+ binfo->kernel_cmdline);
if (rc < 0) {
fprintf(stderr, "couldn't set /chosen/bootargs\n");
goto fail;
@@ -293,21 +293,21 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
}
if (binfo->initrd_size) {
- rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
- binfo->initrd_start);
+ rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+ binfo->initrd_start);
if (rc < 0) {
fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
goto fail;
}
- rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
- binfo->initrd_start + binfo->initrd_size);
+ rc = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+ binfo->initrd_start + binfo->initrd_size);
if (rc < 0) {
fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
goto fail;
}
}
- qemu_devtree_dumpdtb(fdt, size);
+ qemu_fdt_dumpdtb(fdt, size);
cpu_physical_memory_write(addr, fdt, size);
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 3f1d70e..f66c546 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -72,8 +72,8 @@ static int microblaze_load_dtb(hwaddr addr,
}
if (kernel_cmdline) {
- r = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
- kernel_cmdline);
+ r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+ kernel_cmdline);
if (r < 0) {
fprintf(stderr, "couldn't set /chosen/bootargs\n");
}
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 69837a5..0c2b54d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -108,18 +108,18 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
char ser[128];
snprintf(ser, sizeof(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_cells(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_cells(fdt, ser, "interrupts", 42, 2);
- qemu_devtree_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
- qemu_devtree_setprop_string(fdt, "/aliases", alias, ser);
+ qemu_fdt_add_subnode(fdt, ser);
+ qemu_fdt_setprop_string(fdt, ser, "device_type", "serial");
+ qemu_fdt_setprop_string(fdt, ser, "compatible", "ns16550");
+ qemu_fdt_setprop_cells(fdt, ser, "reg", offset, 0x100);
+ qemu_fdt_setprop_cell(fdt, ser, "cell-index", idx);
+ qemu_fdt_setprop_cell(fdt, ser, "clock-frequency", 0);
+ qemu_fdt_setprop_cells(fdt, ser, "interrupts", 42, 2);
+ qemu_fdt_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
+ qemu_fdt_setprop_string(fdt, "/aliases", alias, ser);
if (defcon) {
- qemu_devtree_setprop_string(fdt, "/chosen", "linux,stdout-path", ser);
+ qemu_fdt_setprop_string(fdt, "/chosen", "linux,stdout-path", ser);
}
}
@@ -187,31 +187,31 @@ static int ppce500_load_device_tree(CPUPPCState *env,
}
/* Manipulate device tree in memory. */
- qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 2);
- qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 2);
+ qemu_fdt_setprop_cell(fdt, "/", "#address-cells", 2);
+ qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 2);
- 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));
+ qemu_fdt_add_subnode(fdt, "/memory");
+ qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
+ qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+ sizeof(mem_reg_property));
- qemu_devtree_add_subnode(fdt, "/chosen");
+ qemu_fdt_add_subnode(fdt, "/chosen");
if (initrd_size) {
- ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
- initrd_base);
+ ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+ initrd_base);
if (ret < 0) {
fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
}
- ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
- (initrd_base + initrd_size));
+ ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+ (initrd_base + initrd_size));
if (ret < 0) {
fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
}
}
- ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
- params->kernel_cmdline);
+ ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+ params->kernel_cmdline);
if (ret < 0)
fprintf(stderr, "couldn't set /chosen/bootargs\n");
@@ -221,22 +221,22 @@ static int ppce500_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");
+ qemu_fdt_add_subnode(fdt, "/hypervisor");
+ qemu_fdt_setprop_string(fdt, "/hypervisor", "compatible",
+ "linux,kvm");
kvmppc_get_hypercall(env, hypercall, sizeof(hypercall));
- qemu_devtree_setprop(fdt, "/hypervisor", "hcall-instructions",
- hypercall, sizeof(hypercall));
+ qemu_fdt_setprop(fdt, "/hypervisor", "hcall-instructions",
+ hypercall, sizeof(hypercall));
/* if KVM supports the idle hcall, set property indicating this */
if (kvmppc_get_hasidle(env)) {
- qemu_devtree_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
+ qemu_fdt_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
}
}
/* 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);
+ qemu_fdt_add_subnode(fdt, "/cpus");
+ qemu_fdt_setprop_cell(fdt, "/cpus", "#address-cells", 1);
+ qemu_fdt_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 */
@@ -253,55 +253,56 @@ static int ppce500_load_device_tree(CPUPPCState *env,
snprintf(cpu_name, sizeof(cpu_name), "/cpus/PowerPC,8544@%x",
cpu->cpu_index);
- qemu_devtree_add_subnode(fdt, cpu_name);
- qemu_devtree_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
- qemu_devtree_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
- qemu_devtree_setprop_string(fdt, cpu_name, "device_type", "cpu");
- qemu_devtree_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
- qemu_devtree_setprop_cell(fdt, cpu_name, "d-cache-line-size",
- env->dcache_line_size);
- qemu_devtree_setprop_cell(fdt, cpu_name, "i-cache-line-size",
- env->icache_line_size);
- qemu_devtree_setprop_cell(fdt, cpu_name, "d-cache-size", 0x8000);
- qemu_devtree_setprop_cell(fdt, cpu_name, "i-cache-size", 0x8000);
- qemu_devtree_setprop_cell(fdt, cpu_name, "bus-frequency", 0);
+ qemu_fdt_add_subnode(fdt, cpu_name);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "clock-frequency", clock_freq);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "timebase-frequency", tb_freq);
+ qemu_fdt_setprop_string(fdt, cpu_name, "device_type", "cpu");
+ qemu_fdt_setprop_cell(fdt, cpu_name, "reg", cpu->cpu_index);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-line-size",
+ env->dcache_line_size);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-line-size",
+ env->icache_line_size);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "d-cache-size", 0x8000);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "i-cache-size", 0x8000);
+ qemu_fdt_setprop_cell(fdt, cpu_name, "bus-frequency", 0);
if (cpu->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_u64(fdt, cpu_name, "cpu-release-addr",
- cpu_release_addr);
+ qemu_fdt_setprop_string(fdt, cpu_name, "status", "disabled");
+ qemu_fdt_setprop_string(fdt, cpu_name, "enable-method",
+ "spin-table");
+ qemu_fdt_setprop_u64(fdt, cpu_name, "cpu-release-addr",
+ cpu_release_addr);
} else {
- qemu_devtree_setprop_string(fdt, cpu_name, "status", "okay");
+ qemu_fdt_setprop_string(fdt, cpu_name, "status", "okay");
}
}
- qemu_devtree_add_subnode(fdt, "/aliases");
+ qemu_fdt_add_subnode(fdt, "/aliases");
/* XXX These should go into their respective devices' code */
snprintf(soc, sizeof(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,
- sizeof(compatible_sb));
- qemu_devtree_setprop_cell(fdt, soc, "#address-cells", 1);
- qemu_devtree_setprop_cell(fdt, soc, "#size-cells", 1);
- qemu_devtree_setprop_cells(fdt, soc, "ranges", 0x0,
- MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
- MPC8544_CCSRBAR_SIZE);
+ qemu_fdt_add_subnode(fdt, soc);
+ qemu_fdt_setprop_string(fdt, soc, "device_type", "soc");
+ qemu_fdt_setprop(fdt, soc, "compatible", compatible_sb,
+ sizeof(compatible_sb));
+ qemu_fdt_setprop_cell(fdt, soc, "#address-cells", 1);
+ qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1);
+ qemu_fdt_setprop_cells(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);
+ qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0);
snprintf(mpic, sizeof(mpic), "%s/pic@%llx", soc, MPC8544_MPIC_REGS_OFFSET);
- qemu_devtree_add_subnode(fdt, mpic);
- qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
- qemu_devtree_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
- qemu_devtree_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
- 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);
+ qemu_fdt_add_subnode(fdt, mpic);
+ qemu_fdt_setprop_string(fdt, mpic, "device_type", "open-pic");
+ qemu_fdt_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
+ qemu_fdt_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
+ 0x40000);
+ qemu_fdt_setprop_cell(fdt, mpic, "#address-cells", 0);
+ qemu_fdt_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
+ mpic_ph = qemu_fdt_alloc_phandle(fdt);
+ qemu_fdt_setprop_cell(fdt, mpic, "phandle", mpic_ph);
+ qemu_fdt_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
+ qemu_fdt_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
/*
* We have to generate ser1 first, because Linux takes the first
@@ -315,19 +316,19 @@ static int ppce500_load_device_tree(CPUPPCState *env,
snprintf(gutil, sizeof(gutil), "%s/global-utilities@%llx", soc,
MPC8544_UTIL_OFFSET);
- qemu_devtree_add_subnode(fdt, gutil);
- qemu_devtree_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
- qemu_devtree_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
- qemu_devtree_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
+ qemu_fdt_add_subnode(fdt, gutil);
+ qemu_fdt_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
+ qemu_fdt_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
+ qemu_fdt_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
snprintf(msi, sizeof(msi), "/%s/msi@%llx", soc, MPC8544_MSI_REGS_OFFSET);
- qemu_devtree_add_subnode(fdt, msi);
- qemu_devtree_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
- qemu_devtree_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
- msi_ph = qemu_devtree_alloc_phandle(fdt);
- qemu_devtree_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
- qemu_devtree_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
- qemu_devtree_setprop_cells(fdt, msi, "interrupts",
+ qemu_fdt_add_subnode(fdt, msi);
+ qemu_fdt_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
+ qemu_fdt_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
+ msi_ph = qemu_fdt_alloc_phandle(fdt);
+ qemu_fdt_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
+ qemu_fdt_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
+ qemu_fdt_setprop_cells(fdt, msi, "interrupts",
0xe0, 0x0,
0xe1, 0x0,
0xe2, 0x0,
@@ -336,45 +337,45 @@ static int ppce500_load_device_tree(CPUPPCState *env,
0xe5, 0x0,
0xe6, 0x0,
0xe7, 0x0);
- qemu_devtree_setprop_cell(fdt, msi, "phandle", msi_ph);
- qemu_devtree_setprop_cell(fdt, msi, "linux,phandle", msi_ph);
+ qemu_fdt_setprop_cell(fdt, msi, "phandle", msi_ph);
+ qemu_fdt_setprop_cell(fdt, msi, "linux,phandle", msi_ph);
snprintf(pci, sizeof(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");
- qemu_devtree_setprop_string(fdt, pci, "device_type", "pci");
- qemu_devtree_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
- 0x0, 0x7);
- pci_map = pci_map_create(fdt, qemu_devtree_get_phandle(fdt, mpic),
+ qemu_fdt_add_subnode(fdt, pci);
+ qemu_fdt_setprop_cell(fdt, pci, "cell-index", 0);
+ qemu_fdt_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
+ qemu_fdt_setprop_string(fdt, pci, "device_type", "pci");
+ qemu_fdt_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
+ 0x0, 0x7);
+ pci_map = pci_map_create(fdt, qemu_fdt_get_phandle(fdt, mpic),
params->pci_first_slot, params->pci_nr_slots,
&len);
- qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, len);
- qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
- qemu_devtree_setprop_cells(fdt, pci, "interrupts", 24, 2);
- qemu_devtree_setprop_cells(fdt, pci, "bus-range", 0, 255);
+ qemu_fdt_setprop(fdt, pci, "interrupt-map", pci_map, len);
+ qemu_fdt_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
+ qemu_fdt_setprop_cells(fdt, pci, "interrupts", 24, 2);
+ qemu_fdt_setprop_cells(fdt, pci, "bus-range", 0, 255);
for (i = 0; i < 14; i++) {
pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
}
- qemu_devtree_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
- qemu_devtree_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
- qemu_devtree_setprop_cells(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);
- qemu_devtree_setprop_cell(fdt, pci, "#address-cells", 3);
- qemu_devtree_setprop_string(fdt, "/aliases", "pci0", pci);
+ qemu_fdt_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
+ qemu_fdt_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
+ qemu_fdt_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
+ MPC8544_PCI_REGS_BASE, 0, 0x1000);
+ qemu_fdt_setprop_cell(fdt, pci, "clock-frequency", 66666666);
+ qemu_fdt_setprop_cell(fdt, pci, "#interrupt-cells", 1);
+ qemu_fdt_setprop_cell(fdt, pci, "#size-cells", 2);
+ qemu_fdt_setprop_cell(fdt, pci, "#address-cells", 3);
+ qemu_fdt_setprop_string(fdt, "/aliases", "pci0", pci);
params->fixup_devtree(params, fdt);
if (toplevel_compat) {
- qemu_devtree_setprop(fdt, "/", "compatible", toplevel_compat,
- strlen(toplevel_compat) + 1);
+ qemu_fdt_setprop(fdt, "/", "compatible", toplevel_compat,
+ strlen(toplevel_compat) + 1);
}
done:
- qemu_devtree_dumpdtb(fdt, fdt_size);
+ qemu_fdt_dumpdtb(fdt, fdt_size);
ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
if (ret < 0) {
goto out;
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index c852995..6eecdc5 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -23,9 +23,9 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
const char model[] = "QEMU ppce500";
const char compatible[] = "fsl,qemu-e500";
- qemu_devtree_setprop(fdt, "/", "model", model, sizeof(model));
- qemu_devtree_setprop(fdt, "/", "compatible", compatible,
- sizeof(compatible));
+ qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
+ qemu_fdt_setprop(fdt, "/", "compatible", compatible,
+ sizeof(compatible));
}
static void e500plat_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 444da02..4ed66f1 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -21,9 +21,9 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
const char model[] = "MPC8544DS";
const char compatible[] = "MPC8544DS\0MPC85xxDS";
- qemu_devtree_setprop(fdt, "/", "model", model, sizeof(model));
- qemu_devtree_setprop(fdt, "/", "compatible", compatible,
- sizeof(compatible));
+ qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
+ qemu_fdt_setprop(fdt, "/", "compatible", compatible,
+ sizeof(compatible));
}
static void mpc8544ds_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index 5b039ab..db9d38b 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -77,23 +77,23 @@ static int bamboo_load_device_tree(hwaddr addr,
/* Manipulate device tree in memory. */
- ret = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
- sizeof(mem_reg_property));
+ ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+ sizeof(mem_reg_property));
if (ret < 0)
fprintf(stderr, "couldn't set /memory/reg\n");
- ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
- initrd_base);
+ ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+ initrd_base);
if (ret < 0)
fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
- ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
- (initrd_base + initrd_size));
+ ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+ (initrd_base + initrd_size));
if (ret < 0)
fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
- ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
- kernel_cmdline);
+ ret = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs",
+ kernel_cmdline);
if (ret < 0)
fprintf(stderr, "couldn't set /chosen/bootargs\n");
@@ -105,10 +105,10 @@ static int bamboo_load_device_tree(hwaddr addr,
clock_freq = kvmppc_get_clockfreq();
}
- qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
- clock_freq);
- qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
- tb_freq);
+ qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
+ clock_freq);
+ qemu_fdt_setprop_cell(fdt, "/cpus/cpu@0", "timebase-frequency",
+ tb_freq);
ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
g_free(fdt);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 394ce05..dde1b6a 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -269,24 +269,24 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
return ret;
}
- ret = qemu_devtree_setprop_cell(fdt, "/rtas", "linux,rtas-base",
- rtas_addr);
+ ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-base",
+ rtas_addr);
if (ret < 0) {
fprintf(stderr, "Couldn't add linux,rtas-base property: %s\n",
fdt_strerror(ret));
return ret;
}
- ret = qemu_devtree_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
- rtas_addr);
+ ret = qemu_fdt_setprop_cell(fdt, "/rtas", "linux,rtas-entry",
+ rtas_addr);
if (ret < 0) {
fprintf(stderr, "Couldn't add linux,rtas-entry property: %s\n",
fdt_strerror(ret));
return ret;
}
- ret = qemu_devtree_setprop_cell(fdt, "/rtas", "rtas-size",
- rtas_size);
+ ret = qemu_fdt_setprop_cell(fdt, "/rtas", "rtas-size",
+ rtas_size);
if (ret < 0) {
fprintf(stderr, "Couldn't add rtas-size property: %s\n",
fdt_strerror(ret));
@@ -300,8 +300,8 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
continue;
}
- ret = qemu_devtree_setprop_cell(fdt, "/rtas", call->name,
- i + TOKEN_BASE);
+ ret = qemu_fdt_setprop_cell(fdt, "/rtas", call->name,
+ i + TOKEN_BASE);
if (ret < 0) {
fprintf(stderr, "Couldn't add rtas token for %s: %s\n",
call->name, fdt_strerror(ret));
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 08e77fb..4953375 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -157,7 +157,7 @@ static int xilinx_load_device_tree(hwaddr addr,
}
}
- r = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
+ r = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", kernel_cmdline);
if (r < 0)
fprintf(stderr, "couldn't set /chosen/bootargs\n");
cpu_physical_memory_write(addr, fdt, fdt_size);
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index f0b3f35..19a017e 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -17,27 +17,27 @@
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,
- const char *property, const 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_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 *target_node_path);
-const void *qemu_devtree_getprop(void *fdt, const char *node_path,
- const char *property, int *lenp);
-uint32_t qemu_devtree_getprop_cell(void *fdt, const char *node_path,
- const char *property);
-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);
+int qemu_fdt_setprop(void *fdt, const char *node_path,
+ const char *property, const void *val_array, int size);
+int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
+ const char *property, uint32_t val);
+int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+ const char *property, uint64_t val);
+int qemu_fdt_setprop_string(void *fdt, const char *node_path,
+ const char *property, const char *string);
+int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
+ const char *property,
+ const char *target_node_path);
+const void *qemu_fdt_getprop(void *fdt, const char *node_path,
+ const char *property, int *lenp);
+uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
+ const char *property);
+uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
+uint32_t qemu_fdt_alloc_phandle(void *fdt);
+int qemu_fdt_nop_node(void *fdt, const char *node_path);
+int qemu_fdt_add_subnode(void *fdt, const char *name);
-#define qemu_devtree_setprop_cells(fdt, node_path, property, ...) \
+#define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
do { \
uint32_t qdt_tmp[] = { __VA_ARGS__ }; \
int i; \
@@ -45,10 +45,10 @@ int qemu_devtree_add_subnode(void *fdt, const char *name);
for (i = 0; i < ARRAY_SIZE(qdt_tmp); i++) { \
qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); \
} \
- qemu_devtree_setprop(fdt, node_path, property, qdt_tmp, \
- sizeof(qdt_tmp)); \
+ qemu_fdt_setprop(fdt, node_path, property, qdt_tmp, \
+ sizeof(qdt_tmp)); \
} while (0)
-void qemu_devtree_dumpdtb(void *fdt, int size);
+void qemu_fdt_dumpdtb(void *fdt, int size);
#endif /* __DEVICE_TREE_H__ */
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [RFC PATCH v1 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg
2013-07-12 4:27 [Qemu-devel] [RFC PATCH v1 0/3] Device tree cleanups peter.crosthwaite
2013-07-12 4:28 ` [Qemu-devel] [RFC PATCH v1 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
@ 2013-07-12 4:29 ` peter.crosthwaite
2013-07-20 11:44 ` Andreas Färber
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
2 siblings, 1 reply; 7+ messages in thread
From: peter.crosthwaite @ 2013-07-12 4:29 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, agraf, david
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Looking at the implementation, this doesn't really have a lot to do
with arrays. Its just a pointer to a buffer and is passed through
to the wrapped fn (qemu_fdt_setprop) unchanged. So rename to make it
consistent with libfdt, which in the wrapped function just calls it
"val".
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
device_tree.c | 4 ++--
include/sysemu/device_tree.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/device_tree.c b/device_tree.c
index 9df78c2..048b8e1 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -127,11 +127,11 @@ static int findnode_nofail(void *fdt, const char *node_path)
}
int qemu_fdt_setprop(void *fdt, const char *node_path,
- const char *property, const void *val_array, int size)
+ const char *property, const void *val, int size)
{
int r;
- r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val_array, size);
+ r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size);
if (r < 0) {
fprintf(stderr, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
property, fdt_strerror(r));
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 19a017e..b4650c5 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -18,7 +18,7 @@ void *create_device_tree(int *sizep);
void *load_device_tree(const char *filename_path, int *sizep);
int qemu_fdt_setprop(void *fdt, const char *node_path,
- const char *property, const void *val_array, int size);
+ const char *property, const void *val, int size);
int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
const char *property, uint32_t val);
int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
2013-07-12 4:27 [Qemu-devel] [RFC PATCH v1 0/3] Device tree cleanups peter.crosthwaite
2013-07-12 4:28 ` [Qemu-devel] [RFC PATCH v1 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
@ 2013-07-12 4:29 ` peter.crosthwaite
2013-07-20 2:36 ` Peter Crosthwaite
2013-07-20 12:07 ` Andreas Färber
2 siblings, 2 replies; 7+ messages in thread
From: peter.crosthwaite @ 2013-07-12 4:29 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, agraf, david
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
There are a mix of usages of the qemu_fdt_* API calls, some which
wish to assert and abort QEMU on failure and some of which wish to do
their own error handling. The latter in more correct, but the former
is in the majority and more pragmatic, so facilitate both schemes by
creating asserting and non asserting variants. This patch does this
for qemu fdt_setprop and its wrapping users:
* qemu_fdt_setprop
* qemu_fdt_setprop_u64
* qemu_fdt_setprop_cells
Error reporting is stylistically udpated to use Error ** instead of
integer return codes and exit(1).
Users of these APIs that ignore the return code are converted to using
the _assert variants. Users that check the return code are converted to
use Error ** for their error detection paths.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
If this is ok, I will apply the change pattern to the entire
qemu_fdt API
device_tree.c | 35 ++++++++++++----
hw/arm/boot.c | 7 ++--
hw/ppc/e500.c | 66 +++++++++++++++---------------
hw/ppc/e500plat.c | 6 +--
hw/ppc/mpc8544ds.c | 6 +--
hw/ppc/ppc440_bamboo.c | 8 ++--
include/sysemu/device_tree.h | 97 +++++++++++++++++++++++++++++++++++++++++---
7 files changed, 166 insertions(+), 59 deletions(-)
diff --git a/device_tree.c b/device_tree.c
index 048b8e1..ca2a88d 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -4,8 +4,10 @@
* interface.
*
* Copyright 2008 IBM Corporation.
+ * Copyright 2013 Xilinx Inc.
* Authors: Jerone Young <jyoung5@us.ibm.com>
* Hollis Blanchard <hollisb@us.ibm.com>
+ * Peter Crosthwaite <peter.crosthwaite@xilinx.com>
*
* This work is licensed under the GNU GPL license version 2 or later.
*
@@ -126,19 +128,25 @@ static int findnode_nofail(void *fdt, const char *node_path)
return offset;
}
-int qemu_fdt_setprop(void *fdt, const char *node_path,
- const char *property, const void *val, int size)
+void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
+ const void *val, int size, Error **errp)
{
int r;
r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size);
if (r < 0) {
- fprintf(stderr, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
- property, fdt_strerror(r));
- exit(1);
+ error_setg(errp, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
+ property, fdt_strerror(r));
}
+}
- return r;
+void qemu_fdt_setprop_assert(void *fdt, const char *node_path,
+ const char *property, const void *val, int size)
+{
+ Error *errp = NULL;
+
+ qemu_fdt_setprop(fdt, node_path, property, val, size, &errp);
+ assert_no_error(errp);
}
int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
@@ -156,11 +164,20 @@ int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
return r;
}
-int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
- const char *property, uint64_t val)
+void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+ const char *property, uint64_t val, Error **errp)
{
val = cpu_to_be64(val);
- return qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val));
+ qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val), errp);
+}
+
+void qemu_fdt_setprop_u64_assert(void *fdt, const char *node_path,
+ const char *property, uint64_t val)
+{
+ Error *errp = NULL;
+
+ qemu_fdt_setprop_u64(fdt, node_path, property, val, &errp);
+ assert_no_error(errp);
}
int qemu_fdt_setprop_string(void *fdt, const char *node_path,
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 2768b2b..9164bb9 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -233,6 +233,7 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
char *filename;
int size, rc;
uint32_t acells, scells, hival;
+ Error *errp = NULL;
filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
if (!filename) {
@@ -276,9 +277,9 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
goto fail;
}
- rc = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
- mem_reg_propsize * sizeof(uint32_t));
- if (rc < 0) {
+ qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+ mem_reg_propsize * sizeof(uint32_t), &errp);
+ if (errp) {
fprintf(stderr, "couldn't set /memory/reg\n");
goto fail;
}
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 0c2b54d..4ce5e78 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -111,10 +111,10 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
qemu_fdt_add_subnode(fdt, ser);
qemu_fdt_setprop_string(fdt, ser, "device_type", "serial");
qemu_fdt_setprop_string(fdt, ser, "compatible", "ns16550");
- qemu_fdt_setprop_cells(fdt, ser, "reg", offset, 0x100);
+ qemu_fdt_setprop_cells_assert(fdt, ser, "reg", offset, 0x100);
qemu_fdt_setprop_cell(fdt, ser, "cell-index", idx);
qemu_fdt_setprop_cell(fdt, ser, "clock-frequency", 0);
- qemu_fdt_setprop_cells(fdt, ser, "interrupts", 42, 2);
+ qemu_fdt_setprop_cells_assert(fdt, ser, "interrupts", 42, 2);
qemu_fdt_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
qemu_fdt_setprop_string(fdt, "/aliases", alias, ser);
@@ -192,8 +192,8 @@ static int ppce500_load_device_tree(CPUPPCState *env,
qemu_fdt_add_subnode(fdt, "/memory");
qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
- qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
- sizeof(mem_reg_property));
+ qemu_fdt_setprop_assert(fdt, "/memory", "reg", mem_reg_property,
+ sizeof(mem_reg_property));
qemu_fdt_add_subnode(fdt, "/chosen");
if (initrd_size) {
@@ -225,11 +225,11 @@ static int ppce500_load_device_tree(CPUPPCState *env,
qemu_fdt_setprop_string(fdt, "/hypervisor", "compatible",
"linux,kvm");
kvmppc_get_hypercall(env, hypercall, sizeof(hypercall));
- qemu_fdt_setprop(fdt, "/hypervisor", "hcall-instructions",
- hypercall, sizeof(hypercall));
+ qemu_fdt_setprop_assert(fdt, "/hypervisor", "hcall-instructions",
+ hypercall, sizeof(hypercall));
/* if KVM supports the idle hcall, set property indicating this */
if (kvmppc_get_hasidle(env)) {
- qemu_fdt_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
+ qemu_fdt_setprop_assert(fdt, "/hypervisor", "has-idle", NULL, 0);
}
}
@@ -269,8 +269,8 @@ static int ppce500_load_device_tree(CPUPPCState *env,
qemu_fdt_setprop_string(fdt, cpu_name, "status", "disabled");
qemu_fdt_setprop_string(fdt, cpu_name, "enable-method",
"spin-table");
- qemu_fdt_setprop_u64(fdt, cpu_name, "cpu-release-addr",
- cpu_release_addr);
+ qemu_fdt_setprop_u64_assert(fdt, cpu_name, "cpu-release-addr",
+ cpu_release_addr);
} else {
qemu_fdt_setprop_string(fdt, cpu_name, "status", "okay");
}
@@ -281,13 +281,13 @@ static int ppce500_load_device_tree(CPUPPCState *env,
snprintf(soc, sizeof(soc), "/soc@%llx", MPC8544_CCSRBAR_BASE);
qemu_fdt_add_subnode(fdt, soc);
qemu_fdt_setprop_string(fdt, soc, "device_type", "soc");
- qemu_fdt_setprop(fdt, soc, "compatible", compatible_sb,
- sizeof(compatible_sb));
+ qemu_fdt_setprop_assert(fdt, soc, "compatible", compatible_sb,
+ sizeof(compatible_sb));
qemu_fdt_setprop_cell(fdt, soc, "#address-cells", 1);
qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1);
- qemu_fdt_setprop_cells(fdt, soc, "ranges", 0x0,
- MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
- MPC8544_CCSRBAR_SIZE);
+ qemu_fdt_setprop_cells_assert(fdt, soc, "ranges", 0x0,
+ MPC8544_CCSRBAR_BASE >> 32,
+ MPC8544_CCSRBAR_BASE, MPC8544_CCSRBAR_SIZE);
/* XXX should contain a reasonable value */
qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0);
@@ -295,14 +295,14 @@ static int ppce500_load_device_tree(CPUPPCState *env,
qemu_fdt_add_subnode(fdt, mpic);
qemu_fdt_setprop_string(fdt, mpic, "device_type", "open-pic");
qemu_fdt_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
- qemu_fdt_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
- 0x40000);
+ qemu_fdt_setprop_cells_assert(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
+ 0x40000);
qemu_fdt_setprop_cell(fdt, mpic, "#address-cells", 0);
qemu_fdt_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
mpic_ph = qemu_fdt_alloc_phandle(fdt);
qemu_fdt_setprop_cell(fdt, mpic, "phandle", mpic_ph);
qemu_fdt_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
- qemu_fdt_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
+ qemu_fdt_setprop_assert(fdt, mpic, "interrupt-controller", NULL, 0);
/*
* We have to generate ser1 first, because Linux takes the first
@@ -318,17 +318,19 @@ static int ppce500_load_device_tree(CPUPPCState *env,
MPC8544_UTIL_OFFSET);
qemu_fdt_add_subnode(fdt, gutil);
qemu_fdt_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
- qemu_fdt_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
- qemu_fdt_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
+ qemu_fdt_setprop_cells_assert(fdt, gutil, "reg", MPC8544_UTIL_OFFSET,
+ 0x1000);
+ qemu_fdt_setprop_assert(fdt, gutil, "fsl,has-rstcr", NULL, 0);
snprintf(msi, sizeof(msi), "/%s/msi@%llx", soc, MPC8544_MSI_REGS_OFFSET);
qemu_fdt_add_subnode(fdt, msi);
qemu_fdt_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
- qemu_fdt_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
+ qemu_fdt_setprop_cells_assert(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET,
+ 0x200);
msi_ph = qemu_fdt_alloc_phandle(fdt);
- qemu_fdt_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
+ qemu_fdt_setprop_cells_assert(fdt, msi, "msi-available-ranges", 0x0, 0x100);
qemu_fdt_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
- qemu_fdt_setprop_cells(fdt, msi, "interrupts",
+ qemu_fdt_setprop_cells_assert(fdt, msi, "interrupts",
0xe0, 0x0,
0xe1, 0x0,
0xe2, 0x0,
@@ -345,22 +347,22 @@ static int ppce500_load_device_tree(CPUPPCState *env,
qemu_fdt_setprop_cell(fdt, pci, "cell-index", 0);
qemu_fdt_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
qemu_fdt_setprop_string(fdt, pci, "device_type", "pci");
- qemu_fdt_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
- 0x0, 0x7);
+ qemu_fdt_setprop_cells_assert(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
+ 0x0, 0x7);
pci_map = pci_map_create(fdt, qemu_fdt_get_phandle(fdt, mpic),
params->pci_first_slot, params->pci_nr_slots,
&len);
- qemu_fdt_setprop(fdt, pci, "interrupt-map", pci_map, len);
+ qemu_fdt_setprop_assert(fdt, pci, "interrupt-map", pci_map, len);
qemu_fdt_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
- qemu_fdt_setprop_cells(fdt, pci, "interrupts", 24, 2);
- qemu_fdt_setprop_cells(fdt, pci, "bus-range", 0, 255);
+ qemu_fdt_setprop_cells_assert(fdt, pci, "interrupts", 24, 2);
+ qemu_fdt_setprop_cells_assert(fdt, pci, "bus-range", 0, 255);
for (i = 0; i < 14; i++) {
pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
}
qemu_fdt_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
- qemu_fdt_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
- qemu_fdt_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
- MPC8544_PCI_REGS_BASE, 0, 0x1000);
+ qemu_fdt_setprop_assert(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
+ qemu_fdt_setprop_cells_assert(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
+ MPC8544_PCI_REGS_BASE, 0, 0x1000);
qemu_fdt_setprop_cell(fdt, pci, "clock-frequency", 66666666);
qemu_fdt_setprop_cell(fdt, pci, "#interrupt-cells", 1);
qemu_fdt_setprop_cell(fdt, pci, "#size-cells", 2);
@@ -370,8 +372,8 @@ static int ppce500_load_device_tree(CPUPPCState *env,
params->fixup_devtree(params, fdt);
if (toplevel_compat) {
- qemu_fdt_setprop(fdt, "/", "compatible", toplevel_compat,
- strlen(toplevel_compat) + 1);
+ qemu_fdt_setprop_assert(fdt, "/", "compatible", toplevel_compat,
+ strlen(toplevel_compat) + 1);
}
done:
diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index 6eecdc5..16c0282 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -23,9 +23,9 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
const char model[] = "QEMU ppce500";
const char compatible[] = "fsl,qemu-e500";
- qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
- qemu_fdt_setprop(fdt, "/", "compatible", compatible,
- sizeof(compatible));
+ qemu_fdt_setprop_assert(fdt, "/", "model", model, sizeof(model));
+ qemu_fdt_setprop_assert(fdt, "/", "compatible", compatible,
+ sizeof(compatible));
}
static void e500plat_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
index 4ed66f1..113fd13 100644
--- a/hw/ppc/mpc8544ds.c
+++ b/hw/ppc/mpc8544ds.c
@@ -21,9 +21,9 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
const char model[] = "MPC8544DS";
const char compatible[] = "MPC8544DS\0MPC85xxDS";
- qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
- qemu_fdt_setprop(fdt, "/", "compatible", compatible,
- sizeof(compatible));
+ qemu_fdt_setprop_assert(fdt, "/", "model", model, sizeof(model));
+ qemu_fdt_setprop_assert(fdt, "/", "compatible", compatible,
+ sizeof(compatible));
}
static void mpc8544ds_init(QEMUMachineInitArgs *args)
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index db9d38b..39badfa 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -64,6 +64,7 @@ static int bamboo_load_device_tree(hwaddr addr,
void *fdt;
uint32_t tb_freq = 400000000;
uint32_t clock_freq = 400000000;
+ Error *errp = NULL;
filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
if (!filename) {
@@ -77,10 +78,11 @@ static int bamboo_load_device_tree(hwaddr addr,
/* Manipulate device tree in memory. */
- ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
- sizeof(mem_reg_property));
- if (ret < 0)
+ qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
+ sizeof(mem_reg_property), &errp);
+ if (errp) {
fprintf(stderr, "couldn't set /memory/reg\n");
+ }
ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
initrd_base);
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index b4650c5..adaf5c2 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -4,8 +4,10 @@
* interface.
*
* Copyright 2008 IBM Corporation.
+ * Copyright 2013 Xilinx Inc.
* Authors: Jerone Young <jyoung5@us.ibm.com>
* Hollis Blanchard <hollisb@us.ibm.com>
+ * Peter Crosthwaite <peter.crosthwaite@xilinx.com>
*
* This work is licensed under the GNU GPL license version 2 or later.
*
@@ -14,15 +16,68 @@
#ifndef __DEVICE_TREE_H__
#define __DEVICE_TREE_H__
+#include "qapi/qmp/qerror.h"
+
void *create_device_tree(int *sizep);
void *load_device_tree(const char *filename_path, int *sizep);
-int qemu_fdt_setprop(void *fdt, const char *node_path,
- const char *property, const void *val, int size);
+/**
+ * qemu_fdt_setprop - create or change a property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @val: pointer to data to set the property value to
+ * @size: length of the property value
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop() sets the value of the named property in the given
+ * node to the given value and length, creating the property if it
+ * does not already exist.
+ */
+
+void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
+ const void *val, int size, Error **errp);
+
+/**
+ * qemu_fdt_setprop_assert - create or change a property and assert success
+ *
+ * Same as qemu_fdt_setprop() except no errp argument required, and
+ * asserts the success of the operation.
+ */
+
+void qemu_fdt_setprop_assert(void *fdt, const char *node_path,
+ const char *property, const void *val, int size);
+
int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
const char *property, uint32_t val);
-int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
- const char *property, uint64_t val);
+
+/**
+ * qemu_fdt_setprop_u64 - create or change a 64bit int property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @val: value to set
+ * @errp: Error to populate incase of error
+ *
+ * qemu_fdt_setprop_u64() sets the value of the named property in the given
+ * node to the given uint64_t value. The value is converted to big endian
+ * format as per device tree formatting.
+ */
+
+void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
+ const char *property, uint64_t val, Error **errp);
+
+/**
+ * qemu_fdt_setprop_u64_assert - create or change a 64 bit int property and
+ * assert success
+ *
+ * Same as qemu_fdt_setprop_u64() except no errp argument required, and
+ * asserts the success of the operation.
+ */
+
+void qemu_fdt_setprop_u64_assert(void *fdt, const char *node_path,
+ const char *property, uint64_t val);
+
int qemu_fdt_setprop_string(void *fdt, const char *node_path,
const char *property, const char *string);
int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
@@ -37,7 +92,21 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt);
int qemu_fdt_nop_node(void *fdt, const char *node_path);
int qemu_fdt_add_subnode(void *fdt, const char *name);
-#define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
+/**
+ * qemu_fdt_setprop_cells - create or change a multi-cell property
+ * @fdt: pointer to the device tree blob
+ * @node_path: node-path of the node whose property to change
+ * @property: name of the property to change
+ * @errp: Error to populate incase of error
+ * @...: varargs list of cells to write to property
+ *
+ * qemu_fdt_setprop_cells() sets the value of the named property in the given
+ * node to a list of cells. The varargs are converted to an appropriate length
+ * uint32_t array and converted to big endian. The array is then written as
+ * the property value.
+ */
+
+#define qemu_fdt_setprop_cells(fdt, node_path, property, errp, ...) \
do { \
uint32_t qdt_tmp[] = { __VA_ARGS__ }; \
int i; \
@@ -46,7 +115,23 @@ int qemu_fdt_add_subnode(void *fdt, const char *name);
qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); \
} \
qemu_fdt_setprop(fdt, node_path, property, qdt_tmp, \
- sizeof(qdt_tmp)); \
+ sizeof(qdt_tmp), errp); \
+ } while (0)
+
+/**
+ * qemu_fdt_setprop_cells_assert - create or change a mult-cell property and
+ * assert success
+ *
+ * Same as qemu_fdt_setprop_cells() except no errp argument required, and
+ * asserts the success of the operation.
+ */
+
+#define qemu_fdt_setprop_cells_assert(fdt, node_path, property, ...) \
+ do { \
+ Error *errp = NULL; \
+ \
+ qemu_fdt_setprop_cells(fdt, node_path, property, &errp, __VA_ARGS__); \
+ assert_no_error(errp); \
} while (0)
void qemu_fdt_dumpdtb(void *fdt, int size);
--
1.8.3.rc1.44.gb387c77.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
@ 2013-07-20 2:36 ` Peter Crosthwaite
2013-07-20 12:07 ` Andreas Färber
1 sibling, 0 replies; 7+ messages in thread
From: Peter Crosthwaite @ 2013-07-20 2:36 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, agraf, david
Ping!
If theres not objections to the change pattern id like to proceed with the full
change.
Regards,
Peter
On Fri, Jul 12, 2013 at 2:29 PM, <peter.crosthwaite@xilinx.com> wrote:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> There are a mix of usages of the qemu_fdt_* API calls, some which
> wish to assert and abort QEMU on failure and some of which wish to do
> their own error handling. The latter in more correct, but the former
> is in the majority and more pragmatic, so facilitate both schemes by
> creating asserting and non asserting variants. This patch does this
> for qemu fdt_setprop and its wrapping users:
>
> * qemu_fdt_setprop
> * qemu_fdt_setprop_u64
> * qemu_fdt_setprop_cells
>
> Error reporting is stylistically udpated to use Error ** instead of
> integer return codes and exit(1).
>
> Users of these APIs that ignore the return code are converted to using
> the _assert variants. Users that check the return code are converted to
> use Error ** for their error detection paths.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> If this is ok, I will apply the change pattern to the entire
> qemu_fdt API
>
> device_tree.c | 35 ++++++++++++----
> hw/arm/boot.c | 7 ++--
> hw/ppc/e500.c | 66 +++++++++++++++---------------
> hw/ppc/e500plat.c | 6 +--
> hw/ppc/mpc8544ds.c | 6 +--
> hw/ppc/ppc440_bamboo.c | 8 ++--
> include/sysemu/device_tree.h | 97 +++++++++++++++++++++++++++++++++++++++++---
> 7 files changed, 166 insertions(+), 59 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index 048b8e1..ca2a88d 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -4,8 +4,10 @@
> * interface.
> *
> * Copyright 2008 IBM Corporation.
> + * Copyright 2013 Xilinx Inc.
> * Authors: Jerone Young <jyoung5@us.ibm.com>
> * Hollis Blanchard <hollisb@us.ibm.com>
> + * Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> *
> * This work is licensed under the GNU GPL license version 2 or later.
> *
> @@ -126,19 +128,25 @@ static int findnode_nofail(void *fdt, const char *node_path)
> return offset;
> }
>
> -int qemu_fdt_setprop(void *fdt, const char *node_path,
> - const char *property, const void *val, int size)
> +void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
> + const void *val, int size, Error **errp)
> {
> int r;
>
> r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size);
> if (r < 0) {
> - fprintf(stderr, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
> - property, fdt_strerror(r));
> - exit(1);
> + error_setg(errp, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
> + property, fdt_strerror(r));
> }
> +}
>
> - return r;
> +void qemu_fdt_setprop_assert(void *fdt, const char *node_path,
> + const char *property, const void *val, int size)
> +{
> + Error *errp = NULL;
> +
> + qemu_fdt_setprop(fdt, node_path, property, val, size, &errp);
> + assert_no_error(errp);
> }
>
> int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> @@ -156,11 +164,20 @@ int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> return r;
> }
>
> -int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> - const char *property, uint64_t val)
> +void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> + const char *property, uint64_t val, Error **errp)
> {
> val = cpu_to_be64(val);
> - return qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val));
> + qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val), errp);
> +}
> +
> +void qemu_fdt_setprop_u64_assert(void *fdt, const char *node_path,
> + const char *property, uint64_t val)
> +{
> + Error *errp = NULL;
> +
> + qemu_fdt_setprop_u64(fdt, node_path, property, val, &errp);
> + assert_no_error(errp);
> }
>
> int qemu_fdt_setprop_string(void *fdt, const char *node_path,
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 2768b2b..9164bb9 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -233,6 +233,7 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
> char *filename;
> int size, rc;
> uint32_t acells, scells, hival;
> + Error *errp = NULL;
>
> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
> if (!filename) {
> @@ -276,9 +277,9 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
> goto fail;
> }
>
> - rc = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> - mem_reg_propsize * sizeof(uint32_t));
> - if (rc < 0) {
> + qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> + mem_reg_propsize * sizeof(uint32_t), &errp);
> + if (errp) {
> fprintf(stderr, "couldn't set /memory/reg\n");
> goto fail;
> }
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 0c2b54d..4ce5e78 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -111,10 +111,10 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
> qemu_fdt_add_subnode(fdt, ser);
> qemu_fdt_setprop_string(fdt, ser, "device_type", "serial");
> qemu_fdt_setprop_string(fdt, ser, "compatible", "ns16550");
> - qemu_fdt_setprop_cells(fdt, ser, "reg", offset, 0x100);
> + qemu_fdt_setprop_cells_assert(fdt, ser, "reg", offset, 0x100);
> qemu_fdt_setprop_cell(fdt, ser, "cell-index", idx);
> qemu_fdt_setprop_cell(fdt, ser, "clock-frequency", 0);
> - qemu_fdt_setprop_cells(fdt, ser, "interrupts", 42, 2);
> + qemu_fdt_setprop_cells_assert(fdt, ser, "interrupts", 42, 2);
> qemu_fdt_setprop_phandle(fdt, ser, "interrupt-parent", mpic);
> qemu_fdt_setprop_string(fdt, "/aliases", alias, ser);
>
> @@ -192,8 +192,8 @@ static int ppce500_load_device_tree(CPUPPCState *env,
>
> qemu_fdt_add_subnode(fdt, "/memory");
> qemu_fdt_setprop_string(fdt, "/memory", "device_type", "memory");
> - qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> - sizeof(mem_reg_property));
> + qemu_fdt_setprop_assert(fdt, "/memory", "reg", mem_reg_property,
> + sizeof(mem_reg_property));
>
> qemu_fdt_add_subnode(fdt, "/chosen");
> if (initrd_size) {
> @@ -225,11 +225,11 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> qemu_fdt_setprop_string(fdt, "/hypervisor", "compatible",
> "linux,kvm");
> kvmppc_get_hypercall(env, hypercall, sizeof(hypercall));
> - qemu_fdt_setprop(fdt, "/hypervisor", "hcall-instructions",
> - hypercall, sizeof(hypercall));
> + qemu_fdt_setprop_assert(fdt, "/hypervisor", "hcall-instructions",
> + hypercall, sizeof(hypercall));
> /* if KVM supports the idle hcall, set property indicating this */
> if (kvmppc_get_hasidle(env)) {
> - qemu_fdt_setprop(fdt, "/hypervisor", "has-idle", NULL, 0);
> + qemu_fdt_setprop_assert(fdt, "/hypervisor", "has-idle", NULL, 0);
> }
> }
>
> @@ -269,8 +269,8 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> qemu_fdt_setprop_string(fdt, cpu_name, "status", "disabled");
> qemu_fdt_setprop_string(fdt, cpu_name, "enable-method",
> "spin-table");
> - qemu_fdt_setprop_u64(fdt, cpu_name, "cpu-release-addr",
> - cpu_release_addr);
> + qemu_fdt_setprop_u64_assert(fdt, cpu_name, "cpu-release-addr",
> + cpu_release_addr);
> } else {
> qemu_fdt_setprop_string(fdt, cpu_name, "status", "okay");
> }
> @@ -281,13 +281,13 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> snprintf(soc, sizeof(soc), "/soc@%llx", MPC8544_CCSRBAR_BASE);
> qemu_fdt_add_subnode(fdt, soc);
> qemu_fdt_setprop_string(fdt, soc, "device_type", "soc");
> - qemu_fdt_setprop(fdt, soc, "compatible", compatible_sb,
> - sizeof(compatible_sb));
> + qemu_fdt_setprop_assert(fdt, soc, "compatible", compatible_sb,
> + sizeof(compatible_sb));
> qemu_fdt_setprop_cell(fdt, soc, "#address-cells", 1);
> qemu_fdt_setprop_cell(fdt, soc, "#size-cells", 1);
> - qemu_fdt_setprop_cells(fdt, soc, "ranges", 0x0,
> - MPC8544_CCSRBAR_BASE >> 32, MPC8544_CCSRBAR_BASE,
> - MPC8544_CCSRBAR_SIZE);
> + qemu_fdt_setprop_cells_assert(fdt, soc, "ranges", 0x0,
> + MPC8544_CCSRBAR_BASE >> 32,
> + MPC8544_CCSRBAR_BASE, MPC8544_CCSRBAR_SIZE);
> /* XXX should contain a reasonable value */
> qemu_fdt_setprop_cell(fdt, soc, "bus-frequency", 0);
>
> @@ -295,14 +295,14 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> qemu_fdt_add_subnode(fdt, mpic);
> qemu_fdt_setprop_string(fdt, mpic, "device_type", "open-pic");
> qemu_fdt_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
> - qemu_fdt_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
> - 0x40000);
> + qemu_fdt_setprop_cells_assert(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
> + 0x40000);
> qemu_fdt_setprop_cell(fdt, mpic, "#address-cells", 0);
> qemu_fdt_setprop_cell(fdt, mpic, "#interrupt-cells", 2);
> mpic_ph = qemu_fdt_alloc_phandle(fdt);
> qemu_fdt_setprop_cell(fdt, mpic, "phandle", mpic_ph);
> qemu_fdt_setprop_cell(fdt, mpic, "linux,phandle", mpic_ph);
> - qemu_fdt_setprop(fdt, mpic, "interrupt-controller", NULL, 0);
> + qemu_fdt_setprop_assert(fdt, mpic, "interrupt-controller", NULL, 0);
>
> /*
> * We have to generate ser1 first, because Linux takes the first
> @@ -318,17 +318,19 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> MPC8544_UTIL_OFFSET);
> qemu_fdt_add_subnode(fdt, gutil);
> qemu_fdt_setprop_string(fdt, gutil, "compatible", "fsl,mpc8544-guts");
> - qemu_fdt_setprop_cells(fdt, gutil, "reg", MPC8544_UTIL_OFFSET, 0x1000);
> - qemu_fdt_setprop(fdt, gutil, "fsl,has-rstcr", NULL, 0);
> + qemu_fdt_setprop_cells_assert(fdt, gutil, "reg", MPC8544_UTIL_OFFSET,
> + 0x1000);
> + qemu_fdt_setprop_assert(fdt, gutil, "fsl,has-rstcr", NULL, 0);
>
> snprintf(msi, sizeof(msi), "/%s/msi@%llx", soc, MPC8544_MSI_REGS_OFFSET);
> qemu_fdt_add_subnode(fdt, msi);
> qemu_fdt_setprop_string(fdt, msi, "compatible", "fsl,mpic-msi");
> - qemu_fdt_setprop_cells(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET, 0x200);
> + qemu_fdt_setprop_cells_assert(fdt, msi, "reg", MPC8544_MSI_REGS_OFFSET,
> + 0x200);
> msi_ph = qemu_fdt_alloc_phandle(fdt);
> - qemu_fdt_setprop_cells(fdt, msi, "msi-available-ranges", 0x0, 0x100);
> + qemu_fdt_setprop_cells_assert(fdt, msi, "msi-available-ranges", 0x0, 0x100);
> qemu_fdt_setprop_phandle(fdt, msi, "interrupt-parent", mpic);
> - qemu_fdt_setprop_cells(fdt, msi, "interrupts",
> + qemu_fdt_setprop_cells_assert(fdt, msi, "interrupts",
> 0xe0, 0x0,
> 0xe1, 0x0,
> 0xe2, 0x0,
> @@ -345,22 +347,22 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> qemu_fdt_setprop_cell(fdt, pci, "cell-index", 0);
> qemu_fdt_setprop_string(fdt, pci, "compatible", "fsl,mpc8540-pci");
> qemu_fdt_setprop_string(fdt, pci, "device_type", "pci");
> - qemu_fdt_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
> - 0x0, 0x7);
> + qemu_fdt_setprop_cells_assert(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
> + 0x0, 0x7);
> pci_map = pci_map_create(fdt, qemu_fdt_get_phandle(fdt, mpic),
> params->pci_first_slot, params->pci_nr_slots,
> &len);
> - qemu_fdt_setprop(fdt, pci, "interrupt-map", pci_map, len);
> + qemu_fdt_setprop_assert(fdt, pci, "interrupt-map", pci_map, len);
> qemu_fdt_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
> - qemu_fdt_setprop_cells(fdt, pci, "interrupts", 24, 2);
> - qemu_fdt_setprop_cells(fdt, pci, "bus-range", 0, 255);
> + qemu_fdt_setprop_cells_assert(fdt, pci, "interrupts", 24, 2);
> + qemu_fdt_setprop_cells_assert(fdt, pci, "bus-range", 0, 255);
> for (i = 0; i < 14; i++) {
> pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
> }
> qemu_fdt_setprop_cell(fdt, pci, "fsl,msi", msi_ph);
> - qemu_fdt_setprop(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
> - qemu_fdt_setprop_cells(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
> - MPC8544_PCI_REGS_BASE, 0, 0x1000);
> + qemu_fdt_setprop_assert(fdt, pci, "ranges", pci_ranges, sizeof(pci_ranges));
> + qemu_fdt_setprop_cells_assert(fdt, pci, "reg", MPC8544_PCI_REGS_BASE >> 32,
> + MPC8544_PCI_REGS_BASE, 0, 0x1000);
> qemu_fdt_setprop_cell(fdt, pci, "clock-frequency", 66666666);
> qemu_fdt_setprop_cell(fdt, pci, "#interrupt-cells", 1);
> qemu_fdt_setprop_cell(fdt, pci, "#size-cells", 2);
> @@ -370,8 +372,8 @@ static int ppce500_load_device_tree(CPUPPCState *env,
> params->fixup_devtree(params, fdt);
>
> if (toplevel_compat) {
> - qemu_fdt_setprop(fdt, "/", "compatible", toplevel_compat,
> - strlen(toplevel_compat) + 1);
> + qemu_fdt_setprop_assert(fdt, "/", "compatible", toplevel_compat,
> + strlen(toplevel_compat) + 1);
> }
>
> done:
> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
> index 6eecdc5..16c0282 100644
> --- a/hw/ppc/e500plat.c
> +++ b/hw/ppc/e500plat.c
> @@ -23,9 +23,9 @@ static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
> const char model[] = "QEMU ppce500";
> const char compatible[] = "fsl,qemu-e500";
>
> - qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
> - qemu_fdt_setprop(fdt, "/", "compatible", compatible,
> - sizeof(compatible));
> + qemu_fdt_setprop_assert(fdt, "/", "model", model, sizeof(model));
> + qemu_fdt_setprop_assert(fdt, "/", "compatible", compatible,
> + sizeof(compatible));
> }
>
> static void e500plat_init(QEMUMachineInitArgs *args)
> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
> index 4ed66f1..113fd13 100644
> --- a/hw/ppc/mpc8544ds.c
> +++ b/hw/ppc/mpc8544ds.c
> @@ -21,9 +21,9 @@ static void mpc8544ds_fixup_devtree(PPCE500Params *params, void *fdt)
> const char model[] = "MPC8544DS";
> const char compatible[] = "MPC8544DS\0MPC85xxDS";
>
> - qemu_fdt_setprop(fdt, "/", "model", model, sizeof(model));
> - qemu_fdt_setprop(fdt, "/", "compatible", compatible,
> - sizeof(compatible));
> + qemu_fdt_setprop_assert(fdt, "/", "model", model, sizeof(model));
> + qemu_fdt_setprop_assert(fdt, "/", "compatible", compatible,
> + sizeof(compatible));
> }
>
> static void mpc8544ds_init(QEMUMachineInitArgs *args)
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index db9d38b..39badfa 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -64,6 +64,7 @@ static int bamboo_load_device_tree(hwaddr addr,
> void *fdt;
> uint32_t tb_freq = 400000000;
> uint32_t clock_freq = 400000000;
> + Error *errp = NULL;
>
> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
> if (!filename) {
> @@ -77,10 +78,11 @@ static int bamboo_load_device_tree(hwaddr addr,
>
> /* Manipulate device tree in memory. */
>
> - ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> - sizeof(mem_reg_property));
> - if (ret < 0)
> + qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> + sizeof(mem_reg_property), &errp);
> + if (errp) {
> fprintf(stderr, "couldn't set /memory/reg\n");
> + }
>
> ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> initrd_base);
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index b4650c5..adaf5c2 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -4,8 +4,10 @@
> * interface.
> *
> * Copyright 2008 IBM Corporation.
> + * Copyright 2013 Xilinx Inc.
> * Authors: Jerone Young <jyoung5@us.ibm.com>
> * Hollis Blanchard <hollisb@us.ibm.com>
> + * Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> *
> * This work is licensed under the GNU GPL license version 2 or later.
> *
> @@ -14,15 +16,68 @@
> #ifndef __DEVICE_TREE_H__
> #define __DEVICE_TREE_H__
>
> +#include "qapi/qmp/qerror.h"
> +
> void *create_device_tree(int *sizep);
> void *load_device_tree(const char *filename_path, int *sizep);
>
> -int qemu_fdt_setprop(void *fdt, const char *node_path,
> - const char *property, const void *val, int size);
> +/**
> + * qemu_fdt_setprop - create or change a property
> + * @fdt: pointer to the device tree blob
> + * @node_path: node-path of the node whose property to change
> + * @property: name of the property to change
> + * @val: pointer to data to set the property value to
> + * @size: length of the property value
> + * @errp: Error to populate incase of error
> + *
> + * qemu_fdt_setprop() sets the value of the named property in the given
> + * node to the given value and length, creating the property if it
> + * does not already exist.
> + */
> +
> +void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
> + const void *val, int size, Error **errp);
> +
> +/**
> + * qemu_fdt_setprop_assert - create or change a property and assert success
> + *
> + * Same as qemu_fdt_setprop() except no errp argument required, and
> + * asserts the success of the operation.
> + */
> +
> +void qemu_fdt_setprop_assert(void *fdt, const char *node_path,
> + const char *property, const void *val, int size);
> +
> int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> const char *property, uint32_t val);
> -int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> - const char *property, uint64_t val);
> +
> +/**
> + * qemu_fdt_setprop_u64 - create or change a 64bit int property
> + * @fdt: pointer to the device tree blob
> + * @node_path: node-path of the node whose property to change
> + * @property: name of the property to change
> + * @val: value to set
> + * @errp: Error to populate incase of error
> + *
> + * qemu_fdt_setprop_u64() sets the value of the named property in the given
> + * node to the given uint64_t value. The value is converted to big endian
> + * format as per device tree formatting.
> + */
> +
> +void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> + const char *property, uint64_t val, Error **errp);
> +
> +/**
> + * qemu_fdt_setprop_u64_assert - create or change a 64 bit int property and
> + * assert success
> + *
> + * Same as qemu_fdt_setprop_u64() except no errp argument required, and
> + * asserts the success of the operation.
> + */
> +
> +void qemu_fdt_setprop_u64_assert(void *fdt, const char *node_path,
> + const char *property, uint64_t val);
> +
> int qemu_fdt_setprop_string(void *fdt, const char *node_path,
> const char *property, const char *string);
> int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
> @@ -37,7 +92,21 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt);
> int qemu_fdt_nop_node(void *fdt, const char *node_path);
> int qemu_fdt_add_subnode(void *fdt, const char *name);
>
> -#define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
> +/**
> + * qemu_fdt_setprop_cells - create or change a multi-cell property
> + * @fdt: pointer to the device tree blob
> + * @node_path: node-path of the node whose property to change
> + * @property: name of the property to change
> + * @errp: Error to populate incase of error
> + * @...: varargs list of cells to write to property
> + *
> + * qemu_fdt_setprop_cells() sets the value of the named property in the given
> + * node to a list of cells. The varargs are converted to an appropriate length
> + * uint32_t array and converted to big endian. The array is then written as
> + * the property value.
> + */
> +
> +#define qemu_fdt_setprop_cells(fdt, node_path, property, errp, ...) \
> do { \
> uint32_t qdt_tmp[] = { __VA_ARGS__ }; \
> int i; \
> @@ -46,7 +115,23 @@ int qemu_fdt_add_subnode(void *fdt, const char *name);
> qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); \
> } \
> qemu_fdt_setprop(fdt, node_path, property, qdt_tmp, \
> - sizeof(qdt_tmp)); \
> + sizeof(qdt_tmp), errp); \
> + } while (0)
> +
> +/**
> + * qemu_fdt_setprop_cells_assert - create or change a mult-cell property and
> + * assert success
> + *
> + * Same as qemu_fdt_setprop_cells() except no errp argument required, and
> + * asserts the success of the operation.
> + */
> +
> +#define qemu_fdt_setprop_cells_assert(fdt, node_path, property, ...) \
> + do { \
> + Error *errp = NULL; \
> + \
> + qemu_fdt_setprop_cells(fdt, node_path, property, &errp, __VA_ARGS__); \
> + assert_no_error(errp); \
> } while (0)
>
> void qemu_fdt_dumpdtb(void *fdt, int size);
> --
> 1.8.3.rc1.44.gb387c77.dirty
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
@ 2013-07-20 11:44 ` Andreas Färber
0 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2013-07-20 11:44 UTC (permalink / raw)
To: peter.crosthwaite; +Cc: peter.maydell, david, qemu-devel, agraf
Am 12.07.2013 06:29, schrieb peter.crosthwaite@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Looking at the implementation, this doesn't really have a lot to do
> with arrays. Its just a pointer to a buffer and is passed through
> to the wrapped fn (qemu_fdt_setprop) unchanged. So rename to make it
> consistent with libfdt, which in the wrapped function just calls it
> "val".
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>
> device_tree.c | 4 ++--
> include/sysemu/device_tree.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Andreas Färber <afaerber@suse.de>
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
2013-07-20 2:36 ` Peter Crosthwaite
@ 2013-07-20 12:07 ` Andreas Färber
1 sibling, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2013-07-20 12:07 UTC (permalink / raw)
To: peter.crosthwaite
Cc: Luiz Capitulino, peter.maydell, david, qemu-devel, agraf
Am 12.07.2013 06:29, schrieb peter.crosthwaite@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> There are a mix of usages of the qemu_fdt_* API calls, some which
"some of which"
> wish to assert and abort QEMU on failure and some of which wish to do
> their own error handling. The latter in more correct, but the former
"is more"
> is in the majority and more pragmatic, so facilitate both schemes by
> creating asserting and non asserting variants. This patch does this
> for qemu fdt_setprop and its wrapping users:
>
> * qemu_fdt_setprop
> * qemu_fdt_setprop_u64
> * qemu_fdt_setprop_cells
>
> Error reporting is stylistically udpated to use Error ** instead of
"updated"
> integer return codes and exit(1).
>
> Users of these APIs that ignore the return code are converted to using
> the _assert variants. Users that check the return code are converted to
> use Error ** for their error detection paths.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> If this is ok, I will apply the change pattern to the entire
> qemu_fdt API
>
> device_tree.c | 35 ++++++++++++----
> hw/arm/boot.c | 7 ++--
> hw/ppc/e500.c | 66 +++++++++++++++---------------
> hw/ppc/e500plat.c | 6 +--
> hw/ppc/mpc8544ds.c | 6 +--
> hw/ppc/ppc440_bamboo.c | 8 ++--
> include/sysemu/device_tree.h | 97 +++++++++++++++++++++++++++++++++++++++++---
> 7 files changed, 166 insertions(+), 59 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index 048b8e1..ca2a88d 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -4,8 +4,10 @@
> * interface.
> *
> * Copyright 2008 IBM Corporation.
> + * Copyright 2013 Xilinx Inc.
> * Authors: Jerone Young <jyoung5@us.ibm.com>
> * Hollis Blanchard <hollisb@us.ibm.com>
> + * Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> *
> * This work is licensed under the GNU GPL license version 2 or later.
> *
> @@ -126,19 +128,25 @@ static int findnode_nofail(void *fdt, const char *node_path)
> return offset;
> }
>
> -int qemu_fdt_setprop(void *fdt, const char *node_path,
> - const char *property, const void *val, int size)
> +void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
> + const void *val, int size, Error **errp)
> {
> int r;
>
> r = fdt_setprop(fdt, findnode_nofail(fdt, node_path), property, val, size);
> if (r < 0) {
> - fprintf(stderr, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
> - property, fdt_strerror(r));
> - exit(1);
> + error_setg(errp, "%s: Couldn't set %s/%s: %s\n", __func__, node_path,
> + property, fdt_strerror(r));
The error_set*() functions shouldn't get \n appended. I think it would
be better to drop __func__, too. Otherwise the idea looks sane. Thanks
for looking into this.
> }
> +}
>
> - return r;
> +void qemu_fdt_setprop_assert(void *fdt, const char *node_path,
> + const char *property, const void *val, int size)
> +{
> + Error *errp = NULL;
Please use err for Error* here, errp is used for Error**.
> +
> + qemu_fdt_setprop(fdt, node_path, property, val, size, &errp);
> + assert_no_error(errp);
> }
>
> int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> @@ -156,11 +164,20 @@ int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> return r;
> }
>
> -int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> - const char *property, uint64_t val)
> +void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> + const char *property, uint64_t val, Error **errp)
> {
> val = cpu_to_be64(val);
> - return qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val));
> + qemu_fdt_setprop(fdt, node_path, property, &val, sizeof(val), errp);
> +}
> +
> +void qemu_fdt_setprop_u64_assert(void *fdt, const char *node_path,
> + const char *property, uint64_t val)
> +{
> + Error *errp = NULL;
> +
> + qemu_fdt_setprop_u64(fdt, node_path, property, val, &errp);
> + assert_no_error(errp);
> }
>
> int qemu_fdt_setprop_string(void *fdt, const char *node_path,
[...]
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index db9d38b..39badfa 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -64,6 +64,7 @@ static int bamboo_load_device_tree(hwaddr addr,
> void *fdt;
> uint32_t tb_freq = 400000000;
> uint32_t clock_freq = 400000000;
> + Error *errp = NULL;
err
>
> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
> if (!filename) {
> @@ -77,10 +78,11 @@ static int bamboo_load_device_tree(hwaddr addr,
>
> /* Manipulate device tree in memory. */
>
> - ret = qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> - sizeof(mem_reg_property));
> - if (ret < 0)
> + qemu_fdt_setprop(fdt, "/memory", "reg", mem_reg_property,
> + sizeof(mem_reg_property), &errp);
> + if (errp) {
> fprintf(stderr, "couldn't set /memory/reg\n");
error_free(err) and possibly append it to the error message?
> + }
>
> ret = qemu_fdt_setprop_cell(fdt, "/chosen", "linux,initrd-start",
> initrd_base);
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index b4650c5..adaf5c2 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -4,8 +4,10 @@
> * interface.
> *
> * Copyright 2008 IBM Corporation.
> + * Copyright 2013 Xilinx Inc.
> * Authors: Jerone Young <jyoung5@us.ibm.com>
> * Hollis Blanchard <hollisb@us.ibm.com>
> + * Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> *
> * This work is licensed under the GNU GPL license version 2 or later.
> *
> @@ -14,15 +16,68 @@
> #ifndef __DEVICE_TREE_H__
> #define __DEVICE_TREE_H__
>
> +#include "qapi/qmp/qerror.h"
qapi/error.h?
> +
> void *create_device_tree(int *sizep);
> void *load_device_tree(const char *filename_path, int *sizep);
>
> -int qemu_fdt_setprop(void *fdt, const char *node_path,
> - const char *property, const void *val, int size);
> +/**
> + * qemu_fdt_setprop - create or change a property
* qemu_fdt_setprop:
Description goes below the arguments.
> + * @fdt: pointer to the device tree blob
> + * @node_path: node-path of the node whose property to change
> + * @property: name of the property to change
> + * @val: pointer to data to set the property value to
> + * @size: length of the property value
> + * @errp: Error to populate incase of error
"in case"
> + *
> + * qemu_fdt_setprop() sets the value of the named property in the given
> + * node to the given value and length, creating the property if it
> + * does not already exist.
> + */
> +
> +void qemu_fdt_setprop(void *fdt, const char *node_path, const char *property,
> + const void *val, int size, Error **errp);
> +
> +/**
> + * qemu_fdt_setprop_assert - create or change a property and assert success
> + *
> + * Same as qemu_fdt_setprop() except no errp argument required, and
> + * asserts the success of the operation.
> + */
> +
> +void qemu_fdt_setprop_assert(void *fdt, const char *node_path,
> + const char *property, const void *val, int size);
> +
> int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
> const char *property, uint32_t val);
> -int qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> - const char *property, uint64_t val);
> +
> +/**
> + * qemu_fdt_setprop_u64 - create or change a 64bit int property
> + * @fdt: pointer to the device tree blob
> + * @node_path: node-path of the node whose property to change
> + * @property: name of the property to change
> + * @val: value to set
> + * @errp: Error to populate incase of error
> + *
> + * qemu_fdt_setprop_u64() sets the value of the named property in the given
> + * node to the given uint64_t value. The value is converted to big endian
> + * format as per device tree formatting.
> + */
> +
> +void qemu_fdt_setprop_u64(void *fdt, const char *node_path,
> + const char *property, uint64_t val, Error **errp);
> +
> +/**
> + * qemu_fdt_setprop_u64_assert - create or change a 64 bit int property and
> + * assert success
> + *
> + * Same as qemu_fdt_setprop_u64() except no errp argument required, and
> + * asserts the success of the operation.
> + */
> +
> +void qemu_fdt_setprop_u64_assert(void *fdt, const char *node_path,
> + const char *property, uint64_t val);
> +
> int qemu_fdt_setprop_string(void *fdt, const char *node_path,
> const char *property, const char *string);
> int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
> @@ -37,7 +92,21 @@ uint32_t qemu_fdt_alloc_phandle(void *fdt);
> int qemu_fdt_nop_node(void *fdt, const char *node_path);
> int qemu_fdt_add_subnode(void *fdt, const char *name);
>
> -#define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
> +/**
> + * qemu_fdt_setprop_cells - create or change a multi-cell property
> + * @fdt: pointer to the device tree blob
> + * @node_path: node-path of the node whose property to change
> + * @property: name of the property to change
> + * @errp: Error to populate incase of error
> + * @...: varargs list of cells to write to property
> + *
> + * qemu_fdt_setprop_cells() sets the value of the named property in the given
> + * node to a list of cells. The varargs are converted to an appropriate length
> + * uint32_t array and converted to big endian. The array is then written as
> + * the property value.
> + */
> +
> +#define qemu_fdt_setprop_cells(fdt, node_path, property, errp, ...) \
> do { \
> uint32_t qdt_tmp[] = { __VA_ARGS__ }; \
> int i; \
> @@ -46,7 +115,23 @@ int qemu_fdt_add_subnode(void *fdt, const char *name);
> qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); \
> } \
> qemu_fdt_setprop(fdt, node_path, property, qdt_tmp, \
> - sizeof(qdt_tmp)); \
> + sizeof(qdt_tmp), errp); \
> + } while (0)
> +
> +/**
> + * qemu_fdt_setprop_cells_assert - create or change a mult-cell property and
> + * assert success
> + *
> + * Same as qemu_fdt_setprop_cells() except no errp argument required, and
> + * asserts the success of the operation.
> + */
> +
> +#define qemu_fdt_setprop_cells_assert(fdt, node_path, property, ...) \
> + do { \
> + Error *errp = NULL; \
err
> + \
> + qemu_fdt_setprop_cells(fdt, node_path, property, &errp, __VA_ARGS__); \
> + assert_no_error(errp); \
> } while (0)
>
> void qemu_fdt_dumpdtb(void *fdt, int size);
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-07-20 12:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-12 4:27 [Qemu-devel] [RFC PATCH v1 0/3] Device tree cleanups peter.crosthwaite
2013-07-12 4:28 ` [Qemu-devel] [RFC PATCH v1 1/3] device_tree: s/qemu_devtree/qemu_fdt globally peter.crosthwaite
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 2/3] device_tree: qemu_fdt_setprop: Rename val_array arg peter.crosthwaite
2013-07-20 11:44 ` Andreas Färber
2013-07-12 4:29 ` [Qemu-devel] [RFC PATCH v1 3/3] device_tree: qemu_fdt_setprop: Fixup error reporting peter.crosthwaite
2013-07-20 2:36 ` Peter Crosthwaite
2013-07-20 12:07 ` Andreas Färber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.