Re: [Qemu-ppc] [PATCHv2 11/12] pseries: Consolidate construction of /vdevice device tree node

[Michael Roth]

Quoting David Gibson (2016-10-20 21:56:39)
> Construction of the /vdevice node (and its children) is divided between
> spapr_create_fdt_skel() (at init time), which creates the base node, and
> spapr_populate_vdevice() (at reset time) which creates the nodes for each
> individual virtual device.
> 
> This consolidates both into a single function called from
> spapr_build_fdt().
> 
> Signed-off-by: David Gibson <address@hidden>
> Reviewed-by: Thomas Huth <address@hidden>

Reviewed-by: Michael Roth <address@hidden>

> ---
>  hw/ppc/spapr.c             | 19 ++-----------------
>  hw/ppc/spapr_vio.c         | 23 ++++++++++++++++-------
>  include/hw/ppc/spapr_vio.h |  2 +-
>  3 files changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4bd547f..97369bd 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -316,18 +316,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState 
> *spapr)
>      _FDT((fdt_property_cell(fdt, "#address-cells", 0x2)));
>      _FDT((fdt_property_cell(fdt, "#size-cells", 0x2)));
> 
> -    /* vdevice */
> -    _FDT((fdt_begin_node(fdt, "vdevice")));
> -
> -    _FDT((fdt_property_string(fdt, "device_type", "vdevice")));
> -    _FDT((fdt_property_string(fdt, "compatible", "IBM,vdevice")));
> -    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
> -    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
> -    _FDT((fdt_property_cell(fdt, "#interrupt-cells", 0x2)));
> -    _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0)));
> -
> -    _FDT((fdt_end_node(fdt)));
> -
>      _FDT((fdt_end_node(fdt))); /* close root node */
>      _FDT((fdt_finish(fdt)));
> 
> @@ -943,11 +931,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>          exit(1);
>      }
> 
> -    ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
> -    if (ret < 0) {
> -        error_report("couldn't setup vio devices in fdt");
> -        exit(1);
> -    }
> +    /* /vdevice */
> +    spapr_dt_vdevice(spapr->vio_bus, fdt);
> 
>      if (object_resolve_path_type("", TYPE_SPAPR_RNG, NULL)) {
>          ret = spapr_rng_populate_dt(fdt);
> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> index 2b67df0..cc1e09c 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -36,6 +36,7 @@
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
>  #include "hw/ppc/xics.h"
> +#include "hw/ppc/fdt.h"
>  #include "trace.h"
> 
>  #include <libfdt.h>
> @@ -624,11 +625,21 @@ static int compare_reg(const void *p1, const void *p2)
>      return 1;
>  }
> 
> -int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt)
> +void spapr_dt_vdevice(VIOsPAPRBus *bus, void *fdt)
>  {
>      DeviceState *qdev, **qdevs;
>      BusChild *kid;
>      int i, num, ret = 0;
> +    int node;
> +
> +    _FDT(node = fdt_add_subnode(fdt, 0, "vdevice"));
> +
> +    _FDT(fdt_setprop_string(fdt, node, "device_type", "vdevice"));
> +    _FDT(fdt_setprop_string(fdt, node, "compatible", "IBM,vdevice"));
> +    _FDT(fdt_setprop_cell(fdt, node, "#address-cells", 1));
> +    _FDT(fdt_setprop_cell(fdt, node, "#size-cells", 0));
> +    _FDT(fdt_setprop_cell(fdt, node, "#interrupt-cells", 2));
> +    _FDT(fdt_setprop(fdt, node, "interrupt-controller", NULL, 0));
> 
>      /* Count qdevs on the bus list */
>      num = 0;
> @@ -650,19 +661,17 @@ int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt)
>       * to know that will mean they are in forward order in the tree. */
>      for (i = num - 1; i >= 0; i--) {
>          VIOsPAPRDevice *dev = (VIOsPAPRDevice *)(qdevs[i]);
> +        VIOsPAPRDeviceClass *vdc = VIO_SPAPR_DEVICE_GET_CLASS(dev);
> 
>          ret = vio_make_devnode(dev, fdt);
> -
>          if (ret < 0) {
> -            goto out;
> +            error_report("Couldn't create device node 
> /vdevice/address@hidden"PRIx32,
> +                         vdc->dt_name, dev->reg);
> +            exit(1);
>          }
>      }
> 
> -    ret = 0;
> -out:
>      g_free(qdevs);
> -
> -    return ret;
>  }
> 
>  gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus)
> diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
> index a0e7542..14f5022 100644
> --- a/include/hw/ppc/spapr_vio.h
> +++ b/include/hw/ppc/spapr_vio.h
> @@ -80,7 +80,7 @@ struct VIOsPAPRBus {
> 
>  extern VIOsPAPRBus *spapr_vio_bus_init(void);
>  extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg);
> -extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt);
> +void spapr_dt_vdevice(VIOsPAPRBus *bus, void *fdt);
>  extern gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus);
> 
>  static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev)
> -- 
> 2.7.4
>