Re: [Qemu-devel] [PATCH target-arm v1 9/9] arm: xlnx-zynqmp: Add 2xCortexR5 CPUs

[Alistair Francis]

On Tue, Jun 2, 2015 at 4:04 AM, Peter Crosthwaite
<address@hidden> wrote:
> Add the 2xCortexR5 CPUs to zynqmp board. They are powered off on reset
> (this is true of real hardware).
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>  hw/arm/xlnx-zynqmp.c         | 26 ++++++++++++++++++++++++++
>  include/hw/arm/xlnx-zynqmp.h |  2 ++
>  2 files changed, 28 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 6faa578..bf46f7c 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -71,6 +71,13 @@ static void xlnx_zynqmp_init(Object *obj)
>                                    &error_abort);
>      }
>
> +    for (i = 0; i < XLNX_ZYNQMP_NUM_RCPUS; i++) {
> +        object_initialize(&s->rcpu[i], sizeof(s->rcpu[i]),
> +                          "cortex-r5-" TYPE_ARM_CPU);
> +        object_property_add_child(obj, "rcpu[*]", OBJECT(&s->rcpu[i]),
> +                                  &error_abort);
> +    }
> +

Hey Peter,

Same comment about rcpu, I think it should just be rpu.

>      object_initialize(&s->gic, sizeof(s->gic), TYPE_ARM_GIC);
>      qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
>
> @@ -155,6 +162,25 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
> **errp)
>          qdev_connect_gpio_out(DEVICE(&s->acpu[i]), 1, irq);
>      }
>
> +    for (i = 0; i < XLNX_ZYNQMP_NUM_RCPUS; i++) {
> +        /* RCPUs and held in reset on startup, by the reset controller */
> +        object_property_set_bool(OBJECT(&s->rcpu[i]), true,
> +                                 "start-powered-off", &error_abort);
> +
> +        object_property_set_bool(OBJECT(&s->rcpu[i]), true, "reset-hivecs",
> +                                 &err);
> +        if (err != NULL) {

You don't need the '!= NULL'.

> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        object_property_set_bool(OBJECT(&s->rcpu[i]), true, "realized", 
> &err);
> +        if (err) {
> +            error_propagate((errp), (err));

You don't need the brackets around the errp and err.

Although looking above they all have brackets around them, so I guess
you are conforming with the reset of the style.

Thanks,

Alistair

> +            return;
> +        }
> +    }
> +
>      for (i = 0; i < GIC_NUM_SPI_INTR; i++) {
>          gic_spi[i] = qdev_get_gpio_in(DEVICE(&s->gic), i);
>      }
> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
> index bb67ef6..1272be3 100644
> --- a/include/hw/arm/xlnx-zynqmp.h
> +++ b/include/hw/arm/xlnx-zynqmp.h
> @@ -28,6 +28,7 @@
>                                         TYPE_XLNX_ZYNQMP)
>
>  #define XLNX_ZYNQMP_NUM_ACPUS 4
> +#define XLNX_ZYNQMP_NUM_RCPUS 2
>  #define XLNX_ZYNQMP_NUM_GEMS 4
>  #define XLNX_ZYNQMP_NUM_UARTS 2
>
> @@ -48,6 +49,7 @@ typedef struct XlnxZynqMPState {
>
>      /*< public >*/
>      ARMCPU acpu[XLNX_ZYNQMP_NUM_ACPUS];
> +    ARMCPU rcpu[XLNX_ZYNQMP_NUM_RCPUS];
>      GICState gic;
>      MemoryRegion gic_mr[XLNX_ZYNQMP_GIC_REGIONS][XLNX_ZYNQMP_GIC_ALIASES];
>      CadenceGEMState gem[XLNX_ZYNQMP_NUM_GEMS];
> --
> 2.4.2.3.g2ffcb72
>
>