[Qemu-devel] [PATCH v3 0/2] integrator/cp: Working SD card support

[Qemu-devel] [PATCH v3 0/2] integrator/cp: Working SD card support

[Jan Kiszka]

Addressing Peter Crosthwaite's comments on v2 (thanks!).

Jan

Jan Kiszka (2):
  integrator/cp: Model CP control registers as sysbus device
  integrator/cp: Implement CARDIN and WPROT signals

 hw/arm/integratorcp.c | 95 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 82 insertions(+), 13 deletions(-)

-- 
2.1.4

[Qemu-devel] [PATCH v3 1/2] integrator/cp: Model CP control registers as sysbus device

[Jan Kiszka]

No new features yet, just encapsulation.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/arm/integratorcp.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 8c48b68..2d62275 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -406,6 +406,18 @@ static int icp_pic_init(SysBusDevice *sbd)
 
 /* CP control registers.  */
 
+#define TYPE_ICP_CONTROL_REGS "icp_ctrl_regs"
+#define ICP_CONTROL_REGS(obj) \
+    OBJECT_CHECK(ICPCtrlRegsState, (obj), TYPE_ICP_CONTROL_REGS)
+
+typedef struct ICPCtrlRegsState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+} ICPCtrlRegsState;
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
                                  unsigned size)
 {
@@ -444,15 +456,14 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void icp_control_init(hwaddr base)
+static void icp_control_init(Object *obj)
 {
-    MemoryRegion *io;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
 
-    io = (MemoryRegion *)g_malloc0(sizeof(MemoryRegion));
-    memory_region_init_io(io, NULL, &icp_control_ops, NULL,
-                          "control", 0x00800000);
-    memory_region_add_subregion(get_system_memory(), base, io);
-    /* ??? Save/restore.  */
+    memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
+                          "icp_ctrl_regs", 0x00800000);
+    sysbus_init_mmio(sbd, &s->iomem);
 }
 
 
@@ -541,7 +552,7 @@ static void integratorcp_init(MachineState *machine)
     sysbus_create_simple("pl031", 0x15000000, pic[8]);
     sysbus_create_simple("pl011", 0x16000000, pic[1]);
     sysbus_create_simple("pl011", 0x17000000, pic[2]);
-    icp_control_init(0xcb000000);
+    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
     sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
     sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
     sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
@@ -606,10 +617,18 @@ static const TypeInfo icp_pic_info = {
     .class_init    = icp_pic_class_init,
 };
 
+static const TypeInfo icp_ctrl_regs_info = {
+    .name          = TYPE_ICP_CONTROL_REGS,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ICPCtrlRegsState),
+    .instance_init = icp_control_init,
+};
+
 static void integratorcp_register_types(void)
 {
     type_register_static(&icp_pic_info);
     type_register_static(&core_info);
+    type_register_static(&icp_ctrl_regs_info);
 }
 
 type_init(integratorcp_register_types)
-- 
2.1.4

[Qemu-devel] [PATCH v3 2/2] integrator/cp: Implement CARDIN and WPROT signals

[Jan Kiszka]

This allows to use the SD card emulation of the board: Forward the
signals from the pl181 top the CP control register emulation, report the
current state via CP_INTREG, deliver CARDIN IRQ to the secondary
interrupt controller and also support clearing that line via CP_INTREG.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/arm/integratorcp.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 2d62275..59658d3 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -416,18 +416,29 @@ typedef struct ICPCtrlRegsState {
     /*< public >*/
 
     MemoryRegion iomem;
+
+    qemu_irq mmc_irq;
+    uint32_t intreg_state;
 } ICPCtrlRegsState;
 
+#define ICP_GPIO_MMC_WPROT      "mmc-wprot"
+#define ICP_GPIO_MMC_CARDIN     "mmc-cardin"
+
+#define ICP_INTREG_WPROT        (1 << 0)
+#define ICP_INTREG_CARDIN       (1 << 3)
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
                                  unsigned size)
 {
+    ICPCtrlRegsState *s = opaque;
+
     switch (offset >> 2) {
     case 0: /* CP_IDFIELD */
         return 0x41034003;
     case 1: /* CP_FLASHPROG */
         return 0;
     case 2: /* CP_INTREG */
-        return 0;
+        return s->intreg_state;
     case 3: /* CP_DECODE */
         return 0x11;
     default:
@@ -439,9 +450,14 @@ static uint64_t icp_control_read(void *opaque, hwaddr offset,
 static void icp_control_write(void *opaque, hwaddr offset,
                           uint64_t value, unsigned size)
 {
+    ICPCtrlRegsState *s = opaque;
+
     switch (offset >> 2) {
-    case 1: /* CP_FLASHPROG */
     case 2: /* CP_INTREG */
+        s->intreg_state &= ~(value & ICP_INTREG_CARDIN);
+        qemu_set_irq(s->mmc_irq, !!(s->intreg_state & ICP_INTREG_CARDIN));
+        break;
+    case 1: /* CP_FLASHPROG */
     case 3: /* CP_DECODE */
         /* Nothing interesting implemented yet.  */
         break;
@@ -456,14 +472,41 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void icp_control_mmc_wprot(void *opaque, int line, int level)
+{
+    ICPCtrlRegsState *s = opaque;
+
+    s->intreg_state &= ~ICP_INTREG_WPROT;
+    if (level) {
+        s->intreg_state |= ICP_INTREG_WPROT;
+    }
+}
+
+static void icp_control_mmc_cardin(void *opaque, int line, int level)
+{
+    ICPCtrlRegsState *s = opaque;
+
+    /* line is released by writing to CP_INTREG */
+    if (level) {
+        s->intreg_state |= ICP_INTREG_CARDIN;
+        qemu_set_irq(s->mmc_irq, 1);
+    }
+}
+
 static void icp_control_init(Object *obj)
 {
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
+    DeviceState *dev = DEVICE(obj);
 
     memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
                           "icp_ctrl_regs", 0x00800000);
     sysbus_init_mmio(sbd, &s->iomem);
+
+    qdev_init_gpio_in_named(dev, icp_control_mmc_wprot, ICP_GPIO_MMC_WPROT, 1);
+    qdev_init_gpio_in_named(dev, icp_control_mmc_cardin,
+                            ICP_GPIO_MMC_CARDIN, 1);
+    sysbus_init_irq(sbd, &s->mmc_irq);
 }
 
 
@@ -488,7 +531,7 @@ static void integratorcp_init(MachineState *machine)
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
     qemu_irq pic[32];
-    DeviceState *dev;
+    DeviceState *dev, *sic, *icp;
     int i;
     Error *err = NULL;
 
@@ -546,17 +589,24 @@ static void integratorcp_init(MachineState *machine)
     for (i = 0; i < 32; i++) {
         pic[i] = qdev_get_gpio_in(dev, i);
     }
-    sysbus_create_simple(TYPE_INTEGRATOR_PIC, 0xca000000, pic[26]);
+    sic = sysbus_create_simple(TYPE_INTEGRATOR_PIC, 0xca000000, pic[26]);
     sysbus_create_varargs("integrator_pit", 0x13000000,
                           pic[5], pic[6], pic[7], NULL);
     sysbus_create_simple("pl031", 0x15000000, pic[8]);
     sysbus_create_simple("pl011", 0x16000000, pic[1]);
     sysbus_create_simple("pl011", 0x17000000, pic[2]);
-    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
+    icp = sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000,
+                               qdev_get_gpio_in(sic, 3));
     sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
     sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
     sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
-    sysbus_create_varargs("pl181", 0x1c000000, pic[23], pic[24], NULL);
+
+    dev = sysbus_create_varargs("pl181", 0x1c000000, pic[23], pic[24], NULL);
+    qdev_connect_gpio_out(dev, 0,
+                          qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_WPROT, 0));
+    qdev_connect_gpio_out(dev, 1,
+                          qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_CARDIN, 0));
+
     if (nd_table[0].used)
         smc91c111_init(&nd_table[0], 0xc8000000, pic[27]);
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH v3 1/2] integrator/cp: Model CP control registers as sysbus device

[Peter Crosthwaite]

On Mon, Feb 23, 2015 at 4:21 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> No new features yet, just encapsulation.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/arm/integratorcp.c | 35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index 8c48b68..2d62275 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -406,6 +406,18 @@ static int icp_pic_init(SysBusDevice *sbd)
>
>  /* CP control registers.  */
>
> +#define TYPE_ICP_CONTROL_REGS "icp_ctrl_regs"

Sorry I missed this first time. Type name should use "-" instead of "_".

otherwise

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> +#define ICP_CONTROL_REGS(obj) \
> +    OBJECT_CHECK(ICPCtrlRegsState, (obj), TYPE_ICP_CONTROL_REGS)
> +
> +typedef struct ICPCtrlRegsState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +} ICPCtrlRegsState;
> +
>  static uint64_t icp_control_read(void *opaque, hwaddr offset,
>                                   unsigned size)
>  {
> @@ -444,15 +456,14 @@ static const MemoryRegionOps icp_control_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> -static void icp_control_init(hwaddr base)
> +static void icp_control_init(Object *obj)
>  {
> -    MemoryRegion *io;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
>
> -    io = (MemoryRegion *)g_malloc0(sizeof(MemoryRegion));
> -    memory_region_init_io(io, NULL, &icp_control_ops, NULL,
> -                          "control", 0x00800000);
> -    memory_region_add_subregion(get_system_memory(), base, io);
> -    /* ??? Save/restore.  */
> +    memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
> +                          "icp_ctrl_regs", 0x00800000);
> +    sysbus_init_mmio(sbd, &s->iomem);
>  }
>
>
> @@ -541,7 +552,7 @@ static void integratorcp_init(MachineState *machine)
>      sysbus_create_simple("pl031", 0x15000000, pic[8]);
>      sysbus_create_simple("pl011", 0x16000000, pic[1]);
>      sysbus_create_simple("pl011", 0x17000000, pic[2]);
> -    icp_control_init(0xcb000000);
> +    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
>      sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
>      sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
>      sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
> @@ -606,10 +617,18 @@ static const TypeInfo icp_pic_info = {
>      .class_init    = icp_pic_class_init,
>  };
>
> +static const TypeInfo icp_ctrl_regs_info = {
> +    .name          = TYPE_ICP_CONTROL_REGS,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(ICPCtrlRegsState),
> +    .instance_init = icp_control_init,
> +};
> +
>  static void integratorcp_register_types(void)
>  {
>      type_register_static(&icp_pic_info);
>      type_register_static(&core_info);
> +    type_register_static(&icp_ctrl_regs_info);
>  }
>
>  type_init(integratorcp_register_types)
> --
> 2.1.4
>
>

Re: [Qemu-devel] [PATCH v3 2/2] integrator/cp: Implement CARDIN and WPROT signals

[Peter Crosthwaite]

On Mon, Feb 23, 2015 at 4:21 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> This allows to use the SD card emulation of the board: Forward the
> signals from the pl181 top the CP control register emulation, report the
> current state via CP_INTREG, deliver CARDIN IRQ to the secondary
> interrupt controller and also support clearing that line via CP_INTREG.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>


Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>


> ---
>  hw/arm/integratorcp.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index 2d62275..59658d3 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -416,18 +416,29 @@ typedef struct ICPCtrlRegsState {
>      /*< public >*/
>
>      MemoryRegion iomem;
> +
> +    qemu_irq mmc_irq;
> +    uint32_t intreg_state;
>  } ICPCtrlRegsState;
>
> +#define ICP_GPIO_MMC_WPROT      "mmc-wprot"
> +#define ICP_GPIO_MMC_CARDIN     "mmc-cardin"
> +
> +#define ICP_INTREG_WPROT        (1 << 0)
> +#define ICP_INTREG_CARDIN       (1 << 3)
> +
>  static uint64_t icp_control_read(void *opaque, hwaddr offset,
>                                   unsigned size)
>  {
> +    ICPCtrlRegsState *s = opaque;
> +
>      switch (offset >> 2) {
>      case 0: /* CP_IDFIELD */
>          return 0x41034003;
>      case 1: /* CP_FLASHPROG */
>          return 0;
>      case 2: /* CP_INTREG */
> -        return 0;
> +        return s->intreg_state;
>      case 3: /* CP_DECODE */
>          return 0x11;
>      default:
> @@ -439,9 +450,14 @@ static uint64_t icp_control_read(void *opaque, hwaddr offset,
>  static void icp_control_write(void *opaque, hwaddr offset,
>                            uint64_t value, unsigned size)
>  {
> +    ICPCtrlRegsState *s = opaque;
> +
>      switch (offset >> 2) {
> -    case 1: /* CP_FLASHPROG */
>      case 2: /* CP_INTREG */
> +        s->intreg_state &= ~(value & ICP_INTREG_CARDIN);
> +        qemu_set_irq(s->mmc_irq, !!(s->intreg_state & ICP_INTREG_CARDIN));
> +        break;
> +    case 1: /* CP_FLASHPROG */
>      case 3: /* CP_DECODE */
>          /* Nothing interesting implemented yet.  */
>          break;
> @@ -456,14 +472,41 @@ static const MemoryRegionOps icp_control_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> +static void icp_control_mmc_wprot(void *opaque, int line, int level)
> +{
> +    ICPCtrlRegsState *s = opaque;
> +
> +    s->intreg_state &= ~ICP_INTREG_WPROT;
> +    if (level) {
> +        s->intreg_state |= ICP_INTREG_WPROT;
> +    }
> +}
> +
> +static void icp_control_mmc_cardin(void *opaque, int line, int level)
> +{
> +    ICPCtrlRegsState *s = opaque;
> +
> +    /* line is released by writing to CP_INTREG */
> +    if (level) {
> +        s->intreg_state |= ICP_INTREG_CARDIN;
> +        qemu_set_irq(s->mmc_irq, 1);
> +    }
> +}
> +
>  static void icp_control_init(Object *obj)
>  {
>      SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>      ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
> +    DeviceState *dev = DEVICE(obj);
>
>      memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
>                            "icp_ctrl_regs", 0x00800000);
>      sysbus_init_mmio(sbd, &s->iomem);
> +
> +    qdev_init_gpio_in_named(dev, icp_control_mmc_wprot, ICP_GPIO_MMC_WPROT, 1);
> +    qdev_init_gpio_in_named(dev, icp_control_mmc_cardin,
> +                            ICP_GPIO_MMC_CARDIN, 1);
> +    sysbus_init_irq(sbd, &s->mmc_irq);
>  }
>
>
> @@ -488,7 +531,7 @@ static void integratorcp_init(MachineState *machine)
>      MemoryRegion *ram = g_new(MemoryRegion, 1);
>      MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
>      qemu_irq pic[32];
> -    DeviceState *dev;
> +    DeviceState *dev, *sic, *icp;
>      int i;
>      Error *err = NULL;
>
> @@ -546,17 +589,24 @@ static void integratorcp_init(MachineState *machine)
>      for (i = 0; i < 32; i++) {
>          pic[i] = qdev_get_gpio_in(dev, i);
>      }
> -    sysbus_create_simple(TYPE_INTEGRATOR_PIC, 0xca000000, pic[26]);
> +    sic = sysbus_create_simple(TYPE_INTEGRATOR_PIC, 0xca000000, pic[26]);
>      sysbus_create_varargs("integrator_pit", 0x13000000,
>                            pic[5], pic[6], pic[7], NULL);
>      sysbus_create_simple("pl031", 0x15000000, pic[8]);
>      sysbus_create_simple("pl011", 0x16000000, pic[1]);
>      sysbus_create_simple("pl011", 0x17000000, pic[2]);
> -    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
> +    icp = sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000,
> +                               qdev_get_gpio_in(sic, 3));
>      sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
>      sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
>      sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
> -    sysbus_create_varargs("pl181", 0x1c000000, pic[23], pic[24], NULL);
> +
> +    dev = sysbus_create_varargs("pl181", 0x1c000000, pic[23], pic[24], NULL);
> +    qdev_connect_gpio_out(dev, 0,
> +                          qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_WPROT, 0));
> +    qdev_connect_gpio_out(dev, 1,
> +                          qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_CARDIN, 0));
> +
>      if (nd_table[0].used)
>          smc91c111_init(&nd_table[0], 0xc8000000, pic[27]);
>
> --
> 2.1.4
>
>

[Qemu-devel] [PATCH v4 1/2] integrator/cp: Model CP control registers as sysbus device

[Jan Kiszka]

No new features yet, just encapsulation.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v4:
 - fixed up type name as Peter suggested

 hw/arm/integratorcp.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 8c48b68..5c44c34 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -406,6 +406,18 @@ static int icp_pic_init(SysBusDevice *sbd)
 
 /* CP control registers.  */
 
+#define TYPE_ICP_CONTROL_REGS "icp-ctrl-regs"
+#define ICP_CONTROL_REGS(obj) \
+    OBJECT_CHECK(ICPCtrlRegsState, (obj), TYPE_ICP_CONTROL_REGS)
+
+typedef struct ICPCtrlRegsState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+} ICPCtrlRegsState;
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
                                  unsigned size)
 {
@@ -444,15 +456,14 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void icp_control_init(hwaddr base)
+static void icp_control_init(Object *obj)
 {
-    MemoryRegion *io;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
 
-    io = (MemoryRegion *)g_malloc0(sizeof(MemoryRegion));
-    memory_region_init_io(io, NULL, &icp_control_ops, NULL,
-                          "control", 0x00800000);
-    memory_region_add_subregion(get_system_memory(), base, io);
-    /* ??? Save/restore.  */
+    memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
+                          "icp_ctrl_regs", 0x00800000);
+    sysbus_init_mmio(sbd, &s->iomem);
 }
 
 
@@ -541,7 +552,7 @@ static void integratorcp_init(MachineState *machine)
     sysbus_create_simple("pl031", 0x15000000, pic[8]);
     sysbus_create_simple("pl011", 0x16000000, pic[1]);
     sysbus_create_simple("pl011", 0x17000000, pic[2]);
-    icp_control_init(0xcb000000);
+    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
     sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
     sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
     sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
@@ -606,10 +617,18 @@ static const TypeInfo icp_pic_info = {
     .class_init    = icp_pic_class_init,
 };
 
+static const TypeInfo icp_ctrl_regs_info = {
+    .name          = TYPE_ICP_CONTROL_REGS,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ICPCtrlRegsState),
+    .instance_init = icp_control_init,
+};
+
 static void integratorcp_register_types(void)
 {
     type_register_static(&icp_pic_info);
     type_register_static(&core_info);
+    type_register_static(&icp_ctrl_regs_info);
 }
 
 type_init(integratorcp_register_types)
-- 
2.1.4

Re: [Qemu-devel] [PATCH v4 1/2] integrator/cp: Model CP control registers as sysbus device

[Peter Crosthwaite]

On Mon, Feb 23, 2015 at 11:03 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> No new features yet, just encapsulation.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>
> Changes in v4:
>  - fixed up type name as Peter suggested
>
>  hw/arm/integratorcp.c | 35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
> index 8c48b68..5c44c34 100644
> --- a/hw/arm/integratorcp.c
> +++ b/hw/arm/integratorcp.c
> @@ -406,6 +406,18 @@ static int icp_pic_init(SysBusDevice *sbd)
>
>  /* CP control registers.  */
>
> +#define TYPE_ICP_CONTROL_REGS "icp-ctrl-regs"
> +#define ICP_CONTROL_REGS(obj) \
> +    OBJECT_CHECK(ICPCtrlRegsState, (obj), TYPE_ICP_CONTROL_REGS)
> +
> +typedef struct ICPCtrlRegsState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    MemoryRegion iomem;
> +} ICPCtrlRegsState;
> +
>  static uint64_t icp_control_read(void *opaque, hwaddr offset,
>                                   unsigned size)
>  {
> @@ -444,15 +456,14 @@ static const MemoryRegionOps icp_control_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> -static void icp_control_init(hwaddr base)
> +static void icp_control_init(Object *obj)
>  {
> -    MemoryRegion *io;
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
>
> -    io = (MemoryRegion *)g_malloc0(sizeof(MemoryRegion));
> -    memory_region_init_io(io, NULL, &icp_control_ops, NULL,
> -                          "control", 0x00800000);
> -    memory_region_add_subregion(get_system_memory(), base, io);
> -    /* ??? Save/restore.  */
> +    memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
> +                          "icp_ctrl_regs", 0x00800000);
> +    sysbus_init_mmio(sbd, &s->iomem);
>  }
>
>
> @@ -541,7 +552,7 @@ static void integratorcp_init(MachineState *machine)
>      sysbus_create_simple("pl031", 0x15000000, pic[8]);
>      sysbus_create_simple("pl011", 0x16000000, pic[1]);
>      sysbus_create_simple("pl011", 0x17000000, pic[2]);
> -    icp_control_init(0xcb000000);
> +    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
>      sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
>      sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
>      sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
> @@ -606,10 +617,18 @@ static const TypeInfo icp_pic_info = {
>      .class_init    = icp_pic_class_init,
>  };
>
> +static const TypeInfo icp_ctrl_regs_info = {
> +    .name          = TYPE_ICP_CONTROL_REGS,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(ICPCtrlRegsState),
> +    .instance_init = icp_control_init,
> +};
> +
>  static void integratorcp_register_types(void)
>  {
>      type_register_static(&icp_pic_info);
>      type_register_static(&core_info);
> +    type_register_static(&icp_ctrl_regs_info);
>  }
>
>  type_init(integratorcp_register_types)
> --
> 2.1.4
>

Re: [Qemu-devel] [PATCH v4 1/2] integrator/cp: Model CP control registers as sysbus device

[Peter Maydell]

On 24 February 2015 at 07:03, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> No new features yet, just encapsulation.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v4:
>  - fixed up type name as Peter suggested

I can't see the rest of v4 on the list and neither can
the 'patches' tool -- the latest it has is v3. Can
you resend, please?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 1/2] integrator/cp: Model CP control registers as sysbus device

[Jan Kiszka]

Am 2015-03-10 um 17:03 schrieb Peter Maydell:
> On 24 February 2015 at 07:03, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> No new features yet, just encapsulation.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v4:
>>  - fixed up type name as Peter suggested
> 
> I can't see the rest of v4 on the list and neither can
> the 'patches' tool -- the latest it has is v3. Can
> you resend, please?

There is no v4 of patch 2, it's identical to v3. I can resent if the
tooling requires it, though.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v4 1/2] integrator/cp: Model CP control registers as sysbus device

[Peter Maydell]

On 10 March 2015 at 16:11, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> Am 2015-03-10 um 17:03 schrieb Peter Maydell:
>> On 24 February 2015 at 07:03, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> I can't see the rest of v4 on the list and neither can
>> the 'patches' tool -- the latest it has is v3. Can
>> you resend, please?
>
> There is no v4 of patch 2, it's identical to v3. I can resent if the
> tooling requires it, though.

Yes, please. Tooling tends to treat half-patchsets as
being incomplete and ignores them til the rest arrives.
(This is why http://wiki.qemu.org/Contribute/SubmitAPatch
says to resend the whole series.)

-- PMM

[Qemu-devel] [PATCH v4 1/2] integrator/cp: Model CP control registers as sysbus device

[Jan Kiszka]

No new features yet, just encapsulation.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/arm/integratorcp.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 949ae1e..0dbda3a 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -406,6 +406,18 @@ static int icp_pic_init(SysBusDevice *sbd)
 
 /* CP control registers.  */
 
+#define TYPE_ICP_CONTROL_REGS "icp-ctrl-regs"
+#define ICP_CONTROL_REGS(obj) \
+    OBJECT_CHECK(ICPCtrlRegsState, (obj), TYPE_ICP_CONTROL_REGS)
+
+typedef struct ICPCtrlRegsState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    MemoryRegion iomem;
+} ICPCtrlRegsState;
+
 static uint64_t icp_control_read(void *opaque, hwaddr offset,
                                  unsigned size)
 {
@@ -444,15 +456,14 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void icp_control_init(hwaddr base)
+static void icp_control_init(Object *obj)
 {
-    MemoryRegion *io;
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    ICPCtrlRegsState *s = ICP_CONTROL_REGS(obj);
 
-    io = (MemoryRegion *)g_malloc0(sizeof(MemoryRegion));
-    memory_region_init_io(io, NULL, &icp_control_ops, NULL,
-                          "control", 0x00800000);
-    memory_region_add_subregion(get_system_memory(), base, io);
-    /* ??? Save/restore.  */
+    memory_region_init_io(&s->iomem, OBJECT(s), &icp_control_ops, s,
+                          "icp_ctrl_regs", 0x00800000);
+    sysbus_init_mmio(sbd, &s->iomem);
 }
 
 
@@ -541,7 +552,7 @@ static void integratorcp_init(MachineState *machine)
     sysbus_create_simple("pl031", 0x15000000, pic[8]);
     sysbus_create_simple("pl011", 0x16000000, pic[1]);
     sysbus_create_simple("pl011", 0x17000000, pic[2]);
-    icp_control_init(0xcb000000);
+    sysbus_create_simple(TYPE_ICP_CONTROL_REGS, 0xcb000000, NULL);
     sysbus_create_simple("pl050_keyboard", 0x18000000, pic[3]);
     sysbus_create_simple("pl050_mouse", 0x19000000, pic[4]);
     sysbus_create_simple(TYPE_INTEGRATOR_DEBUG, 0x1a000000, 0);
@@ -606,10 +617,18 @@ static const TypeInfo icp_pic_info = {
     .class_init    = icp_pic_class_init,
 };
 
+static const TypeInfo icp_ctrl_regs_info = {
+    .name          = TYPE_ICP_CONTROL_REGS,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ICPCtrlRegsState),
+    .instance_init = icp_control_init,
+};
+
 static void integratorcp_register_types(void)
 {
     type_register_static(&icp_pic_info);
     type_register_static(&core_info);
+    type_register_static(&icp_ctrl_regs_info);
 }
 
 type_init(integratorcp_register_types)
-- 
2.1.4