Re: [Qemu-devel] [PATCH 2/3] i.MX6UL: Add i.MX6UL SOC

2018-07-17 Thread Peter Maydell
On 30 June 2018 at 22:58, Jean-Christophe Dubois  wrote:
> Signed-off-by: Jean-Christophe Dubois 
> ---
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs|   1 +
>  hw/arm/fsl-imx6ul.c | 649 
>  include/hw/arm/fsl-imx6ul.h | 339 +
>  4 files changed, 990 insertions(+)
>  create mode 100644 hw/arm/fsl-imx6ul.c
>  create mode 100644 include/hw/arm/fsl-imx6ul.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 834d45cfaf..311584fd74 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -133,6 +133,7 @@ CONFIG_FSL_IMX6=y
>  CONFIG_FSL_IMX31=y
>  CONFIG_FSL_IMX25=y
>  CONFIG_FSL_IMX7=y
> +CONFIG_FSL_IMX6UL=y
>
>  CONFIG_IMX_I2C=y
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index d51fcecaf2..e419ad040b 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -36,3 +36,4 @@ obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
>  obj-$(CONFIG_IOTKIT) += iotkit.o
>  obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
>  obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
> +obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> new file mode 100644
> index 00..25d6c3f7c2
> --- /dev/null
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -0,0 +1,649 @@
> +/*
> + * Copyright (c) 2018 Jean-Christophe Dubois 
> + *
> + * i.MX6UL SOC emulation.
> + *
> + * Based on hw/arm/fsl-imx7.c
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "hw/arm/fsl-imx6ul.h"
> +#include "hw/misc/unimp.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/error-report.h"
> +
> +#define NAME_SIZE 20
> +
> +static void fsl_imx6ul_init(Object *obj)
> +{
> +BusState *sysbus = sysbus_get_default();
> +FslIMX6ULState *s = FSL_IMX6UL(obj);
> +char name[NAME_SIZE];
> +int i;
> +
> +
> +for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
> +object_initialize(>cpu[i], sizeof(s->cpu[i]),
> +  ARM_CPU_TYPE_NAME("cortex-a7"));
> +snprintf(name, NAME_SIZE, "cpu%d", i);
> +object_property_add_child(obj, name, OBJECT(>cpu[i]),
> +  _fatal);
> +}


It's been discovered recently that initializing child QOM objects
like this leaks a reference count to them, which can cause crashes
if the device is introspected via the QMP interface.
This has been fixed for various devices including the in-tree FSL
SoC containers in commits 0210b39d0e8..ccf02d73d18 (which just
got pushed to master this afternoon).

Commit e9e4d4d3e18 is the one that fixes fsl-imx6.c, and we should
follow that here. Specifically, rather than separate calls to
object_initialize() and object_property_add_child() we can have
one call to the new object_initialize_child():

   object_initialize_child(obj, name, >cpu[i], sizeof(s->cpu[i]),
   ARM_CPU_TYPE_NAME("cortex-a7"),
   _abort, NULL);

> +
> +/*
> + * A7MPCORE
> + */
> +object_initialize(>a7mpcore, sizeof(s->a7mpcore), 
> TYPE_A15MPCORE_PRIV);
> +qdev_set_parent_bus(DEVICE(>a7mpcore), sysbus);
> +object_property_add_child(obj, "a7mpcore",
> +  OBJECT(>a7mpcore), _fatal);

and for sysbus devices like this there is now a single sysbus_init_child_obj()
which does the work of all three of these calls:

sysbus_init_child_obj(obj, "a7mpcore", >a7mpcore, sizeof(s->a7mpcore),
  TYPE_A7MPCORE_PRIV);

Similarly with the various other child objects here.

thanks
-- PMM



[Qemu-devel] [PATCH 2/3] i.MX6UL: Add i.MX6UL SOC

2018-06-30 Thread Jean-Christophe Dubois
Signed-off-by: Jean-Christophe Dubois 
---
 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   1 +
 hw/arm/fsl-imx6ul.c | 649 
 include/hw/arm/fsl-imx6ul.h | 339 +
 4 files changed, 990 insertions(+)
 create mode 100644 hw/arm/fsl-imx6ul.c
 create mode 100644 include/hw/arm/fsl-imx6ul.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 834d45cfaf..311584fd74 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -133,6 +133,7 @@ CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX31=y
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
+CONFIG_FSL_IMX6UL=y
 
 CONFIG_IMX_I2C=y
 
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index d51fcecaf2..e419ad040b 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -36,3 +36,4 @@ obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
 obj-$(CONFIG_IOTKIT) += iotkit.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
 obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
+obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
new file mode 100644
index 00..25d6c3f7c2
--- /dev/null
+++ b/hw/arm/fsl-imx6ul.c
@@ -0,0 +1,649 @@
+/*
+ * Copyright (c) 2018 Jean-Christophe Dubois 
+ *
+ * i.MX6UL SOC emulation.
+ *
+ * Based on hw/arm/fsl-imx7.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/arm/fsl-imx6ul.h"
+#include "hw/misc/unimp.h"
+#include "sysemu/sysemu.h"
+#include "qemu/error-report.h"
+
+#define NAME_SIZE 20
+
+static void fsl_imx6ul_init(Object *obj)
+{
+BusState *sysbus = sysbus_get_default();
+FslIMX6ULState *s = FSL_IMX6UL(obj);
+char name[NAME_SIZE];
+int i;
+
+
+for (i = 0; i < MIN(smp_cpus, FSL_IMX6UL_NUM_CPUS); i++) {
+object_initialize(>cpu[i], sizeof(s->cpu[i]),
+  ARM_CPU_TYPE_NAME("cortex-a7"));
+snprintf(name, NAME_SIZE, "cpu%d", i);
+object_property_add_child(obj, name, OBJECT(>cpu[i]),
+  _fatal);
+}
+
+/*
+ * A7MPCORE
+ */
+object_initialize(>a7mpcore, sizeof(s->a7mpcore), TYPE_A15MPCORE_PRIV);
+qdev_set_parent_bus(DEVICE(>a7mpcore), sysbus);
+object_property_add_child(obj, "a7mpcore",
+  OBJECT(>a7mpcore), _fatal);
+
+/*
+ * CCM
+ */
+object_initialize(>ccm, sizeof(s->ccm), TYPE_IMX6UL_CCM);
+qdev_set_parent_bus(DEVICE(>ccm), sysbus);
+object_property_add_child(obj, "ccm", OBJECT(>ccm), _fatal);
+
+/*
+ * SRC
+ */
+object_initialize(>src, sizeof(s->src), TYPE_IMX6_SRC);
+qdev_set_parent_bus(DEVICE(>src), sysbus);
+object_property_add_child(obj, "src", OBJECT(>src), _fatal);
+
+/*
+ * GPCv2
+ */
+object_initialize(>gpcv2, sizeof(s->gpcv2), TYPE_IMX_GPCV2);
+qdev_set_parent_bus(DEVICE(>gpcv2), sysbus);
+object_property_add_child(obj, "gpcv2", OBJECT(>gpcv2), _fatal);
+
+/*
+ * GPIOs 1 to 5
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_GPIOS; i++) {
+object_initialize(>gpio[i], sizeof(s->gpio[i]),
+  TYPE_IMX_GPIO);
+qdev_set_parent_bus(DEVICE(>gpio[i]), sysbus);
+snprintf(name, NAME_SIZE, "gpio%d", i);
+object_property_add_child(obj, name,
+  OBJECT(>gpio[i]), _fatal);
+}
+
+/*
+ * GPT 1, 2
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_GPTS; i++) {
+object_initialize(>gpt[i], sizeof(s->gpt[i]), TYPE_IMX7_GPT);
+qdev_set_parent_bus(DEVICE(>gpt[i]), sysbus);
+snprintf(name, NAME_SIZE, "gpt%d", i);
+object_property_add_child(obj, name, OBJECT(>gpt[i]),
+  _fatal);
+}
+
+/*
+ * EPIT 1, 2
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_EPITS; i++) {
+object_initialize(>epit[i], sizeof(s->epit[i]), TYPE_IMX_EPIT);
+qdev_set_parent_bus(DEVICE(>epit[i]), sysbus);
+snprintf(name, NAME_SIZE, "epit%d", i + 1);
+object_property_add_child(obj, name, OBJECT(>epit[i]),
+  _fatal);
+}
+
+/*
+ * eCSPI
+ */
+for (i = 0; i < FSL_IMX6UL_NUM_ECSPIS; i++) {
+object_initialize(>spi[i], sizeof(s->spi[i]), TYPE_IMX_SPI);
+qdev_set_parent_bus(DEVICE(>spi[i]), sysbus_get_default());
+snprintf(name, NAME_SIZE, "spi%d", i + 1);
+