| Message ID | 21d3151db1cf00430db82f67ea6f66e93197e2c9.1373603020.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
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 >
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
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);