Netduino Plus 2 Machine Model

This patch adds the Netduino Plus 2 USART controller (UART also uses the same controller).

It can be used for reading and writing to the guest.

Signed-off-by Alistair Francis <[email protected]>

---
 default-configs/arm-softmmu.mak |   1 +
 hw/char/Makefile.objs           |   1 +
 hw/char/netduino_usart.c        | 252 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 254 insertions(+)
 create mode 100644 hw/char/netduino_usart.c

default-configs/arm-softmmu.mak +1 -0

index f3513fa..46471ad 100644
@@ -78,6 +78,7 @@ CONFIG_NSERIES=y
 CONFIG_REALVIEW=y
 CONFIG_ZAURUS=y
 CONFIG_ZYNQ=y
+CONFIG_NETDUINOP2=y
 
 CONFIG_VERSATILE_PCI=y
 CONFIG_VERSATILE_I2C=y

hw/char/Makefile.objs +1 -0

index 317385d..c03aa98 100644
@@ -15,6 +15,7 @@ obj-$(CONFIG_OMAP) += omap_uart.o
 obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
 obj-$(CONFIG_DIGIC) += digic-uart.o
+obj-$(CONFIG_NETDUINOP2) += netduino_usart.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o

hw/char/netduino_usart.c new file +0 -0

new file mode 100644
index 0000000..437af2b

hw/char/netduino_usart.c new file +16 -0

+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

hw/char/netduino_usart.c new file +11 -0

+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define USART_SR   0x00
+#define USART_DR   0x04
+#define USART_BRR  0x08
+#define USART_CR1  0x0C
+#define USART_CR2  0x10
+#define USART_CR3  0x14
+#define USART_GTPR 0x18

hw/char/netduino_usart.c new file +5 -0

+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    uint32_t usart_sr;

hw/char/netduino_usart.c new file +8 -0

+
+static int usart_can_receive(void *opaque)
+{
+    struct net_usart *s = opaque;
+
+    if (s->usart_cr1 & USART_CR1_UE && s->usart_cr1 & USART_CR1_TE) {
+        return 1;
+    }

hw/char/netduino_usart.c new file +20 -0

+}
+
+static void usart_reset(DeviceState *dev)
+{
+    struct net_usart *s = NETDUINO_USART(dev);
+
+    s->usart_sr = 0x00C00000;
+    s->usart_dr = 0x00000000;
+    s->usart_brr = 0x00000000;
+    s->usart_cr1 = 0x00000000;
+    s->usart_cr2 = 0x00000000;
+    s->usart_cr3 = 0x00000000;
+    s->usart_gtpr = 0x00000000;
+}
+
+static uint64_t netduino_usart_read(void *opaque, hwaddr addr,
+                                    unsigned int size)
+{
+    struct net_usart *s = opaque;
+    uint64_t retvalue;

hw/char/netduino_usart.c new file +79 -0

+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_usart_read: Bad offset %x\n", (int)addr);
+        return 0;
+    }
+
+    return 0;
+}
+
+static void netduino_usart_write(void *opaque, hwaddr addr,
+                       uint64_t val64, unsigned int size)
+{
+    struct net_usart *s = opaque;
+    uint32_t value = (uint32_t) val64;
+    unsigned char ch;
+
+    DPRINTF("Write 0x%x, 0x%x\n", value, (uint) addr);
+
+    switch (addr) {
+    case USART_SR:
+        if (value <= 0x3FF) {
+            s->usart_sr = value;
+        } else {
+            s->usart_sr &= value;
+        }
+        return;
+    case USART_DR:
+        if (value < 0xF000) {
+            ch = value;
+            if (s->chr) {
+                qemu_chr_fe_write(s->chr, &ch, 1);
+            }
+            s->usart_sr |= USART_SR_TC;
+        }
+        return;
+    case USART_BRR:
+        s->usart_brr = value;
+        return;
+    case USART_CR1:
+        s->usart_cr1 = value;
+        return;
+    case USART_CR2:
+        s->usart_cr2 = value;
+        return;
+    case USART_CR3:
+        s->usart_cr3 = value;
+        return;
+    case USART_GTPR:
+        s->usart_gtpr = value;
+        return;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_usart_write: Bad offset %x\n", (int)addr);
+    }
+}
+
+static const MemoryRegionOps netduino_usart_ops = {
+    .read = netduino_usart_read,
+    .write = netduino_usart_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int netduino_usart_init(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);
+    struct net_usart *s = NETDUINO_USART(dev);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &netduino_usart_ops, s,
+                          TYPE_NETDUINO_USART, 0x2000);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    s->chr = qemu_char_get_next_serial();
+
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, usart_can_receive, usart_receive,
+                              NULL, s);
+    }

hw/char/netduino_usart.c new file +16 -1

+    dc->reset = usart_reset;
+}
+
+static const TypeInfo netduino_usart_info = {
+    .name          = TYPE_NETDUINO_USART,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(struct net_usart),
+    .class_init    = netduino_usart_class_init,
+};
+
+static void netduino_usart_register_types(void)
+{
+    type_register_static(&netduino_usart_info);
+}
+
+type_init(netduino_usart_register_types)
--

1.9.1

Quoted 5 lines · Alistair Francis

This patch adds the Netduino Plus 2 USART controller (UART also uses the same controller).

It can be used for reading and writing to the guest.

Do you just mean doing IO with the system?

Quoted 40 lines

Signed-off-by Alistair Francis <[email protected]>

---
 default-configs/arm-softmmu.mak |   1 +
 hw/char/Makefile.objs           |   1 +
 hw/char/netduino_usart.c        | 252 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 254 insertions(+)
 create mode 100644 hw/char/netduino_usart.c

default-configs/arm-softmmu.mak +1 -0

index f3513fa..46471ad 100644
@@ -78,6 +78,7 @@ CONFIG_NSERIES=y
 CONFIG_REALVIEW=y
 CONFIG_ZAURUS=y
 CONFIG_ZYNQ=y
+CONFIG_NETDUINOP2=y
 
 CONFIG_VERSATILE_PCI=y
 CONFIG_VERSATILE_I2C=y

hw/char/Makefile.objs +1 -0

index 317385d..c03aa98 100644
@@ -15,6 +15,7 @@ obj-$(CONFIG_OMAP) += omap_uart.o
 obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
 obj-$(CONFIG_DIGIC) += digic-uart.o
+obj-$(CONFIG_NETDUINOP2) += netduino_usart.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o

hw/char/netduino_usart.c new file +2 -0

new file mode 100644
index 0000000..437af2b
@@ -0,0 +1,252 @@
+/*
+ * Netduino Plus 2 USART

hw/char/netduino_usart.c line 3

Quoted 1 line

hw/char/netduino_usart.c new file +1 -0

+ *

I thing the UART IP is really part of the ST Microcontroller, and should be named as such. Do ST give this controller a meaningful name in their documentation? Looking at the schematic for Netduino it looks like there is very little board level IP. All the sysbus IP is local to the microcontroller. I think the name "netduino" should probably only be limited to the board level.

Quoted 17 lines

hw/char/netduino_usart.c new file +16 -0

+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

hw/char/netduino_usart.c line 29

Quoted 10 lines

hw/char/netduino_usart.c new file +10 -0

+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "sysemu/char.h"
+#include "hw/hw.h"
+
+/* #define DEBUG_NETUSART */

Don't worry about commented out //#defines

hw/char/netduino_usart.c line 32

Quoted 3 lines

hw/char/netduino_usart.c new file +3 -0

+
+#ifdef DEBUG_NETUSART
+#define DPRINTF(fmt, ...) \

You should change this to a always-compiled print using a regular if. Check hw/dma/pl330.c

hw/char/netduino_usart.c line 33

Quoted 2 lines

hw/char/netduino_usart.c new file +1 -0

+do { printf("netduino_usart: " fmt , ## __VA_ARGS__); } while (0)

Multiple lines to keep formatting standard even within #define. use \ as needed.

Use qemu_log instead of printf. (pl330 fails at this too).

Quoted 15 lines

hw/char/netduino_usart.c new file +15 -0

+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define USART_SR   0x00
+#define USART_DR   0x04
+#define USART_BRR  0x08
+#define USART_CR1  0x0C
+#define USART_CR2  0x10
+#define USART_CR3  0x14
+#define USART_GTPR 0x18
+
+#define USART_SR_TXE  (1 << 7)
+#define USART_SR_TC   (1 << 6)
+#define USART_SR_RXNE (1 << 5)

hw/char/netduino_usart.c line 54

Quoted 6 lines

hw/char/netduino_usart.c new file +6 -0

+
+#define USART_CR1_UE  (1 << 13)
+#define USART_CR1_RXNEIE  (1 << 5)
+#define USART_CR1_TE  (1 << 3)
+
+#define TYPE_NETDUINO_USART "netduino_usart"

Change "_" to "-" in QOM typename.

hw/char/netduino_usart.c line 58

Quoted 4 lines

hw/char/netduino_usart.c new file +4 -0

+#define NETDUINO_USART(obj) \
+    OBJECT_CHECK(struct net_usart, (obj), TYPE_NETDUINO_USART)
+
+struct net_usart {

CamelCase in typename. Use a typedef to avoid the repeated struct Foo all throughout.

Quoted 10 lines

hw/char/netduino_usart.c new file +10 -0

+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    uint32_t usart_sr;
+    uint32_t usart_dr;
+    uint32_t usart_brr;
+    uint32_t usart_cr1;
+    uint32_t usart_cr2;
+    uint32_t usart_cr3;

hw/char/netduino_usart.c line 73

Quoted 5 lines

hw/char/netduino_usart.c new file +5 -0

+    uint32_t usart_gtpr;
+
+    CharDriverState *chr;
+    qemu_irq irq;
+};

The struct, QOM typename and register defines should go in a device specific header. Check include/hw/char/digic_uart.h for an example.

Quoted 12 lines

hw/char/netduino_usart.c new file +12 -0

+
+static int usart_can_receive(void *opaque)
+{
+    struct net_usart *s = opaque;
+
+    if (s->usart_cr1 & USART_CR1_UE && s->usart_cr1 & USART_CR1_TE) {
+        return 1;
+    }
+
+    return 0;
+}
+

hw/char/netduino_usart.c line 91

Quoted 6 lines

hw/char/netduino_usart.c new file +6 -0

+static void usart_receive(void *opaque, const uint8_t *buf, int size)
+{
+    struct net_usart *s = opaque;
+    int i, num;
+
+    num = size > 8 ? 8 : size;

MAX macro.

hw/char/netduino_usart.c line 96

Quoted 5 lines

hw/char/netduino_usart.c new file +5 -0

+    s->usart_dr = 0;
+
+    for (i = 0; i < num; i++) {
+        s->usart_dr |= (buf[i] << i);
+    }

This seems strange. dr is an oring of multiple rxed characters? Is RX information lost here when size > 1?

hw/char/netduino_usart.c line 104

Quoted 8 lines

hw/char/netduino_usart.c new file +8 -0

+
+    s->usart_sr |= USART_SR_RXNE;
+
+    if (s->usart_cr1 & USART_CR1_RXNEIE) {
+        qemu_set_irq(s->irq, 1);
+    }
+
+    DPRINTF("Inputting: %c\n", s->usart_dr);

"receiving"

Quoted 20 lines

hw/char/netduino_usart.c new file +20 -0

+}
+
+static void usart_reset(DeviceState *dev)
+{
+    struct net_usart *s = NETDUINO_USART(dev);
+
+    s->usart_sr = 0x00C00000;
+    s->usart_dr = 0x00000000;
+    s->usart_brr = 0x00000000;
+    s->usart_cr1 = 0x00000000;
+    s->usart_cr2 = 0x00000000;
+    s->usart_cr3 = 0x00000000;
+    s->usart_gtpr = 0x00000000;
+}
+
+static uint64_t netduino_usart_read(void *opaque, hwaddr addr,
+                                    unsigned int size)
+{
+    struct net_usart *s = opaque;
+    uint64_t retvalue;

hw/char/netduino_usart.c line 134

Quoted 10 lines

hw/char/netduino_usart.c new file +10 -0

+
+    DPRINTF("Read 0x%x\n", (uint) addr);
+
+    switch (addr) {
+    case USART_SR:
+        retvalue = s->usart_sr;
+        s->usart_sr &= (USART_SR_TC ^ 0xFFFF);
+        return retvalue;
+    case USART_DR:
+        DPRINTF("Value: %x, %c\n", s->usart_dr, s->usart_dr);

PRIx32 instead of %x. Cast the %c version.

hw/char/netduino_usart.c line 146

Quoted 12 lines

hw/char/netduino_usart.c new file +12 -0

+        s->usart_sr |= USART_SR_TXE;
+        return s->usart_dr & 0x3FF;
+    case USART_BRR:
+        return s->usart_brr;
+    case USART_CR1:
+        return s->usart_cr1;
+    case USART_CR2:
+        return s->usart_cr2;
+    case USART_CR3:
+        return s->usart_cr3;
+    case USART_GTPR:
+        return s->usart_gtpr;

If you make your registers an array, you can index the with the offset and remove the repeated s->foo return logic. You can also use a single memset to do all the 0 resets.

Quoted 83 lines

hw/char/netduino_usart.c new file +83 -0

+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_usart_read: Bad offset %x\n", (int)addr);
+        return 0;
+    }
+
+    return 0;
+}
+
+static void netduino_usart_write(void *opaque, hwaddr addr,
+                       uint64_t val64, unsigned int size)
+{
+    struct net_usart *s = opaque;
+    uint32_t value = (uint32_t) val64;
+    unsigned char ch;
+
+    DPRINTF("Write 0x%x, 0x%x\n", value, (uint) addr);
+
+    switch (addr) {
+    case USART_SR:
+        if (value <= 0x3FF) {
+            s->usart_sr = value;
+        } else {
+            s->usart_sr &= value;
+        }
+        return;
+    case USART_DR:
+        if (value < 0xF000) {
+            ch = value;
+            if (s->chr) {
+                qemu_chr_fe_write(s->chr, &ch, 1);
+            }
+            s->usart_sr |= USART_SR_TC;
+        }
+        return;
+    case USART_BRR:
+        s->usart_brr = value;
+        return;
+    case USART_CR1:
+        s->usart_cr1 = value;
+        return;
+    case USART_CR2:
+        s->usart_cr2 = value;
+        return;
+    case USART_CR3:
+        s->usart_cr3 = value;
+        return;
+    case USART_GTPR:
+        s->usart_gtpr = value;
+        return;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_usart_write: Bad offset %x\n", (int)addr);
+    }
+}
+
+static const MemoryRegionOps netduino_usart_ops = {
+    .read = netduino_usart_read,
+    .write = netduino_usart_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int netduino_usart_init(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);
+    struct net_usart *s = NETDUINO_USART(dev);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &netduino_usart_ops, s,
+                          TYPE_NETDUINO_USART, 0x2000);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    s->chr = qemu_char_get_next_serial();
+
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, usart_can_receive, usart_receive,
+                              NULL, s);
+    }
+
+    return 0;
+}
+

hw/char/netduino_usart.c line 235

Quoted 6 lines

hw/char/netduino_usart.c new file +6 -0

+static void netduino_usart_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = netduino_usart_init;

Use of Sysbus::init is deprecated, please use device init and realize instead.

hw/char/netduino_usart.c line 236

Quoted 1 line

hw/char/netduino_usart.c new file +1 -0

+    dc->props = NULL;

Unneeded.

Some comments also apply to the following patches.

Regards,
Peter

Quoted 20 lines

hw/char/netduino_usart.c new file +16 -1

+    dc->reset = usart_reset;
+}
+
+static const TypeInfo netduino_usart_info = {
+    .name          = TYPE_NETDUINO_USART,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(struct net_usart),
+    .class_init    = netduino_usart_class_init,
+};
+
+static void netduino_usart_register_types(void)
+{
+    type_register_static(&netduino_usart_info);
+}
+
+type_init(netduino_usart_register_types)
--

1.9.1

If you make your registers an array, you can index the with the offset and remove the repeated s->foo return logic. You can also use a single memset to do all the 0 resets.

Quoted 3 lines · Peter Crosthwaite

If you make your registers an array, you can index the with the offset and remove the repeated s->foo return logic. You can also use a single memset to do all the 0 resets.

Hmm. I dislike that style personally, especially for devices like this which only have a handful of registers.

-- PMM

This patch adds the Netduino Plus 2 GPIO controller to QEMU. This allows reading and writing to the Netduino GPIO pins.

Signed-off-by Alistair Francis <[email protected]>

---
 hw/gpio/Makefile.objs   |   1 +
 hw/gpio/netduino_gpio.c | 285 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 286 insertions(+)
 create mode 100644 hw/gpio/netduino_gpio.c

hw/gpio/Makefile.objs +1 -0

index 2c8b51f..e61c4d3 100644
@@ -2,5 +2,6 @@ common-obj-$(CONFIG_MAX7310) += max7310.o
 common-obj-$(CONFIG_PL061) += pl061.o
 common-obj-$(CONFIG_PUV3) += puv3_gpio.o
 common-obj-$(CONFIG_ZAURUS) += zaurus.o
+common-obj-$(CONFIG_NETDUINOP2) += netduino_gpio.o
 
 obj-$(CONFIG_OMAP) += omap_gpio.o

hw/gpio/netduino_gpio.c new file +47 -0

new file mode 100644
index 0000000..10d0bbd
@@ -0,0 +1,285 @@
+/*
+ * Netduino Plus 2 GPIO
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+
+/* #define DEBUG_NETGPIO */
+
+#ifdef DEBUG_NETGPIO
+#define DPRINTF(fmt, ...) \
+do { printf("netduino_gpio: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define GPIO_MODER     0x00
+#define GPIO_OTYPER    0x04
+#define GPIO_OSPEEDR   0x08
+#define GPIO_PUPDR     0x0C
+#define GPIO_IDR       0x10
+#define GPIO_ODR       0x14
+#define GPIO_BSRR      0x18
+#define GPIO_BSRR_HIGH 0x1A
+#define GPIO_LCKR      0x1C
+#define GPIO_AFRL      0x20
+#define GPIO_AFRH      0x24
+

hw/gpio/netduino_gpio.c new file +4 -0

+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+

hw/gpio/netduino_gpio.c new file +33 -0

+    /* The GPIO letter (a - k) from the datasheet */
+    uint8_t gpio_letter;
+
+    qemu_irq irq;
+    const unsigned char *id;
+} NETDUINO_GPIOState;
+
+static const VMStateDescription vmstate_netduino_gpio = {
+    .name = "netduino_gpio",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(gpio_moder, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_otyper, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_ospeedr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_pupdr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_idr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_odr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_bsrr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_lckr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_afrl, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_afrh, NETDUINO_GPIOState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void gpio_reset(DeviceState *dev)
+{
+    NETDUINO_GPIOState *s = NETDUINO_GPIO(dev);
+
+    if (s->gpio_letter == 'a') {
+        s->gpio_moder = 0xA8000000;
+        s->gpio_pupdr = 0x64000000;

hw/gpio/netduino_gpio.c new file +24 -0

+
+    s->gpio_otyper = 0x00000000;
+    s->gpio_idr = 0x00000000;
+    s->gpio_odr = 0x00000000;
+    s->gpio_bsrr = 0x00000000;
+    s->gpio_lckr = 0x00000000;
+    s->gpio_afrl = 0x00000000;
+    s->gpio_afrh = 0x00000000;
+    s->gpio_direction = 0x0000;
+}
+
+static uint64_t netduino_gpio_read(void *opaque, hwaddr offset,
+                           unsigned size)
+{
+    NETDUINO_GPIOState *s = (NETDUINO_GPIOState *)opaque;
+
+    DPRINTF("Read 0x%x\n", (uint) offset);
+
+    switch (offset) {
+    case GPIO_MODER:
+        return s->gpio_moder;
+    case GPIO_OTYPER:
+        return s->gpio_otyper;
+    case GPIO_OSPEEDR:

hw/gpio/netduino_gpio.c new file +12 -0

+    case GPIO_LCKR:
+        return s->gpio_lckr;
+    case GPIO_AFRL:
+        return s->gpio_afrl;
+    case GPIO_AFRH:
+        return s->gpio_afrh;
+    }
+    return 0;
+}
+
+static void netduino_gpio_write(void *opaque, hwaddr offset,
+                        uint64_t value, unsigned size)

hw/gpio/netduino_gpio.c new file +21 -0

+
+            if ((s->gpio_moder & mask) == GPIO_MODER_INPUT) {
+                s->gpio_direction |= (1 << (i/2));
+            } else if ((s->gpio_moder & mask) == GPIO_MODER_GENERAL_OUT) {
+                s->gpio_direction &= (0xFFFF ^ (1 << (i/2)));
+            } else {
+                /* Not supported at the moment */
+            }
+        }
+        return;
+    case GPIO_OTYPER:
+        s->gpio_otyper = (uint32_t) value;
+        return;
+    case GPIO_OSPEEDR:
+        s->gpio_ospeedr = (uint32_t) value;
+        return;
+    case GPIO_PUPDR:
+        s->gpio_pupdr = (uint32_t) value;
+        return;
+    case GPIO_IDR:
+        /* Read Only Register */

hw/gpio/netduino_gpio.c new file +22 -0

+        return;
+    case GPIO_AFRL:
+        s->gpio_afrl = (uint32_t) value;
+        return;
+    case GPIO_AFRH:
+        s->gpio_afrh = (uint32_t) value;
+        return;
+    }
+}
+
+static const MemoryRegionOps netduino_gpio_ops = {
+    .read = netduino_gpio_read,
+    .write = netduino_gpio_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static Property net_gpio_properties[] = {
+    DEFINE_PROP_UINT8("gpio-letter", NETDUINO_GPIOState, gpio_letter,
+                      (uint) 'a'),
+    DEFINE_PROP_END_OF_LIST(),
+};
+

hw/gpio/netduino_gpio.c new file +27 -1

+    return 0;
+}
+
+static void netduino_gpio_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = netduino_gpio_initfn;
+    dc->vmsd = &vmstate_netduino_gpio;
+    dc->props = net_gpio_properties;
+    dc->reset = gpio_reset;
+}
+
+static const TypeInfo netduino_gpio_info = {
+    .name          = TYPE_NETDUINO_GPIO,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(NETDUINO_GPIOState),
+    .class_init    = netduino_gpio_class_init,
+};
+
+static void netduino_gpio_register_types(void)
+{
+    type_register_static(&netduino_gpio_info);
+}
+
+type_init(netduino_gpio_register_types)
--

1.9.1

Quoted 2 lines · Alistair Francis

This patch adds the Netduino Plus 2 GPIO controller to QEMU. This allows reading and writing to the Netduino GPIO pins.

Are you sure this isn't actually the GPIO module in the STM32F4xx SoC ? The datasheets and schematic suggest this isn't a board level feature, which would imply it shouldn't have a board level name like "netduino_gpio". Instead you should have an SoC QOM object/device which instantiates all the SoC level components like the GPIO, UART, etc,
and the board model should just create the SoC and any devices which are genuinely separate components from the SoC.

Quoted 76 lines

Signed-off-by Alistair Francis <[email protected]>

---
 hw/gpio/Makefile.objs   |   1 +
 hw/gpio/netduino_gpio.c | 285 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 286 insertions(+)
 create mode 100644 hw/gpio/netduino_gpio.c

hw/gpio/Makefile.objs +1 -0

index 2c8b51f..e61c4d3 100644
@@ -2,5 +2,6 @@ common-obj-$(CONFIG_MAX7310) += max7310.o
 common-obj-$(CONFIG_PL061) += pl061.o
 common-obj-$(CONFIG_PUV3) += puv3_gpio.o
 common-obj-$(CONFIG_ZAURUS) += zaurus.o
+common-obj-$(CONFIG_NETDUINOP2) += netduino_gpio.o
 
 obj-$(CONFIG_OMAP) += omap_gpio.o

hw/gpio/netduino_gpio.c new file +52 -0

new file mode 100644
index 0000000..10d0bbd
@@ -0,0 +1,285 @@
+/*
+ * Netduino Plus 2 GPIO
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+
+/* #define DEBUG_NETGPIO */
+
+#ifdef DEBUG_NETGPIO
+#define DPRINTF(fmt, ...) \
+do { printf("netduino_gpio: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define GPIO_MODER     0x00
+#define GPIO_OTYPER    0x04
+#define GPIO_OSPEEDR   0x08
+#define GPIO_PUPDR     0x0C
+#define GPIO_IDR       0x10
+#define GPIO_ODR       0x14
+#define GPIO_BSRR      0x18
+#define GPIO_BSRR_HIGH 0x1A
+#define GPIO_LCKR      0x1C
+#define GPIO_AFRL      0x20
+#define GPIO_AFRH      0x24
+
+#define GPIO_MODER_INPUT       0
+#define GPIO_MODER_GENERAL_OUT 1
+#define GPIO_MODER_ALT         2
+#define GPIO_MODER_ANALOG      3
+

hw/gpio/netduino_gpio.c line 57

Quoted 5 lines

hw/gpio/netduino_gpio.c new file +5 -0

+#define TYPE_NETDUINO_GPIO "netduino_gpio"
+#define NETDUINO_GPIO(obj) OBJECT_CHECK(NETDUINO_GPIOState, (obj), \
+                           TYPE_NETDUINO_GPIO)
+
+typedef struct NETDUINO_GPIOState {

This should be "Netduino_GPIOState", since the official boardname isn't an all-caps word.

Quoted 8 lines

hw/gpio/netduino_gpio.c new file +8 -0

+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+
+    uint32_t gpio_moder;
+    uint32_t gpio_otyper;
+    uint32_t gpio_ospeedr;
+    uint32_t gpio_pupdr;

hw/gpio/netduino_gpio.c line 71

Quoted 6 lines

hw/gpio/netduino_gpio.c new file +6 -0

+    uint32_t gpio_idr;
+    uint32_t gpio_odr;
+    uint32_t gpio_bsrr;
+    uint32_t gpio_lckr;
+    uint32_t gpio_afrl;
+    uint32_t gpio_afrh;

Do these really all need a 'gpio_' prefix in their names?

hw/gpio/netduino_gpio.c line 77

Quoted 6 lines

hw/gpio/netduino_gpio.c new file +6 -0

+
+    /* This is an internal QEMU Register, used to determine the
+     * GPIO direction as set by gpio_moder
+     * 1: Input; 0: Output
+     */
+    uint16_t gpio_direction;

I can't really figure out what you mean by this comment.

Quoted 43 lines

hw/gpio/netduino_gpio.c new file +42 -0

+    /* The GPIO letter (a - k) from the datasheet */
+    uint8_t gpio_letter;
+
+    qemu_irq irq;
+    const unsigned char *id;
+} NETDUINO_GPIOState;
+
+static const VMStateDescription vmstate_netduino_gpio = {
+    .name = "netduino_gpio",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(gpio_moder, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_otyper, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_ospeedr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_pupdr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_idr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_odr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_bsrr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_lckr, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_afrl, NETDUINO_GPIOState),
+        VMSTATE_UINT32(gpio_afrh, NETDUINO_GPIOState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void gpio_reset(DeviceState *dev)
+{
+    NETDUINO_GPIOState *s = NETDUINO_GPIO(dev);
+
+    if (s->gpio_letter == 'a') {
+        s->gpio_moder = 0xA8000000;
+        s->gpio_pupdr = 0x64000000;
+        s->gpio_ospeedr = 0x00000000;
+    } else if (s->gpio_letter == 'b') {
+        s->gpio_moder = 0x00000280;
+        s->gpio_pupdr = 0x00000100;
+        s->gpio_ospeedr = 0x000000C0;
+    } else {
+        s->gpio_moder = 0x00000000;
+        s->gpio_pupdr = 0x00000000;
+        s->gpio_ospeedr = 0x00000000;

hw/gpio/netduino_gpio.c line 120

Quoted 1 line

hw/gpio/netduino_gpio.c new file +1 -0

+    }

I don't think this is the right way to implement this. You should give the GPIO device some QOM properties which allow it to be configured (eg reset values for various registers) and let the SoC QOM device instantiate and set QOM properties appropriately for each of the GPIO modules in the SoC.

Quoted 24 lines

hw/gpio/netduino_gpio.c new file +24 -0

+
+    s->gpio_otyper = 0x00000000;
+    s->gpio_idr = 0x00000000;
+    s->gpio_odr = 0x00000000;
+    s->gpio_bsrr = 0x00000000;
+    s->gpio_lckr = 0x00000000;
+    s->gpio_afrl = 0x00000000;
+    s->gpio_afrh = 0x00000000;
+    s->gpio_direction = 0x0000;
+}
+
+static uint64_t netduino_gpio_read(void *opaque, hwaddr offset,
+                           unsigned size)
+{
+    NETDUINO_GPIOState *s = (NETDUINO_GPIOState *)opaque;
+
+    DPRINTF("Read 0x%x\n", (uint) offset);
+
+    switch (offset) {
+    case GPIO_MODER:
+        return s->gpio_moder;
+    case GPIO_OTYPER:
+        return s->gpio_otyper;
+    case GPIO_OSPEEDR:

hw/gpio/netduino_gpio.c line 154

Quoted 10 lines

hw/gpio/netduino_gpio.c new file +10 -0

+        return s->gpio_ospeedr;
+    case GPIO_PUPDR:
+        return s->gpio_pupdr;
+    case GPIO_IDR:
+        /* This register changes based on the external GPIO pins */
+        return s->gpio_idr & s->gpio_direction;
+    case GPIO_ODR:
+        return s->gpio_odr;
+    case GPIO_BSRR_HIGH:
+        /* gpio_bsrr reads as zero */

Dup of comment from a couple of lines down?

hw/gpio/netduino_gpio.c line 155

Quoted 1 line

hw/gpio/netduino_gpio.c new file +1 -0

+        return 0xFFFF;

Why does reading the top half of this as a 16 bit read return all-ones when reading the full 32 bits returns all-zeroes?

hw/gpio/netduino_gpio.c line 158

Quoted 3 lines

hw/gpio/netduino_gpio.c new file +3 -0

+    case GPIO_BSRR:
+        /* gpio_bsrr reads as zero */
+        return 0x00000000;

Comment doesn't tell us anything the code doesn't...

Quoted 21 lines

hw/gpio/netduino_gpio.c new file +21 -0

+    case GPIO_LCKR:
+        return s->gpio_lckr;
+    case GPIO_AFRL:
+        return s->gpio_afrl;
+    case GPIO_AFRH:
+        return s->gpio_afrh;
+    }
+    return 0;
+}
+
+static void netduino_gpio_write(void *opaque, hwaddr offset,
+                        uint64_t value, unsigned size)
+{
+    NETDUINO_GPIOState *s = (NETDUINO_GPIOState *)opaque;
+    int i, mask;
+
+    DPRINTF("Write 0x%x, 0x%x\n", (uint) value, (uint) offset);
+
+    switch (offset) {
+    case GPIO_MODER:
+        s->gpio_moder = (uint32_t) value;

hw/gpio/netduino_gpio.c line 180

Quoted 1 line

hw/gpio/netduino_gpio.c new file +1 -0

+        for (i = 0; i < 32; i = i + 2) {

"i += 2" is more idiomatic. In any case I think this loop would be more cleanly phrased as a simple loop from 0 to 15 (giving you one multiply-by-two rather than two divide-by-twos, and probably making the loop termination easier for static analysis tools to understand.)

hw/gpio/netduino_gpio.c line 182

Quoted 2 lines

hw/gpio/netduino_gpio.c new file +2 -0

+            /* Two bits determine the I/O direction/mode */
+            mask = (1 << i) + (1 << (i + 1));

You need to say "1U" if you want to shift it by 31, since shifting into the sign bit of a signed value is undefined behaviour in C. Also this is a funny way of writing "3U << i".

Quoted 21 lines

hw/gpio/netduino_gpio.c new file +21 -0

+
+            if ((s->gpio_moder & mask) == GPIO_MODER_INPUT) {
+                s->gpio_direction |= (1 << (i/2));
+            } else if ((s->gpio_moder & mask) == GPIO_MODER_GENERAL_OUT) {
+                s->gpio_direction &= (0xFFFF ^ (1 << (i/2)));
+            } else {
+                /* Not supported at the moment */
+            }
+        }
+        return;
+    case GPIO_OTYPER:
+        s->gpio_otyper = (uint32_t) value;
+        return;
+    case GPIO_OSPEEDR:
+        s->gpio_ospeedr = (uint32_t) value;
+        return;
+    case GPIO_PUPDR:
+        s->gpio_pupdr = (uint32_t) value;
+        return;
+    case GPIO_IDR:
+        /* Read Only Register */

hw/gpio/netduino_gpio.c line 213

Quoted 10 lines

hw/gpio/netduino_gpio.c new file +10 -0

+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_gpio%c_write: Read Only Register 0x%x\n",
+                      s->gpio_letter, (int)offset);
+        return;
+    case GPIO_ODR:
+        s->gpio_odr = ((uint32_t) value & (s->gpio_direction ^ 0xFFFF));
+        return;
+    case GPIO_BSRR_HIGH:
+        /* Reset the output value */
+        s->gpio_odr &= (uint32_t) (value ^ 0xFFFF);

Any particular reason for using the XOR with 0xffff rather than just inverting?

hw/gpio/netduino_gpio.c line 214

Quoted 1 line

hw/gpio/netduino_gpio.c new file +1 -0

+        s->gpio_bsrr = (uint32_t) (value << 16);

BSRR is write-only and doesn't have any underlying state as far as I can tell from the manual. Indeed this file never reads the gpio_bsrr field in the struct. That suggests you should remove it entirely.

hw/gpio/netduino_gpio.c line 220

Quoted 6 lines

hw/gpio/netduino_gpio.c new file +6 -0

+        DPRINTF("Output: 0x%x\n", s->gpio_odr);
+        return;
+    case GPIO_BSRR:
+        /* Reset the output value */
+        s->gpio_odr &= (uint32_t) ((value >> 16) ^ 0xFFFF);
+        /* Sets the output value */

These comments would be more helpful if they read
/* Top 16 bits are "write one to clear output" */ /* Bottom 16 bits are "write one to set output" */

hw/gpio/netduino_gpio.c line 224

Quoted 4 lines

hw/gpio/netduino_gpio.c new file +4 -0

+        s->gpio_odr |= (uint32_t) (value & 0xFFFF);
+        s->gpio_bsrr = (uint32_t) value;
+        DPRINTF("Output: 0x%x\n", s->gpio_odr);
+        return;

Surely we should actually be doing something with our outputs (ie setting qdev GPIO output lines) here?

hw/gpio/netduino_gpio.c line 226

Quoted 2 lines

hw/gpio/netduino_gpio.c new file +2 -0

+    case GPIO_LCKR:
+        s->gpio_lckr = (uint32_t) value;

This doesn't seem to be implementing the lock functionality the datasheet describes.

Quoted 26 lines

hw/gpio/netduino_gpio.c new file +26 -0

+        return;
+    case GPIO_AFRL:
+        s->gpio_afrl = (uint32_t) value;
+        return;
+    case GPIO_AFRH:
+        s->gpio_afrh = (uint32_t) value;
+        return;
+    }
+}
+
+static const MemoryRegionOps netduino_gpio_ops = {
+    .read = netduino_gpio_read,
+    .write = netduino_gpio_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static Property net_gpio_properties[] = {
+    DEFINE_PROP_UINT8("gpio-letter", NETDUINO_GPIOState, gpio_letter,
+                      (uint) 'a'),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+
+static int netduino_gpio_initfn(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);

hw/gpio/netduino_gpio.c line 258

Quoted 6 lines

hw/gpio/netduino_gpio.c new file +6 -0

+    NETDUINO_GPIOState *s = NETDUINO_GPIO(dev);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &netduino_gpio_ops, s,
+                          "netduino_gpio", 0x2000);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);

Where are the GPIO output lines?

Quoted 28 lines

hw/gpio/netduino_gpio.c new file +27 -1

+    return 0;
+}
+
+static void netduino_gpio_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = netduino_gpio_initfn;
+    dc->vmsd = &vmstate_netduino_gpio;
+    dc->props = net_gpio_properties;
+    dc->reset = gpio_reset;
+}
+
+static const TypeInfo netduino_gpio_info = {
+    .name          = TYPE_NETDUINO_GPIO,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(NETDUINO_GPIOState),
+    .class_init    = netduino_gpio_class_init,
+};
+
+static void netduino_gpio_register_types(void)
+{
+    type_register_static(&netduino_gpio_info);
+}
+
+type_init(netduino_gpio_register_types)
--

Quoted 1 line

1.9.1

thanks
-- PMM

This patch adds the Netduino Plus 2 System Configuration Controller. This is used to configure what memory is mapped at address 0 (although that is not supported) as well as configure how the EXTI interrupts work (also not supported at the moment).

Signed-off-by Alistair Francis <[email protected]>

---
 hw/misc/Makefile.objs     |   1 +
 hw/misc/netduino_syscfg.c | 201 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+)
 create mode 100644 hw/misc/netduino_syscfg.c

hw/misc/Makefile.objs +1 -0

index 979e532..794f510 100644
@@ -39,5 +39,6 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
 obj-$(CONFIG_OMAP) += omap_tap.o
 obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
+obj-$(CONFIG_NETDUINOP2) += netduino_syscfg.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o

hw/misc/netduino_syscfg.c new file +77 -0

new file mode 100644
index 0000000..3db5295
@@ -0,0 +1,201 @@
+/*
+ * Netduino Plus 2 SYSCFG
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "sysemu/char.h"
+#include "hw/hw.h"
+#include "net/net.h"
+
+/* #define DEBUG_NETSYSCFG */
+
+#ifdef DEBUG_NETSYSCFG
+#define DPRINTF(fmt, ...) \
+do { printf("netduino_syscfg: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define SYSCFG_MEMRMP  0x00
+#define SYSCFG_PMC     0x04
+#define SYSCFG_EXTICR1 0x08
+#define SYSCFG_EXTICR2 0x0C
+#define SYSCFG_EXTICR3 0x10
+#define SYSCFG_EXTICR4 0x14
+#define SYSCFG_CMPCR   0x20
+
+#define TYPE_NETDUINO_SYSCFG "netduino_syscfg"
+#define NETDUINO_SYSCFG(obj) \
+    OBJECT_CHECK(struct net_syscfg, (obj), TYPE_NETDUINO_SYSCFG)
+
+struct net_syscfg {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    uint32_t syscfg_memrmp;
+    uint32_t syscfg_pmc;
+    uint32_t syscfg_exticr1;
+    uint32_t syscfg_exticr2;
+    uint32_t syscfg_exticr3;
+    uint32_t syscfg_exticr4;
+    uint32_t syscfg_cmpcr;
+
+    qemu_irq irq;
+};
+
+static void syscfg_reset(DeviceState *dev)
+{
+    struct net_syscfg *s = NETDUINO_SYSCFG(dev);
+
+    s->syscfg_memrmp = 0x00000000;
+    s->syscfg_pmc = 0x00000000;
+    s->syscfg_exticr1 = 0x00000000;
+    s->syscfg_exticr2 = 0x00000000;
+    s->syscfg_exticr3 = 0x00000000;
+    s->syscfg_exticr4 = 0x00000000;
+    s->syscfg_cmpcr = 0x00000000;

hw/misc/netduino_syscfg.c new file +34 -0

+
+    DPRINTF("Read 0x%x\n", (uint) addr);
+
+    switch (addr) {
+    case SYSCFG_MEMRMP:
+        return s->syscfg_memrmp;
+    case SYSCFG_PMC:
+        return s->syscfg_pmc;
+    case SYSCFG_EXTICR1:
+        return s->syscfg_exticr1;
+    case SYSCFG_EXTICR2:
+        return s->syscfg_exticr2;
+    case SYSCFG_EXTICR3:
+        return s->syscfg_exticr3;
+    case SYSCFG_EXTICR4:
+        return s->syscfg_exticr4;
+    case SYSCFG_CMPCR:
+        return s->syscfg_cmpcr;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_syscfg_read: Bad offset %x\n", (int)addr);
+        return 0;
+    }
+
+    return 0;
+}
+
+static void netduino_syscfg_write(void *opaque, hwaddr addr,
+                       uint64_t val64, unsigned int size)
+{
+    struct net_syscfg *s = opaque;
+    uint32_t value = (uint32_t) val64;
+
+    if (addr >= 0x400) {

hw/misc/netduino_syscfg.c new file +70 -1

+        return;
+    case SYSCFG_PMC:
+        /* This isn't supported, so don't allow the guest to write to it
+         * s->syscfg_pmc = value;
+         */
+        return;
+    case SYSCFG_EXTICR1:
+        s->syscfg_exticr1 = (value & 0xFF);
+        return;
+    case SYSCFG_EXTICR2:
+        s->syscfg_exticr2 = (value & 0xFF);
+        return;
+    case SYSCFG_EXTICR3:
+        s->syscfg_exticr3 = (value & 0xFF);
+        return;
+    case SYSCFG_EXTICR4:
+        s->syscfg_exticr4 = (value & 0xFF);
+        return;
+    case SYSCFG_CMPCR:
+        s->syscfg_cmpcr = value;
+        return;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_syscfg_write: Bad offset %x\n", (int)addr);
+    }
+}
+
+static const MemoryRegionOps netduino_syscfg_ops = {
+    .read = netduino_syscfg_read,
+    .write = netduino_syscfg_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int netduino_syscfg_init(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);
+    struct net_syscfg *s = NETDUINO_SYSCFG(dev);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &netduino_syscfg_ops, s,
+                          TYPE_NETDUINO_SYSCFG, 0x2000);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    return 0;
+}
+
+static void netduino_syscfg_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = netduino_syscfg_init;
+    dc->props = NULL;
+    dc->reset = syscfg_reset;
+}
+
+static const TypeInfo netduino_syscfg_info = {
+    .name          = TYPE_NETDUINO_SYSCFG,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(struct net_syscfg),
+    .class_init    = netduino_syscfg_class_init,
+};
+
+static void netduino_syscfg_register_types(void)
+{
+    type_register_static(&netduino_syscfg_info);
+}
+
+type_init(netduino_syscfg_register_types)
--

1.9.1

Quoted 6 lines · Alistair Francis

This patch adds the Netduino Plus 2 System Configuration Controller. This is used to configure what memory is mapped at address 0 (although that is not supported) as well as configure how the EXTI interrupts work (also not supported at the moment).

Curious, Is this core needed to run the examples your posted on cover letter or is it possible to just remove it? Do you need the dummy regs to avoid crashes? (that's worth mentioning in commit msg).

Quoted 101 lines

Signed-off-by Alistair Francis <[email protected]>

---
 hw/misc/Makefile.objs     |   1 +
 hw/misc/netduino_syscfg.c | 201 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 202 insertions(+)
 create mode 100644 hw/misc/netduino_syscfg.c

hw/misc/Makefile.objs +1 -0

index 979e532..794f510 100644
@@ -39,5 +39,6 @@ obj-$(CONFIG_OMAP) += omap_sdrc.o
 obj-$(CONFIG_OMAP) += omap_tap.o
 obj-$(CONFIG_SLAVIO) += slavio_misc.o
 obj-$(CONFIG_ZYNQ) += zynq_slcr.o
+obj-$(CONFIG_NETDUINOP2) += netduino_syscfg.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o

hw/misc/netduino_syscfg.c new file +77 -0

new file mode 100644
index 0000000..3db5295
@@ -0,0 +1,201 @@
+/*
+ * Netduino Plus 2 SYSCFG
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "sysemu/char.h"
+#include "hw/hw.h"
+#include "net/net.h"
+
+/* #define DEBUG_NETSYSCFG */
+
+#ifdef DEBUG_NETSYSCFG
+#define DPRINTF(fmt, ...) \
+do { printf("netduino_syscfg: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define SYSCFG_MEMRMP  0x00
+#define SYSCFG_PMC     0x04
+#define SYSCFG_EXTICR1 0x08
+#define SYSCFG_EXTICR2 0x0C
+#define SYSCFG_EXTICR3 0x10
+#define SYSCFG_EXTICR4 0x14
+#define SYSCFG_CMPCR   0x20
+
+#define TYPE_NETDUINO_SYSCFG "netduino_syscfg"
+#define NETDUINO_SYSCFG(obj) \
+    OBJECT_CHECK(struct net_syscfg, (obj), TYPE_NETDUINO_SYSCFG)
+
+struct net_syscfg {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    uint32_t syscfg_memrmp;
+    uint32_t syscfg_pmc;
+    uint32_t syscfg_exticr1;
+    uint32_t syscfg_exticr2;
+    uint32_t syscfg_exticr3;
+    uint32_t syscfg_exticr4;
+    uint32_t syscfg_cmpcr;
+
+    qemu_irq irq;
+};
+
+static void syscfg_reset(DeviceState *dev)
+{
+    struct net_syscfg *s = NETDUINO_SYSCFG(dev);
+
+    s->syscfg_memrmp = 0x00000000;
+    s->syscfg_pmc = 0x00000000;
+    s->syscfg_exticr1 = 0x00000000;
+    s->syscfg_exticr2 = 0x00000000;
+    s->syscfg_exticr3 = 0x00000000;
+    s->syscfg_exticr4 = 0x00000000;
+    s->syscfg_cmpcr = 0x00000000;

hw/misc/netduino_syscfg.c line 87

Quoted 10 lines

hw/misc/netduino_syscfg.c new file +10 -0

+}
+
+static uint64_t netduino_syscfg_read(void *opaque, hwaddr addr,
+                                     unsigned int size)
+{
+    struct net_syscfg *s = opaque;
+
+    if (addr >= 0x400) {
+        addr = addr >> 7;
+    }

What's this do? I think it's worthy of a comment :).

Quoted 34 lines

hw/misc/netduino_syscfg.c new file +34 -0

+
+    DPRINTF("Read 0x%x\n", (uint) addr);
+
+    switch (addr) {
+    case SYSCFG_MEMRMP:
+        return s->syscfg_memrmp;
+    case SYSCFG_PMC:
+        return s->syscfg_pmc;
+    case SYSCFG_EXTICR1:
+        return s->syscfg_exticr1;
+    case SYSCFG_EXTICR2:
+        return s->syscfg_exticr2;
+    case SYSCFG_EXTICR3:
+        return s->syscfg_exticr3;
+    case SYSCFG_EXTICR4:
+        return s->syscfg_exticr4;
+    case SYSCFG_CMPCR:
+        return s->syscfg_cmpcr;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_syscfg_read: Bad offset %x\n", (int)addr);
+        return 0;
+    }
+
+    return 0;
+}
+
+static void netduino_syscfg_write(void *opaque, hwaddr addr,
+                       uint64_t val64, unsigned int size)
+{
+    struct net_syscfg *s = opaque;
+    uint32_t value = (uint32_t) val64;
+
+    if (addr >= 0x400) {

hw/misc/netduino_syscfg.c line 131

Quoted 10 lines

hw/misc/netduino_syscfg.c new file +10 -0

+        addr = addr >> 7;
+    }
+
+    DPRINTF("Write 0x%x, 0x%x\n", value, (uint) addr);
+
+    switch (addr) {
+    case SYSCFG_MEMRMP:
+        /* This isn't supported, so don't allow the guest to write to it
+         * s->syscfg_memrmp = value;
+         */

qemu_log_mask(LOG_UNIMP

Regards,
Peter

Quoted 74 lines

hw/misc/netduino_syscfg.c new file +70 -1

+        return;
+    case SYSCFG_PMC:
+        /* This isn't supported, so don't allow the guest to write to it
+         * s->syscfg_pmc = value;
+         */
+        return;
+    case SYSCFG_EXTICR1:
+        s->syscfg_exticr1 = (value & 0xFF);
+        return;
+    case SYSCFG_EXTICR2:
+        s->syscfg_exticr2 = (value & 0xFF);
+        return;
+    case SYSCFG_EXTICR3:
+        s->syscfg_exticr3 = (value & 0xFF);
+        return;
+    case SYSCFG_EXTICR4:
+        s->syscfg_exticr4 = (value & 0xFF);
+        return;
+    case SYSCFG_CMPCR:
+        s->syscfg_cmpcr = value;
+        return;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "net_syscfg_write: Bad offset %x\n", (int)addr);
+    }
+}
+
+static const MemoryRegionOps netduino_syscfg_ops = {
+    .read = netduino_syscfg_read,
+    .write = netduino_syscfg_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int netduino_syscfg_init(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);
+    struct net_syscfg *s = NETDUINO_SYSCFG(dev);
+
+    sysbus_init_irq(sbd, &s->irq);
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &netduino_syscfg_ops, s,
+                          TYPE_NETDUINO_SYSCFG, 0x2000);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    return 0;
+}
+
+static void netduino_syscfg_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = netduino_syscfg_init;
+    dc->props = NULL;
+    dc->reset = syscfg_reset;
+}
+
+static const TypeInfo netduino_syscfg_info = {
+    .name          = TYPE_NETDUINO_SYSCFG,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(struct net_syscfg),
+    .class_init    = netduino_syscfg_class_init,
+};
+
+static void netduino_syscfg_register_types(void)
+{
+    type_register_static(&netduino_syscfg_info);
+}
+
+type_init(netduino_syscfg_register_types)
--

1.9.1

This patch adds the Netduino Plus 2 timers: TIM2, TIM3, TIM4 and TIM5 to QEMU.

Signed-off-by Alistair Francis <[email protected]>

---
 hw/timer/Makefile.objs    |   1 +
 hw/timer/netduino_timer.c | 384 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 385 insertions(+)
 create mode 100644 hw/timer/netduino_timer.c

hw/timer/Makefile.objs +1 -0

index 2c86c3d..104fc46 100644
@@ -17,6 +17,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
 common-obj-$(CONFIG_IMX) += imx_gpt.o
 common-obj-$(CONFIG_LM32) += lm32_timer.o
 common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
+common-obj-$(CONFIG_NETDUINOP2) += netduino_timer.o
 
 obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
 obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o

hw/timer/netduino_timer.c new file +53 -0

new file mode 100644
index 0000000..212ba48
@@ -0,0 +1,384 @@
+/*
+ * Netduino Plus 2 USART
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "qemu/timer.h"
+#include "sysemu/sysemu.h"
+
+#define DEBUG_NETTIMER
+
+#ifdef DEBUG_NETTIMER
+#define DPRINTF(fmt, ...) \
+do { printf("netduino_timer: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+#define TIM_CR1      0x00
+#define TIM_CR2      0x04
+#define TIM_SMCR     0x08
+#define TIM_DIER     0x0C
+#define TIM_SR       0x10
+#define TIM_EGR      0x14
+#define TIM_CCMR1    0x18
+#define TIM_CCMR2    0x1C
+#define TIM_CCER     0x20
+#define TIM_CNT      0x24
+#define TIM_PSC      0x28
+#define TIM_ARR      0x2C
+#define TIM_CCR1     0x34
+#define TIM_CCR2     0x38
+#define TIM_CCR3     0x3C
+#define TIM_CCR4     0x40

hw/timer/netduino_timer.c new file +25 -0

+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    QEMUTimer *timer;
+    qemu_irq irq;
+
+    uint32_t tick_offset_vmstate;
+    uint32_t tick_offset;
+
+    uint32_t tim_cr1;
+    uint32_t tim_cr2;
+    uint32_t tim_smcr;
+    uint32_t tim_dier;
+    uint32_t tim_sr;
+    uint32_t tim_egr;
+    uint32_t tim_ccmr1;
+    uint32_t tim_ccmr2;
+    uint32_t tim_ccer;
+    uint32_t tim_cnt;
+    uint32_t tim_psc;
+    uint32_t tim_arr;
+    uint32_t tim_ccr1;
+    uint32_t tim_ccr2;
+    uint32_t tim_ccr3;
+    uint32_t tim_ccr4;

hw/timer/netduino_timer.c new file +8 -0

+
+    if (s->tim_dier == 0x01 && s->tim_cr1 & TIM_CR1_CEN) {
+        netduino_timer_update(s);
+    }
+}
+
+static uint32_t netduino_timer_get_count(NETTIMERState *s)
+{

hw/timer/netduino_timer.c new file +65 -0

+        DPRINTF("Wait Time: 0x%x\n", (uint32_t) (now + ticks));
+    }
+}
+
+static void netduino_timer_reset(DeviceState *dev)
+{
+    struct NETTIMERState *s = NETTIMER(dev);
+
+    s->tim_cr1 = 0;
+    s->tim_cr2 = 0;
+    s->tim_smcr = 0;
+    s->tim_dier = 0;
+    s->tim_sr = 0;
+    s->tim_egr = 0;
+    s->tim_ccmr1 = 0;
+    s->tim_ccmr2 = 0;
+    s->tim_ccer = 0;
+    s->tim_cnt = 0;
+    s->tim_psc = 0;
+    s->tim_arr = 0;
+    s->tim_ccr1 = 0;
+    s->tim_ccr2 = 0;
+    s->tim_ccr3 = 0;
+    s->tim_ccr4 = 0;
+    s->tim_dcr = 0;
+    s->tim_dmar = 0;
+    s->tim_or = 0;
+}
+
+static uint64_t netduino_timer_read(void *opaque, hwaddr offset,
+                           unsigned size)
+{
+    NETTIMERState *s = (NETTIMERState *)opaque;
+
+    DPRINTF("Read 0x%x\n", (uint) offset);
+
+    switch (offset) {
+    case TIM_CR1:
+        return s->tim_cr1;
+    case TIM_CR2:
+        return s->tim_cr2;
+    case TIM_SMCR:
+        return s->tim_smcr;
+    case TIM_DIER:
+        return s->tim_dier;
+    case TIM_SR:
+        return s->tim_sr;
+    case TIM_EGR:
+        return s->tim_egr;
+    case TIM_CCMR1:
+        return s->tim_ccmr1;
+    case TIM_CCMR2:
+        return s->tim_ccmr2;
+    case TIM_CCER:
+        return s->tim_ccer;
+    case TIM_CNT:
+        return s->tim_cnt;
+    case TIM_PSC:
+        return s->tim_psc;
+    case TIM_ARR:
+        return s->tim_arr;
+    case TIM_CCR1:
+        return s->tim_ccr1;
+    case TIM_CCR2:
+        return s->tim_ccr2;

This patch adds the Netduion Plus 2 machine to QEMU.

Signed-off-by Alistair Francis <[email protected]>

---
 hw/arm/Makefile.objs   |   2 +-
 hw/arm/netduinoplus2.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/netduinoplus2.c

hw/arm/Makefile.objs +1 -1

index 6088e53..616f1ae 100644
@@ -2,7 +2,7 @@ obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
-obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
+obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o netduinoplus2.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o

hw/arm/netduinoplus2.c new file +68 -0

new file mode 100644
index 0000000..32acac5
@@ -0,0 +1,202 @@
+/*
+ * Netduino Plus 2 Machine Model
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/ssi.h"
+#include "hw/arm/arm.h"
+#include "hw/devices.h"
+#include "qemu/timer.h"
+#include "net/net.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "qemu/error-report.h"
+#include "sysemu/qtest.h"
+
+#define FLASH_BASE_ADDRESS 0x08000000
+#define FLASH_SIZE (1024 * 1024)
+#define SRAM_BASE_ADDRESS 0x20000000
+#define SRAM_SIZE (192 * 1024)
+
+typedef struct ARMV7MResetArgs {
+    ARMCPU *cpu;
+    uint32_t reset_sp;
+    uint32_t reset_pc;
+} ARMV7MResetArgs;
+
+static void armv7m_reset(void *opaque)
+{
+    ARMV7MResetArgs *args = opaque;
+
+    cpu_reset(CPU(args->cpu));
+
+    args->cpu->env.regs[13] = args->reset_sp & 0xFFFFFFFC;
+    args->cpu->env.thumb = args->reset_pc & 1;
+    args->cpu->env.regs[15] = args->reset_pc & ~1;
+}
+
+static void netduinoplus2_init(MachineState *machine)
+{
+    static const uint32_t gpio_addr[] = { 0x40020000, 0x40020400, 0x40020800,
+        0x40020C00, 0x40021000, 0x40021400, 0x40021800, 0x40021C00,
+        0x40022000, 0x40022400, 0x40022800 };
+    static const uint8_t gpio_letters[] = { 'a', 'b', 'c',
+        'd', 'e', 'f', 'g', 'h',
+        'i', 'j', 'k' };
+    static const uint32_t tim2_5_addr[] = { 0x40000000, 0x40000400,

hw/arm/netduinoplus2.c new file +31 -0

+    MemoryRegion *sram = g_new(MemoryRegion, 1);
+    MemoryRegion *flash = g_new(MemoryRegion, 1);
+    MemoryRegion *flash_alias = g_new(MemoryRegion, 1);
+    MemoryRegion *hack = g_new(MemoryRegion, 1);
+    ARMV7MResetArgs reset_args;
+
+    qemu_irq pic[96];
+    ARMCPU *cpu;
+    CPUARMState *env;
+    DeviceState *nvic;
+    DeviceState *gpio;
+    SysBusDevice *busdev;
+
+    int image_size;
+    uint64_t entry;
+    uint64_t lowaddr;
+    int i;
+    int big_endian = 0;
+
+    /* The Netduinio Plus 2 uses a Cortex-M4, while QEMU currently supports
+     * the Cortex-M3, so that is being used instead
+     */
+    cpu = cpu_arm_init("cortex-m3");
+    env = &cpu->env;
+
+    memory_region_init_ram(flash, NULL, "netduino.flash", FLASH_SIZE);
+    memory_region_init_alias(flash_alias, NULL, "netduino.flash.alias",
+                             flash, 0, FLASH_SIZE);
+
+    vmstate_register_ram_global(flash);
+

hw/arm/netduinoplus2.c new file +15 -0

+    env->nvic = nvic;
+    qdev_init_nofail(nvic);
+    sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
+                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
+    for (i = 0; i < 96; i++) {
+        pic[i] = qdev_get_gpio_in(nvic, i);
+    }
+
+    /* Load the kernel */
+    if (!kernel_filename && !qtest_enabled()) {
+        fprintf(stderr, "Guest image must be specified (using -kernel)\n");
+        exit(1);
+    }
+    if (kernel_filename) {
+        image_size = load_elf(kernel_filename, NULL, NULL, &entry, &lowaddr,

hw/arm/netduinoplus2.c new file +15 -0

+
+    /* System configuration controller */
+    sysbus_create_simple("netduino_syscfg", 0x40013800,
+                         pic[71]);
+
+    /* Attach a UART (uses USART registers) and USART controllers */
+    for (i = 0; i < 7; i++) {
+        sysbus_create_simple("netduino_usart", usart_addr[i],
+                             pic[usart_irq[i]]);
+    }
+
+    /* Timer 2 to 5 */
+    for (i = 0; i < 4; i++) {
+        sysbus_create_simple("netduino_timer", tim2_5_addr[i],
+                             pic[tim2_5_irq[i]]);

hw/arm/netduinoplus2.c new file +31 -1

+
+    /* Hack to map an additional page of ram at the top of the address
+     * space.  This stops qemu complaining about executing code outside RAM
+     * when returning from an exception.
+     */
+    memory_region_init_ram(hack, NULL, "netduino.hack", 0x1000);
+    vmstate_register_ram_global(hack);
+    memory_region_set_readonly(hack, true);
+    memory_region_add_subregion(address_space_mem, 0xfffff000, hack);
+
+    reset_args = (ARMV7MResetArgs) {
+        .cpu = cpu,
+        .reset_pc = entry,
+        .reset_sp = (SRAM_BASE_ADDRESS + (SRAM_SIZE * 2)/3),
+    };
+    qemu_register_reset(armv7m_reset,
+                        g_memdup(&reset_args, sizeof(reset_args)));
+}
+
+static QEMUMachine netduinoplus2_machine = {
+    .name = "netduinoplus2",
+    .desc = "Netduino Plus 2 Machine (Except with a Cortex-M3)",
+    .init = netduinoplus2_init,
+};
+
+static void netduinoplus2_machine_init(void)
+{
+    qemu_register_machine(&netduinoplus2_machine);
+}
+
+machine_init(netduinoplus2_machine_init);
--

1.9.1

Quoted 96 lines · Alistair Francis

This patch adds the Netduion Plus 2 machine to QEMU.

Signed-off-by Alistair Francis <[email protected]>

---
 hw/arm/Makefile.objs   |   2 +-
 hw/arm/netduinoplus2.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 203 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/netduinoplus2.c

hw/arm/Makefile.objs +1 -1

index 6088e53..616f1ae 100644
@@ -2,7 +2,7 @@ obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
-obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
+obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o netduinoplus2.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o

hw/arm/netduinoplus2.c new file +68 -0

new file mode 100644
index 0000000..32acac5
@@ -0,0 +1,202 @@
+/*
+ * Netduino Plus 2 Machine Model
+ *
+ * Copyright (c) 2014 Alistair Francis <[email protected]>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/ssi.h"
+#include "hw/arm/arm.h"
+#include "hw/devices.h"
+#include "qemu/timer.h"
+#include "net/net.h"
+#include "elf.h"
+#include "hw/loader.h"
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "qemu/error-report.h"
+#include "sysemu/qtest.h"
+
+#define FLASH_BASE_ADDRESS 0x08000000
+#define FLASH_SIZE (1024 * 1024)
+#define SRAM_BASE_ADDRESS 0x20000000
+#define SRAM_SIZE (192 * 1024)
+
+typedef struct ARMV7MResetArgs {
+    ARMCPU *cpu;
+    uint32_t reset_sp;
+    uint32_t reset_pc;
+} ARMV7MResetArgs;
+
+static void armv7m_reset(void *opaque)
+{
+    ARMV7MResetArgs *args = opaque;
+
+    cpu_reset(CPU(args->cpu));
+
+    args->cpu->env.regs[13] = args->reset_sp & 0xFFFFFFFC;
+    args->cpu->env.thumb = args->reset_pc & 1;
+    args->cpu->env.regs[15] = args->reset_pc & ~1;
+}
+
+static void netduinoplus2_init(MachineState *machine)
+{
+    static const uint32_t gpio_addr[] = { 0x40020000, 0x40020400, 0x40020800,
+        0x40020C00, 0x40021000, 0x40021400, 0x40021800, 0x40021C00,
+        0x40022000, 0x40022400, 0x40022800 };
+    static const uint8_t gpio_letters[] = { 'a', 'b', 'c',
+        'd', 'e', 'f', 'g', 'h',
+        'i', 'j', 'k' };
+    static const uint32_t tim2_5_addr[] = { 0x40000000, 0x40000400,

hw/arm/netduinoplus2.c line 78

Quoted 10 lines

hw/arm/netduinoplus2.c new file +10 -0

+        0x40000800, 0x40000C00 };
+    static const uint32_t usart_addr[] = { 0x40011000, 0x40004400,
+        0x40004800, 0x40004C00, 0x40005000, 0x40011400, 0x40007800,
+        0x40007C00 };
+    const char *kernel_filename = machine->kernel_filename;
+
+    static const int tim2_5_irq[] = {28, 29, 30, 50};
+    static const int usart_irq[] = {37, 38, 39, 52, 53, 71, 82, 83};
+
+    MemoryRegion *address_space_mem = get_system_memory();

system_memory. address_space is a miselading name for a MR.

Quoted 31 lines

hw/arm/netduinoplus2.c new file +31 -0

+    MemoryRegion *sram = g_new(MemoryRegion, 1);
+    MemoryRegion *flash = g_new(MemoryRegion, 1);
+    MemoryRegion *flash_alias = g_new(MemoryRegion, 1);
+    MemoryRegion *hack = g_new(MemoryRegion, 1);
+    ARMV7MResetArgs reset_args;
+
+    qemu_irq pic[96];
+    ARMCPU *cpu;
+    CPUARMState *env;
+    DeviceState *nvic;
+    DeviceState *gpio;
+    SysBusDevice *busdev;
+
+    int image_size;
+    uint64_t entry;
+    uint64_t lowaddr;
+    int i;
+    int big_endian = 0;
+
+    /* The Netduinio Plus 2 uses a Cortex-M4, while QEMU currently supports
+     * the Cortex-M3, so that is being used instead
+     */
+    cpu = cpu_arm_init("cortex-m3");
+    env = &cpu->env;
+
+    memory_region_init_ram(flash, NULL, "netduino.flash", FLASH_SIZE);
+    memory_region_init_alias(flash_alias, NULL, "netduino.flash.alias",
+                             flash, 0, FLASH_SIZE);
+
+    vmstate_register_ram_global(flash);
+

hw/arm/netduinoplus2.c line 119

Quoted 10 lines

hw/arm/netduinoplus2.c new file +10 -0

+    memory_region_set_readonly(flash, true);
+    memory_region_set_readonly(flash_alias, true);
+
+    memory_region_add_subregion(address_space_mem, FLASH_BASE_ADDRESS, flash);
+    memory_region_add_subregion(address_space_mem, 0, flash_alias);
+
+    memory_region_init_ram(sram, NULL, "netduino.sram", SRAM_SIZE);
+    vmstate_register_ram_global(sram);
+    memory_region_add_subregion(address_space_mem, SRAM_BASE_ADDRESS, sram);
+

Most of this function is the same as armv7m_init(). This seems the be the main difference - the SRAM and FLASH setup. Everything else is board-level additive. The fact that it's different means that either ARMv7M should be parameterized, or Stellaris specific setup should be pushed up to stellaris.c to get armv7m_init cleaned up into a state where it's usable for you.

Specifically, what in armv7m_init() is stopping the code share?

hw/arm/netduinoplus2.c line 121

Quoted 2 lines

hw/arm/netduinoplus2.c new file +2 -0

+    nvic = qdev_create(NULL, "armv7m_nvic");
+    qdev_prop_set_uint32(nvic, "num-irq", 96);

nvic num-irqs sounds config specific so that's one that should probably be a parameter.

Quoted 15 lines

hw/arm/netduinoplus2.c new file +15 -0

+    env->nvic = nvic;
+    qdev_init_nofail(nvic);
+    sysbus_connect_irq(SYS_BUS_DEVICE(nvic), 0,
+                       qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ));
+    for (i = 0; i < 96; i++) {
+        pic[i] = qdev_get_gpio_in(nvic, i);
+    }
+
+    /* Load the kernel */
+    if (!kernel_filename && !qtest_enabled()) {
+        fprintf(stderr, "Guest image must be specified (using -kernel)\n");
+        exit(1);
+    }
+    if (kernel_filename) {
+        image_size = load_elf(kernel_filename, NULL, NULL, &entry, &lowaddr,

hw/arm/netduinoplus2.c line 146

Quoted 10 lines

hw/arm/netduinoplus2.c new file +10 -0

+                              NULL, big_endian, ELF_MACHINE, 1);
+        if (image_size < 0) {
+            image_size = load_image_targphys(kernel_filename, 0, FLASH_SIZE);
+            lowaddr = 0;
+        }
+        if (image_size < 0) {
+            error_report("Could not load kernel '%s'", kernel_filename);
+            exit(1);
+        }
+    }

It seems strange to bootload before you have finished creating the machine (although it should be functionally safe).

Quoted 15 lines

hw/arm/netduinoplus2.c new file +15 -0

+
+    /* System configuration controller */
+    sysbus_create_simple("netduino_syscfg", 0x40013800,
+                         pic[71]);
+
+    /* Attach a UART (uses USART registers) and USART controllers */
+    for (i = 0; i < 7; i++) {
+        sysbus_create_simple("netduino_usart", usart_addr[i],
+                             pic[usart_irq[i]]);
+    }
+
+    /* Timer 2 to 5 */
+    for (i = 0; i < 4; i++) {
+        sysbus_create_simple("netduino_timer", tim2_5_addr[i],
+                             pic[tim2_5_irq[i]]);

hw/arm/netduinoplus2.c line 171

Quoted 10 lines

hw/arm/netduinoplus2.c new file +10 -0

+    }
+
+    /* Attach GPIO devices */
+    for (i = 0; i < 11; i++) {
+        gpio = qdev_create(NULL, "netduino_gpio");
+        qdev_prop_set_uint8(gpio, "gpio-letter", gpio_letters[i]);
+        qdev_init_nofail(gpio);
+        busdev = SYS_BUS_DEVICE(gpio);
+        sysbus_mmio_map(busdev, 0, gpio_addr[i]);
+    }

In the code-shared approach, these devs are added after the armv7_init() call.

Regards,
Peter

Quoted 35 lines

hw/arm/netduinoplus2.c new file +31 -1

+
+    /* Hack to map an additional page of ram at the top of the address
+     * space.  This stops qemu complaining about executing code outside RAM
+     * when returning from an exception.
+     */
+    memory_region_init_ram(hack, NULL, "netduino.hack", 0x1000);
+    vmstate_register_ram_global(hack);
+    memory_region_set_readonly(hack, true);
+    memory_region_add_subregion(address_space_mem, 0xfffff000, hack);
+
+    reset_args = (ARMV7MResetArgs) {
+        .cpu = cpu,
+        .reset_pc = entry,
+        .reset_sp = (SRAM_BASE_ADDRESS + (SRAM_SIZE * 2)/3),
+    };
+    qemu_register_reset(armv7m_reset,
+                        g_memdup(&reset_args, sizeof(reset_args)));
+}
+
+static QEMUMachine netduinoplus2_machine = {
+    .name = "netduinoplus2",
+    .desc = "Netduino Plus 2 Machine (Except with a Cortex-M3)",
+    .init = netduinoplus2_init,
+};
+
+static void netduinoplus2_machine_init(void)
+{
+    qemu_register_machine(&netduinoplus2_machine);
+}
+
+machine_init(netduinoplus2_machine_init);
--

1.9.1

Quoted 8 lines · Peter Crosthwaite

Most of this function is the same as armv7m_init(). This seems the be the main difference - the SRAM and FLASH setup. Everything else is board-level additive. The fact that it's different means that either ARMv7M should be parameterized, or Stellaris specific setup should be pushed up to stellaris.c to get armv7m_init cleaned up into a state where it's usable for you.

Specifically, what in armv7m_init() is stopping the code share?

It is generally similar, although there are enough differences that I don't think it's wroth it. Ideally one day the Netduino Plus 2 will use a Cortex-M4 (one can hope) and this will need to be parameterised. The Netduino also has different base addresses
which would need to be parameterised into armv7m_init(). The Netduio then has the memory alias as well as a different number of interrupt lines.

It is because of all of these differences that I don't think it is worth using the armv7m_init() function. It's really not that much code.

Yeah, but it's not really the right design IMHO. We should have common armv7m init code for the parts which are really "the CPU has all this" (including at least the bitbanding, NVIC, memory mapped system registers, reset/ELF file loading, etc), and board specific code in the board files. Copying this code just because it's easier in the short term leaves us still with the cleanup to do in the long term.

Quoted 18 lines · Alistair Francis

Quoted 8 lines · Peter Crosthwaite

Most of this function is the same as armv7m_init(). This seems the be the main difference - the SRAM and FLASH setup. Everything else is board-level additive. The fact that it's different means that either ARMv7M should be parameterized, or Stellaris specific setup should be pushed up to stellaris.c to get armv7m_init cleaned up into a state where it's usable for you.

Specifically, what in armv7m_init() is stopping the code share?

It is generally similar, although there are enough differences that I don't think it's wroth it. Ideally one day the Netduino Plus 2 will use a Cortex-M4 (one can hope) and this will need to be parameterised. The Netduino also has different base addresses
which would need to be parameterised into armv7m_init(). The Netduio then has the memory alias as well as a different number of interrupt lines.

Quoted 2 lines

It is because of all of these differences that I don't think it is worth using the armv7m_init() function. It's really not that much code.

Yeah, but it's not really the right design IMHO. We should have common armv7m init code for the parts which are really "the CPU has all this" (including at least the bitbanding, NVIC, memory mapped system registers, reset/ELF file loading, etc), and board specific code in the board files. Copying this code just because it's easier in the short term leaves us still with the cleanup to do in the long term.

You don't want to do this by parameterising flash/RAM base addresses, incidentally -- you want to use memory regions so the board sets up its devices in the right places in system memory and then hands that off to the v7M/SoC code level which adds the inside-the-CPU devices.

(There's almost certainly scope for QOMification in the process of separating out the generic v7m from the board code here too.)

thanks
-- PMM

The Netduino Plus 2 has a Cortex-M4 in it, while this model uses a Cortex-M3 as that is supported by QEMU. This means that the code that runs on the Netduino Plus 2 is recompiled for a Cortex-M3 with out Floating Point or DSP optimisations.

For my use of this it doesn't matter if the code has to be recompiled, so I have no desire to add support for the Cortex-M4.

Quoted 5 lines · Alistair Francis

The Netduino Plus 2 has a Cortex-M4 in it, while this model uses a Cortex-M3 as that is supported by QEMU. This means that the code that runs on the Netduino Plus 2 is recompiled for a Cortex-M3 with out Floating Point or DSP optimisations.

Quoted 2 lines

For my use of this it doesn't matter if the code has to be recompiled, so I have no desire to add support for the Cortex-M4.

That does make things a bit awkward. I don't really want to add something that claims to be a model of board X if in fact it won't run anything that isn't specifically built for "QEMU's deficient model of board X". It seems to me that would just be a recipe for lots of user complaints...

thanks
-- PMM