Skip to contentSkip to conversation

integrator/cp: Working SD card support

PATCH Superseded Jan Kiszka 2 participants 5 messages Feb 19, 2015 – Feb 21, 2015

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

v2 cover integrator/cp: Working SD card support

This addresses the review comments on the primitive first version.

Jan

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

 hw/arm/integratorcp.c | 99 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 86 insertions(+), 13 deletions(-)
Signature · 1 line
2.1.4
author
#1 lore raw

PATCH v2 1/2 integrator/cp: Model CP control registers as sysbus device

No new features yet, just encapsulation.

---
 hw/arm/integratorcp.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)
hw/arm/integratorcp.c +35 -8
index 8c48b68..c5aa1f6 100644
@@ -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,15 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void icp_control_init(hwaddr base)
+static int icp_control_init(SysBusDevice *sbd)
 {
-    MemoryRegion *io;
+    ICPCtrlRegsState *s = ICP_CONTROL_REGS(sbd);
 
-    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);
+
+    return 0;
 }
 
 
@@ -541,7 +553,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 +618,25 @@ static const TypeInfo icp_pic_info = {
     .class_init    = icp_pic_class_init,
 };
 
+static void icp_class_init(ObjectClass *klass, void *data)
+{
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = icp_control_init;
+}
+
+static const TypeInfo icp_ctrl_regs_info = {
+    .name          = TYPE_ICP_CONTROL_REGS,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ICPCtrlRegsState),
+    .class_init    = icp_class_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)
Signature · 1 line
2.1.4
#2 lore raw

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

Quoted 63 lines · Jan Kiszka

No new features yet, just encapsulation.

---
 hw/arm/integratorcp.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)
hw/arm/integratorcp.c +20 -8
index 8c48b68..c5aa1f6 100644
@@ -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,15 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void icp_control_init(hwaddr base)
+static int icp_control_init(SysBusDevice *sbd)
 {
-    MemoryRegion *io;
+    ICPCtrlRegsState *s = ICP_CONTROL_REGS(sbd);
 
-    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);
+
+    return 0;
 }
 
 
@@ -541,7 +553,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);

hw/arm/integratorcp.c static const TypeInfo icp_pic_info = { · line 625

Quoted 8 lines
hw/arm/integratorcp.c +5 -0
@@ -606,10 +618,25 @@ static const TypeInfo icp_pic_info = {
     .class_init    = icp_pic_class_init,
 };
 
+static void icp_class_init(ObjectClass *klass, void *data)
+{
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = icp_control_init;

SysBusDevice::init is depracated, please use Object init() or Device::realize instead.

For this one, your device is small enough to not need realize(), you can probably use just an object init then you don't even need a class_init at all. musicpal_misc_init is a good example.

hw/arm/integratorcp.c static const TypeInfo icp_pic_info = { · line 632

Quoted 7 lines
hw/arm/integratorcp.c +7 -0
+}
+
+static const TypeInfo icp_ctrl_regs_info = {
+    .name          = TYPE_ICP_CONTROL_REGS,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(ICPCtrlRegsState),
+    .class_init    = icp_class_init,

.instance_init = icp_init

Regards,
Peter

Quoted 14 lines
hw/arm/integratorcp.c +3 -1
+};
+
 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

author
#3 lore raw

PATCH v2 2/2 integrator/cp: Implement CARDIN and WPROT signals

In reply to 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.

---
 hw/arm/integratorcp.c | 58 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 52 insertions(+), 6 deletions(-)
hw/arm/integratorcp.c +52 -6
index c5aa1f6..39803a9 100644
@@ -416,18 +416,29 @@ typedef struct ICPCtrlRegsState {
     /*< public >*/
 
     MemoryRegion iomem;
+
+    qemu_irq mmc_irq;
+    uint32_t intreg_state;
 } ICPCtrlRegsState;
 
+#define ICP_GPIO_MMC_WPROT      0
+#define ICP_GPIO_MMC_CARDIN     1
+
+#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,39 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void icp_control_gpio_set(void *opaque, int line, int level)
+{
+    ICPCtrlRegsState *s = opaque;
+
+    switch (line) {
+    case ICP_GPIO_MMC_WPROT:
+        s->intreg_state &= ~ICP_INTREG_WPROT;
+        if (level) {
+            s->intreg_state |= ICP_INTREG_WPROT;
+        }
+        break;
+    case ICP_GPIO_MMC_CARDIN:
+        /* line is released by writing to CP_INTREG */
+        if (level) {
+            s->intreg_state |= ICP_INTREG_CARDIN;
+            qemu_set_irq(s->mmc_irq, 1);
+        }
+        break;
+    }
+}
+
 static int icp_control_init(SysBusDevice *sbd)
 {
     ICPCtrlRegsState *s = ICP_CONTROL_REGS(sbd);
+    DeviceState *dev = DEVICE(sbd);
 
     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(dev, icp_control_gpio_set, 2);
+    sysbus_init_irq(sbd, &s->mmc_irq);
+
     return 0;
 }
 
@@ -489,7 +530,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;
 
@@ -547,17 +588,22 @@ 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(icp, ICP_GPIO_MMC_WPROT));
+    qdev_connect_gpio_out(dev, 1, qdev_get_gpio_in(icp, ICP_GPIO_MMC_CARDIN));
+
     if (nd_table[0].used)
         smc91c111_init(&nd_table[0], 0xc8000000, pic[27]);
 
Signature · 1 line
2.1.4
#4 lore raw

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

Quoted 76 lines · 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.

---
 hw/arm/integratorcp.c | 58 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 52 insertions(+), 6 deletions(-)
hw/arm/integratorcp.c +29 -2
index c5aa1f6..39803a9 100644
@@ -416,18 +416,29 @@ typedef struct ICPCtrlRegsState {
     /*< public >*/
 
     MemoryRegion iomem;
+
+    qemu_irq mmc_irq;
+    uint32_t intreg_state;
 } ICPCtrlRegsState;
 
+#define ICP_GPIO_MMC_WPROT      0
+#define ICP_GPIO_MMC_CARDIN     1
+
+#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,39 @@ static const MemoryRegionOps icp_control_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static void icp_control_gpio_set(void *opaque, int line, int level)
+{
+    ICPCtrlRegsState *s = opaque;
+
+    switch (line) {
+    case ICP_GPIO_MMC_WPROT:
+        s->intreg_state &= ~ICP_INTREG_WPROT;
+        if (level) {
+            s->intreg_state |= ICP_INTREG_WPROT;
+        }
+        break;

hw/arm/integratorcp.c static const MemoryRegionOps icp_control_ops = { · line 495

Quoted 10 lines
hw/arm/integratorcp.c +10 -0
+    case ICP_GPIO_MMC_CARDIN:
+        /* line is released by writing to CP_INTREG */
+        if (level) {
+            s->intreg_state |= ICP_INTREG_CARDIN;
+            qemu_set_irq(s->mmc_irq, 1);
+        }
+        break;
+    }
+}
+

qdev_init_gpio_in_named lets you init multiple GPIOs with different handler fns without having to enumerate then and switch-case them like this.

hw/arm/integratorcp.c static const MemoryRegionOps icp_control_ops = { · line 505

Quoted 10 lines
hw/arm/integratorcp.c +2 -0
 static int icp_control_init(SysBusDevice *sbd)
 {
     ICPCtrlRegsState *s = ICP_CONTROL_REGS(sbd);
+    DeviceState *dev = DEVICE(sbd);
 
     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(dev, icp_control_gpio_set, 2);

So you would be something like:

qdev_init_gpio_in_named(dev, icp_writeprot_gpio, "write-prot", 1); qdev_init_gpio_in_named(dev, icp_cardin_gpio, "card-in", 1);

Strings should be macroified (would replace existing enumeration macros).

Regards,
Peter

Quoted 44 lines
hw/arm/integratorcp.c +11 -5
+    sysbus_init_irq(sbd, &s->mmc_irq);
+
     return 0;
 }
 
@@ -489,7 +530,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;
 
@@ -547,17 +588,22 @@ 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(icp, ICP_GPIO_MMC_WPROT));
+    qdev_connect_gpio_out(dev, 1, qdev_get_gpio_in(icp, ICP_GPIO_MMC_CARDIN));
+
     if (nd_table[0].used)
         smc91c111_init(&nd_table[0], 0xc8000000, pic[27]);
 
--

2.1.4