| Message ID | 1452093205-30167-8-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jan 06, 2016 at 03:13:25PM +0000, Eric Auger wrote: > qemu_fdt_setprop self-exists in case of error hence no need to check > the returned value. > > Signed-off-by: Eric Auger <eric.auger@linaro.org> This change is fine, but in general I'm disinclined to invest too much in the qemu interfaces for manipulating flattened trees. I think our device tree manipulation in qemu is now complicated enough that we should move towards using an unflattened (i.e. pointer based) DT representation inside qemu, which is generally more suitable for complex manipulation. That would then get flattened into a blob for the guest in a single pass at reset time.
Hi David, On 01/11/2016 03:45 AM, David Gibson wrote: > On Wed, Jan 06, 2016 at 03:13:25PM +0000, Eric Auger wrote: >> qemu_fdt_setprop self-exists in case of error hence no need to check >> the returned value. >> >> Signed-off-by: Eric Auger <eric.auger@linaro.org> > > This change is fine, but in general I'm disinclined to invest too much > in the qemu interfaces for manipulating flattened trees. > > I think our device tree manipulation in qemu is now complicated enough > that we should move towards using an unflattened (i.e. pointer based) > DT representation inside qemu, which is generally more suitable for > complex manipulation. OK. Is there any user-space library available for un-flattened tree manipulation? I only found references to kernel unflattened tree manipulations (drivers/of/fdt.c, include/linux/of.h) and dtc flattree.c. Besides the indicated direction do I understand correctly that you do not reject the series? Best Regards Eric > > That would then get flattened into a blob for the guest in a single > pass at reset time. >
On Mon, Jan 11, 2016 at 12:18:59PM +0100, Eric Auger wrote: > Hi David, > On 01/11/2016 03:45 AM, David Gibson wrote: > > On Wed, Jan 06, 2016 at 03:13:25PM +0000, Eric Auger wrote: > >> qemu_fdt_setprop self-exists in case of error hence no need to check > >> the returned value. > >> > >> Signed-off-by: Eric Auger <eric.auger@linaro.org> > > > > This change is fine, but in general I'm disinclined to invest too much > > in the qemu interfaces for manipulating flattened trees. > > > > I think our device tree manipulation in qemu is now complicated enough > > that we should move towards using an unflattened (i.e. pointer based) > > DT representation inside qemu, which is generally more suitable for > > complex manipulation. > OK. Is there any user-space library available for un-flattened tree > manipulation? I only found references to kernel unflattened tree > manipulations (drivers/of/fdt.c, include/linux/of.h) and dtc flattree.c. Not that I'm aware of. I've sometimes thought of making one as another companion project to dtc. Or it would be reasonably straightforward to build a qemu specific one using qemu's existing list routines. > Besides the indicated direction do I understand correctly that you do > not reject the series? Yes, that's correct. I think working with unflattened trees is something we should head towards somewhere in the future, but that's certainly not a reason to hold up real improvements based on the existing flattened tree code.
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c index 66fa766..68d7e53 100644 --- a/hw/arm/sysbus-fdt.c +++ b/hw/arm/sysbus-fdt.c @@ -241,12 +241,8 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) reg_attr[2 * i + 1] = cpu_to_be32( memory_region_size(&vdev->regions[i]->mem)); } - ret = qemu_fdt_setprop(fdt, nodename, "reg", reg_attr, - vbasedev->num_regions * 2 * sizeof(uint32_t)); - if (ret) { - error_report("could not set reg property of node %s", nodename); - goto fail_reg; - } + qemu_fdt_setprop(fdt, nodename, "reg", reg_attr, + vbasedev->num_regions * 2 * sizeof(uint32_t)); irq_attr = g_new(uint32_t, vbasedev->num_irqs * 3); for (i = 0; i < vbasedev->num_irqs; i++) { @@ -256,14 +252,9 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) irq_attr[3 * i + 1] = cpu_to_be32(irq_number); irq_attr[3 * i + 2] = cpu_to_be32(GIC_FDT_IRQ_FLAGS_LEVEL_HI); } - ret = qemu_fdt_setprop(fdt, nodename, "interrupts", + qemu_fdt_setprop(fdt, nodename, "interrupts", irq_attr, vbasedev->num_irqs * 3 * sizeof(uint32_t)); - if (ret) { - error_report("could not set interrupts property of node %s", - nodename); - } g_free(irq_attr); -fail_reg: g_free(reg_attr); g_free(nodename); return ret;
qemu_fdt_setprop self-exists in case of error hence no need to check the returned value. Signed-off-by: Eric Auger <eric.auger@linaro.org> --- hw/arm/sysbus-fdt.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-)