Message ID | 1419265427-24238-2-git-send-email-b.reynal@virtualopensystems.com |
---|---|
State | New |
Headers | show |
Hi Baptiste, On 12/22/2014 05:23 PM, Baptiste Reynal wrote: > Creates set_interrupts_fdt_node and set_regions_fdt_node > for code reusability. > > Signed-off-by: Baptiste Reynal <b.reynal@virtualopensystems.com> > --- > hw/arm/sysbus-fdt.c | 102 +++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 73 insertions(+), 29 deletions(-) > > diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c > index 15bb50c..f6ff8a7 100644 > --- a/hw/arm/sysbus-fdt.c > +++ b/hw/arm/sysbus-fdt.c > @@ -58,6 +58,76 @@ typedef struct NodeCreationPair { > /* Device Specific Code */ > > /** > + * set_interrupts_fdt_node > + * > + * Helper function, generate interrupts property for a given node > + */ > +static int set_interrupts_fdt_node(char *nodename, SysBusDevice *sbdev, > + void *opaque, int type, int flags) > +{ > + PlatformBusFdtData *data = opaque; > + PlatformBusDevice *pbus = data->pbus; > + void *fdt = data->fdt; > + uint32_t *irq_attr; > + uint64_t irq_number; > + int i, ret; > + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); > + VFIODevice *vbasedev = &vdev->vbasedev; > + > + irq_attr = g_new(uint32_t, vbasedev->num_irqs*3); > + > + for (i = 0; i < vbasedev->num_irqs; i++) { > + irq_number = platform_bus_get_irqn(pbus, sbdev , i) > + + data->irq_start; > + irq_attr[3*i] = cpu_to_be32(type); > + irq_attr[3*i+1] = cpu_to_be32(irq_number); > + irq_attr[3*i+2] = cpu_to_be32(flags); > + } > + > + ret = qemu_fdt_setprop(fdt, nodename, "interrupts", > + irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t)); > + > + g_free(irq_attr); > + > + return ret; > +} > + > +/** > + * set_regions_fdt_node > + * > + * Helper function, generate reg property for a given node > + */ > +static int set_regions_fdt_node(char *nodename, SysBusDevice *sbdev, > + void *opaque) > +{ > + PlatformBusFdtData *data = opaque; > + PlatformBusDevice *pbus = data->pbus; > + void *fdt = data->fdt; > + uint64_t *reg_attr; > + uint64_t mmio_base; > + int i, ret; > + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); > + VFIODevice *vbasedev = &vdev->vbasedev; > + > + reg_attr = g_new(uint64_t, vbasedev->num_regions*4); > + > + for (i = 0; i < vbasedev->num_regions; i++) { > + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); > + reg_attr[4*i] = 1; > + reg_attr[4*i+1] = mmio_base; > + reg_attr[4*i+2] = 1; > + reg_attr[4*i+3] = memory_region_size(&vdev->regions[i]->mem); > + } > + > + ret = qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "reg", > + vbasedev->num_regions*2, reg_attr); > + > + g_free(reg_attr); > + > + return ret; > +} > + > +/** > * add_calxeda_midway_xgmac_fdt_node > * > * Generates a very simple node with following properties: > @@ -66,16 +136,12 @@ typedef struct NodeCreationPair { > static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) > { > PlatformBusFdtData *data = opaque; > - PlatformBusDevice *pbus = data->pbus; > void *fdt = data->fdt; > const char *parent_node = data->pbus_node_name; > int compat_str_len; > char *nodename; > - int i, ret; > - uint32_t *irq_attr; > - uint64_t *reg_attr; > + int ret; > uint64_t mmio_base; > - uint64_t irq_number; > VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); > VFIODevice *vbasedev = &vdev->vbasedev; > Object *obj = OBJECT(sbdev); > @@ -92,35 +158,15 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) > qemu_fdt_setprop(fdt, nodename, "compatible", > vdev->compat, compat_str_len); > > - reg_attr = g_new(uint64_t, vbasedev->num_regions*4); > - > - for (i = 0; i < vbasedev->num_regions; i++) { > - mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); > - reg_attr[4*i] = 1; > - reg_attr[4*i+1] = mmio_base; > - reg_attr[4*i+2] = 1; > - reg_attr[4*i+3] = memory_region_size(&vdev->regions[i]->mem); > - } > + ret = set_regions_fdt_node(nodename, sbdev, opaque); > > - ret = qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "reg", > - vbasedev->num_regions*2, reg_attr); > if (ret < 0) { > error_report("could not set reg property of node %s", nodename); > goto fail; > } > > - irq_attr = g_new(uint32_t, vbasedev->num_irqs*3); > + ret = set_interrupts_fdt_node(nodename, sbdev, opaque, 0, 0x4); > > - for (i = 0; i < vbasedev->num_irqs; i++) { > - irq_number = platform_bus_get_irqn(pbus, sbdev , i) > - + data->irq_start; > - irq_attr[3*i] = cpu_to_be32(0); > - irq_attr[3*i+1] = cpu_to_be32(irq_number); > - irq_attr[3*i+2] = cpu_to_be32(0x4); > - } > - > - ret = qemu_fdt_setprop(fdt, nodename, "interrupts", > - irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t)); > if (ret < 0) { > error_report("could not set interrupts property of node %s", > nodename); > @@ -128,8 +174,6 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) > } > > g_free(nodename); > - g_free(irq_attr); > - g_free(reg_attr); > > return 0; I think we should also take into account intermediate ret values. Otherwise looks sensible to me. Also you properly handled irq_attr and reg_attr dealloc here as opposed to what I did in that version ;-) maybe the title of your commit may be more explicit? Best Regards Eric > >
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c index 15bb50c..f6ff8a7 100644 --- a/hw/arm/sysbus-fdt.c +++ b/hw/arm/sysbus-fdt.c @@ -58,6 +58,76 @@ typedef struct NodeCreationPair { /* Device Specific Code */ /** + * set_interrupts_fdt_node + * + * Helper function, generate interrupts property for a given node + */ +static int set_interrupts_fdt_node(char *nodename, SysBusDevice *sbdev, + void *opaque, int type, int flags) +{ + PlatformBusFdtData *data = opaque; + PlatformBusDevice *pbus = data->pbus; + void *fdt = data->fdt; + uint32_t *irq_attr; + uint64_t irq_number; + int i, ret; + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); + VFIODevice *vbasedev = &vdev->vbasedev; + + irq_attr = g_new(uint32_t, vbasedev->num_irqs*3); + + for (i = 0; i < vbasedev->num_irqs; i++) { + irq_number = platform_bus_get_irqn(pbus, sbdev , i) + + data->irq_start; + irq_attr[3*i] = cpu_to_be32(type); + irq_attr[3*i+1] = cpu_to_be32(irq_number); + irq_attr[3*i+2] = cpu_to_be32(flags); + } + + ret = qemu_fdt_setprop(fdt, nodename, "interrupts", + irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t)); + + g_free(irq_attr); + + return ret; +} + +/** + * set_regions_fdt_node + * + * Helper function, generate reg property for a given node + */ +static int set_regions_fdt_node(char *nodename, SysBusDevice *sbdev, + void *opaque) +{ + PlatformBusFdtData *data = opaque; + PlatformBusDevice *pbus = data->pbus; + void *fdt = data->fdt; + uint64_t *reg_attr; + uint64_t mmio_base; + int i, ret; + VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); + VFIODevice *vbasedev = &vdev->vbasedev; + + reg_attr = g_new(uint64_t, vbasedev->num_regions*4); + + for (i = 0; i < vbasedev->num_regions; i++) { + mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); + reg_attr[4*i] = 1; + reg_attr[4*i+1] = mmio_base; + reg_attr[4*i+2] = 1; + reg_attr[4*i+3] = memory_region_size(&vdev->regions[i]->mem); + } + + ret = qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "reg", + vbasedev->num_regions*2, reg_attr); + + g_free(reg_attr); + + return ret; +} + +/** * add_calxeda_midway_xgmac_fdt_node * * Generates a very simple node with following properties: @@ -66,16 +136,12 @@ typedef struct NodeCreationPair { static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) { PlatformBusFdtData *data = opaque; - PlatformBusDevice *pbus = data->pbus; void *fdt = data->fdt; const char *parent_node = data->pbus_node_name; int compat_str_len; char *nodename; - int i, ret; - uint32_t *irq_attr; - uint64_t *reg_attr; + int ret; uint64_t mmio_base; - uint64_t irq_number; VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev); VFIODevice *vbasedev = &vdev->vbasedev; Object *obj = OBJECT(sbdev); @@ -92,35 +158,15 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) qemu_fdt_setprop(fdt, nodename, "compatible", vdev->compat, compat_str_len); - reg_attr = g_new(uint64_t, vbasedev->num_regions*4); - - for (i = 0; i < vbasedev->num_regions; i++) { - mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i); - reg_attr[4*i] = 1; - reg_attr[4*i+1] = mmio_base; - reg_attr[4*i+2] = 1; - reg_attr[4*i+3] = memory_region_size(&vdev->regions[i]->mem); - } + ret = set_regions_fdt_node(nodename, sbdev, opaque); - ret = qemu_fdt_setprop_sized_cells_from_array(fdt, nodename, "reg", - vbasedev->num_regions*2, reg_attr); if (ret < 0) { error_report("could not set reg property of node %s", nodename); goto fail; } - irq_attr = g_new(uint32_t, vbasedev->num_irqs*3); + ret = set_interrupts_fdt_node(nodename, sbdev, opaque, 0, 0x4); - for (i = 0; i < vbasedev->num_irqs; i++) { - irq_number = platform_bus_get_irqn(pbus, sbdev , i) - + data->irq_start; - irq_attr[3*i] = cpu_to_be32(0); - irq_attr[3*i+1] = cpu_to_be32(irq_number); - irq_attr[3*i+2] = cpu_to_be32(0x4); - } - - ret = qemu_fdt_setprop(fdt, nodename, "interrupts", - irq_attr, vbasedev->num_irqs*3*sizeof(uint32_t)); if (ret < 0) { error_report("could not set interrupts property of node %s", nodename); @@ -128,8 +174,6 @@ static int add_calxeda_midway_xgmac_fdt_node(SysBusDevice *sbdev, void *opaque) } g_free(nodename); - g_free(irq_attr); - g_free(reg_attr); return 0;
Creates set_interrupts_fdt_node and set_regions_fdt_node for code reusability. Signed-off-by: Baptiste Reynal <b.reynal@virtualopensystems.com> --- hw/arm/sysbus-fdt.c | 102 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 73 insertions(+), 29 deletions(-)