[Qemu-devel] [Qemu-devel RFC v2 0/4] Add support for Smartfusion2 SoC

[Qemu-devel] [Qemu-devel RFC v2 0/4] Add support for Smartfusion2 SoC

[Subbaraya Sundeep]

Hi Qemu-devel,

I am trying to add Smartfusion2 SoC.
SoC is from Microsemi and System on Module(SOM)
board is from Emcraft systems. Smartfusion2 has hardened
Microcontroller(Cortex-M3)based Sub System and FPGA fabric.
At the moment only system timer, sysreg and SPI
controller are modelled.

Testing:
./arm-softmmu/qemu-system-arm -M smartfusion2-som -serial mon:stdio \
-kernel u-boot.bin -display none -drive file=spi.bin,if=mtd,format=raw

U-boot is from Emcraft with modified SPI driver not to use PDMA.
Linux is 4.5 linux with Smartfusion2 SoC dts and clocksource 
driver added by myself @
https://github.com/Subbaraya-Sundeep/linux.git

Baremetal elfs from Microsemi Softconsole IDE are also working.

Changes from v1:
    Added SPI controller.

Thanks,
Sundeep

Subbaraya Sundeep (4):
  msf2: Add Smartfusion2 System timer
  msf2: Microsemi Smartfusion2 System Register block.
  msf2: Add Smartfusion2 SPI controller
  msf2: Add Emcraft's Smartfusion2 SOM kit.

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs            |   2 +-
 hw/arm/msf2_soc.c               | 141 +++++++++++++
 hw/misc/Makefile.objs           |   1 +
 hw/misc/msf2_sysreg.c           | 168 +++++++++++++++
 hw/ssi/Makefile.objs            |   1 +
 hw/ssi/msf2_spi.c               | 449 ++++++++++++++++++++++++++++++++++++++++
 hw/timer/Makefile.objs          |   1 +
 hw/timer/msf2_timer.c           | 273 ++++++++++++++++++++++++
 9 files changed, 1036 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/msf2_soc.c
 create mode 100644 hw/misc/msf2_sysreg.c
 create mode 100644 hw/ssi/msf2_spi.c
 create mode 100644 hw/timer/msf2_timer.c

-- 
2.5.0

[Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[Subbaraya Sundeep]

Modelled System Timer in Microsemi's Smartfusion2 Soc.
Timer has two 32bit down counters and two interrupts.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 hw/timer/Makefile.objs |   1 +
 hw/timer/msf2_timer.c  | 273 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 274 insertions(+)
 create mode 100644 hw/timer/msf2_timer.c

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index dd6f27e..0bdf1e1 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -41,3 +41,4 @@ common-obj-$(CONFIG_STM32F2XX_TIMER) += stm32f2xx_timer.o
 common-obj-$(CONFIG_ASPEED_SOC) += aspeed_timer.o
 
 common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
+common-obj-$(CONFIG_MSF2) += msf2_timer.o
diff --git a/hw/timer/msf2_timer.c b/hw/timer/msf2_timer.c
new file mode 100644
index 0000000..962ada4
--- /dev/null
+++ b/hw/timer/msf2_timer.c
@@ -0,0 +1,273 @@
+/*
+ * Timer block model of Microsemi SmartFusion2.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>.
+ *
+ * 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 "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "qemu/log.h"
+#include "qemu/main-loop.h"
+
+#define D(x)
+
+#define NUM_TIMERS     2
+
+#define R_VAL          0
+#define R_LOADVAL      1
+#define R_BGLOADVAL    2
+#define R_CTRL         3
+#define R_RIS          4
+#define R_MIS          5
+#define R_MAX          6
+
+#define TIMER_CTRL_ENBL     (1 << 0)
+#define TIMER_CTRL_ONESHOT  (1 << 1)
+#define TIMER_CTRL_INTR     (1 << 2)
+#define TIMER_RIS_ACK       (1 << 0)
+#define TIMER_RST_CLR       (1 << 6)
+
+struct msf2_timer {
+    QEMUBH *bh;
+    ptimer_state *ptimer;
+    void *parent;
+    int nr; /* for debug.  */
+
+    unsigned long timer_div;
+
+    uint32_t regs[R_MAX];
+    qemu_irq irq;
+};
+
+#define TYPE_MSF2_TIMER "msf2-timer"
+#define MSF2_TIMER(obj) \
+    OBJECT_CHECK(struct timerblock, (obj), TYPE_MSF2_TIMER)
+
+struct timerblock {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+    uint32_t freq_hz;
+    struct msf2_timer *timers;
+};
+
+static inline unsigned int timer_from_addr(hwaddr addr)
+{
+    /* Timers get a 6x32bit control reg area each.  */
+    return addr / R_MAX;
+}
+
+static void timer_update_irq(struct msf2_timer *st)
+{
+    int isr;
+    int ier;
+
+    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
+    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
+
+    qemu_set_irq(st->irq, (ier && isr));
+}
+
+static uint64_t
+timer_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    struct timerblock *t = opaque;
+    struct msf2_timer *st;
+    uint32_t r = 0;
+    unsigned int timer;
+    int isr;
+    int ier;
+
+    addr >>= 2;
+    timer = timer_from_addr(addr);
+    st = &t->timers[timer];
+
+    if (timer) {
+        addr -= 6;
+    }
+
+    switch (addr) {
+    case R_VAL:
+        r = ptimer_get_count(st->ptimer);
+        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
+        break;
+
+    case R_MIS:
+        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
+        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
+        r = ier && isr;
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(st->regs)) {
+            r = st->regs[addr];
+        }
+        break;
+    }
+    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
+    return r;
+}
+
+static void timer_update(struct msf2_timer *st)
+{
+    uint64_t count;
+
+    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
+
+    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
+        ptimer_stop(st->ptimer);
+        return;
+    }
+
+    count = st->regs[R_LOADVAL];
+    ptimer_set_limit(st->ptimer, count, 1);
+    ptimer_run(st->ptimer, 1);
+}
+
+static void
+timer_write(void *opaque, hwaddr addr,
+            uint64_t val64, unsigned int size)
+{
+    struct timerblock *t = opaque;
+    struct msf2_timer *st;
+    unsigned int timer;
+    uint32_t value = val64;
+
+    addr >>= 2;
+    timer = timer_from_addr(addr);
+    st = &t->timers[timer];
+    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
+             __func__, addr * 4, value, timer));
+
+    if (timer) {
+        addr -= 6;
+    }
+
+    switch (addr) {
+    case R_CTRL:
+        st->regs[R_CTRL] = value;
+        timer_update(st);
+        break;
+
+    case R_RIS:
+        if (value & TIMER_RIS_ACK) {
+            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
+        }
+        break;
+
+    case R_LOADVAL:
+        st->regs[R_LOADVAL] = value;
+        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
+            timer_update(st);
+        }
+        break;
+
+    case R_BGLOADVAL:
+        st->regs[R_BGLOADVAL] = value;
+        st->regs[R_LOADVAL] = value;
+        break;
+
+    case R_VAL:
+    case R_MIS:
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(st->regs)) {
+            st->regs[addr] = value;
+        }
+        break;
+    }
+    timer_update_irq(st);
+}
+
+static const MemoryRegionOps timer_ops = {
+    .read = timer_read,
+    .write = timer_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4
+    }
+};
+
+static void timer_hit(void *opaque)
+{
+    struct msf2_timer *st = opaque;
+    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
+    st->regs[R_RIS] |= TIMER_RIS_ACK;
+
+    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
+        timer_update(st);
+    }
+    timer_update_irq(st);
+}
+
+static void msf2_timer_realize(DeviceState *dev, Error **errp)
+{
+    struct timerblock *t = MSF2_TIMER(dev);
+    unsigned int i;
+
+    /* Init all the ptimers.  */
+    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
+    for (i = 0; i < NUM_TIMERS; i++) {
+        struct msf2_timer *st = &t->timers[i];
+
+        st->parent = t;
+        st->nr = i;
+        st->bh = qemu_bh_new(timer_hit, st);
+        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
+        ptimer_set_freq(st->ptimer, t->freq_hz);
+        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
+    }
+
+    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
+                          R_MAX * 4 * NUM_TIMERS);
+    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
+}
+
+static Property msf2_timer_properties[] = {
+    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
+                                                                83 * 1000000),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void msf2_timer_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = msf2_timer_realize;
+    dc->props = msf2_timer_properties;
+}
+
+static const TypeInfo msf2_timer_info = {
+    .name          = TYPE_MSF2_TIMER,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(struct timerblock),
+    .class_init    = msf2_timer_class_init,
+};
+
+static void msf2_timer_register_types(void)
+{
+    type_register_static(&msf2_timer_info);
+}
+
+type_init(msf2_timer_register_types)
-- 
2.5.0

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[Alistair Francis]

On Sun, Apr 9, 2017 at 4:19 AM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:
> Modelled System Timer in Microsemi's Smartfusion2 Soc.
> Timer has two 32bit down counters and two interrupts.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>

Hey Sundeep,

I have some comments inline below.

> ---
>  hw/timer/Makefile.objs |   1 +
>  hw/timer/msf2_timer.c  | 273 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 274 insertions(+)
>  create mode 100644 hw/timer/msf2_timer.c
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index dd6f27e..0bdf1e1 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -41,3 +41,4 @@ common-obj-$(CONFIG_STM32F2XX_TIMER) += stm32f2xx_timer.o
>  common-obj-$(CONFIG_ASPEED_SOC) += aspeed_timer.o
>
>  common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
> +common-obj-$(CONFIG_MSF2) += msf2_timer.o
> diff --git a/hw/timer/msf2_timer.c b/hw/timer/msf2_timer.c
> new file mode 100644
> index 0000000..962ada4
> --- /dev/null
> +++ b/hw/timer/msf2_timer.c
> @@ -0,0 +1,273 @@
> +/*
> + * Timer block model of Microsemi SmartFusion2.
> + *
> + * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>.
> + *
> + * 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 "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "qemu/log.h"
> +#include "qemu/main-loop.h"
> +
> +#define D(x)

Don't use this for debug prints, you want something more along the
lines of what is at the top of hw/misc/stm32f2xx_syscfg.c. Basically
wrap around the qemu_log() function.

> +
> +#define NUM_TIMERS     2
> +
> +#define R_VAL          0
> +#define R_LOADVAL      1
> +#define R_BGLOADVAL    2
> +#define R_CTRL         3
> +#define R_RIS          4
> +#define R_MIS          5
> +#define R_MAX          6
> +
> +#define TIMER_CTRL_ENBL     (1 << 0)
> +#define TIMER_CTRL_ONESHOT  (1 << 1)
> +#define TIMER_CTRL_INTR     (1 << 2)
> +#define TIMER_RIS_ACK       (1 << 0)
> +#define TIMER_RST_CLR       (1 << 6)
> +
> +struct msf2_timer {
> +    QEMUBH *bh;
> +    ptimer_state *ptimer;
> +    void *parent;
> +    int nr; /* for debug.  */
> +
> +    unsigned long timer_div;
> +
> +    uint32_t regs[R_MAX];
> +    qemu_irq irq;
> +};

Structs in QEMU should always be named in CamelCase.

> +
> +#define TYPE_MSF2_TIMER "msf2-timer"
> +#define MSF2_TIMER(obj) \
> +    OBJECT_CHECK(struct timerblock, (obj), TYPE_MSF2_TIMER)
> +
> +struct timerblock {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion mmio;
> +    uint32_t freq_hz;
> +    struct msf2_timer *timers;
> +};
> +
> +static inline unsigned int timer_from_addr(hwaddr addr)
> +{
> +    /* Timers get a 6x32bit control reg area each.  */
> +    return addr / R_MAX;
> +}
> +
> +static void timer_update_irq(struct msf2_timer *st)
> +{
> +    int isr;
> +    int ier;

I'd declare these both on the same line and declare them as bools.

> +
> +    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
> +    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
> +
> +    qemu_set_irq(st->irq, (ier && isr));
> +}
> +
> +static uint64_t
> +timer_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    struct timerblock *t = opaque;
> +    struct msf2_timer *st;
> +    uint32_t r = 0;
> +    unsigned int timer;
> +    int isr;
> +    int ier;
> +
> +    addr >>= 2;
> +    timer = timer_from_addr(addr);
> +    st = &t->timers[timer];
> +
> +    if (timer) {
> +        addr -= 6;
> +    }

Isn't this timer logic just checking if (addr >> 2) == R_MAX and if it
is set (addr >> 2) back to zero? This seems an overly complex way to
check that.

> +
> +    switch (addr) {
> +    case R_VAL:
> +        r = ptimer_get_count(st->ptimer);
> +        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
> +        break;
> +
> +    case R_MIS:
> +        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
> +        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
> +        r = ier && isr;
> +        break;
> +
> +    default:
> +        if (addr < ARRAY_SIZE(st->regs)) {
> +            r = st->regs[addr];
> +        }
> +        break;
> +    }
> +    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
> +    return r;
> +}
> +
> +static void timer_update(struct msf2_timer *st)
> +{
> +    uint64_t count;
> +
> +    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
> +
> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
> +        ptimer_stop(st->ptimer);
> +        return;
> +    }
> +
> +    count = st->regs[R_LOADVAL];
> +    ptimer_set_limit(st->ptimer, count, 1);
> +    ptimer_run(st->ptimer, 1);
> +}

The update function should be above the read/write functions.

> +
> +static void
> +timer_write(void *opaque, hwaddr addr,
> +            uint64_t val64, unsigned int size)
> +{
> +    struct timerblock *t = opaque;
> +    struct msf2_timer *st;
> +    unsigned int timer;
> +    uint32_t value = val64;
> +
> +    addr >>= 2;
> +    timer = timer_from_addr(addr);
> +    st = &t->timers[timer];
> +    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
> +             __func__, addr * 4, value, timer));
> +
> +    if (timer) {
> +        addr -= 6;
> +    }

Same comment from the read function.

> +
> +    switch (addr) {
> +    case R_CTRL:
> +        st->regs[R_CTRL] = value;
> +        timer_update(st);
> +        break;
> +
> +    case R_RIS:
> +        if (value & TIMER_RIS_ACK) {
> +            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
> +        }
> +        break;
> +
> +    case R_LOADVAL:
> +        st->regs[R_LOADVAL] = value;
> +        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
> +            timer_update(st);
> +        }
> +        break;
> +
> +    case R_BGLOADVAL:
> +        st->regs[R_BGLOADVAL] = value;
> +        st->regs[R_LOADVAL] = value;
> +        break;
> +
> +    case R_VAL:
> +    case R_MIS:
> +        break;
> +
> +    default:
> +        if (addr < ARRAY_SIZE(st->regs)) {
> +            st->regs[addr] = value;
> +        }
> +        break;
> +    }
> +    timer_update_irq(st);
> +}
> +
> +static const MemoryRegionOps timer_ops = {
> +    .read = timer_read,
> +    .write = timer_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4
> +    }
> +};
> +
> +static void timer_hit(void *opaque)
> +{
> +    struct msf2_timer *st = opaque;
> +    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
> +    st->regs[R_RIS] |= TIMER_RIS_ACK;
> +
> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
> +        timer_update(st);
> +    }
> +    timer_update_irq(st);
> +}
> +
> +static void msf2_timer_realize(DeviceState *dev, Error **errp)
> +{
> +    struct timerblock *t = MSF2_TIMER(dev);
> +    unsigned int i;
> +
> +    /* Init all the ptimers.  */
> +    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
> +    for (i = 0; i < NUM_TIMERS; i++) {
> +        struct msf2_timer *st = &t->timers[i];
> +
> +        st->parent = t;
> +        st->nr = i;
> +        st->bh = qemu_bh_new(timer_hit, st);
> +        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
> +        ptimer_set_freq(st->ptimer, t->freq_hz);
> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
> +    }
> +
> +    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
> +                          R_MAX * 4 * NUM_TIMERS);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);

This should be in the devices init() function.

Thanks,

Alistair

> +}
> +
> +static Property msf2_timer_properties[] = {
> +    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
> +                                                                83 * 1000000),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void msf2_timer_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = msf2_timer_realize;
> +    dc->props = msf2_timer_properties;
> +}
> +
> +static const TypeInfo msf2_timer_info = {
> +    .name          = TYPE_MSF2_TIMER,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(struct timerblock),
> +    .class_init    = msf2_timer_class_init,
> +};
> +
> +static void msf2_timer_register_types(void)
> +{
> +    type_register_static(&msf2_timer_info);
> +}
> +
> +type_init(msf2_timer_register_types)
> --
> 2.5.0
>
>

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[sundeep subbaraya]

Hii Alistair,

On Sat, Apr 15, 2017 at 2:58 AM, Alistair Francis <alistair23@gmail.com> wrote:
> On Sun, Apr 9, 2017 at 4:19 AM, Subbaraya Sundeep
> <sundeep.lkml@gmail.com> wrote:
>> Modelled System Timer in Microsemi's Smartfusion2 Soc.
>> Timer has two 32bit down counters and two interrupts.
>>
>> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
>
> Hey Sundeep,
>
> I have some comments inline below.
>
>> ---
>>  hw/timer/Makefile.objs |   1 +
>>  hw/timer/msf2_timer.c  | 273 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 274 insertions(+)
>>  create mode 100644 hw/timer/msf2_timer.c
>>
>> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
>> index dd6f27e..0bdf1e1 100644
>> --- a/hw/timer/Makefile.objs
>> +++ b/hw/timer/Makefile.objs
>> @@ -41,3 +41,4 @@ common-obj-$(CONFIG_STM32F2XX_TIMER) += stm32f2xx_timer.o
>>  common-obj-$(CONFIG_ASPEED_SOC) += aspeed_timer.o
>>
>>  common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
>> +common-obj-$(CONFIG_MSF2) += msf2_timer.o
>> diff --git a/hw/timer/msf2_timer.c b/hw/timer/msf2_timer.c
>> new file mode 100644
>> index 0000000..962ada4
>> --- /dev/null
>> +++ b/hw/timer/msf2_timer.c
>> @@ -0,0 +1,273 @@
>> +/*
>> + * Timer block model of Microsemi SmartFusion2.
>> + *
>> + * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>.
>> + *
>> + * 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 "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/ptimer.h"
>> +#include "qemu/log.h"
>> +#include "qemu/main-loop.h"
>> +
>> +#define D(x)
>
> Don't use this for debug prints, you want something more along the
> lines of what is at the top of hw/misc/stm32f2xx_syscfg.c. Basically
> wrap around the qemu_log() function.

Ok I will use qemu_log like in hw/misc/stm32f2xx_syscfg.c
>
>> +
>> +#define NUM_TIMERS     2
>> +
>> +#define R_VAL          0
>> +#define R_LOADVAL      1
>> +#define R_BGLOADVAL    2
>> +#define R_CTRL         3
>> +#define R_RIS          4
>> +#define R_MIS          5
>> +#define R_MAX          6
>> +
>> +#define TIMER_CTRL_ENBL     (1 << 0)
>> +#define TIMER_CTRL_ONESHOT  (1 << 1)
>> +#define TIMER_CTRL_INTR     (1 << 2)
>> +#define TIMER_RIS_ACK       (1 << 0)
>> +#define TIMER_RST_CLR       (1 << 6)
>> +
>> +struct msf2_timer {
>> +    QEMUBH *bh;
>> +    ptimer_state *ptimer;
>> +    void *parent;
>> +    int nr; /* for debug.  */
>> +
>> +    unsigned long timer_div;
>> +
>> +    uint32_t regs[R_MAX];
>> +    qemu_irq irq;
>> +};
>
> Structs in QEMU should always be named in CamelCase.

Ok I will change.
>
>> +
>> +#define TYPE_MSF2_TIMER "msf2-timer"
>> +#define MSF2_TIMER(obj) \
>> +    OBJECT_CHECK(struct timerblock, (obj), TYPE_MSF2_TIMER)
>> +
>> +struct timerblock {
>> +    SysBusDevice parent_obj;
>> +
>> +    MemoryRegion mmio;
>> +    uint32_t freq_hz;
>> +    struct msf2_timer *timers;
>> +};
>> +
>> +static inline unsigned int timer_from_addr(hwaddr addr)
>> +{
>> +    /* Timers get a 6x32bit control reg area each.  */
>> +    return addr / R_MAX;
>> +}
>> +
>> +static void timer_update_irq(struct msf2_timer *st)
>> +{
>> +    int isr;
>> +    int ier;
>
> I'd declare these both on the same line and declare them as bools.

Ok I will change.
>
>> +
>> +    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>> +    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>> +
>> +    qemu_set_irq(st->irq, (ier && isr));
>> +}
>> +
>> +static uint64_t
>> +timer_read(void *opaque, hwaddr addr, unsigned int size)
>> +{
>> +    struct timerblock *t = opaque;
>> +    struct msf2_timer *st;
>> +    uint32_t r = 0;
>> +    unsigned int timer;
>> +    int isr;
>> +    int ier;
>> +
>> +    addr >>= 2;
>> +    timer = timer_from_addr(addr);
>> +    st = &t->timers[timer];
>> +
>> +    if (timer) {
>> +        addr -= 6;
>> +    }
>
> Isn't this timer logic just checking if (addr >> 2) == R_MAX and if it
> is set (addr >> 2) back to zero? This seems an overly complex way to
> check that.
I did not get you clearly. Do you want me to write like this:
unsigned int timer = 0;

addr >>= 2;
if (addr >= R_MAX) {
    timer = 1;
    addr =  addr - R_MAX;
}

>
>> +
>> +    switch (addr) {
>> +    case R_VAL:
>> +        r = ptimer_get_count(st->ptimer);
>> +        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
>> +        break;
>> +
>> +    case R_MIS:
>> +        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>> +        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>> +        r = ier && isr;
>> +        break;
>> +
>> +    default:
>> +        if (addr < ARRAY_SIZE(st->regs)) {
>> +            r = st->regs[addr];
>> +        }
>> +        break;
>> +    }
>> +    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
>> +    return r;
>> +}
>> +
>> +static void timer_update(struct msf2_timer *st)
>> +{
>> +    uint64_t count;
>> +
>> +    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
>> +
>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
>> +        ptimer_stop(st->ptimer);
>> +        return;
>> +    }
>> +
>> +    count = st->regs[R_LOADVAL];
>> +    ptimer_set_limit(st->ptimer, count, 1);
>> +    ptimer_run(st->ptimer, 1);
>> +}
>
> The update function should be above the read/write functions.
>
Ok I will change

>> +
>> +static void
>> +timer_write(void *opaque, hwaddr addr,
>> +            uint64_t val64, unsigned int size)
>> +{
>> +    struct timerblock *t = opaque;
>> +    struct msf2_timer *st;
>> +    unsigned int timer;
>> +    uint32_t value = val64;
>> +
>> +    addr >>= 2;
>> +    timer = timer_from_addr(addr);
>> +    st = &t->timers[timer];
>> +    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
>> +             __func__, addr * 4, value, timer));
>> +
>> +    if (timer) {
>> +        addr -= 6;
>> +    }
>
> Same comment from the read function.
>
>> +
>> +    switch (addr) {
>> +    case R_CTRL:
>> +        st->regs[R_CTRL] = value;
>> +        timer_update(st);
>> +        break;
>> +
>> +    case R_RIS:
>> +        if (value & TIMER_RIS_ACK) {
>> +            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
>> +        }
>> +        break;
>> +
>> +    case R_LOADVAL:
>> +        st->regs[R_LOADVAL] = value;
>> +        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
>> +            timer_update(st);
>> +        }
>> +        break;
>> +
>> +    case R_BGLOADVAL:
>> +        st->regs[R_BGLOADVAL] = value;
>> +        st->regs[R_LOADVAL] = value;
>> +        break;
>> +
>> +    case R_VAL:
>> +    case R_MIS:
>> +        break;
>> +
>> +    default:
>> +        if (addr < ARRAY_SIZE(st->regs)) {
>> +            st->regs[addr] = value;
>> +        }
>> +        break;
>> +    }
>> +    timer_update_irq(st);
>> +}
>> +
>> +static const MemoryRegionOps timer_ops = {
>> +    .read = timer_read,
>> +    .write = timer_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4
>> +    }
>> +};
>> +
>> +static void timer_hit(void *opaque)
>> +{
>> +    struct msf2_timer *st = opaque;
>> +    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
>> +    st->regs[R_RIS] |= TIMER_RIS_ACK;
>> +
>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
>> +        timer_update(st);
>> +    }
>> +    timer_update_irq(st);
>> +}
>> +
>> +static void msf2_timer_realize(DeviceState *dev, Error **errp)
>> +{
>> +    struct timerblock *t = MSF2_TIMER(dev);
>> +    unsigned int i;
>> +
>> +    /* Init all the ptimers.  */
>> +    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
>> +    for (i = 0; i < NUM_TIMERS; i++) {
>> +        struct msf2_timer *st = &t->timers[i];
>> +
>> +        st->parent = t;
>> +        st->nr = i;
>> +        st->bh = qemu_bh_new(timer_hit, st);
>> +        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
>> +        ptimer_set_freq(st->ptimer, t->freq_hz);
>> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
>> +    }
>> +
>> +    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
>> +                          R_MAX * 4 * NUM_TIMERS);
>> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
>
> This should be in the devices init() function.

I referred Xilinx soft IP models for writing these models and used
same boilerplate code.
I am not clear about realize and init functions yet. Can you please
give a brief about them.
Don't we need to use realize function for new models?

Thanks,
Sundeep
>
> Thanks,
>
> Alistair
>
>> +}
>> +
>> +static Property msf2_timer_properties[] = {
>> +    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
>> +                                                                83 * 1000000),
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void msf2_timer_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->realize = msf2_timer_realize;
>> +    dc->props = msf2_timer_properties;
>> +}
>> +
>> +static const TypeInfo msf2_timer_info = {
>> +    .name          = TYPE_MSF2_TIMER,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(struct timerblock),
>> +    .class_init    = msf2_timer_class_init,
>> +};
>> +
>> +static void msf2_timer_register_types(void)
>> +{
>> +    type_register_static(&msf2_timer_info);
>> +}
>> +
>> +type_init(msf2_timer_register_types)
>> --
>> 2.5.0
>>
>>

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[Alistair Francis]

>>> +
>>> +    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>> +    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>> +
>>> +    qemu_set_irq(st->irq, (ier && isr));
>>> +}
>>> +
>>> +static uint64_t
>>> +timer_read(void *opaque, hwaddr addr, unsigned int size)
>>> +{
>>> +    struct timerblock *t = opaque;
>>> +    struct msf2_timer *st;
>>> +    uint32_t r = 0;
>>> +    unsigned int timer;
>>> +    int isr;
>>> +    int ier;
>>> +
>>> +    addr >>= 2;
>>> +    timer = timer_from_addr(addr);
>>> +    st = &t->timers[timer];
>>> +
>>> +    if (timer) {
>>> +        addr -= 6;
>>> +    }
>>
>> Isn't this timer logic just checking if (addr >> 2) == R_MAX and if it
>> is set (addr >> 2) back to zero? This seems an overly complex way to
>> check that.
> I did not get you clearly. Do you want me to write like this:
> unsigned int timer = 0;
>
> addr >>= 2;
> if (addr >= R_MAX) {
>     timer = 1;
>     addr =  addr - R_MAX;
> }

Yeah, I think this is clearer then what you had earlier.

Although why do you have to subtract R_MAX, shouldn't it just be an
error if accessing values larger then R_MAX?

>
>>
>>> +
>>> +    switch (addr) {
>>> +    case R_VAL:
>>> +        r = ptimer_get_count(st->ptimer);
>>> +        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
>>> +        break;
>>> +
>>> +    case R_MIS:
>>> +        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>> +        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>> +        r = ier && isr;
>>> +        break;
>>> +
>>> +    default:
>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>> +            r = st->regs[addr];
>>> +        }
>>> +        break;
>>> +    }
>>> +    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
>>> +    return r;
>>> +}
>>> +
>>> +static void timer_update(struct msf2_timer *st)
>>> +{
>>> +    uint64_t count;
>>> +
>>> +    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
>>> +
>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
>>> +        ptimer_stop(st->ptimer);
>>> +        return;
>>> +    }
>>> +
>>> +    count = st->regs[R_LOADVAL];
>>> +    ptimer_set_limit(st->ptimer, count, 1);
>>> +    ptimer_run(st->ptimer, 1);
>>> +}
>>
>> The update function should be above the read/write functions.
>>
> Ok I will change
>
>>> +
>>> +static void
>>> +timer_write(void *opaque, hwaddr addr,
>>> +            uint64_t val64, unsigned int size)
>>> +{
>>> +    struct timerblock *t = opaque;
>>> +    struct msf2_timer *st;
>>> +    unsigned int timer;
>>> +    uint32_t value = val64;
>>> +
>>> +    addr >>= 2;
>>> +    timer = timer_from_addr(addr);
>>> +    st = &t->timers[timer];
>>> +    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
>>> +             __func__, addr * 4, value, timer));
>>> +
>>> +    if (timer) {
>>> +        addr -= 6;
>>> +    }
>>
>> Same comment from the read function.
>>
>>> +
>>> +    switch (addr) {
>>> +    case R_CTRL:
>>> +        st->regs[R_CTRL] = value;
>>> +        timer_update(st);
>>> +        break;
>>> +
>>> +    case R_RIS:
>>> +        if (value & TIMER_RIS_ACK) {
>>> +            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
>>> +        }
>>> +        break;
>>> +
>>> +    case R_LOADVAL:
>>> +        st->regs[R_LOADVAL] = value;
>>> +        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
>>> +            timer_update(st);
>>> +        }
>>> +        break;
>>> +
>>> +    case R_BGLOADVAL:
>>> +        st->regs[R_BGLOADVAL] = value;
>>> +        st->regs[R_LOADVAL] = value;
>>> +        break;
>>> +
>>> +    case R_VAL:
>>> +    case R_MIS:
>>> +        break;
>>> +
>>> +    default:
>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>> +            st->regs[addr] = value;
>>> +        }
>>> +        break;
>>> +    }
>>> +    timer_update_irq(st);
>>> +}
>>> +
>>> +static const MemoryRegionOps timer_ops = {
>>> +    .read = timer_read,
>>> +    .write = timer_write,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .valid = {
>>> +        .min_access_size = 4,
>>> +        .max_access_size = 4
>>> +    }
>>> +};
>>> +
>>> +static void timer_hit(void *opaque)
>>> +{
>>> +    struct msf2_timer *st = opaque;
>>> +    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
>>> +    st->regs[R_RIS] |= TIMER_RIS_ACK;
>>> +
>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
>>> +        timer_update(st);
>>> +    }
>>> +    timer_update_irq(st);
>>> +}
>>> +
>>> +static void msf2_timer_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    struct timerblock *t = MSF2_TIMER(dev);
>>> +    unsigned int i;
>>> +
>>> +    /* Init all the ptimers.  */
>>> +    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
>>> +    for (i = 0; i < NUM_TIMERS; i++) {
>>> +        struct msf2_timer *st = &t->timers[i];
>>> +
>>> +        st->parent = t;
>>> +        st->nr = i;
>>> +        st->bh = qemu_bh_new(timer_hit, st);
>>> +        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
>>> +        ptimer_set_freq(st->ptimer, t->freq_hz);
>>> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
>>> +    }
>>> +
>>> +    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
>>> +                          R_MAX * 4 * NUM_TIMERS);
>>> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
>>
>> This should be in the devices init() function.
>
> I referred Xilinx soft IP models for writing these models and used
> same boilerplate code.
> I am not clear about realize and init functions yet. Can you please
> give a brief about them.

Basically the simple explanation is that init is called when the
object is created and realize is called when the object is realized.

Generally for devices it will go something like this:
 1. init
 2. Set properties
 3. Connect things
 4. realize
 5. Map to memory

> Don't we need to use realize function for new models?

AFAIK we still put things like: sysbus_init_irq(),
memory_region_init_io() and sysbus_init_mmio() in the init function.

I don't think we are at a stage yet to not use init functions.

Thanks,

Alistair

>
> Thanks,
> Sundeep
>>
>> Thanks,
>>
>> Alistair
>>
>>> +}
>>> +
>>> +static Property msf2_timer_properties[] = {
>>> +    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
>>> +                                                                83 * 1000000),
>>> +    DEFINE_PROP_END_OF_LIST(),
>>> +};
>>> +
>>> +static void msf2_timer_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->realize = msf2_timer_realize;
>>> +    dc->props = msf2_timer_properties;
>>> +}
>>> +
>>> +static const TypeInfo msf2_timer_info = {
>>> +    .name          = TYPE_MSF2_TIMER,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(struct timerblock),
>>> +    .class_init    = msf2_timer_class_init,
>>> +};
>>> +
>>> +static void msf2_timer_register_types(void)
>>> +{
>>> +    type_register_static(&msf2_timer_info);
>>> +}
>>> +
>>> +type_init(msf2_timer_register_types)
>>> --
>>> 2.5.0
>>>
>>>

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[Peter Maydell]

On 24 April 2017 at 18:44, Alistair Francis <alistair23@gmail.com> wrote:
> Basically the simple explanation is that init is called when the
> object is created and realize is called when the object is realized.
>
> Generally for devices it will go something like this:
>  1. init
>  2. Set properties
>  3. Connect things
>  4. realize
>  5. Map to memory
>
>> Don't we need to use realize function for new models?
>
> AFAIK we still put things like: sysbus_init_irq(),
> memory_region_init_io() and sysbus_init_mmio() in the init function.
>
> I don't think we are at a stage yet to not use init functions.

Two-phase init is here to stay -- some things must be done in
init, some must be done in realize, and some can be done in
either. Some simple devices may find they can do everything
in only one function.

Must be done in init:
 * creating properties (for the cases where that is done "by hand"
   by calling object_property_add_*())
 * calling init on child objects which you want to pass through
   alias properties for

Must be done in realize:
 * anything that can fail such that we need to report the
   error and abandon creation of the device
 * anything which depends on the values of QOM properties that
   the caller might have set

We should probably sit down and write up some guidelines for
how we recommend dealing with the various things that could
be called in either function -- this is basically a code
style and consistency question.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[sundeep subbaraya]

Hi Alistair and Peter,

On Mon, Apr 24, 2017 at 11:28 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 24 April 2017 at 18:44, Alistair Francis <alistair23@gmail.com> wrote:
>> Basically the simple explanation is that init is called when the
>> object is created and realize is called when the object is realized.
>>
>> Generally for devices it will go something like this:
>>  1. init
>>  2. Set properties
>>  3. Connect things
>>  4. realize
>>  5. Map to memory
>>
>>> Don't we need to use realize function for new models?
>>
>> AFAIK we still put things like: sysbus_init_irq(),
>> memory_region_init_io() and sysbus_init_mmio() in the init function.
>>
>> I don't think we are at a stage yet to not use init functions.
>
> Two-phase init is here to stay -- some things must be done in
> init, some must be done in realize, and some can be done in
> either. Some simple devices may find they can do everything
> in only one function.
>
> Must be done in init:
>  * creating properties (for the cases where that is done "by hand"
>    by calling object_property_add_*())
>  * calling init on child objects which you want to pass through
>    alias properties for
>
> Must be done in realize:
>  * anything that can fail such that we need to report the
>    error and abandon creation of the device
>  * anything which depends on the values of QOM properties that
>    the caller might have set
>
> We should probably sit down and write up some guidelines for
> how we recommend dealing with the various things that could
> be called in either function -- this is basically a code
> style and consistency question.

Thanks for the brief. It makes sense for me now.
I will make changes and send the patches.

Thanks,
Sundeep

>
> thanks
> -- PMM

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[sundeep subbaraya]

Hi Alistair,

On Mon, Apr 24, 2017 at 11:14 PM, Alistair Francis <alistair23@gmail.com> wrote:
>>>> +
>>>> +    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>>> +    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>>> +
>>>> +    qemu_set_irq(st->irq, (ier && isr));
>>>> +}
>>>> +
>>>> +static uint64_t
>>>> +timer_read(void *opaque, hwaddr addr, unsigned int size)
>>>> +{
>>>> +    struct timerblock *t = opaque;
>>>> +    struct msf2_timer *st;
>>>> +    uint32_t r = 0;
>>>> +    unsigned int timer;
>>>> +    int isr;
>>>> +    int ier;
>>>> +
>>>> +    addr >>= 2;
>>>> +    timer = timer_from_addr(addr);
>>>> +    st = &t->timers[timer];
>>>> +
>>>> +    if (timer) {
>>>> +        addr -= 6;
>>>> +    }
>>>
>>> Isn't this timer logic just checking if (addr >> 2) == R_MAX and if it
>>> is set (addr >> 2) back to zero? This seems an overly complex way to
>>> check that.
>> I did not get you clearly. Do you want me to write like this:
>> unsigned int timer = 0;
>>
>> addr >>= 2;
>> if (addr >= R_MAX) {
>>     timer = 1;
>>     addr =  addr - R_MAX;
>> }
>
> Yeah, I think this is clearer then what you had earlier.
>
> Although why do you have to subtract R_MAX, shouldn't it just be an
> error if accessing values larger then R_MAX?

Sorry I forgot about replying to this in earlier mail.
There are two independent timer blocks accessing same base address.
Based on offset passed in read/write functions we figure out
which block has to be handled.
0x0 to 0x14 -> timer1
0x18 to 0x2C -> timer2
Here R_MAX is 0x18 hence addr >= R_MAX is valid and refers to timer2.
Although I missed the bounds checking 0 < addr < 0x2C. I will add that
check in read and
write functions.

Thanks,
Sundeep
>
>>
>>>
>>>> +
>>>> +    switch (addr) {
>>>> +    case R_VAL:
>>>> +        r = ptimer_get_count(st->ptimer);
>>>> +        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
>>>> +        break;
>>>> +
>>>> +    case R_MIS:
>>>> +        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>>> +        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>>> +        r = ier && isr;
>>>> +        break;
>>>> +
>>>> +    default:
>>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>>> +            r = st->regs[addr];
>>>> +        }
>>>> +        break;
>>>> +    }
>>>> +    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
>>>> +    return r;
>>>> +}
>>>> +
>>>> +static void timer_update(struct msf2_timer *st)
>>>> +{
>>>> +    uint64_t count;
>>>> +
>>>> +    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
>>>> +
>>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
>>>> +        ptimer_stop(st->ptimer);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    count = st->regs[R_LOADVAL];
>>>> +    ptimer_set_limit(st->ptimer, count, 1);
>>>> +    ptimer_run(st->ptimer, 1);
>>>> +}
>>>
>>> The update function should be above the read/write functions.
>>>
>> Ok I will change
>>
>>>> +
>>>> +static void
>>>> +timer_write(void *opaque, hwaddr addr,
>>>> +            uint64_t val64, unsigned int size)
>>>> +{
>>>> +    struct timerblock *t = opaque;
>>>> +    struct msf2_timer *st;
>>>> +    unsigned int timer;
>>>> +    uint32_t value = val64;
>>>> +
>>>> +    addr >>= 2;
>>>> +    timer = timer_from_addr(addr);
>>>> +    st = &t->timers[timer];
>>>> +    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
>>>> +             __func__, addr * 4, value, timer));
>>>> +
>>>> +    if (timer) {
>>>> +        addr -= 6;
>>>> +    }
>>>
>>> Same comment from the read function.
>>>
>>>> +
>>>> +    switch (addr) {
>>>> +    case R_CTRL:
>>>> +        st->regs[R_CTRL] = value;
>>>> +        timer_update(st);
>>>> +        break;
>>>> +
>>>> +    case R_RIS:
>>>> +        if (value & TIMER_RIS_ACK) {
>>>> +            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
>>>> +        }
>>>> +        break;
>>>> +
>>>> +    case R_LOADVAL:
>>>> +        st->regs[R_LOADVAL] = value;
>>>> +        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
>>>> +            timer_update(st);
>>>> +        }
>>>> +        break;
>>>> +
>>>> +    case R_BGLOADVAL:
>>>> +        st->regs[R_BGLOADVAL] = value;
>>>> +        st->regs[R_LOADVAL] = value;
>>>> +        break;
>>>> +
>>>> +    case R_VAL:
>>>> +    case R_MIS:
>>>> +        break;
>>>> +
>>>> +    default:
>>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>>> +            st->regs[addr] = value;
>>>> +        }
>>>> +        break;
>>>> +    }
>>>> +    timer_update_irq(st);
>>>> +}
>>>> +
>>>> +static const MemoryRegionOps timer_ops = {
>>>> +    .read = timer_read,
>>>> +    .write = timer_write,
>>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>>> +    .valid = {
>>>> +        .min_access_size = 4,
>>>> +        .max_access_size = 4
>>>> +    }
>>>> +};
>>>> +
>>>> +static void timer_hit(void *opaque)
>>>> +{
>>>> +    struct msf2_timer *st = opaque;
>>>> +    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
>>>> +    st->regs[R_RIS] |= TIMER_RIS_ACK;
>>>> +
>>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
>>>> +        timer_update(st);
>>>> +    }
>>>> +    timer_update_irq(st);
>>>> +}
>>>> +
>>>> +static void msf2_timer_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    struct timerblock *t = MSF2_TIMER(dev);
>>>> +    unsigned int i;
>>>> +
>>>> +    /* Init all the ptimers.  */
>>>> +    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
>>>> +    for (i = 0; i < NUM_TIMERS; i++) {
>>>> +        struct msf2_timer *st = &t->timers[i];
>>>> +
>>>> +        st->parent = t;
>>>> +        st->nr = i;
>>>> +        st->bh = qemu_bh_new(timer_hit, st);
>>>> +        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
>>>> +        ptimer_set_freq(st->ptimer, t->freq_hz);
>>>> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
>>>> +    }
>>>> +
>>>> +    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
>>>> +                          R_MAX * 4 * NUM_TIMERS);
>>>> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
>>>
>>> This should be in the devices init() function.
>>
>> I referred Xilinx soft IP models for writing these models and used
>> same boilerplate code.
>> I am not clear about realize and init functions yet. Can you please
>> give a brief about them.
>
> Basically the simple explanation is that init is called when the
> object is created and realize is called when the object is realized.
>
> Generally for devices it will go something like this:
>  1. init
>  2. Set properties
>  3. Connect things
>  4. realize
>  5. Map to memory
>
>> Don't we need to use realize function for new models?
>
> AFAIK we still put things like: sysbus_init_irq(),
> memory_region_init_io() and sysbus_init_mmio() in the init function.
>
> I don't think we are at a stage yet to not use init functions.
>
> Thanks,
>
> Alistair
>
>>
>> Thanks,
>> Sundeep
>>>
>>> Thanks,
>>>
>>> Alistair
>>>
>>>> +}
>>>> +
>>>> +static Property msf2_timer_properties[] = {
>>>> +    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
>>>> +                                                                83 * 1000000),
>>>> +    DEFINE_PROP_END_OF_LIST(),
>>>> +};
>>>> +
>>>> +static void msf2_timer_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +
>>>> +    dc->realize = msf2_timer_realize;
>>>> +    dc->props = msf2_timer_properties;
>>>> +}
>>>> +
>>>> +static const TypeInfo msf2_timer_info = {
>>>> +    .name          = TYPE_MSF2_TIMER,
>>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>> +    .instance_size = sizeof(struct timerblock),
>>>> +    .class_init    = msf2_timer_class_init,
>>>> +};
>>>> +
>>>> +static void msf2_timer_register_types(void)
>>>> +{
>>>> +    type_register_static(&msf2_timer_info);
>>>> +}
>>>> +
>>>> +type_init(msf2_timer_register_types)
>>>> --
>>>> 2.5.0
>>>>
>>>>

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[Alistair Francis]

On Tue, Apr 25, 2017 at 3:36 AM, sundeep subbaraya
<sundeep.lkml@gmail.com> wrote:
> Hi Alistair,
>
> On Mon, Apr 24, 2017 at 11:14 PM, Alistair Francis <alistair23@gmail.com> wrote:
>>>>> +
>>>>> +    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>>>> +    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>>>> +
>>>>> +    qemu_set_irq(st->irq, (ier && isr));
>>>>> +}
>>>>> +
>>>>> +static uint64_t
>>>>> +timer_read(void *opaque, hwaddr addr, unsigned int size)
>>>>> +{
>>>>> +    struct timerblock *t = opaque;
>>>>> +    struct msf2_timer *st;
>>>>> +    uint32_t r = 0;
>>>>> +    unsigned int timer;
>>>>> +    int isr;
>>>>> +    int ier;
>>>>> +
>>>>> +    addr >>= 2;
>>>>> +    timer = timer_from_addr(addr);
>>>>> +    st = &t->timers[timer];
>>>>> +
>>>>> +    if (timer) {
>>>>> +        addr -= 6;
>>>>> +    }
>>>>
>>>> Isn't this timer logic just checking if (addr >> 2) == R_MAX and if it
>>>> is set (addr >> 2) back to zero? This seems an overly complex way to
>>>> check that.
>>> I did not get you clearly. Do you want me to write like this:
>>> unsigned int timer = 0;
>>>
>>> addr >>= 2;
>>> if (addr >= R_MAX) {
>>>     timer = 1;
>>>     addr =  addr - R_MAX;
>>> }
>>
>> Yeah, I think this is clearer then what you had earlier.
>>
>> Although why do you have to subtract R_MAX, shouldn't it just be an
>> error if accessing values larger then R_MAX?
>
> Sorry I forgot about replying to this in earlier mail.
> There are two independent timer blocks accessing same base address.
> Based on offset passed in read/write functions we figure out
> which block has to be handled.
> 0x0 to 0x14 -> timer1
> 0x18 to 0x2C -> timer2
> Here R_MAX is 0x18 hence addr >= R_MAX is valid and refers to timer2.
> Although I missed the bounds checking 0 < addr < 0x2C. I will add that
> check in read and
> write functions.

Ok, when you send the next version can you explain this in comments
that way it's clear what you are trying to do.

Thanks,

Alistair

>
> Thanks,
> Sundeep
>>
>>>
>>>>
>>>>> +
>>>>> +    switch (addr) {
>>>>> +    case R_VAL:
>>>>> +        r = ptimer_get_count(st->ptimer);
>>>>> +        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
>>>>> +        break;
>>>>> +
>>>>> +    case R_MIS:
>>>>> +        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>>>> +        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>>>> +        r = ier && isr;
>>>>> +        break;
>>>>> +
>>>>> +    default:
>>>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>>>> +            r = st->regs[addr];
>>>>> +        }
>>>>> +        break;
>>>>> +    }
>>>>> +    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
>>>>> +    return r;
>>>>> +}
>>>>> +
>>>>> +static void timer_update(struct msf2_timer *st)
>>>>> +{
>>>>> +    uint64_t count;
>>>>> +
>>>>> +    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
>>>>> +
>>>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
>>>>> +        ptimer_stop(st->ptimer);
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    count = st->regs[R_LOADVAL];
>>>>> +    ptimer_set_limit(st->ptimer, count, 1);
>>>>> +    ptimer_run(st->ptimer, 1);
>>>>> +}
>>>>
>>>> The update function should be above the read/write functions.
>>>>
>>> Ok I will change
>>>
>>>>> +
>>>>> +static void
>>>>> +timer_write(void *opaque, hwaddr addr,
>>>>> +            uint64_t val64, unsigned int size)
>>>>> +{
>>>>> +    struct timerblock *t = opaque;
>>>>> +    struct msf2_timer *st;
>>>>> +    unsigned int timer;
>>>>> +    uint32_t value = val64;
>>>>> +
>>>>> +    addr >>= 2;
>>>>> +    timer = timer_from_addr(addr);
>>>>> +    st = &t->timers[timer];
>>>>> +    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
>>>>> +             __func__, addr * 4, value, timer));
>>>>> +
>>>>> +    if (timer) {
>>>>> +        addr -= 6;
>>>>> +    }
>>>>
>>>> Same comment from the read function.
>>>>
>>>>> +
>>>>> +    switch (addr) {
>>>>> +    case R_CTRL:
>>>>> +        st->regs[R_CTRL] = value;
>>>>> +        timer_update(st);
>>>>> +        break;
>>>>> +
>>>>> +    case R_RIS:
>>>>> +        if (value & TIMER_RIS_ACK) {
>>>>> +            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
>>>>> +        }
>>>>> +        break;
>>>>> +
>>>>> +    case R_LOADVAL:
>>>>> +        st->regs[R_LOADVAL] = value;
>>>>> +        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
>>>>> +            timer_update(st);
>>>>> +        }
>>>>> +        break;
>>>>> +
>>>>> +    case R_BGLOADVAL:
>>>>> +        st->regs[R_BGLOADVAL] = value;
>>>>> +        st->regs[R_LOADVAL] = value;
>>>>> +        break;
>>>>> +
>>>>> +    case R_VAL:
>>>>> +    case R_MIS:
>>>>> +        break;
>>>>> +
>>>>> +    default:
>>>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>>>> +            st->regs[addr] = value;
>>>>> +        }
>>>>> +        break;
>>>>> +    }
>>>>> +    timer_update_irq(st);
>>>>> +}
>>>>> +
>>>>> +static const MemoryRegionOps timer_ops = {
>>>>> +    .read = timer_read,
>>>>> +    .write = timer_write,
>>>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>>>> +    .valid = {
>>>>> +        .min_access_size = 4,
>>>>> +        .max_access_size = 4
>>>>> +    }
>>>>> +};
>>>>> +
>>>>> +static void timer_hit(void *opaque)
>>>>> +{
>>>>> +    struct msf2_timer *st = opaque;
>>>>> +    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
>>>>> +    st->regs[R_RIS] |= TIMER_RIS_ACK;
>>>>> +
>>>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
>>>>> +        timer_update(st);
>>>>> +    }
>>>>> +    timer_update_irq(st);
>>>>> +}
>>>>> +
>>>>> +static void msf2_timer_realize(DeviceState *dev, Error **errp)
>>>>> +{
>>>>> +    struct timerblock *t = MSF2_TIMER(dev);
>>>>> +    unsigned int i;
>>>>> +
>>>>> +    /* Init all the ptimers.  */
>>>>> +    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
>>>>> +    for (i = 0; i < NUM_TIMERS; i++) {
>>>>> +        struct msf2_timer *st = &t->timers[i];
>>>>> +
>>>>> +        st->parent = t;
>>>>> +        st->nr = i;
>>>>> +        st->bh = qemu_bh_new(timer_hit, st);
>>>>> +        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
>>>>> +        ptimer_set_freq(st->ptimer, t->freq_hz);
>>>>> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
>>>>> +    }
>>>>> +
>>>>> +    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
>>>>> +                          R_MAX * 4 * NUM_TIMERS);
>>>>> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
>>>>
>>>> This should be in the devices init() function.
>>>
>>> I referred Xilinx soft IP models for writing these models and used
>>> same boilerplate code.
>>> I am not clear about realize and init functions yet. Can you please
>>> give a brief about them.
>>
>> Basically the simple explanation is that init is called when the
>> object is created and realize is called when the object is realized.
>>
>> Generally for devices it will go something like this:
>>  1. init
>>  2. Set properties
>>  3. Connect things
>>  4. realize
>>  5. Map to memory
>>
>>> Don't we need to use realize function for new models?
>>
>> AFAIK we still put things like: sysbus_init_irq(),
>> memory_region_init_io() and sysbus_init_mmio() in the init function.
>>
>> I don't think we are at a stage yet to not use init functions.
>>
>> Thanks,
>>
>> Alistair
>>
>>>
>>> Thanks,
>>> Sundeep
>>>>
>>>> Thanks,
>>>>
>>>> Alistair
>>>>
>>>>> +}
>>>>> +
>>>>> +static Property msf2_timer_properties[] = {
>>>>> +    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
>>>>> +                                                                83 * 1000000),
>>>>> +    DEFINE_PROP_END_OF_LIST(),
>>>>> +};
>>>>> +
>>>>> +static void msf2_timer_class_init(ObjectClass *klass, void *data)
>>>>> +{
>>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>>> +
>>>>> +    dc->realize = msf2_timer_realize;
>>>>> +    dc->props = msf2_timer_properties;
>>>>> +}
>>>>> +
>>>>> +static const TypeInfo msf2_timer_info = {
>>>>> +    .name          = TYPE_MSF2_TIMER,
>>>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>>> +    .instance_size = sizeof(struct timerblock),
>>>>> +    .class_init    = msf2_timer_class_init,
>>>>> +};
>>>>> +
>>>>> +static void msf2_timer_register_types(void)
>>>>> +{
>>>>> +    type_register_static(&msf2_timer_info);
>>>>> +}
>>>>> +
>>>>> +type_init(msf2_timer_register_types)
>>>>> --
>>>>> 2.5.0
>>>>>
>>>>>

Re: [Qemu-devel] [Qemu-devel RFC v2 1/4] msf2: Add Smartfusion2 System timer

[sundeep subbaraya]

Hi Alistair,

On Fri, Apr 28, 2017 at 12:23 AM, Alistair Francis <alistair23@gmail.com> wrote:
> On Tue, Apr 25, 2017 at 3:36 AM, sundeep subbaraya
> <sundeep.lkml@gmail.com> wrote:
>> Hi Alistair,
>>
>> On Mon, Apr 24, 2017 at 11:14 PM, Alistair Francis <alistair23@gmail.com> wrote:
>>>>>> +
>>>>>> +    isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>>>>> +    ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>>>>> +
>>>>>> +    qemu_set_irq(st->irq, (ier && isr));
>>>>>> +}
>>>>>> +
>>>>>> +static uint64_t
>>>>>> +timer_read(void *opaque, hwaddr addr, unsigned int size)
>>>>>> +{
>>>>>> +    struct timerblock *t = opaque;
>>>>>> +    struct msf2_timer *st;
>>>>>> +    uint32_t r = 0;
>>>>>> +    unsigned int timer;
>>>>>> +    int isr;
>>>>>> +    int ier;
>>>>>> +
>>>>>> +    addr >>= 2;
>>>>>> +    timer = timer_from_addr(addr);
>>>>>> +    st = &t->timers[timer];
>>>>>> +
>>>>>> +    if (timer) {
>>>>>> +        addr -= 6;
>>>>>> +    }
>>>>>
>>>>> Isn't this timer logic just checking if (addr >> 2) == R_MAX and if it
>>>>> is set (addr >> 2) back to zero? This seems an overly complex way to
>>>>> check that.
>>>> I did not get you clearly. Do you want me to write like this:
>>>> unsigned int timer = 0;
>>>>
>>>> addr >>= 2;
>>>> if (addr >= R_MAX) {
>>>>     timer = 1;
>>>>     addr =  addr - R_MAX;
>>>> }
>>>
>>> Yeah, I think this is clearer then what you had earlier.
>>>
>>> Although why do you have to subtract R_MAX, shouldn't it just be an
>>> error if accessing values larger then R_MAX?
>>
>> Sorry I forgot about replying to this in earlier mail.
>> There are two independent timer blocks accessing same base address.
>> Based on offset passed in read/write functions we figure out
>> which block has to be handled.
>> 0x0 to 0x14 -> timer1
>> 0x18 to 0x2C -> timer2
>> Here R_MAX is 0x18 hence addr >= R_MAX is valid and refers to timer2.
>> Although I missed the bounds checking 0 < addr < 0x2C. I will add that
>> check in read and
>> write functions.
>
> Ok, when you send the next version can you explain this in comments
> that way it's clear what you are trying to do.

Sure Alistair.

Thanks,
Sundeep
>
> Thanks,
>
> Alistair
>
>>
>> Thanks,
>> Sundeep
>>>
>>>>
>>>>>
>>>>>> +
>>>>>> +    switch (addr) {
>>>>>> +    case R_VAL:
>>>>>> +        r = ptimer_get_count(st->ptimer);
>>>>>> +        D(qemu_log("msf2_timer t=%d read counter=%x\n", timer, r));
>>>>>> +        break;
>>>>>> +
>>>>>> +    case R_MIS:
>>>>>> +        isr = !!(st->regs[R_RIS] & TIMER_RIS_ACK);
>>>>>> +        ier = !!(st->regs[R_CTRL] & TIMER_CTRL_INTR);
>>>>>> +        r = ier && isr;
>>>>>> +        break;
>>>>>> +
>>>>>> +    default:
>>>>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>>>>> +            r = st->regs[addr];
>>>>>> +        }
>>>>>> +        break;
>>>>>> +    }
>>>>>> +    D(fprintf(stderr, "%s timer=%d %x=%x\n", __func__, timer, addr * 4, r));
>>>>>> +    return r;
>>>>>> +}
>>>>>> +
>>>>>> +static void timer_update(struct msf2_timer *st)
>>>>>> +{
>>>>>> +    uint64_t count;
>>>>>> +
>>>>>> +    D(fprintf(stderr, "%s timer=%d\n", __func__, st->nr));
>>>>>> +
>>>>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ENBL)) {
>>>>>> +        ptimer_stop(st->ptimer);
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    count = st->regs[R_LOADVAL];
>>>>>> +    ptimer_set_limit(st->ptimer, count, 1);
>>>>>> +    ptimer_run(st->ptimer, 1);
>>>>>> +}
>>>>>
>>>>> The update function should be above the read/write functions.
>>>>>
>>>> Ok I will change
>>>>
>>>>>> +
>>>>>> +static void
>>>>>> +timer_write(void *opaque, hwaddr addr,
>>>>>> +            uint64_t val64, unsigned int size)
>>>>>> +{
>>>>>> +    struct timerblock *t = opaque;
>>>>>> +    struct msf2_timer *st;
>>>>>> +    unsigned int timer;
>>>>>> +    uint32_t value = val64;
>>>>>> +
>>>>>> +    addr >>= 2;
>>>>>> +    timer = timer_from_addr(addr);
>>>>>> +    st = &t->timers[timer];
>>>>>> +    D(fprintf(stderr, "%s addr=%x val=%x (timer=%d)\n",
>>>>>> +             __func__, addr * 4, value, timer));
>>>>>> +
>>>>>> +    if (timer) {
>>>>>> +        addr -= 6;
>>>>>> +    }
>>>>>
>>>>> Same comment from the read function.
>>>>>
>>>>>> +
>>>>>> +    switch (addr) {
>>>>>> +    case R_CTRL:
>>>>>> +        st->regs[R_CTRL] = value;
>>>>>> +        timer_update(st);
>>>>>> +        break;
>>>>>> +
>>>>>> +    case R_RIS:
>>>>>> +        if (value & TIMER_RIS_ACK) {
>>>>>> +            st->regs[R_RIS] &= ~TIMER_RIS_ACK;
>>>>>> +        }
>>>>>> +        break;
>>>>>> +
>>>>>> +    case R_LOADVAL:
>>>>>> +        st->regs[R_LOADVAL] = value;
>>>>>> +        if (st->regs[R_CTRL] & TIMER_CTRL_ENBL) {
>>>>>> +            timer_update(st);
>>>>>> +        }
>>>>>> +        break;
>>>>>> +
>>>>>> +    case R_BGLOADVAL:
>>>>>> +        st->regs[R_BGLOADVAL] = value;
>>>>>> +        st->regs[R_LOADVAL] = value;
>>>>>> +        break;
>>>>>> +
>>>>>> +    case R_VAL:
>>>>>> +    case R_MIS:
>>>>>> +        break;
>>>>>> +
>>>>>> +    default:
>>>>>> +        if (addr < ARRAY_SIZE(st->regs)) {
>>>>>> +            st->regs[addr] = value;
>>>>>> +        }
>>>>>> +        break;
>>>>>> +    }
>>>>>> +    timer_update_irq(st);
>>>>>> +}
>>>>>> +
>>>>>> +static const MemoryRegionOps timer_ops = {
>>>>>> +    .read = timer_read,
>>>>>> +    .write = timer_write,
>>>>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>>>>> +    .valid = {
>>>>>> +        .min_access_size = 4,
>>>>>> +        .max_access_size = 4
>>>>>> +    }
>>>>>> +};
>>>>>> +
>>>>>> +static void timer_hit(void *opaque)
>>>>>> +{
>>>>>> +    struct msf2_timer *st = opaque;
>>>>>> +    D(fprintf(stderr, "%s %d\n", __func__, st->nr));
>>>>>> +    st->regs[R_RIS] |= TIMER_RIS_ACK;
>>>>>> +
>>>>>> +    if (!(st->regs[R_CTRL] & TIMER_CTRL_ONESHOT)) {
>>>>>> +        timer_update(st);
>>>>>> +    }
>>>>>> +    timer_update_irq(st);
>>>>>> +}
>>>>>> +
>>>>>> +static void msf2_timer_realize(DeviceState *dev, Error **errp)
>>>>>> +{
>>>>>> +    struct timerblock *t = MSF2_TIMER(dev);
>>>>>> +    unsigned int i;
>>>>>> +
>>>>>> +    /* Init all the ptimers.  */
>>>>>> +    t->timers = g_malloc0((sizeof t->timers[0]) * NUM_TIMERS);
>>>>>> +    for (i = 0; i < NUM_TIMERS; i++) {
>>>>>> +        struct msf2_timer *st = &t->timers[i];
>>>>>> +
>>>>>> +        st->parent = t;
>>>>>> +        st->nr = i;
>>>>>> +        st->bh = qemu_bh_new(timer_hit, st);
>>>>>> +        st->ptimer = ptimer_init(st->bh, PTIMER_POLICY_DEFAULT);
>>>>>> +        ptimer_set_freq(st->ptimer, t->freq_hz);
>>>>>> +        sysbus_init_irq(SYS_BUS_DEVICE(dev), &st->irq);
>>>>>> +    }
>>>>>> +
>>>>>> +    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, "msf2-timer",
>>>>>> +                          R_MAX * 4 * NUM_TIMERS);
>>>>>> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &t->mmio);
>>>>>
>>>>> This should be in the devices init() function.
>>>>
>>>> I referred Xilinx soft IP models for writing these models and used
>>>> same boilerplate code.
>>>> I am not clear about realize and init functions yet. Can you please
>>>> give a brief about them.
>>>
>>> Basically the simple explanation is that init is called when the
>>> object is created and realize is called when the object is realized.
>>>
>>> Generally for devices it will go something like this:
>>>  1. init
>>>  2. Set properties
>>>  3. Connect things
>>>  4. realize
>>>  5. Map to memory
>>>
>>>> Don't we need to use realize function for new models?
>>>
>>> AFAIK we still put things like: sysbus_init_irq(),
>>> memory_region_init_io() and sysbus_init_mmio() in the init function.
>>>
>>> I don't think we are at a stage yet to not use init functions.
>>>
>>> Thanks,
>>>
>>> Alistair
>>>
>>>>
>>>> Thanks,
>>>> Sundeep
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Alistair
>>>>>
>>>>>> +}
>>>>>> +
>>>>>> +static Property msf2_timer_properties[] = {
>>>>>> +    DEFINE_PROP_UINT32("clock-frequency", struct timerblock, freq_hz,
>>>>>> +                                                                83 * 1000000),
>>>>>> +    DEFINE_PROP_END_OF_LIST(),
>>>>>> +};
>>>>>> +
>>>>>> +static void msf2_timer_class_init(ObjectClass *klass, void *data)
>>>>>> +{
>>>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>>>> +
>>>>>> +    dc->realize = msf2_timer_realize;
>>>>>> +    dc->props = msf2_timer_properties;
>>>>>> +}
>>>>>> +
>>>>>> +static const TypeInfo msf2_timer_info = {
>>>>>> +    .name          = TYPE_MSF2_TIMER,
>>>>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>>>> +    .instance_size = sizeof(struct timerblock),
>>>>>> +    .class_init    = msf2_timer_class_init,
>>>>>> +};
>>>>>> +
>>>>>> +static void msf2_timer_register_types(void)
>>>>>> +{
>>>>>> +    type_register_static(&msf2_timer_info);
>>>>>> +}
>>>>>> +
>>>>>> +type_init(msf2_timer_register_types)
>>>>>> --
>>>>>> 2.5.0
>>>>>>
>>>>>>

[Qemu-devel] [Qemu-devel RFC v2 2/4] msf2: Microsemi Smartfusion2 System Register block.

[Subbaraya Sundeep]

Added Sytem register block of Smartfusion2.
This block has PLL registers which are accessed by guest.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 hw/misc/Makefile.objs |   1 +
 hw/misc/msf2_sysreg.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 169 insertions(+)
 create mode 100644 hw/misc/msf2_sysreg.c

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index c8b4893..aee53df 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -56,3 +56,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-$(CONFIG_MSF2) += msf2_sysreg.o
diff --git a/hw/misc/msf2_sysreg.c b/hw/misc/msf2_sysreg.c
new file mode 100644
index 0000000..4873463
--- /dev/null
+++ b/hw/misc/msf2_sysreg.c
@@ -0,0 +1,168 @@
+/*
+ * System Register block model of Microsemi SmartFusion2.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/hw.h"
+#include "qemu/timer.h"
+#include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+
+#ifndef MSF2_SYSREG_ERR_DEBUG
+#define MSF2_SYSREG_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT(...) do { \
+        if (MSF2_SYSREG_ERR_DEBUG) { \
+            fprintf(stderr,  ": %s: ", __func__); \
+            fprintf(stderr, ## __VA_ARGS__); \
+        } \
+    } while (0);
+
+#define R_PSS_RST_CTRL_SOFT_RST 0x1
+
+enum {
+    ESRAM_CR        = 0x00 / 4,
+    ESRAM_MAX_LAT,
+    DDR_CR,
+    ENVM_CR,
+    ENVM_REMAP_BASE_CR,
+    ENVM_REMAP_FAB_CR,
+    CC_CR,
+    CC_REGION_CR,
+    CC_LOCK_BASE_ADDR_CR,
+    CC_FLUSH_INDX_CR,
+    DDRB_BUF_TIMER_CR,
+    DDRB_NB_ADDR_CR,
+    DDRB_NB_SIZE_CR,
+    DDRB_CR,
+
+    SOFT_RESET_CR  = 0x48 / 4,
+    M3_CR,
+
+    GPIO_SYSRESET_SEL_CR = 0x58 / 4,
+
+    MDDR_CR = 0x60 / 4,
+
+    MSSDDR_PLL_STATUS_LOW_CR = 0x90 / 4,
+    MSSDDR_PLL_STATUS_HIGH_CR,
+    MSSDDR_FACC1_CR,
+    MSSDDR_FACC2_CR,
+
+    MSSDDR_PLL_STATUS = 0x150 / 4,
+
+};
+
+#define MSF2_SYSREG_MMIO_SIZE     0x300
+#define MSF2_SYSREG_NUM_REGS      (MSF2_SYSREG_MMIO_SIZE / 4)
+
+#define TYPE_MSF2_SYSREG          "msf2-sysreg"
+#define MSF2_SYSREG(obj)  OBJECT_CHECK(Sf2SysregState, (obj), TYPE_MSF2_SYSREG)
+
+typedef struct Sf2SysregState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+
+    uint32_t regs[MSF2_SYSREG_NUM_REGS];
+} Sf2SysregState;
+
+static void msf2_sysreg_reset(DeviceState *d)
+{
+    Sf2SysregState *s = MSF2_SYSREG(d);
+
+    DB_PRINT("RESET\n");
+
+    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x02420041;
+    s->regs[MSSDDR_FACC1_CR] = 0x0A482124;
+    s->regs[MSSDDR_PLL_STATUS] = 0x3;
+}
+
+static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
+    unsigned size)
+{
+    Sf2SysregState *s = opaque;
+    offset /= 4;
+    uint32_t ret = s->regs[offset];
+
+    DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx32 "\n", offset * 4, ret);
+
+   return ret;
+}
+
+static void msf2_sysreg_write(void *opaque, hwaddr offset,
+                          uint64_t val, unsigned size)
+{
+    Sf2SysregState *s = (Sf2SysregState *)opaque;
+    offset /= 4;
+
+    DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx64 "\n", offset * 4, val);
+
+    switch (offset) {
+    case MSSDDR_PLL_STATUS:
+        break;
+
+    default:
+        s->regs[offset] = val;
+        break;
+    }
+}
+
+static const MemoryRegionOps sysreg_ops = {
+    .read = msf2_sysreg_read,
+    .write = msf2_sysreg_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void msf2_sysreg_init(Object *obj)
+{
+    Sf2SysregState *s = MSF2_SYSREG(obj);
+
+    memory_region_init_io(&s->iomem, obj, &sysreg_ops, s, "sysreg",
+                          MSF2_SYSREG_MMIO_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static const VMStateDescription vmstate_msf2_sysreg = {
+    .name = "msf2_sysreg",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, Sf2SysregState, MSF2_SYSREG_NUM_REGS),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void msf2_sysreg_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_msf2_sysreg;
+    dc->reset = msf2_sysreg_reset;
+}
+
+static const TypeInfo msf2_sysreg_info = {
+    .class_init = msf2_sysreg_class_init,
+    .name  = TYPE_MSF2_SYSREG,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(Sf2SysregState),
+    .instance_init = msf2_sysreg_init,
+};
+
+static void msf2_sysreg_register_types(void)
+{
+    type_register_static(&msf2_sysreg_info);
+}
+
+type_init(msf2_sysreg_register_types)
-- 
2.5.0

Re: [Qemu-devel] [Qemu-devel RFC v2 2/4] msf2: Microsemi Smartfusion2 System Register block.

[Alistair Francis]

On Sun, Apr 9, 2017 at 4:19 AM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:
> Added Sytem register block of Smartfusion2.
> This block has PLL registers which are accessed by guest.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
> ---
>  hw/misc/Makefile.objs |   1 +
>  hw/misc/msf2_sysreg.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 169 insertions(+)
>  create mode 100644 hw/misc/msf2_sysreg.c
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index c8b4893..aee53df 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -56,3 +56,4 @@ obj-$(CONFIG_EDU) += edu.o
>  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
>  obj-$(CONFIG_AUX) += auxbus.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
> +obj-$(CONFIG_MSF2) += msf2_sysreg.o
> diff --git a/hw/misc/msf2_sysreg.c b/hw/misc/msf2_sysreg.c
> new file mode 100644
> index 0000000..4873463
> --- /dev/null
> +++ b/hw/misc/msf2_sysreg.c
> @@ -0,0 +1,168 @@
> +/*
> + * System Register block model of Microsemi SmartFusion2.
> + *
> + * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/hw.h"
> +#include "qemu/timer.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/log.h"
> +
> +#ifndef MSF2_SYSREG_ERR_DEBUG
> +#define MSF2_SYSREG_ERR_DEBUG 0
> +#endif
> +
> +#define DB_PRINT(...) do { \
> +        if (MSF2_SYSREG_ERR_DEBUG) { \
> +            fprintf(stderr,  ": %s: ", __func__); \
> +            fprintf(stderr, ## __VA_ARGS__); \

You should use qemu_log() here.

> +        } \
> +    } while (0);
> +
> +#define R_PSS_RST_CTRL_SOFT_RST 0x1
> +
> +enum {
> +    ESRAM_CR        = 0x00 / 4,
> +    ESRAM_MAX_LAT,
> +    DDR_CR,
> +    ENVM_CR,
> +    ENVM_REMAP_BASE_CR,
> +    ENVM_REMAP_FAB_CR,
> +    CC_CR,
> +    CC_REGION_CR,
> +    CC_LOCK_BASE_ADDR_CR,
> +    CC_FLUSH_INDX_CR,
> +    DDRB_BUF_TIMER_CR,
> +    DDRB_NB_ADDR_CR,
> +    DDRB_NB_SIZE_CR,
> +    DDRB_CR,
> +
> +    SOFT_RESET_CR  = 0x48 / 4,
> +    M3_CR,
> +
> +    GPIO_SYSRESET_SEL_CR = 0x58 / 4,
> +
> +    MDDR_CR = 0x60 / 4,
> +
> +    MSSDDR_PLL_STATUS_LOW_CR = 0x90 / 4,
> +    MSSDDR_PLL_STATUS_HIGH_CR,
> +    MSSDDR_FACC1_CR,
> +    MSSDDR_FACC2_CR,
> +
> +    MSSDDR_PLL_STATUS = 0x150 / 4,
> +
> +};
> +
> +#define MSF2_SYSREG_MMIO_SIZE     0x300
> +#define MSF2_SYSREG_NUM_REGS      (MSF2_SYSREG_MMIO_SIZE / 4)
> +
> +#define TYPE_MSF2_SYSREG          "msf2-sysreg"
> +#define MSF2_SYSREG(obj)  OBJECT_CHECK(Sf2SysregState, (obj), TYPE_MSF2_SYSREG)
> +
> +typedef struct Sf2SysregState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +
> +    uint32_t regs[MSF2_SYSREG_NUM_REGS];
> +} Sf2SysregState;
> +
> +static void msf2_sysreg_reset(DeviceState *d)
> +{
> +    Sf2SysregState *s = MSF2_SYSREG(d);
> +
> +    DB_PRINT("RESET\n");
> +
> +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x02420041;
> +    s->regs[MSSDDR_FACC1_CR] = 0x0A482124;
> +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
> +}
> +
> +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
> +    unsigned size)
> +{
> +    Sf2SysregState *s = opaque;
> +    offset /= 4;
> +    uint32_t ret = s->regs[offset];

Probably should check that this offset value is inside the array.

> +
> +    DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx32 "\n", offset * 4, ret);
> +
> +   return ret;
> +}
> +
> +static void msf2_sysreg_write(void *opaque, hwaddr offset,
> +                          uint64_t val, unsigned size)
> +{
> +    Sf2SysregState *s = (Sf2SysregState *)opaque;
> +    offset /= 4;
> +
> +    DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx64 "\n", offset * 4, val);
> +
> +    switch (offset) {
> +    case MSSDDR_PLL_STATUS:
> +        break;
> +
> +    default:
> +        s->regs[offset] = val;

Check the bounds here as well.

> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps sysreg_ops = {
> +    .read = msf2_sysreg_read,
> +    .write = msf2_sysreg_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void msf2_sysreg_init(Object *obj)
> +{
> +    Sf2SysregState *s = MSF2_SYSREG(obj);
> +
> +    memory_region_init_io(&s->iomem, obj, &sysreg_ops, s, "sysreg",
> +                          MSF2_SYSREG_MMIO_SIZE);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
> +
> +static const VMStateDescription vmstate_msf2_sysreg = {
> +    .name = "msf2_sysreg",
> +    .version_id = 2,
> +    .minimum_version_id = 2,

This version_id should start at 1, you only increase it when you make
changes to the code that affects migration once it's in the tree.

Thanks,

Alistair

> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, Sf2SysregState, MSF2_SYSREG_NUM_REGS),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void msf2_sysreg_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->vmsd = &vmstate_msf2_sysreg;
> +    dc->reset = msf2_sysreg_reset;
> +}
> +
> +static const TypeInfo msf2_sysreg_info = {
> +    .class_init = msf2_sysreg_class_init,
> +    .name  = TYPE_MSF2_SYSREG,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size  = sizeof(Sf2SysregState),
> +    .instance_init = msf2_sysreg_init,
> +};
> +
> +static void msf2_sysreg_register_types(void)
> +{
> +    type_register_static(&msf2_sysreg_info);
> +}
> +
> +type_init(msf2_sysreg_register_types)
> --
> 2.5.0
>
>

Re: [Qemu-devel] [Qemu-devel RFC v2 2/4] msf2: Microsemi Smartfusion2 System Register block.

[sundeep subbaraya]

Hi Alistair,

On Sat, Apr 15, 2017 at 3:02 AM, Alistair Francis <alistair23@gmail.com> wrote:
> On Sun, Apr 9, 2017 at 4:19 AM, Subbaraya Sundeep
> <sundeep.lkml@gmail.com> wrote:
>> Added Sytem register block of Smartfusion2.
>> This block has PLL registers which are accessed by guest.
>>
>> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
>> ---
>>  hw/misc/Makefile.objs |   1 +
>>  hw/misc/msf2_sysreg.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 169 insertions(+)
>>  create mode 100644 hw/misc/msf2_sysreg.c
>>
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index c8b4893..aee53df 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -56,3 +56,4 @@ obj-$(CONFIG_EDU) += edu.o
>>  obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
>>  obj-$(CONFIG_AUX) += auxbus.o
>>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>> +obj-$(CONFIG_MSF2) += msf2_sysreg.o
>> diff --git a/hw/misc/msf2_sysreg.c b/hw/misc/msf2_sysreg.c
>> new file mode 100644
>> index 0000000..4873463
>> --- /dev/null
>> +++ b/hw/misc/msf2_sysreg.c
>> @@ -0,0 +1,168 @@
>> +/*
>> + * System Register block model of Microsemi SmartFusion2.
>> + *
>> + * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version
>> + * 2 of the License, or (at your option) any later version.
>> + *
>> + * You should have received a copy of the GNU General Public License along
>> + * with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/hw.h"
>> +#include "qemu/timer.h"
>> +#include "hw/sysbus.h"
>> +#include "sysemu/sysemu.h"
>> +#include "qemu/log.h"
>> +
>> +#ifndef MSF2_SYSREG_ERR_DEBUG
>> +#define MSF2_SYSREG_ERR_DEBUG 0
>> +#endif
>> +
>> +#define DB_PRINT(...) do { \
>> +        if (MSF2_SYSREG_ERR_DEBUG) { \
>> +            fprintf(stderr,  ": %s: ", __func__); \
>> +            fprintf(stderr, ## __VA_ARGS__); \
>
> You should use qemu_log() here.
>
Ok I will change.
>> +        } \
>> +    } while (0);
>> +
>> +#define R_PSS_RST_CTRL_SOFT_RST 0x1
>> +
>> +enum {
>> +    ESRAM_CR        = 0x00 / 4,
>> +    ESRAM_MAX_LAT,
>> +    DDR_CR,
>> +    ENVM_CR,
>> +    ENVM_REMAP_BASE_CR,
>> +    ENVM_REMAP_FAB_CR,
>> +    CC_CR,
>> +    CC_REGION_CR,
>> +    CC_LOCK_BASE_ADDR_CR,
>> +    CC_FLUSH_INDX_CR,
>> +    DDRB_BUF_TIMER_CR,
>> +    DDRB_NB_ADDR_CR,
>> +    DDRB_NB_SIZE_CR,
>> +    DDRB_CR,
>> +
>> +    SOFT_RESET_CR  = 0x48 / 4,
>> +    M3_CR,
>> +
>> +    GPIO_SYSRESET_SEL_CR = 0x58 / 4,
>> +
>> +    MDDR_CR = 0x60 / 4,
>> +
>> +    MSSDDR_PLL_STATUS_LOW_CR = 0x90 / 4,
>> +    MSSDDR_PLL_STATUS_HIGH_CR,
>> +    MSSDDR_FACC1_CR,
>> +    MSSDDR_FACC2_CR,
>> +
>> +    MSSDDR_PLL_STATUS = 0x150 / 4,
>> +
>> +};
>> +
>> +#define MSF2_SYSREG_MMIO_SIZE     0x300
>> +#define MSF2_SYSREG_NUM_REGS      (MSF2_SYSREG_MMIO_SIZE / 4)
>> +
>> +#define TYPE_MSF2_SYSREG          "msf2-sysreg"
>> +#define MSF2_SYSREG(obj)  OBJECT_CHECK(Sf2SysregState, (obj), TYPE_MSF2_SYSREG)
>> +
>> +typedef struct Sf2SysregState {
>> +    SysBusDevice parent_obj;
>> +
>> +    MemoryRegion iomem;
>> +
>> +    uint32_t regs[MSF2_SYSREG_NUM_REGS];
>> +} Sf2SysregState;
>> +
>> +static void msf2_sysreg_reset(DeviceState *d)
>> +{
>> +    Sf2SysregState *s = MSF2_SYSREG(d);
>> +
>> +    DB_PRINT("RESET\n");
>> +
>> +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x02420041;
>> +    s->regs[MSSDDR_FACC1_CR] = 0x0A482124;
>> +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
>> +}
>> +
>> +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
>> +    unsigned size)
>> +{
>> +    Sf2SysregState *s = opaque;
>> +    offset /= 4;
>> +    uint32_t ret = s->regs[offset];
>
> Probably should check that this offset value is inside the array.
>
Ok I will change.

>> +
>> +    DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx32 "\n", offset * 4, ret);
>> +
>> +   return ret;
>> +}
>> +
>> +static void msf2_sysreg_write(void *opaque, hwaddr offset,
>> +                          uint64_t val, unsigned size)
>> +{
>> +    Sf2SysregState *s = (Sf2SysregState *)opaque;
>> +    offset /= 4;
>> +
>> +    DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx64 "\n", offset * 4, val);
>> +
>> +    switch (offset) {
>> +    case MSSDDR_PLL_STATUS:
>> +        break;
>> +
>> +    default:
>> +        s->regs[offset] = val;
>
> Check the bounds here as well.
>
Ok I will change.

>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps sysreg_ops = {
>> +    .read = msf2_sysreg_read,
>> +    .write = msf2_sysreg_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void msf2_sysreg_init(Object *obj)
>> +{
>> +    Sf2SysregState *s = MSF2_SYSREG(obj);
>> +
>> +    memory_region_init_io(&s->iomem, obj, &sysreg_ops, s, "sysreg",
>> +                          MSF2_SYSREG_MMIO_SIZE);
>> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
>> +}
>> +
>> +static const VMStateDescription vmstate_msf2_sysreg = {
>> +    .name = "msf2_sysreg",
>> +    .version_id = 2,
>> +    .minimum_version_id = 2,
>
> This version_id should start at 1, you only increase it when you make
> changes to the code that affects migration once it's in the tree.
>
Ok I will change.

Thanks,
Sundeep

> Thanks,
>
> Alistair
>
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32_ARRAY(regs, Sf2SysregState, MSF2_SYSREG_NUM_REGS),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +static void msf2_sysreg_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->vmsd = &vmstate_msf2_sysreg;
>> +    dc->reset = msf2_sysreg_reset;
>> +}
>> +
>> +static const TypeInfo msf2_sysreg_info = {
>> +    .class_init = msf2_sysreg_class_init,
>> +    .name  = TYPE_MSF2_SYSREG,
>> +    .parent = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size  = sizeof(Sf2SysregState),
>> +    .instance_init = msf2_sysreg_init,
>> +};
>> +
>> +static void msf2_sysreg_register_types(void)
>> +{
>> +    type_register_static(&msf2_sysreg_info);
>> +}
>> +
>> +type_init(msf2_sysreg_register_types)
>> --
>> 2.5.0
>>
>>

[Qemu-devel] [Qemu-devel RFC v2 3/4] msf2: Add Smartfusion2 SPI controller

[Subbaraya Sundeep]

Modelled Microsemi's Smartfusion2 SPI controller.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 hw/ssi/Makefile.objs |   1 +
 hw/ssi/msf2_spi.c    | 449 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 450 insertions(+)
 create mode 100644 hw/ssi/msf2_spi.c

diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index 487add2..86445d7 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -4,6 +4,7 @@ common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
 common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
 common-obj-$(CONFIG_ASPEED_SOC) += aspeed_smc.o
 common-obj-$(CONFIG_STM32F2XX_SPI) += stm32f2xx_spi.o
+common-obj-$(CONFIG_MSF2) += msf2_spi.o
 
 obj-$(CONFIG_OMAP) += omap_spi.o
 obj-$(CONFIG_IMX) += imx_spi.o
diff --git a/hw/ssi/msf2_spi.c b/hw/ssi/msf2_spi.c
new file mode 100644
index 0000000..6054cd8
--- /dev/null
+++ b/hw/ssi/msf2_spi.c
@@ -0,0 +1,449 @@
+/*
+ * SPI controller model of Microsemi Smartfusion2.
+ *
+ * Copyright (C) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
+ *
+ * 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 "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+#include "qemu/fifo32.h"
+
+#include "hw/ssi/ssi.h"
+
+#ifdef MSF2_SPI_ERR_DEBUG
+#define DB_PRINT(...) do { \
+    fprintf(stderr,  ": %s: ", __func__); \
+    fprintf(stderr, ## __VA_ARGS__); \
+    } while (0);
+#else
+    #define DB_PRINT(...)
+#endif
+
+#define FIFO_CAPACITY     32
+#define FIFO_CAPACITY     32
+
+#define R_CONTROL         0
+#define R_DFSIZE          1
+#define R_STATUS          2
+#define R_INTCLR          3
+#define R_RX              4
+#define R_TX              5
+#define R_CLKGEN          6
+#define R_SS              7
+#define R_MIS             8
+#define R_RIS             9
+#define R_CONTROL2        10
+#define R_COMMAND         11
+#define R_PKTSIZE         12
+#define R_CMDSIZE         13
+#define R_HWSTATUS        14
+#define R_STAT8           15
+#define R_MAX             16
+
+#define S_RXFIFOFUL       (1 << 4)
+#define S_RXFIFOFULNXT    (1 << 5)
+#define S_RXFIFOEMP       (1 << 6)
+#define S_RXFIFOEMPNXT    (1 << 7)
+#define S_TXFIFOFUL       (1 << 8)
+#define S_TXFIFOFULNXT    (1 << 9)
+#define S_TXFIFOEMP       (1 << 10)
+#define S_TXFIFOEMPNXT    (1 << 11)
+#define S_FRAMESTART      (1 << 12)
+#define S_SSEL            (1 << 13)
+#define S_ACTIVE          (1 << 14)
+
+#define C_ENABLE          (1 << 0)
+#define C_MODE            (1 << 1)
+#define C_INTRXDATA       (1 << 4)
+#define C_INTTXDATA       (1 << 5)
+#define C_INTRXOVRFLO     (1 << 6)
+#define C_SPS             (1 << 26)
+#define C_BIGFIFO         (1 << 29)
+#define C_RESET           (1 << 31)
+
+#define FRAMESZ_MASK      0x1F
+#define FMCOUNT_MASK      0x00FFFF00
+#define FMCOUNT_SHIFT     8
+
+#define TXDONE            (1 << 0)
+#define RXRDY             (1 << 1)
+#define RXCHOVRF          (1 << 2)
+
+#define TYPE_MSF2_SPI   "msf2-spi"
+#define MSF2_SPI(obj)   OBJECT_CHECK(Msf2SPI, (obj), TYPE_MSF2_SPI)
+
+typedef struct Msf2SPI {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+
+    qemu_irq irq;
+
+    qemu_irq cs_line;
+
+    SSIBus *spi;
+
+    Fifo32 rx_fifo;
+    Fifo32 tx_fifo;
+
+    int fifo_depth;
+    uint32_t frame_count;
+    bool enabled;
+
+    uint32_t regs[R_MAX];
+} Msf2SPI;
+
+static void txfifo_reset(Msf2SPI *s)
+{
+    fifo32_reset(&s->tx_fifo);
+
+    s->regs[R_STATUS] &= ~S_TXFIFOFUL;
+    s->regs[R_STATUS] |= S_TXFIFOEMP;
+}
+
+static void rxfifo_reset(Msf2SPI *s)
+{
+    fifo32_reset(&s->rx_fifo);
+
+    s->regs[R_STATUS] &= ~S_RXFIFOFUL;
+    s->regs[R_STATUS] |= S_RXFIFOEMP;
+}
+
+static void set_fifodepth(Msf2SPI *s)
+{
+    int size = s->regs[R_DFSIZE] & FRAMESZ_MASK;
+
+    if (0 <= size && size <= 8) {
+        s->fifo_depth = 32;
+    }
+    if (9 <= size && size <= 16) {
+        s->fifo_depth = 16;
+    }
+    if (17 <= size && size <= 32) {
+        s->fifo_depth = 8;
+    }
+}
+
+static void msf2_spi_do_reset(Msf2SPI *s)
+{
+    memset(s->regs, 0, sizeof s->regs);
+    s->regs[R_CONTROL] = 0x80000102;
+    s->regs[R_DFSIZE] = 0x4;
+    s->regs[R_STATUS] = 0x2440;
+    s->regs[R_CLKGEN] = 0x7;
+    s->regs[R_STAT8] = 0x7;
+    s->regs[R_RIS] = 0x0;
+
+    s->fifo_depth = 4;
+    s->frame_count = 1;
+    s->enabled = false;
+
+    rxfifo_reset(s);
+    txfifo_reset(s);
+}
+
+static void update_mis(Msf2SPI *s)
+{
+    uint32_t reg = s->regs[R_CONTROL];
+    uint32_t tmp;
+
+    /*
+     * form the Control register interrupt enable bits
+     * same as RIS, MIS and Interrupt clear registers for simplicity
+     */
+    tmp = ((reg & C_INTRXOVRFLO) >> 4) | ((reg & C_INTRXDATA) >> 3) |
+           ((reg & C_INTTXDATA) >> 5);
+    s->regs[R_MIS] |= tmp & s->regs[R_RIS];
+}
+
+static void spi_update_irq(Msf2SPI *s)
+{
+    int irq;
+
+    update_mis(s);
+    irq = !!(s->regs[R_MIS]);
+
+    qemu_set_irq(s->irq, irq);
+}
+
+static void msf2_spi_reset(DeviceState *d)
+{
+    msf2_spi_do_reset(MSF2_SPI(d));
+}
+
+static uint64_t
+spi_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    Msf2SPI *s = opaque;
+    uint32_t r = 0;
+
+    addr >>= 2;
+    switch (addr) {
+    case R_RX:
+        s->regs[R_STATUS] &= ~S_RXFIFOFUL;
+        s->regs[R_STATUS] &= ~RXCHOVRF;
+        r = fifo32_pop(&s->rx_fifo);
+        if (fifo32_is_empty(&s->rx_fifo)) {
+            s->regs[R_STATUS] |= S_RXFIFOEMP;
+        }
+        break;
+
+    case R_MIS:
+        update_mis(s);
+        r = s->regs[R_MIS];
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(s->regs)) {
+            r = s->regs[addr];
+        }
+        break;
+    }
+
+    DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr * 4, r);
+    spi_update_irq(s);
+    return r;
+}
+
+static void assert_cs(Msf2SPI *s)
+{
+    qemu_set_irq(s->cs_line, 0);
+}
+
+static void deassert_cs(Msf2SPI *s)
+{
+    qemu_set_irq(s->cs_line, 1);
+}
+
+static void spi_flush_txfifo(Msf2SPI *s)
+{
+    uint32_t tx;
+    uint32_t rx;
+    bool sps = !!(s->regs[R_CONTROL] & C_SPS);
+
+    /*
+     * Chip Select(CS) is automatically controlled by this controller.
+     * If SPS bit is set in Control register then CS is asserted
+     * until all the frames set in frame count of Control register are
+     * transferred. If SPS is not set then CS pulses between frames.
+     * Note that Slave Select register specifies which of the CS line
+     * has to be controlled automatically by controller. Bits SS[7:1] are for
+     * masters in FPGA fabric since we model only Microcontroller subsystem
+     * of Smartfusion2 we control only one CS(SS[0]) line.
+     */
+    while (!fifo32_is_empty(&s->tx_fifo) && s->frame_count) {
+        assert_cs(s);
+
+        s->regs[R_STATUS] &= ~TXDONE;
+        s->regs[R_STATUS] &= ~RXRDY;
+
+        tx = fifo32_pop(&s->tx_fifo);
+        DB_PRINT("data tx:%x\n", tx);
+        rx = ssi_transfer(s->spi, tx);
+        DB_PRINT("data rx:%x\n", rx);
+
+        if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
+            s->regs[R_STATUS] |= RXCHOVRF;
+            s->regs[R_RIS] |= RXCHOVRF;
+        } else {
+            fifo32_push(&s->rx_fifo, rx);
+            s->regs[R_STATUS] &= ~S_RXFIFOEMP;
+            if (fifo32_num_used(&s->rx_fifo) == (s->fifo_depth - 1)) {
+                s->regs[R_STATUS] |= S_RXFIFOFULNXT;
+            }
+            if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
+                s->regs[R_STATUS] |= S_RXFIFOFUL;
+            }
+        }
+        s->frame_count--;
+        if (!sps) {
+            deassert_cs(s);
+            assert_cs(s);
+        }
+    }
+
+    if (!sps) {
+        deassert_cs(s);
+    }
+
+    if (!s->frame_count) {
+        s->frame_count = (s->regs[R_CONTROL] & FMCOUNT_MASK) >> FMCOUNT_SHIFT;
+        if (sps) {
+            deassert_cs(s);
+        }
+        s->regs[R_RIS] |= TXDONE;
+        s->regs[R_RIS] |= RXRDY;
+        s->regs[R_STATUS] |= TXDONE;
+        s->regs[R_STATUS] |= RXRDY;
+   }
+}
+
+static void spi_write(void *opaque, hwaddr addr,
+            uint64_t val64, unsigned int size)
+{
+    Msf2SPI *s = opaque;
+    uint32_t value = val64;
+
+    DB_PRINT("addr=" TARGET_FMT_plx " = %x\n", addr, value);
+    addr >>= 2;
+
+    switch (addr) {
+    case R_TX:
+        /* adding to already full FIFO */
+        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
+            break;
+        }
+        s->regs[R_STATUS] &= ~S_TXFIFOEMP;
+        fifo32_push(&s->tx_fifo, value);
+        if (fifo32_num_used(&s->tx_fifo) == (s->fifo_depth - 1)) {
+            s->regs[R_STATUS] |= S_TXFIFOFULNXT;
+        }
+        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
+            s->regs[R_STATUS] |= S_TXFIFOFUL;
+        }
+        if (s->enabled) {
+            spi_flush_txfifo(s);
+        }
+        break;
+
+    case R_CONTROL:
+        s->regs[R_CONTROL] = value;
+        if (value & C_BIGFIFO) {
+            set_fifodepth(s);
+        } else {
+            s->fifo_depth = 4;
+        }
+        if (value & C_ENABLE) {
+            s->enabled = true;
+        } else {
+            s->enabled = false;
+        }
+        s->frame_count = (value & FMCOUNT_MASK) >> FMCOUNT_SHIFT;
+        if (value & C_RESET) {
+            msf2_spi_do_reset(s);
+        }
+        break;
+
+    case R_DFSIZE:
+        if (s->enabled) {
+            break;
+        }
+        s->regs[R_DFSIZE] = value;
+        break;
+
+    case R_INTCLR:
+        s->regs[R_INTCLR] = value;
+        if (value & TXDONE) {
+            s->regs[R_RIS] &= ~TXDONE;
+        }
+        if (value & RXRDY) {
+            s->regs[R_RIS] &= ~RXRDY;
+        }
+        if (value & RXCHOVRF) {
+            s->regs[R_RIS] &= ~RXCHOVRF;
+        }
+        break;
+
+    case R_MIS:
+    case R_STATUS:
+    case R_RIS:
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(s->regs)) {
+            s->regs[addr] = value;
+        }
+        break;
+    }
+
+    spi_update_irq(s);
+}
+
+static const MemoryRegionOps spi_ops = {
+    .read = spi_read,
+    .write = spi_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4
+    }
+};
+
+static int msf2_spi_init(SysBusDevice *sbd)
+{
+    DeviceState *dev = DEVICE(sbd);
+    Msf2SPI *s = MSF2_SPI(dev);
+
+    DB_PRINT("\n");
+
+    s->spi = ssi_create_bus(dev, "spi");
+
+    sysbus_init_irq(sbd, &s->irq);
+    ssi_auto_connect_slaves(dev, &s->cs_line, s->spi);
+    sysbus_init_irq(sbd, &s->cs_line);
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &spi_ops, s,
+                          "msf2-spi", R_MAX * 4);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    fifo32_create(&s->tx_fifo, FIFO_CAPACITY);
+    fifo32_create(&s->rx_fifo, FIFO_CAPACITY);
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_msf2_spi = {
+    .name = "msf2_spi",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_FIFO32(tx_fifo, Msf2SPI),
+        VMSTATE_FIFO32(rx_fifo, Msf2SPI),
+        VMSTATE_UINT32_ARRAY(regs, Msf2SPI, R_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void msf2_spi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+
+    k->init = msf2_spi_init;
+    dc->reset = msf2_spi_reset;
+    dc->vmsd = &vmstate_msf2_spi;
+}
+
+static const TypeInfo msf2_spi_info = {
+    .name           = TYPE_MSF2_SPI,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(Msf2SPI),
+    .class_init     = msf2_spi_class_init,
+};
+
+static void msf2_spi_register_types(void)
+{
+    type_register_static(&msf2_spi_info);
+}
+
+type_init(msf2_spi_register_types)
-- 
2.5.0

[Qemu-devel] [Qemu-devel RFC v2 4/4] msf2: Add Emcraft's Smartfusion2 SOM kit.

[Subbaraya Sundeep]

Emulated Emcraft's Smartfusion2 System On Module starter
kit.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs            |   2 +-
 hw/arm/msf2_soc.c               | 141 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/msf2_soc.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 1e3bd2b..379f7e1 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -121,3 +121,4 @@ CONFIG_ACPI=y
 CONFIG_SMBIOS=y
 CONFIG_ASPEED_SOC=y
 CONFIG_GPIO_KEY=y
+CONFIG_MSF2=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 4c5c4ee..cce2759 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,7 +1,7 @@
 obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-y += integratorcp.o mainstone.o musicpal.o nseries.o
-obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
+obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o msf2_soc.o
 obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
 obj-$(CONFIG_ACPI) += virt-acpi-build.o
 obj-y += netduino2.o
diff --git a/hw/arm/msf2_soc.c b/hw/arm/msf2_soc.c
new file mode 100644
index 0000000..7277b51
--- /dev/null
+++ b/hw/arm/msf2_soc.c
@@ -0,0 +1,141 @@
+/*
+ * Smartfusion2 SOM starter kit(from Emcraft) emulation.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
+ *
+ * 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 "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/arm/arm.h"
+#include "exec/address-spaces.h"
+#include "hw/sysbus.h"
+#include "hw/char/serial.h"
+#include "hw/boards.h"
+#include "sysemu/block-backend.h"
+#include "hw/ssi/ssi.h"
+
+#define MSF2_NUM_USARTS       1
+#define MSF2_NUM_TIMERS       2
+
+#define ENVM_BASE_ADDRESS     0x60000000
+#define ENVM_SIZE             (128 * 1024)
+
+#define DDR_BASE_ADDRESS      0xA0000000
+#define DDR_SIZE              (64 * 1024 * 1024)
+
+#define SRAM_BASE_ADDRESS     0x20000000
+#define SRAM_SIZE             (64 * 1024)
+
+#define MSF2_TIMER_BASE       0x40004000
+#define MSF2_UART0_BASE       0x40000000
+#define MSF2_SYSREG_BASE      0x40038000
+#define MSF2_SPI0_BASE        0x40001000
+
+#define MSF2_SPI0_IRQ         2
+#define MSF2_UART0_IRQ        10
+#define MSF2_TIMER_IRQ0       14
+#define MSF2_TIMER_IRQ1       15
+
+static void msf2_init(MachineState *machine)
+{
+    const char *kernel_filename = NULL;
+    DeviceState *dev, *nvic;
+    MemoryRegion *system_memory = get_system_memory();
+    MemoryRegion *nvm = g_new(MemoryRegion, 1);
+    MemoryRegion *nvm_alias = g_new(MemoryRegion, 1);
+    MemoryRegion *sram = g_new(MemoryRegion, 1);
+    MemoryRegion *ddr = g_new(MemoryRegion, 1);
+    QemuOpts *machine_opts = qemu_get_machine_opts();
+    SysBusDevice *busdev;
+    DriveInfo *dinfo = drive_get_next(IF_MTD);
+    qemu_irq cs_line;
+    SSIBus *spi;
+
+    kernel_filename = qemu_opt_get(machine_opts, "kernel");
+
+    memory_region_init_ram(nvm, NULL, "MSF2.envm", ENVM_SIZE,
+                           &error_fatal);
+    memory_region_init_alias(nvm_alias, NULL, "MSF2.flash.alias",
+                             nvm, 0, ENVM_SIZE);
+    vmstate_register_ram_global(nvm);
+
+    memory_region_set_readonly(nvm, true);
+    memory_region_set_readonly(nvm_alias, true);
+
+    memory_region_add_subregion(system_memory, ENVM_BASE_ADDRESS, nvm);
+    memory_region_add_subregion(system_memory, 0, nvm_alias);
+
+    memory_region_init_ram(ddr, NULL, "MSF2.ddr", DDR_SIZE,
+                           &error_fatal);
+    vmstate_register_ram_global(ddr);
+    memory_region_add_subregion(system_memory, DDR_BASE_ADDRESS, ddr);
+
+    memory_region_init_ram(sram, NULL, "MSF2.sram", SRAM_SIZE,
+                           &error_fatal);
+    vmstate_register_ram_global(sram);
+    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
+
+    nvic = armv7m_init(system_memory, ENVM_SIZE, 96,
+                       kernel_filename, "cortex-m3");
+
+	if (serial_hds[0]) {
+                serial_mm_init(get_system_memory(), MSF2_UART0_BASE, 2,
+                       qdev_get_gpio_in(nvic, MSF2_UART0_IRQ),
+                       115200, serial_hds[0], DEVICE_NATIVE_ENDIAN);
+    }
+    dev = qdev_create(NULL, "msf2-timer");
+    qdev_prop_set_uint32(dev, "clock-frequency", 83 * 1000000);
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MSF2_TIMER_BASE);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
+                           qdev_get_gpio_in(nvic, MSF2_TIMER_IRQ0));
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 1,
+                           qdev_get_gpio_in(nvic, MSF2_TIMER_IRQ1));
+
+    dev = qdev_create(NULL, "msf2-sysreg");
+    qdev_init_nofail(dev);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MSF2_SYSREG_BASE);
+
+    dev = qdev_create(NULL, "msf2-spi");
+    qdev_init_nofail(dev);
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, MSF2_SPI0_BASE);
+    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, MSF2_SPI0_IRQ));
+
+    spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
+    dev = ssi_create_slave_no_init(spi, "s25sl12801");
+    qdev_prop_set_uint8(dev, "spansion-cr2nv", 1);
+    if (dinfo)
+		qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
+                                    &error_fatal);
+    qdev_init_nofail(dev);
+    cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
+    sysbus_connect_irq(busdev, 1, cs_line);
+}
+
+static void msf2_machine_init(MachineClass *mc)
+{
+    mc->desc = "SmartFusion2 SOM kit from Emcraft";
+    mc->init = msf2_init;
+}
+
+DEFINE_MACHINE("smartfusion2-som", msf2_machine_init)
-- 
2.5.0

Re: [Qemu-devel] [Qemu-devel RFC v2 4/4] msf2: Add Emcraft's Smartfusion2 SOM kit.

[Alistair Francis]

On Sun, Apr 9, 2017 at 4:19 AM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:
> Emulated Emcraft's Smartfusion2 System On Module starter
> kit.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>

Hey Sundeep,

Cool patch, I have some comments below.

> ---
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs            |   2 +-
>  hw/arm/msf2_soc.c               | 141 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/msf2_soc.c
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 1e3bd2b..379f7e1 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -121,3 +121,4 @@ CONFIG_ACPI=y
>  CONFIG_SMBIOS=y
>  CONFIG_ASPEED_SOC=y
>  CONFIG_GPIO_KEY=y
> +CONFIG_MSF2=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 4c5c4ee..cce2759 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,7 +1,7 @@
>  obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
>  obj-$(CONFIG_DIGIC) += digic_boards.o
>  obj-y += integratorcp.o mainstone.o musicpal.o nseries.o
> -obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
> +obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o msf2_soc.o
>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
>  obj-$(CONFIG_ACPI) += virt-acpi-build.o
>  obj-y += netduino2.o
> diff --git a/hw/arm/msf2_soc.c b/hw/arm/msf2_soc.c
> new file mode 100644
> index 0000000..7277b51
> --- /dev/null
> +++ b/hw/arm/msf2_soc.c
> @@ -0,0 +1,141 @@
> +/*
> + * Smartfusion2 SOM starter kit(from Emcraft) emulation.
> + *
> + * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
> + *
> + * 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 "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "hw/arm/arm.h"
> +#include "exec/address-spaces.h"
> +#include "hw/sysbus.h"
> +#include "hw/char/serial.h"
> +#include "hw/boards.h"
> +#include "sysemu/block-backend.h"
> +#include "hw/ssi/ssi.h"
> +
> +#define MSF2_NUM_USARTS       1
> +#define MSF2_NUM_TIMERS       2
> +
> +#define ENVM_BASE_ADDRESS     0x60000000
> +#define ENVM_SIZE             (128 * 1024)
> +
> +#define DDR_BASE_ADDRESS      0xA0000000
> +#define DDR_SIZE              (64 * 1024 * 1024)
> +
> +#define SRAM_BASE_ADDRESS     0x20000000
> +#define SRAM_SIZE             (64 * 1024)
> +
> +#define MSF2_TIMER_BASE       0x40004000
> +#define MSF2_UART0_BASE       0x40000000
> +#define MSF2_SYSREG_BASE      0x40038000
> +#define MSF2_SPI0_BASE        0x40001000
> +
> +#define MSF2_SPI0_IRQ         2
> +#define MSF2_UART0_IRQ        10
> +#define MSF2_TIMER_IRQ0       14
> +#define MSF2_TIMER_IRQ1       15
> +
> +static void msf2_init(MachineState *machine)
> +{
> +    const char *kernel_filename = NULL;
> +    DeviceState *dev, *nvic;
> +    MemoryRegion *system_memory = get_system_memory();
> +    MemoryRegion *nvm = g_new(MemoryRegion, 1);
> +    MemoryRegion *nvm_alias = g_new(MemoryRegion, 1);
> +    MemoryRegion *sram = g_new(MemoryRegion, 1);
> +    MemoryRegion *ddr = g_new(MemoryRegion, 1);
> +    QemuOpts *machine_opts = qemu_get_machine_opts();
> +    SysBusDevice *busdev;
> +    DriveInfo *dinfo = drive_get_next(IF_MTD);
> +    qemu_irq cs_line;
> +    SSIBus *spi;
> +
> +    kernel_filename = qemu_opt_get(machine_opts, "kernel");

You can just get the kernel filename directly from: machine->kernel_filename

> +
> +    memory_region_init_ram(nvm, NULL, "MSF2.envm", ENVM_SIZE,
> +                           &error_fatal);
> +    memory_region_init_alias(nvm_alias, NULL, "MSF2.flash.alias",
> +                             nvm, 0, ENVM_SIZE);
> +    vmstate_register_ram_global(nvm);
> +
> +    memory_region_set_readonly(nvm, true);
> +    memory_region_set_readonly(nvm_alias, true);
> +
> +    memory_region_add_subregion(system_memory, ENVM_BASE_ADDRESS, nvm);
> +    memory_region_add_subregion(system_memory, 0, nvm_alias);
> +
> +    memory_region_init_ram(ddr, NULL, "MSF2.ddr", DDR_SIZE,
> +                           &error_fatal);
> +    vmstate_register_ram_global(ddr);
> +    memory_region_add_subregion(system_memory, DDR_BASE_ADDRESS, ddr);
> +
> +    memory_region_init_ram(sram, NULL, "MSF2.sram", SRAM_SIZE,
> +                           &error_fatal);
> +    vmstate_register_ram_global(sram);
> +    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
> +
> +    nvic = armv7m_init(system_memory, ENVM_SIZE, 96,
> +                       kernel_filename, "cortex-m3");
> +
> +       if (serial_hds[0]) {
> +                serial_mm_init(get_system_memory(), MSF2_UART0_BASE, 2,
> +                       qdev_get_gpio_in(nvic, MSF2_UART0_IRQ),
> +                       115200, serial_hds[0], DEVICE_NATIVE_ENDIAN);
> +    }
> +    dev = qdev_create(NULL, "msf2-timer");
> +    qdev_prop_set_uint32(dev, "clock-frequency", 83 * 1000000);
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MSF2_TIMER_BASE);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> +                           qdev_get_gpio_in(nvic, MSF2_TIMER_IRQ0));
> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 1,
> +                           qdev_get_gpio_in(nvic, MSF2_TIMER_IRQ1));
> +
> +    dev = qdev_create(NULL, "msf2-sysreg");
> +    qdev_init_nofail(dev);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MSF2_SYSREG_BASE);
> +
> +    dev = qdev_create(NULL, "msf2-spi");
> +    qdev_init_nofail(dev);
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, MSF2_SPI0_BASE);
> +    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, MSF2_SPI0_IRQ));
> +
> +    spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
> +    dev = ssi_create_slave_no_init(spi, "s25sl12801");
> +    qdev_prop_set_uint8(dev, "spansion-cr2nv", 1);
> +    if (dinfo)
> +               qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
> +                                    &error_fatal);
> +    qdev_init_nofail(dev);
> +    cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
> +    sysbus_connect_irq(busdev, 1, cs_line);

Instead of calling all of these in the init function you should split
it up over the machines init and realize function.

Look at the stm32f205_soc or xlnx-zynqmp files for examples of how to do this.

It also moves away from calling qdev_create() and qdev_init_nofail()
and instead manually creates the objects.

Otherwise this patch looks pretty good.

Thanks,

Alistair

> +}
> +
> +static void msf2_machine_init(MachineClass *mc)
> +{
> +    mc->desc = "SmartFusion2 SOM kit from Emcraft";
> +    mc->init = msf2_init;
> +}
> +
> +DEFINE_MACHINE("smartfusion2-som", msf2_machine_init)
> --
> 2.5.0
>
>

Re: [Qemu-devel] [Qemu-devel RFC v2 4/4] msf2: Add Emcraft's Smartfusion2 SOM kit.

[sundeep subbaraya]

Hii Alistair,

On Sat, Apr 15, 2017 at 2:42 AM, Alistair Francis <alistair23@gmail.com> wrote:
> On Sun, Apr 9, 2017 at 4:19 AM, Subbaraya Sundeep
> <sundeep.lkml@gmail.com> wrote:
>> Emulated Emcraft's Smartfusion2 System On Module starter
>> kit.
>>
>> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
>
> Hey Sundeep,
>
> Cool patch, I have some comments below.
>
>> ---
>>  default-configs/arm-softmmu.mak |   1 +
>>  hw/arm/Makefile.objs            |   2 +-
>>  hw/arm/msf2_soc.c               | 141 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 143 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/arm/msf2_soc.c
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 1e3bd2b..379f7e1 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -121,3 +121,4 @@ CONFIG_ACPI=y
>>  CONFIG_SMBIOS=y
>>  CONFIG_ASPEED_SOC=y
>>  CONFIG_GPIO_KEY=y
>> +CONFIG_MSF2=y
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 4c5c4ee..cce2759 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -1,7 +1,7 @@
>>  obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
>>  obj-$(CONFIG_DIGIC) += digic_boards.o
>>  obj-y += integratorcp.o mainstone.o musicpal.o nseries.o
>> -obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
>> +obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o msf2_soc.o
>>  obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
>>  obj-$(CONFIG_ACPI) += virt-acpi-build.o
>>  obj-y += netduino2.o
>> diff --git a/hw/arm/msf2_soc.c b/hw/arm/msf2_soc.c
>> new file mode 100644
>> index 0000000..7277b51
>> --- /dev/null
>> +++ b/hw/arm/msf2_soc.c
>> @@ -0,0 +1,141 @@
>> +/*
>> + * Smartfusion2 SOM starter kit(from Emcraft) emulation.
>> + *
>> + * Copyright (c) 2017 Subbaraya Sundeep <sundeep.lkml@gmail.com>
>> + *
>> + * 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 "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "qemu-common.h"
>> +#include "hw/arm/arm.h"
>> +#include "exec/address-spaces.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/char/serial.h"
>> +#include "hw/boards.h"
>> +#include "sysemu/block-backend.h"
>> +#include "hw/ssi/ssi.h"
>> +
>> +#define MSF2_NUM_USARTS       1
>> +#define MSF2_NUM_TIMERS       2
>> +
>> +#define ENVM_BASE_ADDRESS     0x60000000
>> +#define ENVM_SIZE             (128 * 1024)
>> +
>> +#define DDR_BASE_ADDRESS      0xA0000000
>> +#define DDR_SIZE              (64 * 1024 * 1024)
>> +
>> +#define SRAM_BASE_ADDRESS     0x20000000
>> +#define SRAM_SIZE             (64 * 1024)
>> +
>> +#define MSF2_TIMER_BASE       0x40004000
>> +#define MSF2_UART0_BASE       0x40000000
>> +#define MSF2_SYSREG_BASE      0x40038000
>> +#define MSF2_SPI0_BASE        0x40001000
>> +
>> +#define MSF2_SPI0_IRQ         2
>> +#define MSF2_UART0_IRQ        10
>> +#define MSF2_TIMER_IRQ0       14
>> +#define MSF2_TIMER_IRQ1       15
>> +
>> +static void msf2_init(MachineState *machine)
>> +{
>> +    const char *kernel_filename = NULL;
>> +    DeviceState *dev, *nvic;
>> +    MemoryRegion *system_memory = get_system_memory();
>> +    MemoryRegion *nvm = g_new(MemoryRegion, 1);
>> +    MemoryRegion *nvm_alias = g_new(MemoryRegion, 1);
>> +    MemoryRegion *sram = g_new(MemoryRegion, 1);
>> +    MemoryRegion *ddr = g_new(MemoryRegion, 1);
>> +    QemuOpts *machine_opts = qemu_get_machine_opts();
>> +    SysBusDevice *busdev;
>> +    DriveInfo *dinfo = drive_get_next(IF_MTD);
>> +    qemu_irq cs_line;
>> +    SSIBus *spi;
>> +
>> +    kernel_filename = qemu_opt_get(machine_opts, "kernel");
>
> You can just get the kernel filename directly from: machine->kernel_filename
>
I have to check again. I remember getting NULL from machine->kernel_filename.
I will comment on this again.
>> +
>> +    memory_region_init_ram(nvm, NULL, "MSF2.envm", ENVM_SIZE,
>> +                           &error_fatal);
>> +    memory_region_init_alias(nvm_alias, NULL, "MSF2.flash.alias",
>> +                             nvm, 0, ENVM_SIZE);
>> +    vmstate_register_ram_global(nvm);
>> +
>> +    memory_region_set_readonly(nvm, true);
>> +    memory_region_set_readonly(nvm_alias, true);
>> +
>> +    memory_region_add_subregion(system_memory, ENVM_BASE_ADDRESS, nvm);
>> +    memory_region_add_subregion(system_memory, 0, nvm_alias);
>> +
>> +    memory_region_init_ram(ddr, NULL, "MSF2.ddr", DDR_SIZE,
>> +                           &error_fatal);
>> +    vmstate_register_ram_global(ddr);
>> +    memory_region_add_subregion(system_memory, DDR_BASE_ADDRESS, ddr);
>> +
>> +    memory_region_init_ram(sram, NULL, "MSF2.sram", SRAM_SIZE,
>> +                           &error_fatal);
>> +    vmstate_register_ram_global(sram);
>> +    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
>> +
>> +    nvic = armv7m_init(system_memory, ENVM_SIZE, 96,
>> +                       kernel_filename, "cortex-m3");
>> +
>> +       if (serial_hds[0]) {
>> +                serial_mm_init(get_system_memory(), MSF2_UART0_BASE, 2,
>> +                       qdev_get_gpio_in(nvic, MSF2_UART0_IRQ),
>> +                       115200, serial_hds[0], DEVICE_NATIVE_ENDIAN);
>> +    }
>> +    dev = qdev_create(NULL, "msf2-timer");
>> +    qdev_prop_set_uint32(dev, "clock-frequency", 83 * 1000000);
>> +    qdev_init_nofail(dev);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MSF2_TIMER_BASE);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
>> +                           qdev_get_gpio_in(nvic, MSF2_TIMER_IRQ0));
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 1,
>> +                           qdev_get_gpio_in(nvic, MSF2_TIMER_IRQ1));
>> +
>> +    dev = qdev_create(NULL, "msf2-sysreg");
>> +    qdev_init_nofail(dev);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MSF2_SYSREG_BASE);
>> +
>> +    dev = qdev_create(NULL, "msf2-spi");
>> +    qdev_init_nofail(dev);
>> +    busdev = SYS_BUS_DEVICE(dev);
>> +    sysbus_mmio_map(busdev, 0, MSF2_SPI0_BASE);
>> +    sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, MSF2_SPI0_IRQ));
>> +
>> +    spi = (SSIBus *)qdev_get_child_bus(dev, "spi");
>> +    dev = ssi_create_slave_no_init(spi, "s25sl12801");
>> +    qdev_prop_set_uint8(dev, "spansion-cr2nv", 1);
>> +    if (dinfo)
>> +               qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
>> +                                    &error_fatal);
>> +    qdev_init_nofail(dev);
>> +    cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
>> +    sysbus_connect_irq(busdev, 1, cs_line);
>
> Instead of calling all of these in the init function you should split
> it up over the machines init and realize function.
>
> Look at the stm32f205_soc or xlnx-zynqmp files for examples of how to do this.
>
> It also moves away from calling qdev_create() and qdev_init_nofail()
> and instead manually creates the objects.
>
I am still learning all these. Please correct me if am wrong.
I need to create a SoC file and a board file like stm32f205 and
xlnx-zynqmp now right?

> Otherwise this patch looks pretty good.

Thank you :)
Sundeep

>
> Thanks,
>
> Alistair
>
>> +}
>> +
>> +static void msf2_machine_init(MachineClass *mc)
>> +{
>> +    mc->desc = "SmartFusion2 SOM kit from Emcraft";
>> +    mc->init = msf2_init;
>> +}
>> +
>> +DEFINE_MACHINE("smartfusion2-som", msf2_machine_init)
>> --
>> 2.5.0
>>
>>

Re: [Qemu-devel] [Qemu-devel RFC v2 4/4] msf2: Add Emcraft's Smartfusion2 SOM kit.

[Alistair Francis]

>>
>> Instead of calling all of these in the init function you should split
>> it up over the machines init and realize function.
>>
>> Look at the stm32f205_soc or xlnx-zynqmp files for examples of how to do this.
>>
>> It also moves away from calling qdev_create() and qdev_init_nofail()
>> and instead manually creates the objects.
>>
> I am still learning all these. Please correct me if am wrong.
> I need to create a SoC file and a board file like stm32f205 and
> xlnx-zynqmp now right?

Hey Sundeep,

I don't think you have to do it like that. I think for some
SoCs/boards it makes sense. For example the ZynqMP SoCs are included
on multiple different boards (EP108 and ZCU102) so it makes sense to
have a SoC and a board separately defined. On the other hand if you
had a SoC that is always on the same board it doesn't make as much
sense.

It is probably is a good idea to split it between a board and an SoC
unless you have a good reason not to though.

Thanks,

Alistair

>
>> Otherwise this patch looks pretty good.
>
> Thank you :)
> Sundeep
>
>>
>> Thanks,
>>
>> Alistair
>>
>>> +}
>>> +
>>> +static void msf2_machine_init(MachineClass *mc)
>>> +{
>>> +    mc->desc = "SmartFusion2 SOM kit from Emcraft";
>>> +    mc->init = msf2_init;
>>> +}
>>> +
>>> +DEFINE_MACHINE("smartfusion2-som", msf2_machine_init)
>>> --
>>> 2.5.0
>>>
>>>

Re: [Qemu-devel] [Qemu-devel RFC v2 4/4] msf2: Add Emcraft's Smartfusion2 SOM kit.

[sundeep subbaraya]

Hi Alistair,

On Mon, Apr 24, 2017 at 11:23 PM, Alistair Francis <alistair23@gmail.com> wrote:
>>>
>>> Instead of calling all of these in the init function you should split
>>> it up over the machines init and realize function.
>>>
>>> Look at the stm32f205_soc or xlnx-zynqmp files for examples of how to do this.
>>>
>>> It also moves away from calling qdev_create() and qdev_init_nofail()
>>> and instead manually creates the objects.
>>>
>> I am still learning all these. Please correct me if am wrong.
>> I need to create a SoC file and a board file like stm32f205 and
>> xlnx-zynqmp now right?
>
> Hey Sundeep,
>
> I don't think you have to do it like that. I think for some
> SoCs/boards it makes sense. For example the ZynqMP SoCs are included
> on multiple different boards (EP108 and ZCU102) so it makes sense to
> have a SoC and a board separately defined. On the other hand if you
> had a SoC that is always on the same board it doesn't make as much
> sense.
>
> It is probably is a good idea to split it between a board and an SoC
> unless you have a good reason not to though.

There are multiples boards with the SoC. I will split into seperate files.

Thanks,
Sundeep
>
> Thanks,
>
> Alistair
>
>>
>>> Otherwise this patch looks pretty good.
>>
>> Thank you :)
>> Sundeep
>>
>>>
>>> Thanks,
>>>
>>> Alistair
>>>
>>>> +}
>>>> +
>>>> +static void msf2_machine_init(MachineClass *mc)
>>>> +{
>>>> +    mc->desc = "SmartFusion2 SOM kit from Emcraft";
>>>> +    mc->init = msf2_init;
>>>> +}
>>>> +
>>>> +DEFINE_MACHINE("smartfusion2-som", msf2_machine_init)
>>>> --
>>>> 2.5.0
>>>>
>>>>

Re: [Qemu-devel] [Qemu-devel RFC v2 0/4] Add support for Smartfusion2 SoC

[sundeep subbaraya]

Hi Qemu-devel,

This is my first attempt in Qemu.
Please let me know am doing correct or not.
SoC is cortex M3 based so no bootrom stuff and unlike other
SoCs Qemu need not load dtb and kernel in DDR. Hence am using
u-boot (supplied with -kernel) as bootloader in eNVM and it loads
kernel from SPI flash to DDR same like real hardware scenario.
Also let me know any other Maintainers need to be CCed.

Thank you,
Sundeep

On Sun, Apr 9, 2017 at 4:49 PM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:
> Hi Qemu-devel,
>
> I am trying to add Smartfusion2 SoC.
> SoC is from Microsemi and System on Module(SOM)
> board is from Emcraft systems. Smartfusion2 has hardened
> Microcontroller(Cortex-M3)based Sub System and FPGA fabric.
> At the moment only system timer, sysreg and SPI
> controller are modelled.
>
> Testing:
> ./arm-softmmu/qemu-system-arm -M smartfusion2-som -serial mon:stdio \
> -kernel u-boot.bin -display none -drive file=spi.bin,if=mtd,format=raw
>
> U-boot is from Emcraft with modified SPI driver not to use PDMA.
> Linux is 4.5 linux with Smartfusion2 SoC dts and clocksource
> driver added by myself @
> https://github.com/Subbaraya-Sundeep/linux.git
>
> Baremetal elfs from Microsemi Softconsole IDE are also working.
>
> Changes from v1:
>     Added SPI controller.
>
> Thanks,
> Sundeep
>
> Subbaraya Sundeep (4):
>   msf2: Add Smartfusion2 System timer
>   msf2: Microsemi Smartfusion2 System Register block.
>   msf2: Add Smartfusion2 SPI controller
>   msf2: Add Emcraft's Smartfusion2 SOM kit.
>
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs            |   2 +-
>  hw/arm/msf2_soc.c               | 141 +++++++++++++
>  hw/misc/Makefile.objs           |   1 +
>  hw/misc/msf2_sysreg.c           | 168 +++++++++++++++
>  hw/ssi/Makefile.objs            |   1 +
>  hw/ssi/msf2_spi.c               | 449 ++++++++++++++++++++++++++++++++++++++++
>  hw/timer/Makefile.objs          |   1 +
>  hw/timer/msf2_timer.c           | 273 ++++++++++++++++++++++++
>  9 files changed, 1036 insertions(+), 1 deletion(-)
>  create mode 100644 hw/arm/msf2_soc.c
>  create mode 100644 hw/misc/msf2_sysreg.c
>  create mode 100644 hw/ssi/msf2_spi.c
>  create mode 100644 hw/timer/msf2_timer.c
>
> --
> 2.5.0
>

Re: [Qemu-devel] [Qemu-devel RFC v2 0/4] Add support for Smartfusion2 SoC

[Peter Maydell]

On 13 April 2017 at 04:21, sundeep subbaraya <sundeep.lkml@gmail.com> wrote:
> Hi Qemu-devel,
>
> This is my first attempt in Qemu.
> Please let me know am doing correct or not.
> SoC is cortex M3 based so no bootrom stuff and unlike other
> SoCs Qemu need not load dtb and kernel in DDR. Hence am using
> u-boot (supplied with -kernel) as bootloader in eNVM and it loads
> kernel from SPI flash to DDR same like real hardware scenario.
> Also let me know any other Maintainers need to be CCed.

Hi -- thanks for the patchset. I've put it onto my queue of
patches to review, but it may be a little while until I
can get to it (especially since it's the Easter holiday
in the UK right now).

-- PMM

Re: [Qemu-devel] [Qemu-devel RFC v2 0/4] Add support for Smartfusion2 SoC

[sundeep subbaraya]

Hi,

On Thu, Apr 13, 2017 at 3:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 April 2017 at 04:21, sundeep subbaraya <sundeep.lkml@gmail.com> wrote:
>> Hi Qemu-devel,
>>
>> This is my first attempt in Qemu.
>> Please let me know am doing correct or not.
>> SoC is cortex M3 based so no bootrom stuff and unlike other
>> SoCs Qemu need not load dtb and kernel in DDR. Hence am using
>> u-boot (supplied with -kernel) as bootloader in eNVM and it loads
>> kernel from SPI flash to DDR same like real hardware scenario.
>> Also let me know any other Maintainers need to be CCed.
>
> Hi -- thanks for the patchset. I've put it onto my queue of
> patches to review, but it may be a little while until I
> can get to it (especially since it's the Easter holiday
> in the UK right now).

Thanks Peter. Happy holiday :)

Sundeep
>
> -- PMM