All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC
@ 2017-07-03  4:45 Subbaraya Sundeep
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer Subbaraya Sundeep
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Subbaraya Sundeep @ 2017-07-03  4:45 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, crosthwaite.peter, alistair23, f4bug, 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

Binaries u-boot.bin and spi.bin are at:
https://github.com/Subbaraya-Sundeep/qemu-test-binaries.git

U-boot is from Emcraft with modified
    - SPI driver not to use PDMA.
    - ugly hack to pass dtb to kernel in r1.
@
https://github.com/Subbaraya-Sundeep/emcraft-uboot-sf2.git

Linux is 4.5 linux with Smartfusion2 SoC dts and clocksource
driver added by myself @
https://github.com/Subbaraya-Sundeep/linux.git

v6:
    Moved some defines from header files to source files
    Added properties m3clk, apb0div, apb0div1 properties
    to soc.
    Added properties apb0divisor, apb1divisor to sysreg
    Update system_clock_source in msf2-soc.c
    Changed machine name smartfusion2-som->emcraft-sf2

v5
    As per Philippe comments:
        Added abort in Sysreg if guest tries to remap memory
        other than default mapping.
        Use of CONFIG_MSF2 in Makefile for soc.c
        Fixed incorrect logic in timer model.
        Renamed msf2-timer.c -> mss-timer.c
                msf2-spi.c -> mss-spi.c also type names
        Renamed function msf2_init->emcraft_sf2_init in msf2-som.c
        Added part-name,eNVM-size,eSRAM-size,pclk0 and pclk1
            properties to soc.
        Pass soc part-name,memory size and clock rate properties from som.
v4:
    Fixed build failure by using PRIx macros.
v3:
    Added SoC file and board file as per Alistair comments.
v2:
    Added SPI controller so that u-boot loads kernel from spi flash.
v1:
    Initial patch set with timer and sysreg

Thanks,
Sundeep

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

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs            |   2 +
 hw/arm/msf2-soc.c               | 216 +++++++++++++++++++++
 hw/arm/msf2-som.c               |  94 +++++++++
 hw/misc/Makefile.objs           |   1 +
 hw/misc/msf2-sysreg.c           | 200 +++++++++++++++++++
 hw/ssi/Makefile.objs            |   1 +
 hw/ssi/mss-spi.c                | 415 ++++++++++++++++++++++++++++++++++++++++
 hw/timer/Makefile.objs          |   1 +
 hw/timer/mss-timer.c            | 261 +++++++++++++++++++++++++
 include/hw/arm/msf2-soc.h       |  67 +++++++
 include/hw/misc/msf2-sysreg.h   |  82 ++++++++
 include/hw/ssi/mss-spi.h        |  62 ++++++
 include/hw/timer/mss-timer.h    |  67 +++++++
 14 files changed, 1470 insertions(+)
 create mode 100644 hw/arm/msf2-soc.c
 create mode 100644 hw/arm/msf2-som.c
 create mode 100644 hw/misc/msf2-sysreg.c
 create mode 100644 hw/ssi/mss-spi.c
 create mode 100644 hw/timer/mss-timer.c
 create mode 100644 include/hw/arm/msf2-soc.h
 create mode 100644 include/hw/misc/msf2-sysreg.h
 create mode 100644 include/hw/ssi/mss-spi.h
 create mode 100644 include/hw/timer/mss-timer.h

-- 
2.5.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer
  2017-07-03  4:45 [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC Subbaraya Sundeep
@ 2017-07-03  4:45 ` Subbaraya Sundeep
  2017-07-05 17:56   ` Alistair Francis
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block Subbaraya Sundeep
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Subbaraya Sundeep @ 2017-07-03  4:45 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, crosthwaite.peter, alistair23, f4bug, 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/mss-timer.c         | 261 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/timer/mss-timer.h |  67 +++++++++++
 3 files changed, 329 insertions(+)
 create mode 100644 hw/timer/mss-timer.c
 create mode 100644 include/hw/timer/mss-timer.h

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index dd6f27e..fc4d2da 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) += mss-timer.o
diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
new file mode 100644
index 0000000..e46d118
--- /dev/null
+++ b/hw/timer/mss-timer.c
@@ -0,0 +1,261 @@
+/*
+ * Block model of System timer present in
+ * Microsemi's SmartFusion2 and SmartFusion SoCs.
+ *
+ * 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 "hw/timer/mss-timer.h"
+
+#ifndef MSS_TIMER_ERR_DEBUG
+#define MSS_TIMER_ERR_DEBUG  0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+    if (MSS_TIMER_ERR_DEBUG >= lvl) { \
+        qemu_log("%s: " fmt "\n", __func__, ## args); \
+    } \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+#define R_TIM_VAL         0
+#define R_TIM_LOADVAL     1
+#define R_TIM_BGLOADVAL   2
+#define R_TIM_CTRL        3
+#define R_TIM_RIS         4
+#define R_TIM_MIS         5
+
+#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)
+#define TIMER_MODE          (1 << 0)
+
+static void timer_update_irq(struct Msf2Timer *st)
+{
+    bool isr, ier;
+
+    isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
+    ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
+    qemu_set_irq(st->irq, (ier && isr));
+}
+
+static void timer_update(struct Msf2Timer *st)
+{
+    uint64_t count;
+
+    if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL)) {
+        ptimer_stop(st->ptimer);
+        return;
+    }
+
+    count = st->regs[R_TIM_LOADVAL];
+    ptimer_set_limit(st->ptimer, count, 1);
+    ptimer_run(st->ptimer, 1);
+}
+
+static uint64_t
+timer_read(void *opaque, hwaddr offset, unsigned int size)
+{
+    MSSTimerState *t = opaque;
+    hwaddr addr;
+    struct Msf2Timer *st;
+    uint32_t ret = 0;
+    int timer = 0;
+    int isr;
+    int ier;
+
+    addr = offset >> 2;
+    /*
+     * Two independent timers has same base address.
+     * Based on address passed figure out which timer is being used.
+     */
+    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
+        timer = 1;
+        addr -= R_TIM1_MAX;
+    }
+
+    st = &t->timers[timer];
+
+    switch (addr) {
+    case R_TIM_VAL:
+        ret = ptimer_get_count(st->ptimer);
+        break;
+
+    case R_TIM_MIS:
+        isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
+        ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
+        ret = ier & isr;
+        break;
+
+    default:
+        if (addr < NUM_TIMERS * R_TIM1_MAX) {
+            ret = st->regs[addr];
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
+        }
+        break;
+    }
+
+    DB_PRINT("timer=%d 0x%" HWADDR_PRIx "=0x%" PRIx32, timer, offset,
+            ret);
+    return ret;
+}
+
+static void
+timer_write(void *opaque, hwaddr offset,
+            uint64_t val64, unsigned int size)
+{
+    MSSTimerState *t = opaque;
+    hwaddr addr;
+    struct Msf2Timer *st;
+    int timer = 0;
+    uint32_t value = val64;
+
+    addr = offset >> 2;
+    /*
+     * Two independent timers has same base address.
+     * Based on addr passed figure out which timer is being used.
+     */
+    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
+        timer = 1;
+        addr -= R_TIM1_MAX;
+    }
+
+    st = &t->timers[timer];
+
+    DB_PRINT("addr=0x%" HWADDR_PRIx " val=0x%" PRIx32 " (timer=%d)", offset,
+            value, timer);
+
+    switch (addr) {
+    case R_TIM_CTRL:
+        st->regs[R_TIM_CTRL] = value;
+        timer_update(st);
+        break;
+
+    case R_TIM_RIS:
+        if (value & TIMER_RIS_ACK) {
+            st->regs[R_TIM_RIS] &= ~TIMER_RIS_ACK;
+        }
+        break;
+
+    case R_TIM_LOADVAL:
+        st->regs[R_TIM_LOADVAL] = value;
+        if (st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL) {
+            timer_update(st);
+        }
+        break;
+
+    case R_TIM_BGLOADVAL:
+        st->regs[R_TIM_BGLOADVAL] = value;
+        st->regs[R_TIM_LOADVAL] = value;
+        break;
+
+    case R_TIM_VAL:
+    case R_TIM_MIS:
+        break;
+
+    default:
+        if (addr < NUM_TIMERS * R_TIM1_MAX) {
+            st->regs[addr] = value;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
+            return;
+        }
+        break;
+    }
+    timer_update_irq(st);
+}
+
+static const MemoryRegionOps timer_ops = {
+    .read = timer_read,
+    .write = timer_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4
+    }
+};
+
+static void timer_hit(void *opaque)
+{
+    struct Msf2Timer *st = opaque;
+
+    st->regs[R_TIM_RIS] |= TIMER_RIS_ACK;
+
+    if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ONESHOT)) {
+        timer_update(st);
+    }
+    timer_update_irq(st);
+}
+
+static void mss_timer_init(Object *obj)
+{
+    MSSTimerState *t = MSS_TIMER(obj);
+    int i;
+
+    /* Init all the ptimers.  */
+    for (i = 0; i < NUM_TIMERS; i++) {
+        struct Msf2Timer *st = &t->timers[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(obj), &st->irq);
+    }
+
+    memory_region_init_io(&t->mmio, OBJECT(t), &timer_ops, t, TYPE_MSS_TIMER,
+                          NUM_TIMERS * R_TIM1_MAX * 4);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &t->mmio);
+}
+
+static Property mss_timer_properties[] = {
+    /* Libero GUI shows 100Mhz as default for clocks */
+    DEFINE_PROP_UINT32("clock-frequency", MSSTimerState, freq_hz,
+                      100 * 1000000),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mss_timer_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->props = mss_timer_properties;
+}
+
+static const TypeInfo mss_timer_info = {
+    .name          = TYPE_MSS_TIMER,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(MSSTimerState),
+    .instance_init = mss_timer_init,
+    .class_init    = mss_timer_class_init,
+};
+
+static void mss_timer_register_types(void)
+{
+    type_register_static(&mss_timer_info);
+}
+
+type_init(mss_timer_register_types)
diff --git a/include/hw/timer/mss-timer.h b/include/hw/timer/mss-timer.h
new file mode 100644
index 0000000..85aed49
--- /dev/null
+++ b/include/hw/timer/mss-timer.h
@@ -0,0 +1,67 @@
+/*
+ * Microsemi SmartFusion2 Timer.
+ *
+ * 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.
+ */
+
+#ifndef HW_MSS_TIMER_H
+#define HW_MSS_TIMER_H
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+
+#define TYPE_MSS_TIMER     "mss-timer"
+#define MSS_TIMER(obj)     OBJECT_CHECK(MSSTimerState, \
+                              (obj), TYPE_MSS_TIMER)
+
+/*
+ * There are two 32-bit down counting timers.
+ * Timers 1 and 2 can be concatenated into a single 64-bit Timer
+ * that operates either in Periodic mode or in One-shot mode.
+ * Writing 1 to the TIM64_MODE register bit 0 sets the Timers in 64-bit mode.
+ * In 64-bit mode, writing to the 32-bit registers has no effect.
+ * Similarly, in 32-bit mode, writing to the 64-bit mode registers
+ * has no effect. Only two 32-bit timers are supported currently.
+ */
+#define NUM_TIMERS        2
+
+#define R_TIM1_MAX        6
+
+struct Msf2Timer {
+    QEMUBH *bh;
+    ptimer_state *ptimer;
+
+    uint32_t regs[R_TIM1_MAX];
+    qemu_irq irq;
+};
+
+typedef struct MSSTimerState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+    uint32_t freq_hz;
+    struct Msf2Timer timers[NUM_TIMERS];
+} MSSTimerState;
+
+#endif /* HW_MSS_TIMER_H */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-03  4:45 [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC Subbaraya Sundeep
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer Subbaraya Sundeep
@ 2017-07-03  4:45 ` Subbaraya Sundeep
  2017-07-05 18:06   ` Alistair Francis
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller Subbaraya Sundeep
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Subbaraya Sundeep @ 2017-07-03  4:45 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, crosthwaite.peter, alistair23, f4bug, 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         | 200 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
 3 files changed, 283 insertions(+)
 create mode 100644 hw/misc/msf2-sysreg.c
 create mode 100644 include/hw/misc/msf2-sysreg.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index c8b4893..0f52354 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..64ee141
--- /dev/null
+++ b/hw/misc/msf2-sysreg.c
@@ -0,0 +1,200 @@
+/*
+ * 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 "hw/misc/msf2-sysreg.h"
+
+#ifndef MSF2_SYSREG_ERR_DEBUG
+#define MSF2_SYSREG_ERR_DEBUG  0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
+        qemu_log("%s: " fmt "\n", __func__, ## args); \
+    } \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static inline int msf2_divbits(uint32_t div)
+{
+    int ret = 0;
+
+    switch (div) {
+    case 1:
+        ret = 0;
+        break;
+    case 2:
+        ret = 1;
+        break;
+    case 4:
+        ret = 2;
+        break;
+    case 8:
+        ret = 4;
+        break;
+    case 16:
+        ret = 5;
+        break;
+    case 32:
+        ret = 6;
+        break;
+    default:
+        break;
+    }
+
+    return ret;
+}
+
+static void msf2_sysreg_reset(DeviceState *d)
+{
+    MSF2SysregState *s = MSF2_SYSREG(d);
+
+    DB_PRINT("RESET");
+
+    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
+    s->regs[MSSDDR_PLL_STATUS] = 0x3;
+    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
+                               msf2_divbits(s->apb1div) << 2;
+}
+
+static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
+    unsigned size)
+{
+    MSF2SysregState *s = opaque;
+    offset /= 4;
+    uint32_t ret = 0;
+
+    if (offset < ARRAY_SIZE(s->regs)) {
+        ret = s->regs[offset];
+        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
+                    offset * 4, ret);
+    } else {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
+                    offset * 4);
+    }
+
+    return ret;
+}
+
+static void msf2_sysreg_write(void *opaque, hwaddr offset,
+                          uint64_t val, unsigned size)
+{
+    MSF2SysregState *s = (MSF2SysregState *)opaque;
+    uint32_t newval = val;
+    uint32_t oldval;
+
+    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
+            offset, val);
+
+    offset /= 4;
+
+    switch (offset) {
+    case MSSDDR_PLL_STATUS:
+        break;
+
+    case ESRAM_CR:
+        oldval = s->regs[ESRAM_CR];
+        if (oldval ^ newval) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                       TYPE_MSF2_SYSREG": eSRAM remapping not supported\n");
+            abort();
+        }
+        break;
+
+    case DDR_CR:
+        oldval = s->regs[DDR_CR];
+        if (oldval ^ newval) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                       TYPE_MSF2_SYSREG": DDR remapping not supported\n");
+            abort();
+        }
+        break;
+
+    case ENVM_REMAP_BASE_CR:
+        oldval = s->regs[ENVM_REMAP_BASE_CR];
+        if (oldval ^ newval) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                       TYPE_MSF2_SYSREG": eNVM remapping not supported\n");
+            abort();
+        }
+        break;
+
+    default:
+        if (offset < ARRAY_SIZE(s->regs)) {
+            s->regs[offset] = val;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
+                        offset * 4);
+        }
+        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)
+{
+    MSF2SysregState *s = MSF2_SYSREG(obj);
+
+    memory_region_init_io(&s->iomem, obj, &sysreg_ops, s, TYPE_MSF2_SYSREG,
+                          MSF2_SYSREG_MMIO_SIZE);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static const VMStateDescription vmstate_msf2_sysreg = {
+    .name = TYPE_MSF2_SYSREG,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, MSF2SysregState, MSF2_SYSREG_MMIO_SIZE / 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static Property msf2_sysreg_properties[] = {
+    /* default divisors in Libero GUI */
+    DEFINE_PROP_UINT32("apb0divisor", MSF2SysregState, apb0div, 2),
+    DEFINE_PROP_UINT32("apb1divisor", MSF2SysregState, apb1div, 2),
+    DEFINE_PROP_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;
+    dc->props = msf2_sysreg_properties;
+}
+
+static const TypeInfo msf2_sysreg_info = {
+    .name  = TYPE_MSF2_SYSREG,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .class_init = msf2_sysreg_class_init,
+    .instance_size  = sizeof(MSF2SysregState),
+    .instance_init = msf2_sysreg_init,
+};
+
+static void msf2_sysreg_register_types(void)
+{
+    type_register_static(&msf2_sysreg_info);
+}
+
+type_init(msf2_sysreg_register_types)
diff --git a/include/hw/misc/msf2-sysreg.h b/include/hw/misc/msf2-sysreg.h
new file mode 100644
index 0000000..5e9ea99
--- /dev/null
+++ b/include/hw/misc/msf2-sysreg.h
@@ -0,0 +1,82 @@
+/*
+ * Microsemi SmartFusion2 SYSREG
+ *
+ * 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.
+ */
+
+#ifndef HW_MSF2_SYSREG_H
+#define HW_MSF2_SYSREG_H
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+
+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 TYPE_MSF2_SYSREG          "msf2-sysreg"
+#define MSF2_SYSREG(obj)  OBJECT_CHECK(MSF2SysregState, (obj), TYPE_MSF2_SYSREG)
+
+typedef struct MSF2SysregState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+
+    uint32_t apb0div;
+    uint32_t apb1div;
+
+    uint32_t regs[MSF2_SYSREG_MMIO_SIZE / 4];
+} MSF2SysregState;
+
+#endif /* HW_MSF2_SYSREG_H */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller
  2017-07-03  4:45 [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC Subbaraya Sundeep
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer Subbaraya Sundeep
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block Subbaraya Sundeep
@ 2017-07-03  4:45 ` Subbaraya Sundeep
  2017-07-05 18:18   ` Alistair Francis
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC Subbaraya Sundeep
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit Subbaraya Sundeep
  4 siblings, 1 reply; 22+ messages in thread
From: Subbaraya Sundeep @ 2017-07-03  4:45 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, crosthwaite.peter, alistair23, f4bug, Subbaraya Sundeep

Modelled Microsemi's Smartfusion2 SPI controller.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 hw/ssi/Makefile.objs     |   1 +
 hw/ssi/mss-spi.c         | 414 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/ssi/mss-spi.h |  62 +++++++
 3 files changed, 477 insertions(+)
 create mode 100644 hw/ssi/mss-spi.c
 create mode 100644 include/hw/ssi/mss-spi.h

diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index 487add2..f5bcc65 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) += mss-spi.o
 
 obj-$(CONFIG_OMAP) += omap_spi.o
 obj-$(CONFIG_IMX) += imx_spi.o
diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
new file mode 100644
index 0000000..a572abc
--- /dev/null
+++ b/hw/ssi/mss-spi.c
@@ -0,0 +1,414 @@
+/*
+ * Block model of SPI controller present in
+ * Microsemi's SmartFusion2 and SmartFusion SoCs.
+ *
+ * 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 "hw/ssi/mss-spi.h"
+
+#ifndef MSS_SPI_ERR_DEBUG
+#define MSS_SPI_ERR_DEBUG   0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+    if (MSS_SPI_ERR_DEBUG >= lvl) { \
+        qemu_log("%s: " fmt "\n", __func__, ## args); \
+    } \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+#define FIFO_CAPACITY         32
+#define FIFO_CAPACITY         32
+
+#define R_SPI_CONTROL         0
+#define R_SPI_DFSIZE          1
+#define R_SPI_STATUS          2
+#define R_SPI_INTCLR          3
+#define R_SPI_RX              4
+#define R_SPI_TX              5
+#define R_SPI_CLKGEN          6
+#define R_SPI_SS              7
+#define R_SPI_MIS             8
+#define R_SPI_RIS             9
+
+#define S_TXDONE             (1 << 0)
+#define S_RXRDY              (1 << 1)
+#define S_RXCHOVRF           (1 << 2)
+#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
+
+static void txfifo_reset(MSSSpiState *s)
+{
+    fifo32_reset(&s->tx_fifo);
+
+    s->regs[R_SPI_STATUS] &= ~S_TXFIFOFUL;
+    s->regs[R_SPI_STATUS] |= S_TXFIFOEMP;
+}
+
+static void rxfifo_reset(MSSSpiState *s)
+{
+    fifo32_reset(&s->rx_fifo);
+
+    s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
+    s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
+}
+
+static void set_fifodepth(MSSSpiState *s)
+{
+    unsigned int size = s->regs[R_SPI_DFSIZE] & FRAMESZ_MASK;
+
+    if (size <= 8) {
+        s->fifo_depth = 32;
+    } else if (size <= 16) {
+        s->fifo_depth = 16;
+    } else if (size <= 32) {
+        s->fifo_depth = 8;
+    } else {
+        s->fifo_depth = 4;
+    }
+}
+
+static void mss_spi_do_reset(MSSSpiState *s)
+{
+    memset(s->regs, 0, sizeof s->regs);
+    s->regs[R_SPI_CONTROL] = 0x80000102;
+    s->regs[R_SPI_DFSIZE] = 0x4;
+    s->regs[R_SPI_STATUS] = S_SSEL | S_TXFIFOEMP | S_RXFIFOEMP;
+    s->regs[R_SPI_CLKGEN] = 0x7;
+    s->regs[R_SPI_RIS] = 0x0;
+
+    s->fifo_depth = 4;
+    s->frame_count = 1;
+    s->enabled = false;
+
+    rxfifo_reset(s);
+    txfifo_reset(s);
+}
+
+static void update_mis(MSSSpiState *s)
+{
+    uint32_t reg = s->regs[R_SPI_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_SPI_MIS] |= tmp & s->regs[R_SPI_RIS];
+}
+
+static void spi_update_irq(MSSSpiState *s)
+{
+    int irq;
+
+    update_mis(s);
+    irq = !!(s->regs[R_SPI_MIS]);
+
+    qemu_set_irq(s->irq, irq);
+}
+
+static void mss_spi_reset(DeviceState *d)
+{
+    mss_spi_do_reset(MSS_SPI(d));
+}
+
+static uint64_t
+spi_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    MSSSpiState *s = opaque;
+    uint32_t ret = 0;
+
+    addr >>= 2;
+    switch (addr) {
+    case R_SPI_RX:
+        s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
+        s->regs[R_SPI_STATUS] &= ~S_RXCHOVRF;
+        ret = fifo32_pop(&s->rx_fifo);
+        if (fifo32_is_empty(&s->rx_fifo)) {
+            s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
+        }
+        break;
+
+    case R_SPI_MIS:
+        update_mis(s);
+        ret = s->regs[R_SPI_MIS];
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(s->regs)) {
+            ret = s->regs[addr];
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                         "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__,
+                         addr * 4);
+        }
+        break;
+    }
+
+    DB_PRINT("addr=0x%" HWADDR_PRIx " = 0x%" PRIx32, addr * 4, ret);
+    spi_update_irq(s);
+    return ret;
+}
+
+static void assert_cs(MSSSpiState *s)
+{
+    qemu_set_irq(s->cs_line, 0);
+}
+
+static void deassert_cs(MSSSpiState *s)
+{
+    qemu_set_irq(s->cs_line, 1);
+}
+
+static void spi_flush_txfifo(MSSSpiState *s)
+{
+    uint32_t tx;
+    uint32_t rx;
+    bool sps = !!(s->regs[R_SPI_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_SPI_STATUS] &= ~(S_TXDONE | S_RXRDY);
+
+        tx = fifo32_pop(&s->tx_fifo);
+        DB_PRINT("data tx:0x%" PRIx32, tx);
+        rx = ssi_transfer(s->spi, tx);
+        DB_PRINT("data rx:0x%" PRIx32, rx);
+
+        if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
+            s->regs[R_SPI_STATUS] |= S_RXCHOVRF;
+            s->regs[R_SPI_RIS] |= S_RXCHOVRF;
+        } else {
+            fifo32_push(&s->rx_fifo, rx);
+            s->regs[R_SPI_STATUS] &= ~S_RXFIFOEMP;
+            if (fifo32_num_used(&s->rx_fifo) == (s->fifo_depth - 1)) {
+                s->regs[R_SPI_STATUS] |= S_RXFIFOFULNXT;
+            }
+            if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
+                s->regs[R_SPI_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_SPI_CONTROL] & FMCOUNT_MASK) >>
+                            FMCOUNT_SHIFT;
+        if (sps) {
+            deassert_cs(s);
+        }
+        s->regs[R_SPI_RIS] |= S_TXDONE | S_RXRDY;
+        s->regs[R_SPI_STATUS] |= S_TXDONE | S_RXRDY;
+   }
+}
+
+static void spi_write(void *opaque, hwaddr addr,
+            uint64_t val64, unsigned int size)
+{
+    MSSSpiState *s = opaque;
+    uint32_t value = val64;
+
+    DB_PRINT("addr=0x%" HWADDR_PRIx " =0x%" PRIx32, addr, value);
+    addr >>= 2;
+
+    switch (addr) {
+    case R_SPI_TX:
+        /* adding to already full FIFO */
+        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
+            break;
+        }
+        s->regs[R_SPI_STATUS] &= ~S_TXFIFOEMP;
+        fifo32_push(&s->tx_fifo, value);
+        if (fifo32_num_used(&s->tx_fifo) == (s->fifo_depth - 1)) {
+            s->regs[R_SPI_STATUS] |= S_TXFIFOFULNXT;
+        }
+        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
+            s->regs[R_SPI_STATUS] |= S_TXFIFOFUL;
+        }
+        if (s->enabled) {
+            spi_flush_txfifo(s);
+        }
+        break;
+
+    case R_SPI_CONTROL:
+        s->regs[R_SPI_CONTROL] = value;
+        if (value & C_BIGFIFO) {
+            set_fifodepth(s);
+        } else {
+            s->fifo_depth = 4;
+        }
+        s->enabled = value & C_ENABLE;
+        s->frame_count = (value & FMCOUNT_MASK) >> FMCOUNT_SHIFT;
+        if (value & C_RESET) {
+            mss_spi_do_reset(s);
+        }
+        break;
+
+    case R_SPI_DFSIZE:
+        if (s->enabled) {
+            break;
+        }
+        s->regs[R_SPI_DFSIZE] = value;
+        break;
+
+    case R_SPI_INTCLR:
+        s->regs[R_SPI_INTCLR] = value;
+        if (value & S_TXDONE) {
+            s->regs[R_SPI_RIS] &= ~S_TXDONE;
+        }
+        if (value & S_RXRDY) {
+            s->regs[R_SPI_RIS] &= ~S_RXRDY;
+        }
+        if (value & S_RXCHOVRF) {
+            s->regs[R_SPI_RIS] &= ~S_RXCHOVRF;
+        }
+        break;
+
+    case R_SPI_MIS:
+    case R_SPI_STATUS:
+    case R_SPI_RIS:
+            qemu_log_mask(LOG_GUEST_ERROR,
+                         "%s: Write to read only register 0x%" HWADDR_PRIx "\n",
+                         __func__, addr * 4);
+        break;
+
+    default:
+        if (addr < ARRAY_SIZE(s->regs)) {
+            s->regs[addr] = value;
+        } else {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                         "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__,
+                         addr * 4);
+        }
+        break;
+    }
+
+    spi_update_irq(s);
+}
+
+static const MemoryRegionOps spi_ops = {
+    .read = spi_read,
+    .write = spi_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4
+    }
+};
+
+static void mss_spi_realize(DeviceState *dev, Error **errp)
+{
+    MSSSpiState *s = MSS_SPI(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    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,
+                          TYPE_MSS_SPI, R_SPI_MAX * 4);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    fifo32_create(&s->tx_fifo, FIFO_CAPACITY);
+    fifo32_create(&s->rx_fifo, FIFO_CAPACITY);
+}
+
+static const VMStateDescription vmstate_mss_spi = {
+    .name = TYPE_MSS_SPI,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_FIFO32(tx_fifo, MSSSpiState),
+        VMSTATE_FIFO32(rx_fifo, MSSSpiState),
+        VMSTATE_UINT32_ARRAY(regs, MSSSpiState, R_SPI_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void mss_spi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = mss_spi_realize;
+    dc->reset = mss_spi_reset;
+    dc->vmsd = &vmstate_mss_spi;
+}
+
+static const TypeInfo mss_spi_info = {
+    .name           = TYPE_MSS_SPI,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(MSSSpiState),
+    .class_init     = mss_spi_class_init,
+};
+
+static void mss_spi_register_types(void)
+{
+    type_register_static(&mss_spi_info);
+}
+
+type_init(mss_spi_register_types)
diff --git a/include/hw/ssi/mss-spi.h b/include/hw/ssi/mss-spi.h
new file mode 100644
index 0000000..6c8c0b1
--- /dev/null
+++ b/include/hw/ssi/mss-spi.h
@@ -0,0 +1,62 @@
+/*
+ * Microsemi SmartFusion2 SPI
+ *
+ * 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.
+ */
+
+#ifndef HW_MSS_SPI_H
+#define HW_MSS_SPI_H
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/ssi/ssi.h"
+#include "qemu/fifo32.h"
+#include "sysemu/sysemu.h"
+#include "qemu/log.h"
+
+#define TYPE_MSS_SPI   "mss-spi"
+#define MSS_SPI(obj)   OBJECT_CHECK(MSSSpiState, (obj), TYPE_MSS_SPI)
+
+#define R_SPI_MAX             16
+
+typedef struct MSSSpiState {
+    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_SPI_MAX];
+} MSSSpiState;
+
+#endif /* HW_MSS_SPI_H */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC.
  2017-07-03  4:45 [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC Subbaraya Sundeep
                   ` (2 preceding siblings ...)
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller Subbaraya Sundeep
@ 2017-07-03  4:45 ` Subbaraya Sundeep
  2017-07-05 18:25   ` Alistair Francis
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit Subbaraya Sundeep
  4 siblings, 1 reply; 22+ messages in thread
From: Subbaraya Sundeep @ 2017-07-03  4:45 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, crosthwaite.peter, alistair23, f4bug, Subbaraya Sundeep

Smartfusion2 SoC has hardened Microcontroller subsystem
and flash based FPGA fabric. This patch adds support for
Microcontroller subsystem in the SoC.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs            |   1 +
 hw/arm/msf2-soc.c               | 216 ++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/msf2-soc.h       |  67 +++++++++++++
 4 files changed, 285 insertions(+)
 create mode 100644 hw/arm/msf2-soc.c
 create mode 100644 include/hw/arm/msf2-soc.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 78d7af0..7062512 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -122,3 +122,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..c828061 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -18,3 +18,4 @@ obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
 obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
+obj-$(CONFIG_MSF2) += msf2-soc.o
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
new file mode 100644
index 0000000..d45827f
--- /dev/null
+++ b/hw/arm/msf2-soc.c
@@ -0,0 +1,216 @@
+/*
+ * SmartFusion2 SoC 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/char/serial.h"
+#include "hw/boards.h"
+#include "sysemu/block-backend.h"
+#include "hw/arm/msf2-soc.h"
+
+#define MSF2_TIMER_BASE     0x40004000
+#define MSF2_SYSREG_BASE    0x40038000
+
+#define ENVM_BASE_ADDRESS     0x60000000
+
+#define SRAM_BASE_ADDRESS     0x20000000
+
+#define MSF2_ENVM_SIZE        (512 * K_BYTE)
+#define MSF2_ESRAM_SIZE       (64 * K_BYTE)
+
+static const uint32_t spi_addr[MSF2_NUM_SPIS] = { 0x40001000 , 0x40011000 };
+static const uint32_t uart_addr[MSF2_NUM_UARTS] = { 0x40000000 , 0x40010000 };
+
+static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 };
+static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 };
+static const int timer_irq[MSF2_NUM_TIMERS] = { 14, 15 };
+
+static void m2sxxx_soc_initfn(Object *obj)
+{
+    MSF2State *s = MSF2_SOC(obj);
+    int i;
+
+    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
+    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
+
+    object_initialize(&s->sysreg, sizeof(s->sysreg), TYPE_MSF2_SYSREG);
+    qdev_set_parent_bus(DEVICE(&s->sysreg), sysbus_get_default());
+
+    object_initialize(&s->timer, sizeof(s->timer), TYPE_MSS_TIMER);
+    qdev_set_parent_bus(DEVICE(&s->timer), sysbus_get_default());
+
+    for (i = 0; i < MSF2_NUM_SPIS; i++) {
+        object_initialize(&s->spi[i], sizeof(s->spi[i]),
+                          TYPE_MSS_SPI);
+        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+    }
+}
+
+static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
+{
+    MSF2State *s = MSF2_SOC(dev_soc);
+    DeviceState *dev, *armv7m;
+    SysBusDevice *busdev;
+    Error *err = NULL;
+    int i;
+
+    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);
+
+    memory_region_init_ram(nvm, NULL, "MSF2.eNVM", s->envm_size,
+                           &error_fatal);
+
+    /*
+     * On power-on, the eNVM region 0x60000000 is automatically
+     * remapped to the Cortex-M3 processor executable region
+     * start address (0x0). We do not support remapping other eNVM,
+     * eSRAM and DDR regions by guest(via Sysreg) currently.
+     */
+    memory_region_init_alias(nvm_alias, NULL, "MSF2.eNVM.alias",
+                             nvm, 0, s->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(sram, NULL, "MSF2.eSRAM", s->esram_size,
+                           &error_fatal);
+    vmstate_register_ram_global(sram);
+    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
+
+    armv7m = DEVICE(&s->armv7m);
+    qdev_prop_set_uint32(armv7m, "num-irq", 81);
+    qdev_prop_set_string(armv7m, "cpu-model", "cortex-m3");
+    object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
+                                     "memory", &error_abort);
+    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
+
+    for (i = 0; i < MSF2_NUM_UARTS; i++) {
+        if (serial_hds[i]) {
+            serial_mm_init(get_system_memory(), uart_addr[i], 2,
+                           qdev_get_gpio_in(armv7m, uart_irq[i]),
+                           115200, serial_hds[i], DEVICE_NATIVE_ENDIAN);
+        }
+    }
+
+    dev = DEVICE(&s->timer);
+    /* APB0 clock is the timer input clock */
+    qdev_prop_set_uint32(dev, "clock-frequency", s->m3clk / s->apb0div);
+    object_property_set_bool(OBJECT(&s->timer), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, MSF2_TIMER_BASE);
+    sysbus_connect_irq(busdev, 0,
+                           qdev_get_gpio_in(armv7m, timer_irq[0]));
+    sysbus_connect_irq(busdev, 1,
+                           qdev_get_gpio_in(armv7m, timer_irq[1]));
+
+    dev = DEVICE(&s->sysreg);
+    qdev_prop_set_uint32(dev, "apb0divisor", s->apb0div);
+    qdev_prop_set_uint32(dev, "apb1divisor", s->apb1div);
+    object_property_set_bool(OBJECT(&s->sysreg), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+    busdev = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(busdev, 0, MSF2_SYSREG_BASE);
+
+    for (i = 0; i < MSF2_NUM_SPIS; i++) {
+        gchar *bus_name = g_strdup_printf("spi%d", i);
+
+        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
+        if (err != NULL) {
+            g_free(bus_name);
+            error_propagate(errp, err);
+            return;
+        }
+
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
+                           qdev_get_gpio_in(armv7m, spi_irq[i]));
+
+        /* Alias controller SPI bus to the SoC itself */
+        object_property_add_alias(OBJECT(s), bus_name,
+                                  OBJECT(&s->spi[i]), "spi",
+                                  &error_abort);
+        g_free(bus_name);
+    }
+}
+
+static Property m2sxxx_soc_properties[] = {
+    /*
+     * part name specifies the type of SmartFusion2 device variant(this
+     * property is for information purpose only.
+     */
+    DEFINE_PROP_STRING("part-name", MSF2State, part_name),
+    DEFINE_PROP_UINT64("eNVM-size", MSF2State, envm_size, MSF2_ENVM_SIZE),
+    DEFINE_PROP_UINT64("eSRAM-size", MSF2State, esram_size, MSF2_ESRAM_SIZE),
+    /* Libero GUI shows 100Mhz as default for clocks */
+    DEFINE_PROP_UINT32("m3clk", MSF2State, m3clk, 100 * 1000000),
+    /* default divisors in Libero GUI */
+    DEFINE_PROP_UINT32("apb0div", MSF2State, apb0div, 2),
+    DEFINE_PROP_UINT32("apb1div", MSF2State, apb1div, 2),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void m2sxxx_soc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = m2sxxx_soc_realize;
+    dc->props = m2sxxx_soc_properties;
+}
+
+static const TypeInfo m2sxxx_soc_info = {
+    .name          = TYPE_MSF2_SOC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(MSF2State),
+    .instance_init = m2sxxx_soc_initfn,
+    .class_init    = m2sxxx_soc_class_init,
+};
+
+static void m2sxxx_soc_types(void)
+{
+    type_register_static(&m2sxxx_soc_info);
+}
+
+type_init(m2sxxx_soc_types)
diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
new file mode 100644
index 0000000..9ed677a
--- /dev/null
+++ b/include/hw/arm/msf2-soc.h
@@ -0,0 +1,67 @@
+/*
+ * Microsemi Smartfusion2 SoC
+ *
+ * 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.
+ */
+
+#ifndef HW_ARM_MSF2_SOC_H
+#define HW_ARM_MSF2_SOC_H
+
+#include "hw/misc/msf2-sysreg.h"
+#include "hw/timer/mss-timer.h"
+#include "hw/ssi/mss-spi.h"
+#include "hw/arm/armv7m.h"
+#include "qemu/cutils.h"
+
+#define TYPE_MSF2_SOC     "msf2-soc"
+#define MSF2_SOC(obj)     OBJECT_CHECK(MSF2State, (obj), TYPE_MSF2_SOC)
+
+#define MSF2_NUM_SPIS         2
+#define MSF2_NUM_UARTS        2
+
+/*
+ * System timer consists of two programmable 32-bit
+ * decrementing counters that generate individual interrupts to
+ * the Cortex-M3 processor
+ */
+#define MSF2_NUM_TIMERS       2
+
+typedef struct MSF2State {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    ARMv7MState armv7m;
+
+    char *part_name;
+    uint64_t envm_size;
+    uint64_t esram_size;
+
+    uint32_t m3clk;
+    uint32_t apb0div;
+    uint32_t apb1div;
+
+    MSF2SysregState sysreg;
+    MSSTimerState timer;
+    MSSSpiState spi[MSF2_NUM_SPIS];
+} MSF2State;
+
+#endif
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [Qemu devel v6 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit.
  2017-07-03  4:45 [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC Subbaraya Sundeep
                   ` (3 preceding siblings ...)
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC Subbaraya Sundeep
@ 2017-07-03  4:45 ` Subbaraya Sundeep
  4 siblings, 0 replies; 22+ messages in thread
From: Subbaraya Sundeep @ 2017-07-03  4:45 UTC (permalink / raw)
  To: qemu-devel, qemu-arm
  Cc: peter.maydell, crosthwaite.peter, alistair23, f4bug, Subbaraya Sundeep

Emulated Emcraft's Smartfusion2 System On Module starter
kit.

Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
---
 hw/arm/Makefile.objs |  1 +
 hw/arm/msf2-som.c    | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)
 create mode 100644 hw/arm/msf2-som.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index c828061..2073934 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -5,6 +5,7 @@ obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
 obj-$(CONFIG_ACPI) += virt-acpi-build.o
 obj-y += netduino2.o
+obj-$(CONFIG_MSF2) += msf2-som.o
 obj-y += sysbus-fdt.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
new file mode 100644
index 0000000..ea945b1
--- /dev/null
+++ b/hw/arm/msf2-som.c
@@ -0,0 +1,94 @@
+/*
+ * 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 "hw/boards.h"
+#include "hw/arm/msf2-soc.h"
+#include "hw/arm/arm.h"
+#include "exec/address-spaces.h"
+
+#define DDR_BASE_ADDRESS      0xA0000000
+#define DDR_SIZE              (64 * M_BYTE)
+
+#define M2S010_ENVM_SIZE      (256 * K_BYTE)
+#define M2S010_ESRAM_SIZE     (64 * K_BYTE)
+
+static void emcraft_sf2_init(MachineState *machine)
+{
+    DeviceState *dev;
+    DeviceState *spi_flash;
+    MSF2State *soc;
+    DriveInfo *dinfo = drive_get_next(IF_MTD);
+    qemu_irq cs_line;
+    SSIBus *spi_bus;
+    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *ddr = g_new(MemoryRegion, 1);
+
+    memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE,
+                           &error_fatal);
+    vmstate_register_ram_global(ddr);
+    memory_region_add_subregion(sysmem, DDR_BASE_ADDRESS, ddr);
+
+    dev = qdev_create(NULL, TYPE_MSF2_SOC);
+    qdev_prop_set_string(dev, "part-name", "M2S010");
+    qdev_prop_set_uint64(dev, "eNVM-size", M2S010_ENVM_SIZE);
+    qdev_prop_set_uint64(dev, "eSRAM-size", M2S010_ESRAM_SIZE);
+
+    /*
+     * CPU clock and peripheral clocks(APB0, APB1)are configurable
+     * in Libero. CPU clock is divided by APB0 and APB1 divisors for
+     * peripherals. Emcraft's SoM kit comes with these settings by default.
+     */
+    qdev_prop_set_uint32(dev, "m3clk", 142 * 1000000);
+    qdev_prop_set_uint32(dev, "apb0div", 2);
+    qdev_prop_set_uint32(dev, "apb1div", 2);
+
+    object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
+
+    soc = MSF2_SOC(dev);
+
+    /* Attach SPI flash to SPI0 controller */
+    spi_bus = (SSIBus *)qdev_get_child_bus(dev, "spi0");
+    spi_flash = ssi_create_slave_no_init(spi_bus, "s25sl12801");
+    qdev_prop_set_uint8(spi_flash, "spansion-cr2nv", 1);
+    if (dinfo) {
+        qdev_prop_set_drive(spi_flash, "drive", blk_by_legacy_dinfo(dinfo),
+                                    &error_fatal);
+    }
+    qdev_init_nofail(spi_flash);
+    cs_line = qdev_get_gpio_in_named(spi_flash, SSI_GPIO_CS, 0);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&soc->spi[0]), 1, cs_line);
+
+    armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+                       soc->envm_size);
+}
+
+static void emcraft_sf2_machine_init(MachineClass *mc)
+{
+    mc->desc = "SmartFusion2 SOM kit from Emcraft";
+    mc->init = emcraft_sf2_init;
+}
+
+DEFINE_MACHINE("emcraft-sf2", emcraft_sf2_machine_init)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer Subbaraya Sundeep
@ 2017-07-05 17:56   ` Alistair Francis
  2017-07-05 17:58     ` Alistair Francis
  2017-07-07  6:59     ` sundeep subbaraya
  0 siblings, 2 replies; 22+ messages in thread
From: Alistair Francis @ 2017-07-05 17:56 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

On Sun, Jul 2, 2017 at 9:45 PM, 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>
> ---
>  hw/timer/Makefile.objs       |   1 +
>  hw/timer/mss-timer.c         | 261 +++++++++++++++++++++++++++++++++++++++++++
>  include/hw/timer/mss-timer.h |  67 +++++++++++
>  3 files changed, 329 insertions(+)
>  create mode 100644 hw/timer/mss-timer.c
>  create mode 100644 include/hw/timer/mss-timer.h
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index dd6f27e..fc4d2da 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) += mss-timer.o
> diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
> new file mode 100644
> index 0000000..e46d118
> --- /dev/null
> +++ b/hw/timer/mss-timer.c
> @@ -0,0 +1,261 @@
> +/*
> + * Block model of System timer present in
> + * Microsemi's SmartFusion2 and SmartFusion SoCs.
> + *
> + * 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 "hw/timer/mss-timer.h"
> +
> +#ifndef MSS_TIMER_ERR_DEBUG
> +#define MSS_TIMER_ERR_DEBUG  0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> +    if (MSS_TIMER_ERR_DEBUG >= lvl) { \
> +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> +    } \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +#define R_TIM_VAL         0
> +#define R_TIM_LOADVAL     1
> +#define R_TIM_BGLOADVAL   2
> +#define R_TIM_CTRL        3
> +#define R_TIM_RIS         4
> +#define R_TIM_MIS         5
> +
> +#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)
> +#define TIMER_MODE          (1 << 0)
> +
> +static void timer_update_irq(struct Msf2Timer *st)
> +{
> +    bool isr, ier;
> +
> +    isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
> +    ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
> +    qemu_set_irq(st->irq, (ier && isr));
> +}
> +
> +static void timer_update(struct Msf2Timer *st)
> +{
> +    uint64_t count;
> +
> +    if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL)) {
> +        ptimer_stop(st->ptimer);
> +        return;
> +    }
> +
> +    count = st->regs[R_TIM_LOADVAL];
> +    ptimer_set_limit(st->ptimer, count, 1);
> +    ptimer_run(st->ptimer, 1);
> +}
> +
> +static uint64_t
> +timer_read(void *opaque, hwaddr offset, unsigned int size)
> +{
> +    MSSTimerState *t = opaque;
> +    hwaddr addr;
> +    struct Msf2Timer *st;
> +    uint32_t ret = 0;
> +    int timer = 0;
> +    int isr;
> +    int ier;
> +
> +    addr = offset >> 2;
> +    /*
> +     * Two independent timers has same base address.
> +     * Based on address passed figure out which timer is being used.
> +     */
> +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
> +        timer = 1;
> +        addr -= R_TIM1_MAX;
> +    }
> +
> +    st = &t->timers[timer];
> +
> +    switch (addr) {
> +    case R_TIM_VAL:
> +        ret = ptimer_get_count(st->ptimer);
> +        break;
> +
> +    case R_TIM_MIS:
> +        isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
> +        ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
> +        ret = ier & isr;
> +        break;
> +
> +    default:
> +        if (addr < NUM_TIMERS * R_TIM1_MAX) {

Shouldn't this just be: addr < R_TIM1_MAX?

At this point you have already subjected the offset for multiple timers.

> +            ret = st->regs[addr];
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");

Maybe add a return here so you don't print the other debug message after this.

> +        }
> +        break;
> +    }
> +
> +    DB_PRINT("timer=%d 0x%" HWADDR_PRIx "=0x%" PRIx32, timer, offset,
> +            ret);
> +    return ret;
> +}
> +
> +static void
> +timer_write(void *opaque, hwaddr offset,
> +            uint64_t val64, unsigned int size)
> +{
> +    MSSTimerState *t = opaque;
> +    hwaddr addr;
> +    struct Msf2Timer *st;
> +    int timer = 0;
> +    uint32_t value = val64;
> +
> +    addr = offset >> 2;
> +    /*
> +     * Two independent timers has same base address.
> +     * Based on addr passed figure out which timer is being used.
> +     */
> +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
> +        timer = 1;
> +        addr -= R_TIM1_MAX;
> +    }
> +
> +    st = &t->timers[timer];
> +
> +    DB_PRINT("addr=0x%" HWADDR_PRIx " val=0x%" PRIx32 " (timer=%d)", offset,
> +            value, timer);
> +
> +    switch (addr) {
> +    case R_TIM_CTRL:
> +        st->regs[R_TIM_CTRL] = value;
> +        timer_update(st);
> +        break;
> +
> +    case R_TIM_RIS:
> +        if (value & TIMER_RIS_ACK) {
> +            st->regs[R_TIM_RIS] &= ~TIMER_RIS_ACK;
> +        }
> +        break;
> +
> +    case R_TIM_LOADVAL:
> +        st->regs[R_TIM_LOADVAL] = value;
> +        if (st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL) {
> +            timer_update(st);
> +        }
> +        break;
> +
> +    case R_TIM_BGLOADVAL:
> +        st->regs[R_TIM_BGLOADVAL] = value;
> +        st->regs[R_TIM_LOADVAL] = value;
> +        break;
> +
> +    case R_TIM_VAL:
> +    case R_TIM_MIS:
> +        break;
> +
> +    default:
> +        if (addr < NUM_TIMERS * R_TIM1_MAX) {

Same comment as above.

> +            st->regs[addr] = value;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
> +            return;
> +        }
> +        break;
> +    }
> +    timer_update_irq(st);
> +}

Once those comments above are addressed:

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,
Alistair

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer
  2017-07-05 17:56   ` Alistair Francis
@ 2017-07-05 17:58     ` Alistair Francis
  2017-07-07  7:00       ` sundeep subbaraya
  2017-07-07  6:59     ` sundeep subbaraya
  1 sibling, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2017-07-05 17:58 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

On Wed, Jul 5, 2017 at 10:56 AM, Alistair Francis <alistair23@gmail.com> wrote:
> On Sun, Jul 2, 2017 at 9:45 PM, 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>
>> ---
>>  hw/timer/Makefile.objs       |   1 +
>>  hw/timer/mss-timer.c         | 261 +++++++++++++++++++++++++++++++++++++++++++
>>  include/hw/timer/mss-timer.h |  67 +++++++++++
>>  3 files changed, 329 insertions(+)
>>  create mode 100644 hw/timer/mss-timer.c
>>  create mode 100644 include/hw/timer/mss-timer.h
>>
>> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
>> index dd6f27e..fc4d2da 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) += mss-timer.o
>> diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
>> new file mode 100644
>> index 0000000..e46d118
>> --- /dev/null
>> +++ b/hw/timer/mss-timer.c
>> @@ -0,0 +1,261 @@
>> +/*
>> + * Block model of System timer present in
>> + * Microsemi's SmartFusion2 and SmartFusion SoCs.
>> + *
>> + * 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 "hw/timer/mss-timer.h"

Also just noticed (this applies to the whole series) that you need the
headers in the C file.

Move every #include that you can from the mss-timer.h files to this
file, headers should be local to where they are called. Also make sure
you include the os-dep header in every file.

Thanks,
Alistair

>> +
>> +#ifndef MSS_TIMER_ERR_DEBUG
>> +#define MSS_TIMER_ERR_DEBUG  0
>> +#endif
>> +
>> +#define DB_PRINT_L(lvl, fmt, args...) do { \
>> +    if (MSS_TIMER_ERR_DEBUG >= lvl) { \
>> +        qemu_log("%s: " fmt "\n", __func__, ## args); \
>> +    } \
>> +} while (0);
>> +
>> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>> +
>> +#define R_TIM_VAL         0
>> +#define R_TIM_LOADVAL     1
>> +#define R_TIM_BGLOADVAL   2
>> +#define R_TIM_CTRL        3
>> +#define R_TIM_RIS         4
>> +#define R_TIM_MIS         5
>> +
>> +#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)
>> +#define TIMER_MODE          (1 << 0)
>> +
>> +static void timer_update_irq(struct Msf2Timer *st)
>> +{
>> +    bool isr, ier;
>> +
>> +    isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
>> +    ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
>> +    qemu_set_irq(st->irq, (ier && isr));
>> +}
>> +
>> +static void timer_update(struct Msf2Timer *st)
>> +{
>> +    uint64_t count;
>> +
>> +    if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL)) {
>> +        ptimer_stop(st->ptimer);
>> +        return;
>> +    }
>> +
>> +    count = st->regs[R_TIM_LOADVAL];
>> +    ptimer_set_limit(st->ptimer, count, 1);
>> +    ptimer_run(st->ptimer, 1);
>> +}
>> +
>> +static uint64_t
>> +timer_read(void *opaque, hwaddr offset, unsigned int size)
>> +{
>> +    MSSTimerState *t = opaque;
>> +    hwaddr addr;
>> +    struct Msf2Timer *st;
>> +    uint32_t ret = 0;
>> +    int timer = 0;
>> +    int isr;
>> +    int ier;
>> +
>> +    addr = offset >> 2;
>> +    /*
>> +     * Two independent timers has same base address.
>> +     * Based on address passed figure out which timer is being used.
>> +     */
>> +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
>> +        timer = 1;
>> +        addr -= R_TIM1_MAX;
>> +    }
>> +
>> +    st = &t->timers[timer];
>> +
>> +    switch (addr) {
>> +    case R_TIM_VAL:
>> +        ret = ptimer_get_count(st->ptimer);
>> +        break;
>> +
>> +    case R_TIM_MIS:
>> +        isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
>> +        ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
>> +        ret = ier & isr;
>> +        break;
>> +
>> +    default:
>> +        if (addr < NUM_TIMERS * R_TIM1_MAX) {
>
> Shouldn't this just be: addr < R_TIM1_MAX?
>
> At this point you have already subjected the offset for multiple timers.
>
>> +            ret = st->regs[addr];
>> +        } else {
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
>
> Maybe add a return here so you don't print the other debug message after this.
>
>> +        }
>> +        break;
>> +    }
>> +
>> +    DB_PRINT("timer=%d 0x%" HWADDR_PRIx "=0x%" PRIx32, timer, offset,
>> +            ret);
>> +    return ret;
>> +}
>> +
>> +static void
>> +timer_write(void *opaque, hwaddr offset,
>> +            uint64_t val64, unsigned int size)
>> +{
>> +    MSSTimerState *t = opaque;
>> +    hwaddr addr;
>> +    struct Msf2Timer *st;
>> +    int timer = 0;
>> +    uint32_t value = val64;
>> +
>> +    addr = offset >> 2;
>> +    /*
>> +     * Two independent timers has same base address.
>> +     * Based on addr passed figure out which timer is being used.
>> +     */
>> +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
>> +        timer = 1;
>> +        addr -= R_TIM1_MAX;
>> +    }
>> +
>> +    st = &t->timers[timer];
>> +
>> +    DB_PRINT("addr=0x%" HWADDR_PRIx " val=0x%" PRIx32 " (timer=%d)", offset,
>> +            value, timer);
>> +
>> +    switch (addr) {
>> +    case R_TIM_CTRL:
>> +        st->regs[R_TIM_CTRL] = value;
>> +        timer_update(st);
>> +        break;
>> +
>> +    case R_TIM_RIS:
>> +        if (value & TIMER_RIS_ACK) {
>> +            st->regs[R_TIM_RIS] &= ~TIMER_RIS_ACK;
>> +        }
>> +        break;
>> +
>> +    case R_TIM_LOADVAL:
>> +        st->regs[R_TIM_LOADVAL] = value;
>> +        if (st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL) {
>> +            timer_update(st);
>> +        }
>> +        break;
>> +
>> +    case R_TIM_BGLOADVAL:
>> +        st->regs[R_TIM_BGLOADVAL] = value;
>> +        st->regs[R_TIM_LOADVAL] = value;
>> +        break;
>> +
>> +    case R_TIM_VAL:
>> +    case R_TIM_MIS:
>> +        break;
>> +
>> +    default:
>> +        if (addr < NUM_TIMERS * R_TIM1_MAX) {
>
> Same comment as above.
>
>> +            st->regs[addr] = value;
>> +        } else {
>> +            qemu_log_mask(LOG_GUEST_ERROR,
>> +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
>> +            return;
>> +        }
>> +        break;
>> +    }
>> +    timer_update_irq(st);
>> +}
>
> Once those comments above are addressed:
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>
> Thanks,
> Alistair

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block Subbaraya Sundeep
@ 2017-07-05 18:06   ` Alistair Francis
  2017-07-07  7:08     ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2017-07-05 18:06 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
>  3 files changed, 283 insertions(+)
>  create mode 100644 hw/misc/msf2-sysreg.c
>  create mode 100644 include/hw/misc/msf2-sysreg.h
>
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index c8b4893..0f52354 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..64ee141
> --- /dev/null
> +++ b/hw/misc/msf2-sysreg.c
> @@ -0,0 +1,200 @@
> +/*
> + * 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 "hw/misc/msf2-sysreg.h"

Same #include comment from patch 1.

> +
> +#ifndef MSF2_SYSREG_ERR_DEBUG
> +#define MSF2_SYSREG_ERR_DEBUG  0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
> +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> +    } \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +static inline int msf2_divbits(uint32_t div)
> +{
> +    int ret = 0;
> +
> +    switch (div) {
> +    case 1:
> +        ret = 0;
> +        break;
> +    case 2:
> +        ret = 1;
> +        break;
> +    case 4:
> +        ret = 2;
> +        break;
> +    case 8:
> +        ret = 4;
> +        break;
> +    case 16:
> +        ret = 5;
> +        break;
> +    case 32:
> +        ret = 6;
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void msf2_sysreg_reset(DeviceState *d)
> +{
> +    MSF2SysregState *s = MSF2_SYSREG(d);
> +
> +    DB_PRINT("RESET");
> +
> +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
> +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
> +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
> +                               msf2_divbits(s->apb1div) << 2;
> +}
> +
> +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
> +    unsigned size)
> +{
> +    MSF2SysregState *s = opaque;
> +    offset /= 4;

Probably best to use a bitshift.

> +    uint32_t ret = 0;
> +
> +    if (offset < ARRAY_SIZE(s->regs)) {
> +        ret = s->regs[offset];
> +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
> +                    offset * 4, ret);

Bitshift here as well.

> +    } else {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
> +                    offset * 4);
> +    }
> +
> +    return ret;
> +}
> +
> +static void msf2_sysreg_write(void *opaque, hwaddr offset,
> +                          uint64_t val, unsigned size)
> +{
> +    MSF2SysregState *s = (MSF2SysregState *)opaque;
> +    uint32_t newval = val;
> +    uint32_t oldval;
> +
> +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
> +            offset, val);
> +
> +    offset /= 4;

Same here

> +
> +    switch (offset) {
> +    case MSSDDR_PLL_STATUS:
> +        break;
> +
> +    case ESRAM_CR:
> +        oldval = s->regs[ESRAM_CR];
> +        if (oldval ^ newval) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                       TYPE_MSF2_SYSREG": eSRAM remapping not supported\n");
> +            abort();

The guest should not be able to kill QEMU, a guest error should never
result in an abort.

> +        }
> +        break;
> +
> +    case DDR_CR:
> +        oldval = s->regs[DDR_CR];
> +        if (oldval ^ newval) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                       TYPE_MSF2_SYSREG": DDR remapping not supported\n");
> +            abort();
> +        }
> +        break;
> +
> +    case ENVM_REMAP_BASE_CR:
> +        oldval = s->regs[ENVM_REMAP_BASE_CR];
> +        if (oldval ^ newval) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                       TYPE_MSF2_SYSREG": eNVM remapping not supported\n");
> +            abort();
> +        }
> +        break;
> +
> +    default:
> +        if (offset < ARRAY_SIZE(s->regs)) {
> +            s->regs[offset] = val;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                        "%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
> +                        offset * 4);
> +        }
> +        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)
> +{
> +    MSF2SysregState *s = MSF2_SYSREG(obj);
> +
> +    memory_region_init_io(&s->iomem, obj, &sysreg_ops, s, TYPE_MSF2_SYSREG,
> +                          MSF2_SYSREG_MMIO_SIZE);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
> +
> +static const VMStateDescription vmstate_msf2_sysreg = {
> +    .name = TYPE_MSF2_SYSREG,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, MSF2SysregState, MSF2_SYSREG_MMIO_SIZE / 4),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};

This just reminded me that I don't think your first patch has a
VMState. Can you add that

Thanks,
Alistair

> +
> +static Property msf2_sysreg_properties[] = {
> +    /* default divisors in Libero GUI */
> +    DEFINE_PROP_UINT32("apb0divisor", MSF2SysregState, apb0div, 2),
> +    DEFINE_PROP_UINT32("apb1divisor", MSF2SysregState, apb1div, 2),
> +    DEFINE_PROP_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;
> +    dc->props = msf2_sysreg_properties;
> +}
> +
> +static const TypeInfo msf2_sysreg_info = {
> +    .name  = TYPE_MSF2_SYSREG,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .class_init = msf2_sysreg_class_init,
> +    .instance_size  = sizeof(MSF2SysregState),
> +    .instance_init = msf2_sysreg_init,
> +};
> +
> +static void msf2_sysreg_register_types(void)
> +{
> +    type_register_static(&msf2_sysreg_info);
> +}
> +
> +type_init(msf2_sysreg_register_types)
> diff --git a/include/hw/misc/msf2-sysreg.h b/include/hw/misc/msf2-sysreg.h
> new file mode 100644
> index 0000000..5e9ea99
> --- /dev/null
> +++ b/include/hw/misc/msf2-sysreg.h
> @@ -0,0 +1,82 @@
> +/*
> + * Microsemi SmartFusion2 SYSREG
> + *
> + * 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.
> + */
> +
> +#ifndef HW_MSF2_SYSREG_H
> +#define HW_MSF2_SYSREG_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/hw.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/log.h"
> +
> +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 TYPE_MSF2_SYSREG          "msf2-sysreg"
> +#define MSF2_SYSREG(obj)  OBJECT_CHECK(MSF2SysregState, (obj), TYPE_MSF2_SYSREG)
> +
> +typedef struct MSF2SysregState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +
> +    uint32_t apb0div;
> +    uint32_t apb1div;
> +
> +    uint32_t regs[MSF2_SYSREG_MMIO_SIZE / 4];
> +} MSF2SysregState;
> +
> +#endif /* HW_MSF2_SYSREG_H */
> --
> 2.5.0
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller Subbaraya Sundeep
@ 2017-07-05 18:18   ` Alistair Francis
  2017-07-07 12:03     ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2017-07-05 18:18 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

On Sun, Jul 2, 2017 at 9:45 PM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:
> Modelled Microsemi's Smartfusion2 SPI controller.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
> ---
>  hw/ssi/Makefile.objs     |   1 +
>  hw/ssi/mss-spi.c         | 414 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ssi/mss-spi.h |  62 +++++++
>  3 files changed, 477 insertions(+)
>  create mode 100644 hw/ssi/mss-spi.c
>  create mode 100644 include/hw/ssi/mss-spi.h
>
> diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
> index 487add2..f5bcc65 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) += mss-spi.o
>
>  obj-$(CONFIG_OMAP) += omap_spi.o
>  obj-$(CONFIG_IMX) += imx_spi.o
> diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
> new file mode 100644
> index 0000000..a572abc
> --- /dev/null
> +++ b/hw/ssi/mss-spi.c
> @@ -0,0 +1,414 @@
> +/*
> + * Block model of SPI controller present in
> + * Microsemi's SmartFusion2 and SmartFusion SoCs.
> + *
> + * 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 "hw/ssi/mss-spi.h"

Same comment as earlier patches.

> +
> +#ifndef MSS_SPI_ERR_DEBUG
> +#define MSS_SPI_ERR_DEBUG   0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> +    if (MSS_SPI_ERR_DEBUG >= lvl) { \
> +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> +    } \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +#define FIFO_CAPACITY         32
> +#define FIFO_CAPACITY         32
> +
> +#define R_SPI_CONTROL         0
> +#define R_SPI_DFSIZE          1
> +#define R_SPI_STATUS          2
> +#define R_SPI_INTCLR          3
> +#define R_SPI_RX              4
> +#define R_SPI_TX              5
> +#define R_SPI_CLKGEN          6
> +#define R_SPI_SS              7
> +#define R_SPI_MIS             8
> +#define R_SPI_RIS             9
> +
> +#define S_TXDONE             (1 << 0)
> +#define S_RXRDY              (1 << 1)
> +#define S_RXCHOVRF           (1 << 2)
> +#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
> +
> +static void txfifo_reset(MSSSpiState *s)
> +{
> +    fifo32_reset(&s->tx_fifo);
> +
> +    s->regs[R_SPI_STATUS] &= ~S_TXFIFOFUL;
> +    s->regs[R_SPI_STATUS] |= S_TXFIFOEMP;
> +}
> +
> +static void rxfifo_reset(MSSSpiState *s)
> +{
> +    fifo32_reset(&s->rx_fifo);
> +
> +    s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
> +    s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
> +}
> +
> +static void set_fifodepth(MSSSpiState *s)
> +{
> +    unsigned int size = s->regs[R_SPI_DFSIZE] & FRAMESZ_MASK;
> +
> +    if (size <= 8) {
> +        s->fifo_depth = 32;
> +    } else if (size <= 16) {
> +        s->fifo_depth = 16;
> +    } else if (size <= 32) {
> +        s->fifo_depth = 8;
> +    } else {
> +        s->fifo_depth = 4;
> +    }
> +}
> +
> +static void mss_spi_do_reset(MSSSpiState *s)
> +{
> +    memset(s->regs, 0, sizeof s->regs);
> +    s->regs[R_SPI_CONTROL] = 0x80000102;
> +    s->regs[R_SPI_DFSIZE] = 0x4;
> +    s->regs[R_SPI_STATUS] = S_SSEL | S_TXFIFOEMP | S_RXFIFOEMP;
> +    s->regs[R_SPI_CLKGEN] = 0x7;
> +    s->regs[R_SPI_RIS] = 0x0;
> +
> +    s->fifo_depth = 4;
> +    s->frame_count = 1;
> +    s->enabled = false;
> +
> +    rxfifo_reset(s);
> +    txfifo_reset(s);
> +}
> +
> +static void update_mis(MSSSpiState *s)
> +{
> +    uint32_t reg = s->regs[R_SPI_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_SPI_MIS] |= tmp & s->regs[R_SPI_RIS];
> +}
> +
> +static void spi_update_irq(MSSSpiState *s)
> +{
> +    int irq;
> +
> +    update_mis(s);
> +    irq = !!(s->regs[R_SPI_MIS]);
> +
> +    qemu_set_irq(s->irq, irq);
> +}
> +
> +static void mss_spi_reset(DeviceState *d)
> +{
> +    mss_spi_do_reset(MSS_SPI(d));
> +}
> +
> +static uint64_t
> +spi_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    MSSSpiState *s = opaque;
> +    uint32_t ret = 0;
> +
> +    addr >>= 2;
> +    switch (addr) {
> +    case R_SPI_RX:
> +        s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
> +        s->regs[R_SPI_STATUS] &= ~S_RXCHOVRF;
> +        ret = fifo32_pop(&s->rx_fifo);
> +        if (fifo32_is_empty(&s->rx_fifo)) {
> +            s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
> +        }
> +        break;
> +
> +    case R_SPI_MIS:
> +        update_mis(s);
> +        ret = s->regs[R_SPI_MIS];
> +        break;
> +
> +    default:
> +        if (addr < ARRAY_SIZE(s->regs)) {
> +            ret = s->regs[addr];
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                         "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__,
> +                         addr * 4);

I think return here to avoid the next print statement and irq update.

> +        }
> +        break;
> +    }
> +
> +    DB_PRINT("addr=0x%" HWADDR_PRIx " = 0x%" PRIx32, addr * 4, ret);
> +    spi_update_irq(s);
> +    return ret;
> +}
> +
> +static void assert_cs(MSSSpiState *s)
> +{
> +    qemu_set_irq(s->cs_line, 0);
> +}
> +
> +static void deassert_cs(MSSSpiState *s)
> +{
> +    qemu_set_irq(s->cs_line, 1);
> +}
> +
> +static void spi_flush_txfifo(MSSSpiState *s)
> +{
> +    uint32_t tx;
> +    uint32_t rx;
> +    bool sps = !!(s->regs[R_SPI_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_SPI_STATUS] &= ~(S_TXDONE | S_RXRDY);
> +
> +        tx = fifo32_pop(&s->tx_fifo);
> +        DB_PRINT("data tx:0x%" PRIx32, tx);
> +        rx = ssi_transfer(s->spi, tx);
> +        DB_PRINT("data rx:0x%" PRIx32, rx);
> +
> +        if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
> +            s->regs[R_SPI_STATUS] |= S_RXCHOVRF;
> +            s->regs[R_SPI_RIS] |= S_RXCHOVRF;
> +        } else {
> +            fifo32_push(&s->rx_fifo, rx);
> +            s->regs[R_SPI_STATUS] &= ~S_RXFIFOEMP;
> +            if (fifo32_num_used(&s->rx_fifo) == (s->fifo_depth - 1)) {
> +                s->regs[R_SPI_STATUS] |= S_RXFIFOFULNXT;
> +            }
> +            if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {

I'd use an else if to indicate that both can't happen at the same time.

> +                s->regs[R_SPI_STATUS] |= S_RXFIFOFUL;
> +            }
> +        }
> +        s->frame_count--;
> +        if (!sps) {
> +            deassert_cs(s);
> +            assert_cs(s);

Do you need the asert here? Can't you just wait for it to hit the
start of the loop?

Does it deassert assert and deassert after a transfer has finished?

> +        }
> +    }
> +
> +    if (!sps) {
> +        deassert_cs(s);
> +    }
> +
> +    if (!s->frame_count) {
> +        s->frame_count = (s->regs[R_SPI_CONTROL] & FMCOUNT_MASK) >>
> +                            FMCOUNT_SHIFT;
> +        if (sps) {
> +            deassert_cs(s);
> +        }
> +        s->regs[R_SPI_RIS] |= S_TXDONE | S_RXRDY;
> +        s->regs[R_SPI_STATUS] |= S_TXDONE | S_RXRDY;
> +   }
> +}
> +
> +static void spi_write(void *opaque, hwaddr addr,
> +            uint64_t val64, unsigned int size)
> +{
> +    MSSSpiState *s = opaque;
> +    uint32_t value = val64;
> +
> +    DB_PRINT("addr=0x%" HWADDR_PRIx " =0x%" PRIx32, addr, value);
> +    addr >>= 2;
> +
> +    switch (addr) {
> +    case R_SPI_TX:
> +        /* adding to already full FIFO */
> +        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
> +            break;
> +        }
> +        s->regs[R_SPI_STATUS] &= ~S_TXFIFOEMP;
> +        fifo32_push(&s->tx_fifo, value);
> +        if (fifo32_num_used(&s->tx_fifo) == (s->fifo_depth - 1)) {
> +            s->regs[R_SPI_STATUS] |= S_TXFIFOFULNXT;
> +        }

else if here as well

Thanks,
Alistair

> +        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
> +            s->regs[R_SPI_STATUS] |= S_TXFIFOFUL;
> +        }
> +        if (s->enabled) {
> +            spi_flush_txfifo(s);
> +        }
> +        break;
> +
> +    case R_SPI_CONTROL:
> +        s->regs[R_SPI_CONTROL] = value;
> +        if (value & C_BIGFIFO) {
> +            set_fifodepth(s);
> +        } else {
> +            s->fifo_depth = 4;
> +        }
> +        s->enabled = value & C_ENABLE;
> +        s->frame_count = (value & FMCOUNT_MASK) >> FMCOUNT_SHIFT;
> +        if (value & C_RESET) {
> +            mss_spi_do_reset(s);
> +        }
> +        break;
> +
> +    case R_SPI_DFSIZE:
> +        if (s->enabled) {
> +            break;
> +        }
> +        s->regs[R_SPI_DFSIZE] = value;
> +        break;
> +
> +    case R_SPI_INTCLR:
> +        s->regs[R_SPI_INTCLR] = value;
> +        if (value & S_TXDONE) {
> +            s->regs[R_SPI_RIS] &= ~S_TXDONE;
> +        }
> +        if (value & S_RXRDY) {
> +            s->regs[R_SPI_RIS] &= ~S_RXRDY;
> +        }
> +        if (value & S_RXCHOVRF) {
> +            s->regs[R_SPI_RIS] &= ~S_RXCHOVRF;
> +        }
> +        break;
> +
> +    case R_SPI_MIS:
> +    case R_SPI_STATUS:
> +    case R_SPI_RIS:
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                         "%s: Write to read only register 0x%" HWADDR_PRIx "\n",
> +                         __func__, addr * 4);
> +        break;
> +
> +    default:
> +        if (addr < ARRAY_SIZE(s->regs)) {
> +            s->regs[addr] = value;
> +        } else {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                         "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__,
> +                         addr * 4);
> +        }
> +        break;
> +    }
> +
> +    spi_update_irq(s);
> +}
> +
> +static const MemoryRegionOps spi_ops = {
> +    .read = spi_read,
> +    .write = spi_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 4
> +    }
> +};
> +
> +static void mss_spi_realize(DeviceState *dev, Error **errp)
> +{
> +    MSSSpiState *s = MSS_SPI(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    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,
> +                          TYPE_MSS_SPI, R_SPI_MAX * 4);
> +    sysbus_init_mmio(sbd, &s->mmio);
> +
> +    fifo32_create(&s->tx_fifo, FIFO_CAPACITY);
> +    fifo32_create(&s->rx_fifo, FIFO_CAPACITY);
> +}
> +
> +static const VMStateDescription vmstate_mss_spi = {
> +    .name = TYPE_MSS_SPI,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_FIFO32(tx_fifo, MSSSpiState),
> +        VMSTATE_FIFO32(rx_fifo, MSSSpiState),
> +        VMSTATE_UINT32_ARRAY(regs, MSSSpiState, R_SPI_MAX),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static void mss_spi_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = mss_spi_realize;
> +    dc->reset = mss_spi_reset;
> +    dc->vmsd = &vmstate_mss_spi;
> +}
> +
> +static const TypeInfo mss_spi_info = {
> +    .name           = TYPE_MSS_SPI,
> +    .parent         = TYPE_SYS_BUS_DEVICE,
> +    .instance_size  = sizeof(MSSSpiState),
> +    .class_init     = mss_spi_class_init,
> +};
> +
> +static void mss_spi_register_types(void)
> +{
> +    type_register_static(&mss_spi_info);
> +}
> +
> +type_init(mss_spi_register_types)
> diff --git a/include/hw/ssi/mss-spi.h b/include/hw/ssi/mss-spi.h
> new file mode 100644
> index 0000000..6c8c0b1
> --- /dev/null
> +++ b/include/hw/ssi/mss-spi.h
> @@ -0,0 +1,62 @@
> +/*
> + * Microsemi SmartFusion2 SPI
> + *
> + * 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.
> + */
> +
> +#ifndef HW_MSS_SPI_H
> +#define HW_MSS_SPI_H
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "hw/hw.h"
> +#include "hw/ssi/ssi.h"
> +#include "qemu/fifo32.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/log.h"
> +
> +#define TYPE_MSS_SPI   "mss-spi"
> +#define MSS_SPI(obj)   OBJECT_CHECK(MSSSpiState, (obj), TYPE_MSS_SPI)
> +
> +#define R_SPI_MAX             16
> +
> +typedef struct MSSSpiState {
> +    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_SPI_MAX];
> +} MSSSpiState;
> +
> +#endif /* HW_MSS_SPI_H */
> --
> 2.5.0
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC.
  2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC Subbaraya Sundeep
@ 2017-07-05 18:25   ` Alistair Francis
  2017-07-07  7:09     ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2017-07-05 18:25 UTC (permalink / raw)
  To: Subbaraya Sundeep
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

On Sun, Jul 2, 2017 at 9:45 PM, Subbaraya Sundeep
<sundeep.lkml@gmail.com> wrote:

The patch title shouldn't end in a full stop.

> Smartfusion2 SoC has hardened Microcontroller subsystem
> and flash based FPGA fabric. This patch adds support for
> Microcontroller subsystem in the SoC.
>
> Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>

Once you have fixed up the title.

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,
Alistair


> ---
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs            |   1 +
>  hw/arm/msf2-soc.c               | 216 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/msf2-soc.h       |  67 +++++++++++++
>  4 files changed, 285 insertions(+)
>  create mode 100644 hw/arm/msf2-soc.c
>  create mode 100644 include/hw/arm/msf2-soc.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 78d7af0..7062512 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -122,3 +122,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..c828061 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -18,3 +18,4 @@ obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
>  obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
>  obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
> +obj-$(CONFIG_MSF2) += msf2-soc.o
> diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> new file mode 100644
> index 0000000..d45827f
> --- /dev/null
> +++ b/hw/arm/msf2-soc.c
> @@ -0,0 +1,216 @@
> +/*
> + * SmartFusion2 SoC 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/char/serial.h"
> +#include "hw/boards.h"
> +#include "sysemu/block-backend.h"
> +#include "hw/arm/msf2-soc.h"
> +
> +#define MSF2_TIMER_BASE     0x40004000
> +#define MSF2_SYSREG_BASE    0x40038000
> +
> +#define ENVM_BASE_ADDRESS     0x60000000
> +
> +#define SRAM_BASE_ADDRESS     0x20000000
> +
> +#define MSF2_ENVM_SIZE        (512 * K_BYTE)
> +#define MSF2_ESRAM_SIZE       (64 * K_BYTE)
> +
> +static const uint32_t spi_addr[MSF2_NUM_SPIS] = { 0x40001000 , 0x40011000 };
> +static const uint32_t uart_addr[MSF2_NUM_UARTS] = { 0x40000000 , 0x40010000 };
> +
> +static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 };
> +static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 };
> +static const int timer_irq[MSF2_NUM_TIMERS] = { 14, 15 };
> +
> +static void m2sxxx_soc_initfn(Object *obj)
> +{
> +    MSF2State *s = MSF2_SOC(obj);
> +    int i;
> +
> +    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
> +    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
> +
> +    object_initialize(&s->sysreg, sizeof(s->sysreg), TYPE_MSF2_SYSREG);
> +    qdev_set_parent_bus(DEVICE(&s->sysreg), sysbus_get_default());
> +
> +    object_initialize(&s->timer, sizeof(s->timer), TYPE_MSS_TIMER);
> +    qdev_set_parent_bus(DEVICE(&s->timer), sysbus_get_default());
> +
> +    for (i = 0; i < MSF2_NUM_SPIS; i++) {
> +        object_initialize(&s->spi[i], sizeof(s->spi[i]),
> +                          TYPE_MSS_SPI);
> +        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
> +    }
> +}
> +
> +static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
> +{
> +    MSF2State *s = MSF2_SOC(dev_soc);
> +    DeviceState *dev, *armv7m;
> +    SysBusDevice *busdev;
> +    Error *err = NULL;
> +    int i;
> +
> +    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);
> +
> +    memory_region_init_ram(nvm, NULL, "MSF2.eNVM", s->envm_size,
> +                           &error_fatal);
> +
> +    /*
> +     * On power-on, the eNVM region 0x60000000 is automatically
> +     * remapped to the Cortex-M3 processor executable region
> +     * start address (0x0). We do not support remapping other eNVM,
> +     * eSRAM and DDR regions by guest(via Sysreg) currently.
> +     */
> +    memory_region_init_alias(nvm_alias, NULL, "MSF2.eNVM.alias",
> +                             nvm, 0, s->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(sram, NULL, "MSF2.eSRAM", s->esram_size,
> +                           &error_fatal);
> +    vmstate_register_ram_global(sram);
> +    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
> +
> +    armv7m = DEVICE(&s->armv7m);
> +    qdev_prop_set_uint32(armv7m, "num-irq", 81);
> +    qdev_prop_set_string(armv7m, "cpu-model", "cortex-m3");
> +    object_property_set_link(OBJECT(&s->armv7m), OBJECT(get_system_memory()),
> +                                     "memory", &error_abort);
> +    object_property_set_bool(OBJECT(&s->armv7m), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
> +
> +    for (i = 0; i < MSF2_NUM_UARTS; i++) {
> +        if (serial_hds[i]) {
> +            serial_mm_init(get_system_memory(), uart_addr[i], 2,
> +                           qdev_get_gpio_in(armv7m, uart_irq[i]),
> +                           115200, serial_hds[i], DEVICE_NATIVE_ENDIAN);
> +        }
> +    }
> +
> +    dev = DEVICE(&s->timer);
> +    /* APB0 clock is the timer input clock */
> +    qdev_prop_set_uint32(dev, "clock-frequency", s->m3clk / s->apb0div);
> +    object_property_set_bool(OBJECT(&s->timer), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, MSF2_TIMER_BASE);
> +    sysbus_connect_irq(busdev, 0,
> +                           qdev_get_gpio_in(armv7m, timer_irq[0]));
> +    sysbus_connect_irq(busdev, 1,
> +                           qdev_get_gpio_in(armv7m, timer_irq[1]));
> +
> +    dev = DEVICE(&s->sysreg);
> +    qdev_prop_set_uint32(dev, "apb0divisor", s->apb0div);
> +    qdev_prop_set_uint32(dev, "apb1divisor", s->apb1div);
> +    object_property_set_bool(OBJECT(&s->sysreg), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    busdev = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(busdev, 0, MSF2_SYSREG_BASE);
> +
> +    for (i = 0; i < MSF2_NUM_SPIS; i++) {
> +        gchar *bus_name = g_strdup_printf("spi%d", i);
> +
> +        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized", &err);
> +        if (err != NULL) {
> +            g_free(bus_name);
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
> +                           qdev_get_gpio_in(armv7m, spi_irq[i]));
> +
> +        /* Alias controller SPI bus to the SoC itself */
> +        object_property_add_alias(OBJECT(s), bus_name,
> +                                  OBJECT(&s->spi[i]), "spi",
> +                                  &error_abort);
> +        g_free(bus_name);
> +    }
> +}
> +
> +static Property m2sxxx_soc_properties[] = {
> +    /*
> +     * part name specifies the type of SmartFusion2 device variant(this
> +     * property is for information purpose only.
> +     */
> +    DEFINE_PROP_STRING("part-name", MSF2State, part_name),
> +    DEFINE_PROP_UINT64("eNVM-size", MSF2State, envm_size, MSF2_ENVM_SIZE),
> +    DEFINE_PROP_UINT64("eSRAM-size", MSF2State, esram_size, MSF2_ESRAM_SIZE),
> +    /* Libero GUI shows 100Mhz as default for clocks */
> +    DEFINE_PROP_UINT32("m3clk", MSF2State, m3clk, 100 * 1000000),
> +    /* default divisors in Libero GUI */
> +    DEFINE_PROP_UINT32("apb0div", MSF2State, apb0div, 2),
> +    DEFINE_PROP_UINT32("apb1div", MSF2State, apb1div, 2),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void m2sxxx_soc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = m2sxxx_soc_realize;
> +    dc->props = m2sxxx_soc_properties;
> +}
> +
> +static const TypeInfo m2sxxx_soc_info = {
> +    .name          = TYPE_MSF2_SOC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(MSF2State),
> +    .instance_init = m2sxxx_soc_initfn,
> +    .class_init    = m2sxxx_soc_class_init,
> +};
> +
> +static void m2sxxx_soc_types(void)
> +{
> +    type_register_static(&m2sxxx_soc_info);
> +}
> +
> +type_init(m2sxxx_soc_types)
> diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
> new file mode 100644
> index 0000000..9ed677a
> --- /dev/null
> +++ b/include/hw/arm/msf2-soc.h
> @@ -0,0 +1,67 @@
> +/*
> + * Microsemi Smartfusion2 SoC
> + *
> + * 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.
> + */
> +
> +#ifndef HW_ARM_MSF2_SOC_H
> +#define HW_ARM_MSF2_SOC_H
> +
> +#include "hw/misc/msf2-sysreg.h"
> +#include "hw/timer/mss-timer.h"
> +#include "hw/ssi/mss-spi.h"
> +#include "hw/arm/armv7m.h"
> +#include "qemu/cutils.h"
> +
> +#define TYPE_MSF2_SOC     "msf2-soc"
> +#define MSF2_SOC(obj)     OBJECT_CHECK(MSF2State, (obj), TYPE_MSF2_SOC)
> +
> +#define MSF2_NUM_SPIS         2
> +#define MSF2_NUM_UARTS        2
> +
> +/*
> + * System timer consists of two programmable 32-bit
> + * decrementing counters that generate individual interrupts to
> + * the Cortex-M3 processor
> + */
> +#define MSF2_NUM_TIMERS       2
> +
> +typedef struct MSF2State {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    ARMv7MState armv7m;
> +
> +    char *part_name;
> +    uint64_t envm_size;
> +    uint64_t esram_size;
> +
> +    uint32_t m3clk;
> +    uint32_t apb0div;
> +    uint32_t apb1div;
> +
> +    MSF2SysregState sysreg;
> +    MSSTimerState timer;
> +    MSSSpiState spi[MSF2_NUM_SPIS];
> +} MSF2State;
> +
> +#endif
> --
> 2.5.0
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer
  2017-07-05 17:56   ` Alistair Francis
  2017-07-05 17:58     ` Alistair Francis
@ 2017-07-07  6:59     ` sundeep subbaraya
  1 sibling, 0 replies; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-07  6:59 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

Hi Alistair,

On Wed, Jul 5, 2017 at 11:26 PM, Alistair Francis <alistair23@gmail.com>
wrote:

> On Sun, Jul 2, 2017 at 9:45 PM, 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>
> > ---
> >  hw/timer/Makefile.objs       |   1 +
> >  hw/timer/mss-timer.c         | 261 ++++++++++++++++++++++++++++++
> +++++++++++++
> >  include/hw/timer/mss-timer.h |  67 +++++++++++
> >  3 files changed, 329 insertions(+)
> >  create mode 100644 hw/timer/mss-timer.c
> >  create mode 100644 include/hw/timer/mss-timer.h
> >
> > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> > index dd6f27e..fc4d2da 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) += mss-timer.o
> > diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
> > new file mode 100644
> > index 0000000..e46d118
> > --- /dev/null
> > +++ b/hw/timer/mss-timer.c
> > @@ -0,0 +1,261 @@
> > +/*
> > + * Block model of System timer present in
> > + * Microsemi's SmartFusion2 and SmartFusion SoCs.
> > + *
> > + * 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 "hw/timer/mss-timer.h"
> > +
> > +#ifndef MSS_TIMER_ERR_DEBUG
> > +#define MSS_TIMER_ERR_DEBUG  0
> > +#endif
> > +
> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
> > +    if (MSS_TIMER_ERR_DEBUG >= lvl) { \
> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> > +    } \
> > +} while (0);
> > +
> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> > +
> > +#define R_TIM_VAL         0
> > +#define R_TIM_LOADVAL     1
> > +#define R_TIM_BGLOADVAL   2
> > +#define R_TIM_CTRL        3
> > +#define R_TIM_RIS         4
> > +#define R_TIM_MIS         5
> > +
> > +#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)
> > +#define TIMER_MODE          (1 << 0)
> > +
> > +static void timer_update_irq(struct Msf2Timer *st)
> > +{
> > +    bool isr, ier;
> > +
> > +    isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
> > +    ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
> > +    qemu_set_irq(st->irq, (ier && isr));
> > +}
> > +
> > +static void timer_update(struct Msf2Timer *st)
> > +{
> > +    uint64_t count;
> > +
> > +    if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL)) {
> > +        ptimer_stop(st->ptimer);
> > +        return;
> > +    }
> > +
> > +    count = st->regs[R_TIM_LOADVAL];
> > +    ptimer_set_limit(st->ptimer, count, 1);
> > +    ptimer_run(st->ptimer, 1);
> > +}
> > +
> > +static uint64_t
> > +timer_read(void *opaque, hwaddr offset, unsigned int size)
> > +{
> > +    MSSTimerState *t = opaque;
> > +    hwaddr addr;
> > +    struct Msf2Timer *st;
> > +    uint32_t ret = 0;
> > +    int timer = 0;
> > +    int isr;
> > +    int ier;
> > +
> > +    addr = offset >> 2;
> > +    /*
> > +     * Two independent timers has same base address.
> > +     * Based on address passed figure out which timer is being used.
> > +     */
> > +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
> > +        timer = 1;
> > +        addr -= R_TIM1_MAX;
> > +    }
> > +
> > +    st = &t->timers[timer];
> > +
> > +    switch (addr) {
> > +    case R_TIM_VAL:
> > +        ret = ptimer_get_count(st->ptimer);
> > +        break;
> > +
> > +    case R_TIM_MIS:
> > +        isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
> > +        ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
> > +        ret = ier & isr;
> > +        break;
> > +
> > +    default:
> > +        if (addr < NUM_TIMERS * R_TIM1_MAX) {
>
> Shouldn't this just be: addr < R_TIM1_MAX?


> At this point you have already subjected the offset for multiple timers.
>

Yes. You are right. NUM_TIMERS * R_TIM1_MAX will also work but offsets are
already changed so I will change to  addr < R_TIM1_MAX.


> > +            ret = st->regs[addr];
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
>
> Maybe add a return here so you don't print the other debug message after
> this.
>
> Ok I will change.

> > +        }
> > +        break;
> > +    }
> > +
> > +    DB_PRINT("timer=%d 0x%" HWADDR_PRIx "=0x%" PRIx32, timer, offset,
> > +            ret);
> > +    return ret;
> > +}
> > +
> > +static void
> > +timer_write(void *opaque, hwaddr offset,
> > +            uint64_t val64, unsigned int size)
> > +{
> > +    MSSTimerState *t = opaque;
> > +    hwaddr addr;
> > +    struct Msf2Timer *st;
> > +    int timer = 0;
> > +    uint32_t value = val64;
> > +
> > +    addr = offset >> 2;
> > +    /*
> > +     * Two independent timers has same base address.
> > +     * Based on addr passed figure out which timer is being used.
> > +     */
> > +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
> > +        timer = 1;
> > +        addr -= R_TIM1_MAX;
> > +    }
> > +
> > +    st = &t->timers[timer];
> > +
> > +    DB_PRINT("addr=0x%" HWADDR_PRIx " val=0x%" PRIx32 " (timer=%d)",
> offset,
> > +            value, timer);
> > +
> > +    switch (addr) {
> > +    case R_TIM_CTRL:
> > +        st->regs[R_TIM_CTRL] = value;
> > +        timer_update(st);
> > +        break;
> > +
> > +    case R_TIM_RIS:
> > +        if (value & TIMER_RIS_ACK) {
> > +            st->regs[R_TIM_RIS] &= ~TIMER_RIS_ACK;
> > +        }
> > +        break;
> > +
> > +    case R_TIM_LOADVAL:
> > +        st->regs[R_TIM_LOADVAL] = value;
> > +        if (st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL) {
> > +            timer_update(st);
> > +        }
> > +        break;
> > +
> > +    case R_TIM_BGLOADVAL:
> > +        st->regs[R_TIM_BGLOADVAL] = value;
> > +        st->regs[R_TIM_LOADVAL] = value;
> > +        break;
> > +
> > +    case R_TIM_VAL:
> > +    case R_TIM_MIS:
> > +        break;
> > +
> > +    default:
> > +        if (addr < NUM_TIMERS * R_TIM1_MAX) {
>
> Same comment as above.
>

Ok.

>
> > +            st->regs[addr] = value;
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
> > +            return;
> > +        }
> > +        break;
> > +    }
> > +    timer_update_irq(st);
> > +}
>
> Once those comments above are addressed:
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>

Thank you.
Sundeep


>
> Thanks,
> Alistair
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer
  2017-07-05 17:58     ` Alistair Francis
@ 2017-07-07  7:00       ` sundeep subbaraya
  0 siblings, 0 replies; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-07  7:00 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

Hi Alistair,

On Wed, Jul 5, 2017 at 11:28 PM, Alistair Francis <alistair23@gmail.com>
wrote:

> On Wed, Jul 5, 2017 at 10:56 AM, Alistair Francis <alistair23@gmail.com>
> wrote:
> > On Sun, Jul 2, 2017 at 9:45 PM, 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>
> >> ---
> >>  hw/timer/Makefile.objs       |   1 +
> >>  hw/timer/mss-timer.c         | 261 ++++++++++++++++++++++++++++++
> +++++++++++++
> >>  include/hw/timer/mss-timer.h |  67 +++++++++++
> >>  3 files changed, 329 insertions(+)
> >>  create mode 100644 hw/timer/mss-timer.c
> >>  create mode 100644 include/hw/timer/mss-timer.h
> >>
> >> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> >> index dd6f27e..fc4d2da 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) += mss-timer.o
> >> diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
> >> new file mode 100644
> >> index 0000000..e46d118
> >> --- /dev/null
> >> +++ b/hw/timer/mss-timer.c
> >> @@ -0,0 +1,261 @@
> >> +/*
> >> + * Block model of System timer present in
> >> + * Microsemi's SmartFusion2 and SmartFusion SoCs.
> >> + *
> >> + * 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 "hw/timer/mss-timer.h"
>
> Also just noticed (this applies to the whole series) that you need the
> headers in the C file.
>
> Move every #include that you can from the mss-timer.h files to this
> file, headers should be local to where they are called. Also make sure
> you include the os-dep header in every file.
>

Ok will fix this.

Thanks,
Sundeep


>
> Thanks,
> Alistair
>
> >> +
> >> +#ifndef MSS_TIMER_ERR_DEBUG
> >> +#define MSS_TIMER_ERR_DEBUG  0
> >> +#endif
> >> +
> >> +#define DB_PRINT_L(lvl, fmt, args...) do { \
> >> +    if (MSS_TIMER_ERR_DEBUG >= lvl) { \
> >> +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> >> +    } \
> >> +} while (0);
> >> +
> >> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> >> +
> >> +#define R_TIM_VAL         0
> >> +#define R_TIM_LOADVAL     1
> >> +#define R_TIM_BGLOADVAL   2
> >> +#define R_TIM_CTRL        3
> >> +#define R_TIM_RIS         4
> >> +#define R_TIM_MIS         5
> >> +
> >> +#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)
> >> +#define TIMER_MODE          (1 << 0)
> >> +
> >> +static void timer_update_irq(struct Msf2Timer *st)
> >> +{
> >> +    bool isr, ier;
> >> +
> >> +    isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
> >> +    ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
> >> +    qemu_set_irq(st->irq, (ier && isr));
> >> +}
> >> +
> >> +static void timer_update(struct Msf2Timer *st)
> >> +{
> >> +    uint64_t count;
> >> +
> >> +    if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL)) {
> >> +        ptimer_stop(st->ptimer);
> >> +        return;
> >> +    }
> >> +
> >> +    count = st->regs[R_TIM_LOADVAL];
> >> +    ptimer_set_limit(st->ptimer, count, 1);
> >> +    ptimer_run(st->ptimer, 1);
> >> +}
> >> +
> >> +static uint64_t
> >> +timer_read(void *opaque, hwaddr offset, unsigned int size)
> >> +{
> >> +    MSSTimerState *t = opaque;
> >> +    hwaddr addr;
> >> +    struct Msf2Timer *st;
> >> +    uint32_t ret = 0;
> >> +    int timer = 0;
> >> +    int isr;
> >> +    int ier;
> >> +
> >> +    addr = offset >> 2;
> >> +    /*
> >> +     * Two independent timers has same base address.
> >> +     * Based on address passed figure out which timer is being used.
> >> +     */
> >> +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
> >> +        timer = 1;
> >> +        addr -= R_TIM1_MAX;
> >> +    }
> >> +
> >> +    st = &t->timers[timer];
> >> +
> >> +    switch (addr) {
> >> +    case R_TIM_VAL:
> >> +        ret = ptimer_get_count(st->ptimer);
> >> +        break;
> >> +
> >> +    case R_TIM_MIS:
> >> +        isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
> >> +        ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
> >> +        ret = ier & isr;
> >> +        break;
> >> +
> >> +    default:
> >> +        if (addr < NUM_TIMERS * R_TIM1_MAX) {
> >
> > Shouldn't this just be: addr < R_TIM1_MAX?
> >
> > At this point you have already subjected the offset for multiple timers.
> >
> >> +            ret = st->regs[addr];
> >> +        } else {
> >> +            qemu_log_mask(LOG_GUEST_ERROR,
> >> +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
> >
> > Maybe add a return here so you don't print the other debug message after
> this.
> >
> >> +        }
> >> +        break;
> >> +    }
> >> +
> >> +    DB_PRINT("timer=%d 0x%" HWADDR_PRIx "=0x%" PRIx32, timer, offset,
> >> +            ret);
> >> +    return ret;
> >> +}
> >> +
> >> +static void
> >> +timer_write(void *opaque, hwaddr offset,
> >> +            uint64_t val64, unsigned int size)
> >> +{
> >> +    MSSTimerState *t = opaque;
> >> +    hwaddr addr;
> >> +    struct Msf2Timer *st;
> >> +    int timer = 0;
> >> +    uint32_t value = val64;
> >> +
> >> +    addr = offset >> 2;
> >> +    /*
> >> +     * Two independent timers has same base address.
> >> +     * Based on addr passed figure out which timer is being used.
> >> +     */
> >> +    if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
> >> +        timer = 1;
> >> +        addr -= R_TIM1_MAX;
> >> +    }
> >> +
> >> +    st = &t->timers[timer];
> >> +
> >> +    DB_PRINT("addr=0x%" HWADDR_PRIx " val=0x%" PRIx32 " (timer=%d)",
> offset,
> >> +            value, timer);
> >> +
> >> +    switch (addr) {
> >> +    case R_TIM_CTRL:
> >> +        st->regs[R_TIM_CTRL] = value;
> >> +        timer_update(st);
> >> +        break;
> >> +
> >> +    case R_TIM_RIS:
> >> +        if (value & TIMER_RIS_ACK) {
> >> +            st->regs[R_TIM_RIS] &= ~TIMER_RIS_ACK;
> >> +        }
> >> +        break;
> >> +
> >> +    case R_TIM_LOADVAL:
> >> +        st->regs[R_TIM_LOADVAL] = value;
> >> +        if (st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL) {
> >> +            timer_update(st);
> >> +        }
> >> +        break;
> >> +
> >> +    case R_TIM_BGLOADVAL:
> >> +        st->regs[R_TIM_BGLOADVAL] = value;
> >> +        st->regs[R_TIM_LOADVAL] = value;
> >> +        break;
> >> +
> >> +    case R_TIM_VAL:
> >> +    case R_TIM_MIS:
> >> +        break;
> >> +
> >> +    default:
> >> +        if (addr < NUM_TIMERS * R_TIM1_MAX) {
> >
> > Same comment as above.
> >
> >> +            st->regs[addr] = value;
> >> +        } else {
> >> +            qemu_log_mask(LOG_GUEST_ERROR,
> >> +                        TYPE_MSS_TIMER": 64-bit mode not supported\n");
> >> +            return;
> >> +        }
> >> +        break;
> >> +    }
> >> +    timer_update_irq(st);
> >> +}
> >
> > Once those comments above are addressed:
> >
> > Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
> >
> > Thanks,
> > Alistair
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-05 18:06   ` Alistair Francis
@ 2017-07-07  7:08     ` sundeep subbaraya
  2017-07-07 16:33       ` Alistair Francis
  0 siblings, 1 reply; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-07  7:08 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

Hi Alistair,

On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <alistair23@gmail.com>
wrote:

> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200 ++++++++++++++++++++++++++++++
> ++++++++++++
> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
> >  3 files changed, 283 insertions(+)
> >  create mode 100644 hw/misc/msf2-sysreg.c
> >  create mode 100644 include/hw/misc/msf2-sysreg.h
> >
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index c8b4893..0f52354 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..64ee141
> > --- /dev/null
> > +++ b/hw/misc/msf2-sysreg.c
> > @@ -0,0 +1,200 @@
> > +/*
> > + * 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 "hw/misc/msf2-sysreg.h"
>
> Same #include comment from patch 1.
>

Ok will change.

>
> > +
> > +#ifndef MSF2_SYSREG_ERR_DEBUG
> > +#define MSF2_SYSREG_ERR_DEBUG  0
> > +#endif
> > +
> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> > +    } \
> > +} while (0);
> > +
> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> > +
> > +static inline int msf2_divbits(uint32_t div)
> > +{
> > +    int ret = 0;
> > +
> > +    switch (div) {
> > +    case 1:
> > +        ret = 0;
> > +        break;
> > +    case 2:
> > +        ret = 1;
> > +        break;
> > +    case 4:
> > +        ret = 2;
> > +        break;
> > +    case 8:
> > +        ret = 4;
> > +        break;
> > +    case 16:
> > +        ret = 5;
> > +        break;
> > +    case 32:
> > +        ret = 6;
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +static void msf2_sysreg_reset(DeviceState *d)
> > +{
> > +    MSF2SysregState *s = MSF2_SYSREG(d);
> > +
> > +    DB_PRINT("RESET");
> > +
> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
> > +                               msf2_divbits(s->apb1div) << 2;
> > +}
> > +
> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
> > +    unsigned size)
> > +{
> > +    MSF2SysregState *s = opaque;
> > +    offset /= 4;
>
> Probably best to use a bitshift.
>

Ok will change.

>
> > +    uint32_t ret = 0;
> > +
> > +    if (offset < ARRAY_SIZE(s->regs)) {
> > +        ret = s->regs[offset];
> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
> > +                    offset * 4, ret);
>
> Bitshift here as well.
>

Ok will change.

>
> > +    } else {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
> > +                    offset * 4);
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
> > +                          uint64_t val, unsigned size)
> > +{
> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
> > +    uint32_t newval = val;
> > +    uint32_t oldval;
> > +
> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
> > +            offset, val);
> > +
> > +    offset /= 4;
>
> Same here


Ok will change

>
> > +
> > +    switch (offset) {
> > +    case MSSDDR_PLL_STATUS:
> > +        break;
> > +
> > +    case ESRAM_CR:
> > +        oldval = s->regs[ESRAM_CR];
> > +        if (oldval ^ newval) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
> supported\n");
> > +            abort();
>
> The guest should not be able to kill QEMU, a guest error should never
> result in an abort.
>

Philippe suggested to abort because:
If guest tries to remap since firmware do a remap, the code flow will be
completely wrong.
Reporting a GUEST_ERROR here is not enough since code flow continuing would
be
pretty hard to understand/debug.
We decided to abort for now.


> > +        }
> > +        break;
> > +
> > +    case DDR_CR:
> > +        oldval = s->regs[DDR_CR];
> > +        if (oldval ^ newval) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                       TYPE_MSF2_SYSREG": DDR remapping not
> supported\n");
> > +            abort();
> > +        }
> > +        break;
> > +
> > +    case ENVM_REMAP_BASE_CR:
> > +        oldval = s->regs[ENVM_REMAP_BASE_CR];
> > +        if (oldval ^ newval) {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                       TYPE_MSF2_SYSREG": eNVM remapping not
> supported\n");
> > +            abort();
> > +        }
> > +        break;
> > +
> > +    default:
> > +        if (offset < ARRAY_SIZE(s->regs)) {
> > +            s->regs[offset] = val;
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                        "%s: Bad offset 0x%08" HWADDR_PRIx "\n",
> __func__,
> > +                        offset * 4);
> > +        }
> > +        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)
> > +{
> > +    MSF2SysregState *s = MSF2_SYSREG(obj);
> > +
> > +    memory_region_init_io(&s->iomem, obj, &sysreg_ops, s,
> TYPE_MSF2_SYSREG,
> > +                          MSF2_SYSREG_MMIO_SIZE);
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> > +}
> > +
> > +static const VMStateDescription vmstate_msf2_sysreg = {
> > +    .name = TYPE_MSF2_SYSREG,
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINT32_ARRAY(regs, MSF2SysregState,
> MSF2_SYSREG_MMIO_SIZE / 4),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
>
> This just reminded me that I don't think your first patch has a
> VMState. Can you add that
>

Sure thanks for pointing it.

Thanks,
Sundeep


>
> Thanks,
> Alistair
>
> > +
> > +static Property msf2_sysreg_properties[] = {
> > +    /* default divisors in Libero GUI */
> > +    DEFINE_PROP_UINT32("apb0divisor", MSF2SysregState, apb0div, 2),
> > +    DEFINE_PROP_UINT32("apb1divisor", MSF2SysregState, apb1div, 2),
> > +    DEFINE_PROP_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;
> > +    dc->props = msf2_sysreg_properties;
> > +}
> > +
> > +static const TypeInfo msf2_sysreg_info = {
> > +    .name  = TYPE_MSF2_SYSREG,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .class_init = msf2_sysreg_class_init,
> > +    .instance_size  = sizeof(MSF2SysregState),
> > +    .instance_init = msf2_sysreg_init,
> > +};
> > +
> > +static void msf2_sysreg_register_types(void)
> > +{
> > +    type_register_static(&msf2_sysreg_info);
> > +}
> > +
> > +type_init(msf2_sysreg_register_types)
> > diff --git a/include/hw/misc/msf2-sysreg.h
> b/include/hw/misc/msf2-sysreg.h
> > new file mode 100644
> > index 0000000..5e9ea99
> > --- /dev/null
> > +++ b/include/hw/misc/msf2-sysreg.h
> > @@ -0,0 +1,82 @@
> > +/*
> > + * Microsemi SmartFusion2 SYSREG
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef HW_MSF2_SYSREG_H
> > +#define HW_MSF2_SYSREG_H
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/hw.h"
> > +#include "sysemu/sysemu.h"
> > +#include "qemu/log.h"
> > +
> > +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 TYPE_MSF2_SYSREG          "msf2-sysreg"
> > +#define MSF2_SYSREG(obj)  OBJECT_CHECK(MSF2SysregState, (obj),
> TYPE_MSF2_SYSREG)
> > +
> > +typedef struct MSF2SysregState {
> > +    SysBusDevice parent_obj;
> > +
> > +    MemoryRegion iomem;
> > +
> > +    uint32_t apb0div;
> > +    uint32_t apb1div;
> > +
> > +    uint32_t regs[MSF2_SYSREG_MMIO_SIZE / 4];
> > +} MSF2SysregState;
> > +
> > +#endif /* HW_MSF2_SYSREG_H */
> > --
> > 2.5.0
> >
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC.
  2017-07-05 18:25   ` Alistair Francis
@ 2017-07-07  7:09     ` sundeep subbaraya
  0 siblings, 0 replies; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-07  7:09 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

Hi Alistair,

On Wed, Jul 5, 2017 at 11:55 PM, Alistair Francis <alistair23@gmail.com>
wrote:

> On Sun, Jul 2, 2017 at 9:45 PM, Subbaraya Sundeep
> <sundeep.lkml@gmail.com> wrote:
>
> The patch title shouldn't end in a full stop.
>

Ok will remove .

>
> > Smartfusion2 SoC has hardened Microcontroller subsystem
> > and flash based FPGA fabric. This patch adds support for
> > Microcontroller subsystem in the SoC.
> >
> > Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
>
> Once you have fixed up the title.
>
> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
>

Thank you,
Sundeep

>
> Thanks,
> Alistair
>
>
> > ---
> >  default-configs/arm-softmmu.mak |   1 +
> >  hw/arm/Makefile.objs            |   1 +
> >  hw/arm/msf2-soc.c               | 216 ++++++++++++++++++++++++++++++
> ++++++++++
> >  include/hw/arm/msf2-soc.h       |  67 +++++++++++++
> >  4 files changed, 285 insertions(+)
> >  create mode 100644 hw/arm/msf2-soc.c
> >  create mode 100644 include/hw/arm/msf2-soc.h
> >
> > diff --git a/default-configs/arm-softmmu.mak
> b/default-configs/arm-softmmu.mak
> > index 78d7af0..7062512 100644
> > --- a/default-configs/arm-softmmu.mak
> > +++ b/default-configs/arm-softmmu.mak
> > @@ -122,3 +122,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..c828061 100644
> > --- a/hw/arm/Makefile.objs
> > +++ b/hw/arm/Makefile.objs
> > @@ -18,3 +18,4 @@ obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
> >  obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
> >  obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
> >  obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
> > +obj-$(CONFIG_MSF2) += msf2-soc.o
> > diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
> > new file mode 100644
> > index 0000000..d45827f
> > --- /dev/null
> > +++ b/hw/arm/msf2-soc.c
> > @@ -0,0 +1,216 @@
> > +/*
> > + * SmartFusion2 SoC 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/char/serial.h"
> > +#include "hw/boards.h"
> > +#include "sysemu/block-backend.h"
> > +#include "hw/arm/msf2-soc.h"
> > +
> > +#define MSF2_TIMER_BASE     0x40004000
> > +#define MSF2_SYSREG_BASE    0x40038000
> > +
> > +#define ENVM_BASE_ADDRESS     0x60000000
> > +
> > +#define SRAM_BASE_ADDRESS     0x20000000
> > +
> > +#define MSF2_ENVM_SIZE        (512 * K_BYTE)
> > +#define MSF2_ESRAM_SIZE       (64 * K_BYTE)
> > +
> > +static const uint32_t spi_addr[MSF2_NUM_SPIS] = { 0x40001000 ,
> 0x40011000 };
> > +static const uint32_t uart_addr[MSF2_NUM_UARTS] = { 0x40000000 ,
> 0x40010000 };
> > +
> > +static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 };
> > +static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 };
> > +static const int timer_irq[MSF2_NUM_TIMERS] = { 14, 15 };
> > +
> > +static void m2sxxx_soc_initfn(Object *obj)
> > +{
> > +    MSF2State *s = MSF2_SOC(obj);
> > +    int i;
> > +
> > +    object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
> > +    qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
> > +
> > +    object_initialize(&s->sysreg, sizeof(s->sysreg), TYPE_MSF2_SYSREG);
> > +    qdev_set_parent_bus(DEVICE(&s->sysreg), sysbus_get_default());
> > +
> > +    object_initialize(&s->timer, sizeof(s->timer), TYPE_MSS_TIMER);
> > +    qdev_set_parent_bus(DEVICE(&s->timer), sysbus_get_default());
> > +
> > +    for (i = 0; i < MSF2_NUM_SPIS; i++) {
> > +        object_initialize(&s->spi[i], sizeof(s->spi[i]),
> > +                          TYPE_MSS_SPI);
> > +        qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
> > +    }
> > +}
> > +
> > +static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
> > +{
> > +    MSF2State *s = MSF2_SOC(dev_soc);
> > +    DeviceState *dev, *armv7m;
> > +    SysBusDevice *busdev;
> > +    Error *err = NULL;
> > +    int i;
> > +
> > +    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);
> > +
> > +    memory_region_init_ram(nvm, NULL, "MSF2.eNVM", s->envm_size,
> > +                           &error_fatal);
> > +
> > +    /*
> > +     * On power-on, the eNVM region 0x60000000 is automatically
> > +     * remapped to the Cortex-M3 processor executable region
> > +     * start address (0x0). We do not support remapping other eNVM,
> > +     * eSRAM and DDR regions by guest(via Sysreg) currently.
> > +     */
> > +    memory_region_init_alias(nvm_alias, NULL, "MSF2.eNVM.alias",
> > +                             nvm, 0, s->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(sram, NULL, "MSF2.eSRAM", s->esram_size,
> > +                           &error_fatal);
> > +    vmstate_register_ram_global(sram);
> > +    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS,
> sram);
> > +
> > +    armv7m = DEVICE(&s->armv7m);
> > +    qdev_prop_set_uint32(armv7m, "num-irq", 81);
> > +    qdev_prop_set_string(armv7m, "cpu-model", "cortex-m3");
> > +    object_property_set_link(OBJECT(&s->armv7m),
> OBJECT(get_system_memory()),
> > +                                     "memory", &error_abort);
> > +    object_property_set_bool(OBJECT(&s->armv7m), true, "realized",
> &err);
> > +    if (err != NULL) {
> > +        error_propagate(errp, err);
> > +        return;
> > +    }
> > +
> > +    system_clock_scale = NANOSECONDS_PER_SECOND / s->m3clk;
> > +
> > +    for (i = 0; i < MSF2_NUM_UARTS; i++) {
> > +        if (serial_hds[i]) {
> > +            serial_mm_init(get_system_memory(), uart_addr[i], 2,
> > +                           qdev_get_gpio_in(armv7m, uart_irq[i]),
> > +                           115200, serial_hds[i], DEVICE_NATIVE_ENDIAN);
> > +        }
> > +    }
> > +
> > +    dev = DEVICE(&s->timer);
> > +    /* APB0 clock is the timer input clock */
> > +    qdev_prop_set_uint32(dev, "clock-frequency", s->m3clk / s->apb0div);
> > +    object_property_set_bool(OBJECT(&s->timer), true, "realized",
> &err);
> > +    if (err != NULL) {
> > +        error_propagate(errp, err);
> > +        return;
> > +    }
> > +    busdev = SYS_BUS_DEVICE(dev);
> > +    sysbus_mmio_map(busdev, 0, MSF2_TIMER_BASE);
> > +    sysbus_connect_irq(busdev, 0,
> > +                           qdev_get_gpio_in(armv7m, timer_irq[0]));
> > +    sysbus_connect_irq(busdev, 1,
> > +                           qdev_get_gpio_in(armv7m, timer_irq[1]));
> > +
> > +    dev = DEVICE(&s->sysreg);
> > +    qdev_prop_set_uint32(dev, "apb0divisor", s->apb0div);
> > +    qdev_prop_set_uint32(dev, "apb1divisor", s->apb1div);
> > +    object_property_set_bool(OBJECT(&s->sysreg), true, "realized",
> &err);
> > +    if (err != NULL) {
> > +        error_propagate(errp, err);
> > +        return;
> > +    }
> > +    busdev = SYS_BUS_DEVICE(dev);
> > +    sysbus_mmio_map(busdev, 0, MSF2_SYSREG_BASE);
> > +
> > +    for (i = 0; i < MSF2_NUM_SPIS; i++) {
> > +        gchar *bus_name = g_strdup_printf("spi%d", i);
> > +
> > +        object_property_set_bool(OBJECT(&s->spi[i]), true, "realized",
> &err);
> > +        if (err != NULL) {
> > +            g_free(bus_name);
> > +            error_propagate(errp, err);
> > +            return;
> > +        }
> > +
> > +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, spi_addr[i]);
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi[i]), 0,
> > +                           qdev_get_gpio_in(armv7m, spi_irq[i]));
> > +
> > +        /* Alias controller SPI bus to the SoC itself */
> > +        object_property_add_alias(OBJECT(s), bus_name,
> > +                                  OBJECT(&s->spi[i]), "spi",
> > +                                  &error_abort);
> > +        g_free(bus_name);
> > +    }
> > +}
> > +
> > +static Property m2sxxx_soc_properties[] = {
> > +    /*
> > +     * part name specifies the type of SmartFusion2 device variant(this
> > +     * property is for information purpose only.
> > +     */
> > +    DEFINE_PROP_STRING("part-name", MSF2State, part_name),
> > +    DEFINE_PROP_UINT64("eNVM-size", MSF2State, envm_size,
> MSF2_ENVM_SIZE),
> > +    DEFINE_PROP_UINT64("eSRAM-size", MSF2State, esram_size,
> MSF2_ESRAM_SIZE),
> > +    /* Libero GUI shows 100Mhz as default for clocks */
> > +    DEFINE_PROP_UINT32("m3clk", MSF2State, m3clk, 100 * 1000000),
> > +    /* default divisors in Libero GUI */
> > +    DEFINE_PROP_UINT32("apb0div", MSF2State, apb0div, 2),
> > +    DEFINE_PROP_UINT32("apb1div", MSF2State, apb1div, 2),
> > +    DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void m2sxxx_soc_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = m2sxxx_soc_realize;
> > +    dc->props = m2sxxx_soc_properties;
> > +}
> > +
> > +static const TypeInfo m2sxxx_soc_info = {
> > +    .name          = TYPE_MSF2_SOC,
> > +    .parent        = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(MSF2State),
> > +    .instance_init = m2sxxx_soc_initfn,
> > +    .class_init    = m2sxxx_soc_class_init,
> > +};
> > +
> > +static void m2sxxx_soc_types(void)
> > +{
> > +    type_register_static(&m2sxxx_soc_info);
> > +}
> > +
> > +type_init(m2sxxx_soc_types)
> > diff --git a/include/hw/arm/msf2-soc.h b/include/hw/arm/msf2-soc.h
> > new file mode 100644
> > index 0000000..9ed677a
> > --- /dev/null
> > +++ b/include/hw/arm/msf2-soc.h
> > @@ -0,0 +1,67 @@
> > +/*
> > + * Microsemi Smartfusion2 SoC
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef HW_ARM_MSF2_SOC_H
> > +#define HW_ARM_MSF2_SOC_H
> > +
> > +#include "hw/misc/msf2-sysreg.h"
> > +#include "hw/timer/mss-timer.h"
> > +#include "hw/ssi/mss-spi.h"
> > +#include "hw/arm/armv7m.h"
> > +#include "qemu/cutils.h"
> > +
> > +#define TYPE_MSF2_SOC     "msf2-soc"
> > +#define MSF2_SOC(obj)     OBJECT_CHECK(MSF2State, (obj), TYPE_MSF2_SOC)
> > +
> > +#define MSF2_NUM_SPIS         2
> > +#define MSF2_NUM_UARTS        2
> > +
> > +/*
> > + * System timer consists of two programmable 32-bit
> > + * decrementing counters that generate individual interrupts to
> > + * the Cortex-M3 processor
> > + */
> > +#define MSF2_NUM_TIMERS       2
> > +
> > +typedef struct MSF2State {
> > +    /*< private >*/
> > +    SysBusDevice parent_obj;
> > +    /*< public >*/
> > +
> > +    ARMv7MState armv7m;
> > +
> > +    char *part_name;
> > +    uint64_t envm_size;
> > +    uint64_t esram_size;
> > +
> > +    uint32_t m3clk;
> > +    uint32_t apb0div;
> > +    uint32_t apb1div;
> > +
> > +    MSF2SysregState sysreg;
> > +    MSSTimerState timer;
> > +    MSSSpiState spi[MSF2_NUM_SPIS];
> > +} MSF2State;
> > +
> > +#endif
> > --
> > 2.5.0
> >
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller
  2017-07-05 18:18   ` Alistair Francis
@ 2017-07-07 12:03     ` sundeep subbaraya
  0 siblings, 0 replies; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-07 12:03 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

Hi Alistair,

On Wed, Jul 5, 2017 at 11:48 PM, Alistair Francis <alistair23@gmail.com>
wrote:

> On Sun, Jul 2, 2017 at 9:45 PM, Subbaraya Sundeep
> <sundeep.lkml@gmail.com> wrote:
> > Modelled Microsemi's Smartfusion2 SPI controller.
> >
> > Signed-off-by: Subbaraya Sundeep <sundeep.lkml@gmail.com>
> > ---
> >  hw/ssi/Makefile.objs     |   1 +
> >  hw/ssi/mss-spi.c         | 414 ++++++++++++++++++++++++++++++
> +++++++++++++++++
> >  include/hw/ssi/mss-spi.h |  62 +++++++
> >  3 files changed, 477 insertions(+)
> >  create mode 100644 hw/ssi/mss-spi.c
> >  create mode 100644 include/hw/ssi/mss-spi.h
> >
> > diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
> > index 487add2..f5bcc65 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) += mss-spi.o
> >
> >  obj-$(CONFIG_OMAP) += omap_spi.o
> >  obj-$(CONFIG_IMX) += imx_spi.o
> > diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
> > new file mode 100644
> > index 0000000..a572abc
> > --- /dev/null
> > +++ b/hw/ssi/mss-spi.c
> > @@ -0,0 +1,414 @@
> > +/*
> > + * Block model of SPI controller present in
> > + * Microsemi's SmartFusion2 and SmartFusion SoCs.
> > + *
> > + * 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 "hw/ssi/mss-spi.h"
>
> Same comment as earlier patches.
>

Ok will fix it.

>
> > +
> > +#ifndef MSS_SPI_ERR_DEBUG
> > +#define MSS_SPI_ERR_DEBUG   0
> > +#endif
> > +
> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
> > +    if (MSS_SPI_ERR_DEBUG >= lvl) { \
> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> > +    } \
> > +} while (0);
> > +
> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> > +
> > +#define FIFO_CAPACITY         32
> > +#define FIFO_CAPACITY         32
> > +
> > +#define R_SPI_CONTROL         0
> > +#define R_SPI_DFSIZE          1
> > +#define R_SPI_STATUS          2
> > +#define R_SPI_INTCLR          3
> > +#define R_SPI_RX              4
> > +#define R_SPI_TX              5
> > +#define R_SPI_CLKGEN          6
> > +#define R_SPI_SS              7
> > +#define R_SPI_MIS             8
> > +#define R_SPI_RIS             9
> > +
> > +#define S_TXDONE             (1 << 0)
> > +#define S_RXRDY              (1 << 1)
> > +#define S_RXCHOVRF           (1 << 2)
> > +#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
> > +
> > +static void txfifo_reset(MSSSpiState *s)
> > +{
> > +    fifo32_reset(&s->tx_fifo);
> > +
> > +    s->regs[R_SPI_STATUS] &= ~S_TXFIFOFUL;
> > +    s->regs[R_SPI_STATUS] |= S_TXFIFOEMP;
> > +}
> > +
> > +static void rxfifo_reset(MSSSpiState *s)
> > +{
> > +    fifo32_reset(&s->rx_fifo);
> > +
> > +    s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
> > +    s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
> > +}
> > +
> > +static void set_fifodepth(MSSSpiState *s)
> > +{
> > +    unsigned int size = s->regs[R_SPI_DFSIZE] & FRAMESZ_MASK;
> > +
> > +    if (size <= 8) {
> > +        s->fifo_depth = 32;
> > +    } else if (size <= 16) {
> > +        s->fifo_depth = 16;
> > +    } else if (size <= 32) {
> > +        s->fifo_depth = 8;
> > +    } else {
> > +        s->fifo_depth = 4;
> > +    }
> > +}
> > +
> > +static void mss_spi_do_reset(MSSSpiState *s)
> > +{
> > +    memset(s->regs, 0, sizeof s->regs);
> > +    s->regs[R_SPI_CONTROL] = 0x80000102;
> > +    s->regs[R_SPI_DFSIZE] = 0x4;
> > +    s->regs[R_SPI_STATUS] = S_SSEL | S_TXFIFOEMP | S_RXFIFOEMP;
> > +    s->regs[R_SPI_CLKGEN] = 0x7;
> > +    s->regs[R_SPI_RIS] = 0x0;
> > +
> > +    s->fifo_depth = 4;
> > +    s->frame_count = 1;
> > +    s->enabled = false;
> > +
> > +    rxfifo_reset(s);
> > +    txfifo_reset(s);
> > +}
> > +
> > +static void update_mis(MSSSpiState *s)
> > +{
> > +    uint32_t reg = s->regs[R_SPI_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_SPI_MIS] |= tmp & s->regs[R_SPI_RIS];
> > +}
> > +
> > +static void spi_update_irq(MSSSpiState *s)
> > +{
> > +    int irq;
> > +
> > +    update_mis(s);
> > +    irq = !!(s->regs[R_SPI_MIS]);
> > +
> > +    qemu_set_irq(s->irq, irq);
> > +}
> > +
> > +static void mss_spi_reset(DeviceState *d)
> > +{
> > +    mss_spi_do_reset(MSS_SPI(d));
> > +}
> > +
> > +static uint64_t
> > +spi_read(void *opaque, hwaddr addr, unsigned int size)
> > +{
> > +    MSSSpiState *s = opaque;
> > +    uint32_t ret = 0;
> > +
> > +    addr >>= 2;
> > +    switch (addr) {
> > +    case R_SPI_RX:
> > +        s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
> > +        s->regs[R_SPI_STATUS] &= ~S_RXCHOVRF;
> > +        ret = fifo32_pop(&s->rx_fifo);
> > +        if (fifo32_is_empty(&s->rx_fifo)) {
> > +            s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
> > +        }
> > +        break;
> > +
> > +    case R_SPI_MIS:
> > +        update_mis(s);
> > +        ret = s->regs[R_SPI_MIS];
> > +        break;
> > +
> > +    default:
> > +        if (addr < ARRAY_SIZE(s->regs)) {
> > +            ret = s->regs[addr];
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                         "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> __func__,
> > +                         addr * 4);
>
> I think return here to avoid the next print statement and irq update.
>

Ok will fix it.

>
> > +        }
> > +        break;
> > +    }
> > +
> > +    DB_PRINT("addr=0x%" HWADDR_PRIx " = 0x%" PRIx32, addr * 4, ret);
> > +    spi_update_irq(s);
> > +    return ret;
> > +}
> > +
> > +static void assert_cs(MSSSpiState *s)
> > +{
> > +    qemu_set_irq(s->cs_line, 0);
> > +}
> > +
> > +static void deassert_cs(MSSSpiState *s)
> > +{
> > +    qemu_set_irq(s->cs_line, 1);
> > +}
> > +
> > +static void spi_flush_txfifo(MSSSpiState *s)
> > +{
> > +    uint32_t tx;
> > +    uint32_t rx;
> > +    bool sps = !!(s->regs[R_SPI_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_SPI_STATUS] &= ~(S_TXDONE | S_RXRDY);
> > +
> > +        tx = fifo32_pop(&s->tx_fifo);
> > +        DB_PRINT("data tx:0x%" PRIx32, tx);
> > +        rx = ssi_transfer(s->spi, tx);
> > +        DB_PRINT("data rx:0x%" PRIx32, rx);
> > +
> > +        if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
> > +            s->regs[R_SPI_STATUS] |= S_RXCHOVRF;
> > +            s->regs[R_SPI_RIS] |= S_RXCHOVRF;
> > +        } else {
> > +            fifo32_push(&s->rx_fifo, rx);
> > +            s->regs[R_SPI_STATUS] &= ~S_RXFIFOEMP;
> > +            if (fifo32_num_used(&s->rx_fifo) == (s->fifo_depth - 1)) {
> > +                s->regs[R_SPI_STATUS] |= S_RXFIFOFULNXT;
> > +            }
> > +            if (fifo32_num_used(&s->rx_fifo) == s->fifo_depth) {
>
> I'd use an else if to indicate that both can't happen at the same time.
>

Ok will add else statement.

>
> > +                s->regs[R_SPI_STATUS] |= S_RXFIFOFUL;
> > +            }
> > +        }
> > +        s->frame_count--;
> > +        if (!sps) {
> > +            deassert_cs(s);
> > +            assert_cs(s);
>
> Do you need the asert here? Can't you just wait for it to hit the
> start of the loop?
>
> Does it deassert assert and deassert after a transfer has finished?


You are right. I guess I have written like that to indicate a pulse in the
loop :).
I will remove the assert.

>


> > +        }
> > +    }
> > +
> > +    if (!sps) {
> > +        deassert_cs(s);
> > +    }
> > +
> > +    if (!s->frame_count) {
> > +        s->frame_count = (s->regs[R_SPI_CONTROL] & FMCOUNT_MASK) >>
> > +                            FMCOUNT_SHIFT;
> > +        if (sps) {
> > +            deassert_cs(s);
> > +        }
> > +        s->regs[R_SPI_RIS] |= S_TXDONE | S_RXRDY;
> > +        s->regs[R_SPI_STATUS] |= S_TXDONE | S_RXRDY;
> > +   }
> > +}
> > +
> > +static void spi_write(void *opaque, hwaddr addr,
> > +            uint64_t val64, unsigned int size)
> > +{
> > +    MSSSpiState *s = opaque;
> > +    uint32_t value = val64;
> > +
> > +    DB_PRINT("addr=0x%" HWADDR_PRIx " =0x%" PRIx32, addr, value);
> > +    addr >>= 2;
> > +
> > +    switch (addr) {
> > +    case R_SPI_TX:
> > +        /* adding to already full FIFO */
> > +        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
> > +            break;
> > +        }
> > +        s->regs[R_SPI_STATUS] &= ~S_TXFIFOEMP;
> > +        fifo32_push(&s->tx_fifo, value);
> > +        if (fifo32_num_used(&s->tx_fifo) == (s->fifo_depth - 1)) {
> > +            s->regs[R_SPI_STATUS] |= S_TXFIFOFULNXT;
> > +        }
>
> else if here as well
>
> Ok will add else if.

Thanks,
Sundeep


> Thanks,
> Alistair
>
> > +        if (fifo32_num_used(&s->tx_fifo) == s->fifo_depth) {
> > +            s->regs[R_SPI_STATUS] |= S_TXFIFOFUL;
> > +        }
> > +        if (s->enabled) {
> > +            spi_flush_txfifo(s);
> > +        }
> > +        break;
> > +
> > +    case R_SPI_CONTROL:
> > +        s->regs[R_SPI_CONTROL] = value;
> > +        if (value & C_BIGFIFO) {
> > +            set_fifodepth(s);
> > +        } else {
> > +            s->fifo_depth = 4;
> > +        }
> > +        s->enabled = value & C_ENABLE;
> > +        s->frame_count = (value & FMCOUNT_MASK) >> FMCOUNT_SHIFT;
> > +        if (value & C_RESET) {
> > +            mss_spi_do_reset(s);
> > +        }
> > +        break;
> > +
> > +    case R_SPI_DFSIZE:
> > +        if (s->enabled) {
> > +            break;
> > +        }
> > +        s->regs[R_SPI_DFSIZE] = value;
> > +        break;
> > +
> > +    case R_SPI_INTCLR:
> > +        s->regs[R_SPI_INTCLR] = value;
> > +        if (value & S_TXDONE) {
> > +            s->regs[R_SPI_RIS] &= ~S_TXDONE;
> > +        }
> > +        if (value & S_RXRDY) {
> > +            s->regs[R_SPI_RIS] &= ~S_RXRDY;
> > +        }
> > +        if (value & S_RXCHOVRF) {
> > +            s->regs[R_SPI_RIS] &= ~S_RXCHOVRF;
> > +        }
> > +        break;
> > +
> > +    case R_SPI_MIS:
> > +    case R_SPI_STATUS:
> > +    case R_SPI_RIS:
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                         "%s: Write to read only register 0x%"
> HWADDR_PRIx "\n",
> > +                         __func__, addr * 4);
> > +        break;
> > +
> > +    default:
> > +        if (addr < ARRAY_SIZE(s->regs)) {
> > +            s->regs[addr] = value;
> > +        } else {
> > +            qemu_log_mask(LOG_GUEST_ERROR,
> > +                         "%s: Bad offset 0x%" HWADDR_PRIx "\n",
> __func__,
> > +                         addr * 4);
> > +        }
> > +        break;
> > +    }
> > +
> > +    spi_update_irq(s);
> > +}
> > +
> > +static const MemoryRegionOps spi_ops = {
> > +    .read = spi_read,
> > +    .write = spi_write,
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +    .valid = {
> > +        .min_access_size = 1,
> > +        .max_access_size = 4
> > +    }
> > +};
> > +
> > +static void mss_spi_realize(DeviceState *dev, Error **errp)
> > +{
> > +    MSSSpiState *s = MSS_SPI(dev);
> > +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > +
> > +    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,
> > +                          TYPE_MSS_SPI, R_SPI_MAX * 4);
> > +    sysbus_init_mmio(sbd, &s->mmio);
> > +
> > +    fifo32_create(&s->tx_fifo, FIFO_CAPACITY);
> > +    fifo32_create(&s->rx_fifo, FIFO_CAPACITY);
> > +}
> > +
> > +static const VMStateDescription vmstate_mss_spi = {
> > +    .name = TYPE_MSS_SPI,
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_FIFO32(tx_fifo, MSSSpiState),
> > +        VMSTATE_FIFO32(rx_fifo, MSSSpiState),
> > +        VMSTATE_UINT32_ARRAY(regs, MSSSpiState, R_SPI_MAX),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static void mss_spi_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +
> > +    dc->realize = mss_spi_realize;
> > +    dc->reset = mss_spi_reset;
> > +    dc->vmsd = &vmstate_mss_spi;
> > +}
> > +
> > +static const TypeInfo mss_spi_info = {
> > +    .name           = TYPE_MSS_SPI,
> > +    .parent         = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size  = sizeof(MSSSpiState),
> > +    .class_init     = mss_spi_class_init,
> > +};
> > +
> > +static void mss_spi_register_types(void)
> > +{
> > +    type_register_static(&mss_spi_info);
> > +}
> > +
> > +type_init(mss_spi_register_types)
> > diff --git a/include/hw/ssi/mss-spi.h b/include/hw/ssi/mss-spi.h
> > new file mode 100644
> > index 0000000..6c8c0b1
> > --- /dev/null
> > +++ b/include/hw/ssi/mss-spi.h
> > @@ -0,0 +1,62 @@
> > +/*
> > + * Microsemi SmartFusion2 SPI
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef HW_MSS_SPI_H
> > +#define HW_MSS_SPI_H
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/hw.h"
> > +#include "hw/ssi/ssi.h"
> > +#include "qemu/fifo32.h"
> > +#include "sysemu/sysemu.h"
> > +#include "qemu/log.h"
> > +
> > +#define TYPE_MSS_SPI   "mss-spi"
> > +#define MSS_SPI(obj)   OBJECT_CHECK(MSSSpiState, (obj), TYPE_MSS_SPI)
> > +
> > +#define R_SPI_MAX             16
> > +
> > +typedef struct MSSSpiState {
> > +    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_SPI_MAX];
> > +} MSSSpiState;
> > +
> > +#endif /* HW_MSS_SPI_H */
> > --
> > 2.5.0
> >
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-07  7:08     ` sundeep subbaraya
@ 2017-07-07 16:33       ` Alistair Francis
  2017-07-10  8:25         ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: Alistair Francis @ 2017-07-07 16:33 UTC (permalink / raw)
  To: sundeep subbaraya
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

On Fri, Jul 7, 2017 at 12:08 AM, sundeep subbaraya
<sundeep.lkml@gmail.com> wrote:
> Hi Alistair,
>
> On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <alistair23@gmail.com>
> wrote:
>>
>> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200
>> > ++++++++++++++++++++++++++++++++++++++++++
>> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
>> >  3 files changed, 283 insertions(+)
>> >  create mode 100644 hw/misc/msf2-sysreg.c
>> >  create mode 100644 include/hw/misc/msf2-sysreg.h
>> >
>> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> > index c8b4893..0f52354 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..64ee141
>> > --- /dev/null
>> > +++ b/hw/misc/msf2-sysreg.c
>> > @@ -0,0 +1,200 @@
>> > +/*
>> > + * 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 "hw/misc/msf2-sysreg.h"
>>
>> Same #include comment from patch 1.
>
>
> Ok will change.
>>
>>
>> > +
>> > +#ifndef MSF2_SYSREG_ERR_DEBUG
>> > +#define MSF2_SYSREG_ERR_DEBUG  0
>> > +#endif
>> > +
>> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
>> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
>> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
>> > +    } \
>> > +} while (0);
>> > +
>> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>> > +
>> > +static inline int msf2_divbits(uint32_t div)
>> > +{
>> > +    int ret = 0;
>> > +
>> > +    switch (div) {
>> > +    case 1:
>> > +        ret = 0;
>> > +        break;
>> > +    case 2:
>> > +        ret = 1;
>> > +        break;
>> > +    case 4:
>> > +        ret = 2;
>> > +        break;
>> > +    case 8:
>> > +        ret = 4;
>> > +        break;
>> > +    case 16:
>> > +        ret = 5;
>> > +        break;
>> > +    case 32:
>> > +        ret = 6;
>> > +        break;
>> > +    default:
>> > +        break;
>> > +    }
>> > +
>> > +    return ret;
>> > +}
>> > +
>> > +static void msf2_sysreg_reset(DeviceState *d)
>> > +{
>> > +    MSF2SysregState *s = MSF2_SYSREG(d);
>> > +
>> > +    DB_PRINT("RESET");
>> > +
>> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
>> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
>> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
>> > +                               msf2_divbits(s->apb1div) << 2;
>> > +}
>> > +
>> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
>> > +    unsigned size)
>> > +{
>> > +    MSF2SysregState *s = opaque;
>> > +    offset /= 4;
>>
>> Probably best to use a bitshift.
>
>
> Ok will change.
>>
>>
>> > +    uint32_t ret = 0;
>> > +
>> > +    if (offset < ARRAY_SIZE(s->regs)) {
>> > +        ret = s->regs[offset];
>> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
>> > +                    offset * 4, ret);
>>
>> Bitshift here as well.
>
>
> Ok will change.
>>
>>
>> > +    } else {
>> > +        qemu_log_mask(LOG_GUEST_ERROR,
>> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
>> > +                    offset * 4);
>> > +    }
>> > +
>> > +    return ret;
>> > +}
>> > +
>> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
>> > +                          uint64_t val, unsigned size)
>> > +{
>> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
>> > +    uint32_t newval = val;
>> > +    uint32_t oldval;
>> > +
>> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
>> > +            offset, val);
>> > +
>> > +    offset /= 4;
>>
>> Same here
>
>
> Ok will change
>>
>>
>> > +
>> > +    switch (offset) {
>> > +    case MSSDDR_PLL_STATUS:
>> > +        break;
>> > +
>> > +    case ESRAM_CR:
>> > +        oldval = s->regs[ESRAM_CR];
>> > +        if (oldval ^ newval) {
>> > +            qemu_log_mask(LOG_GUEST_ERROR,
>> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
>> > supported\n");
>> > +            abort();
>>
>> The guest should not be able to kill QEMU, a guest error should never
>> result in an abort.
>
>
> Philippe suggested to abort because:
> If guest tries to remap since firmware do a remap, the code flow will be
> completely wrong.
> Reporting a GUEST_ERROR here is not enough since code flow continuing would
> be
> pretty hard to understand/debug.

I don't see how it will be that hard to debug as QEMU will tell you
that the guest is doing something wrong.

You can't allow the guest to abort or exit QEMU. It's a security
liability issue and specifically mentioned as not allowed:
https://github.com/qemu/qemu/blob/master/HACKING#L230

Thanks,
Alistair

> We decided to abort for now.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-07 16:33       ` Alistair Francis
@ 2017-07-10  8:25         ` sundeep subbaraya
  2017-07-13  2:21           ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-10  8:25 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite, Philippe Mathieu-Daudé

Hi Alistair,

On Fri, Jul 7, 2017 at 10:03 PM, Alistair Francis <alistair23@gmail.com>
wrote:

> On Fri, Jul 7, 2017 at 12:08 AM, sundeep subbaraya
> <sundeep.lkml@gmail.com> wrote:
> > Hi Alistair,
> >
> > On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <alistair23@gmail.com>
> > wrote:
> >>
> >> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200
> >> > ++++++++++++++++++++++++++++++++++++++++++
> >> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
> >> >  3 files changed, 283 insertions(+)
> >> >  create mode 100644 hw/misc/msf2-sysreg.c
> >> >  create mode 100644 include/hw/misc/msf2-sysreg.h
> >> >
> >> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> >> > index c8b4893..0f52354 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..64ee141
> >> > --- /dev/null
> >> > +++ b/hw/misc/msf2-sysreg.c
> >> > @@ -0,0 +1,200 @@
> >> > +/*
> >> > + * 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 "hw/misc/msf2-sysreg.h"
> >>
> >> Same #include comment from patch 1.
> >
> >
> > Ok will change.
> >>
> >>
> >> > +
> >> > +#ifndef MSF2_SYSREG_ERR_DEBUG
> >> > +#define MSF2_SYSREG_ERR_DEBUG  0
> >> > +#endif
> >> > +
> >> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
> >> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
> >> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
> >> > +    } \
> >> > +} while (0);
> >> > +
> >> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> >> > +
> >> > +static inline int msf2_divbits(uint32_t div)
> >> > +{
> >> > +    int ret = 0;
> >> > +
> >> > +    switch (div) {
> >> > +    case 1:
> >> > +        ret = 0;
> >> > +        break;
> >> > +    case 2:
> >> > +        ret = 1;
> >> > +        break;
> >> > +    case 4:
> >> > +        ret = 2;
> >> > +        break;
> >> > +    case 8:
> >> > +        ret = 4;
> >> > +        break;
> >> > +    case 16:
> >> > +        ret = 5;
> >> > +        break;
> >> > +    case 32:
> >> > +        ret = 6;
> >> > +        break;
> >> > +    default:
> >> > +        break;
> >> > +    }
> >> > +
> >> > +    return ret;
> >> > +}
> >> > +
> >> > +static void msf2_sysreg_reset(DeviceState *d)
> >> > +{
> >> > +    MSF2SysregState *s = MSF2_SYSREG(d);
> >> > +
> >> > +    DB_PRINT("RESET");
> >> > +
> >> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
> >> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
> >> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
> >> > +                               msf2_divbits(s->apb1div) << 2;
> >> > +}
> >> > +
> >> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
> >> > +    unsigned size)
> >> > +{
> >> > +    MSF2SysregState *s = opaque;
> >> > +    offset /= 4;
> >>
> >> Probably best to use a bitshift.
> >
> >
> > Ok will change.
> >>
> >>
> >> > +    uint32_t ret = 0;
> >> > +
> >> > +    if (offset < ARRAY_SIZE(s->regs)) {
> >> > +        ret = s->regs[offset];
> >> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
> >> > +                    offset * 4, ret);
> >>
> >> Bitshift here as well.
> >
> >
> > Ok will change.
> >>
> >>
> >> > +    } else {
> >> > +        qemu_log_mask(LOG_GUEST_ERROR,
> >> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n",
> __func__,
> >> > +                    offset * 4);
> >> > +    }
> >> > +
> >> > +    return ret;
> >> > +}
> >> > +
> >> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
> >> > +                          uint64_t val, unsigned size)
> >> > +{
> >> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
> >> > +    uint32_t newval = val;
> >> > +    uint32_t oldval;
> >> > +
> >> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
> >> > +            offset, val);
> >> > +
> >> > +    offset /= 4;
> >>
> >> Same here
> >
> >
> > Ok will change
> >>
> >>
> >> > +
> >> > +    switch (offset) {
> >> > +    case MSSDDR_PLL_STATUS:
> >> > +        break;
> >> > +
> >> > +    case ESRAM_CR:
> >> > +        oldval = s->regs[ESRAM_CR];
> >> > +        if (oldval ^ newval) {
> >> > +            qemu_log_mask(LOG_GUEST_ERROR,
> >> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
> >> > supported\n");
> >> > +            abort();
> >>
> >> The guest should not be able to kill QEMU, a guest error should never
> >> result in an abort.
> >
> >
> > Philippe suggested to abort because:
> > If guest tries to remap since firmware do a remap, the code flow will be
> > completely wrong.
> > Reporting a GUEST_ERROR here is not enough since code flow continuing
> would
> > be
> > pretty hard to understand/debug.
>
> I don't see how it will be that hard to debug as QEMU will tell you
> that the guest is doing something wrong.
>
> You can't allow the guest to abort or exit QEMU. It's a security
> liability issue and specifically mentioned as not allowed:
> https://github.com/qemu/qemu/blob/master/HACKING#L230
>
> Ok. Lets hear from Philippe. Philippe?

Thanks,
Sundeep


> Thanks,
> Alistair
>
> > We decided to abort for now.
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-10  8:25         ` sundeep subbaraya
@ 2017-07-13  2:21           ` sundeep subbaraya
  2017-07-21  9:20             ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-13  2:21 UTC (permalink / raw)
  To: Alistair Francis, Philippe Mathieu-Daudé
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite

Hi Phiiippe,

Gentle reminder.

Thanks,
Sundeep

On Mon, Jul 10, 2017 at 1:55 PM, sundeep subbaraya <sundeep.lkml@gmail.com>
wrote:

> Hi Alistair,
>
> On Fri, Jul 7, 2017 at 10:03 PM, Alistair Francis <alistair23@gmail.com>
> wrote:
>
>> On Fri, Jul 7, 2017 at 12:08 AM, sundeep subbaraya
>> <sundeep.lkml@gmail.com> wrote:
>> > Hi Alistair,
>> >
>> > On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <alistair23@gmail.com
>> >
>> > wrote:
>> >>
>> >> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200
>> >> > ++++++++++++++++++++++++++++++++++++++++++
>> >> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
>> >> >  3 files changed, 283 insertions(+)
>> >> >  create mode 100644 hw/misc/msf2-sysreg.c
>> >> >  create mode 100644 include/hw/misc/msf2-sysreg.h
>> >> >
>> >> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> >> > index c8b4893..0f52354 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..64ee141
>> >> > --- /dev/null
>> >> > +++ b/hw/misc/msf2-sysreg.c
>> >> > @@ -0,0 +1,200 @@
>> >> > +/*
>> >> > + * 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 "hw/misc/msf2-sysreg.h"
>> >>
>> >> Same #include comment from patch 1.
>> >
>> >
>> > Ok will change.
>> >>
>> >>
>> >> > +
>> >> > +#ifndef MSF2_SYSREG_ERR_DEBUG
>> >> > +#define MSF2_SYSREG_ERR_DEBUG  0
>> >> > +#endif
>> >> > +
>> >> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
>> >> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
>> >> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
>> >> > +    } \
>> >> > +} while (0);
>> >> > +
>> >> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>> >> > +
>> >> > +static inline int msf2_divbits(uint32_t div)
>> >> > +{
>> >> > +    int ret = 0;
>> >> > +
>> >> > +    switch (div) {
>> >> > +    case 1:
>> >> > +        ret = 0;
>> >> > +        break;
>> >> > +    case 2:
>> >> > +        ret = 1;
>> >> > +        break;
>> >> > +    case 4:
>> >> > +        ret = 2;
>> >> > +        break;
>> >> > +    case 8:
>> >> > +        ret = 4;
>> >> > +        break;
>> >> > +    case 16:
>> >> > +        ret = 5;
>> >> > +        break;
>> >> > +    case 32:
>> >> > +        ret = 6;
>> >> > +        break;
>> >> > +    default:
>> >> > +        break;
>> >> > +    }
>> >> > +
>> >> > +    return ret;
>> >> > +}
>> >> > +
>> >> > +static void msf2_sysreg_reset(DeviceState *d)
>> >> > +{
>> >> > +    MSF2SysregState *s = MSF2_SYSREG(d);
>> >> > +
>> >> > +    DB_PRINT("RESET");
>> >> > +
>> >> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
>> >> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
>> >> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
>> >> > +                               msf2_divbits(s->apb1div) << 2;
>> >> > +}
>> >> > +
>> >> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
>> >> > +    unsigned size)
>> >> > +{
>> >> > +    MSF2SysregState *s = opaque;
>> >> > +    offset /= 4;
>> >>
>> >> Probably best to use a bitshift.
>> >
>> >
>> > Ok will change.
>> >>
>> >>
>> >> > +    uint32_t ret = 0;
>> >> > +
>> >> > +    if (offset < ARRAY_SIZE(s->regs)) {
>> >> > +        ret = s->regs[offset];
>> >> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
>> >> > +                    offset * 4, ret);
>> >>
>> >> Bitshift here as well.
>> >
>> >
>> > Ok will change.
>> >>
>> >>
>> >> > +    } else {
>> >> > +        qemu_log_mask(LOG_GUEST_ERROR,
>> >> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n",
>> __func__,
>> >> > +                    offset * 4);
>> >> > +    }
>> >> > +
>> >> > +    return ret;
>> >> > +}
>> >> > +
>> >> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
>> >> > +                          uint64_t val, unsigned size)
>> >> > +{
>> >> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
>> >> > +    uint32_t newval = val;
>> >> > +    uint32_t oldval;
>> >> > +
>> >> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
>> >> > +            offset, val);
>> >> > +
>> >> > +    offset /= 4;
>> >>
>> >> Same here
>> >
>> >
>> > Ok will change
>> >>
>> >>
>> >> > +
>> >> > +    switch (offset) {
>> >> > +    case MSSDDR_PLL_STATUS:
>> >> > +        break;
>> >> > +
>> >> > +    case ESRAM_CR:
>> >> > +        oldval = s->regs[ESRAM_CR];
>> >> > +        if (oldval ^ newval) {
>> >> > +            qemu_log_mask(LOG_GUEST_ERROR,
>> >> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
>> >> > supported\n");
>> >> > +            abort();
>> >>
>> >> The guest should not be able to kill QEMU, a guest error should never
>> >> result in an abort.
>> >
>> >
>> > Philippe suggested to abort because:
>> > If guest tries to remap since firmware do a remap, the code flow will be
>> > completely wrong.
>> > Reporting a GUEST_ERROR here is not enough since code flow continuing
>> would
>> > be
>> > pretty hard to understand/debug.
>>
>> I don't see how it will be that hard to debug as QEMU will tell you
>> that the guest is doing something wrong.
>>
>> You can't allow the guest to abort or exit QEMU. It's a security
>> liability issue and specifically mentioned as not allowed:
>> https://github.com/qemu/qemu/blob/master/HACKING#L230
>>
>> Ok. Lets hear from Philippe. Philippe?
>
> Thanks,
> Sundeep
>
>
>> Thanks,
>> Alistair
>>
>> > We decided to abort for now.
>>
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-13  2:21           ` sundeep subbaraya
@ 2017-07-21  9:20             ` sundeep subbaraya
  2017-08-01  6:08               ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: sundeep subbaraya @ 2017-07-21  9:20 UTC (permalink / raw)
  To: Alistair Francis, Philippe Mathieu-Daudé
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite

Hi,

Ping

On Thu, Jul 13, 2017 at 7:51 AM, sundeep subbaraya <sundeep.lkml@gmail.com>
wrote:

> Hi Phiiippe,
>
> Gentle reminder.
>
> Thanks,
> Sundeep
>
>
> On Mon, Jul 10, 2017 at 1:55 PM, sundeep subbaraya <sundeep.lkml@gmail.com
> > wrote:
>
>> Hi Alistair,
>>
>> On Fri, Jul 7, 2017 at 10:03 PM, Alistair Francis <alistair23@gmail.com>
>> wrote:
>>
>>> On Fri, Jul 7, 2017 at 12:08 AM, sundeep subbaraya
>>> <sundeep.lkml@gmail.com> wrote:
>>> > Hi Alistair,
>>> >
>>> > On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <
>>> alistair23@gmail.com>
>>> > wrote:
>>> >>
>>> >> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200
>>> >> > ++++++++++++++++++++++++++++++++++++++++++
>>> >> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
>>> >> >  3 files changed, 283 insertions(+)
>>> >> >  create mode 100644 hw/misc/msf2-sysreg.c
>>> >> >  create mode 100644 include/hw/misc/msf2-sysreg.h
>>> >> >
>>> >> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>> >> > index c8b4893..0f52354 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..64ee141
>>> >> > --- /dev/null
>>> >> > +++ b/hw/misc/msf2-sysreg.c
>>> >> > @@ -0,0 +1,200 @@
>>> >> > +/*
>>> >> > + * 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 "hw/misc/msf2-sysreg.h"
>>> >>
>>> >> Same #include comment from patch 1.
>>> >
>>> >
>>> > Ok will change.
>>> >>
>>> >>
>>> >> > +
>>> >> > +#ifndef MSF2_SYSREG_ERR_DEBUG
>>> >> > +#define MSF2_SYSREG_ERR_DEBUG  0
>>> >> > +#endif
>>> >> > +
>>> >> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
>>> >> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
>>> >> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
>>> >> > +    } \
>>> >> > +} while (0);
>>> >> > +
>>> >> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>>> >> > +
>>> >> > +static inline int msf2_divbits(uint32_t div)
>>> >> > +{
>>> >> > +    int ret = 0;
>>> >> > +
>>> >> > +    switch (div) {
>>> >> > +    case 1:
>>> >> > +        ret = 0;
>>> >> > +        break;
>>> >> > +    case 2:
>>> >> > +        ret = 1;
>>> >> > +        break;
>>> >> > +    case 4:
>>> >> > +        ret = 2;
>>> >> > +        break;
>>> >> > +    case 8:
>>> >> > +        ret = 4;
>>> >> > +        break;
>>> >> > +    case 16:
>>> >> > +        ret = 5;
>>> >> > +        break;
>>> >> > +    case 32:
>>> >> > +        ret = 6;
>>> >> > +        break;
>>> >> > +    default:
>>> >> > +        break;
>>> >> > +    }
>>> >> > +
>>> >> > +    return ret;
>>> >> > +}
>>> >> > +
>>> >> > +static void msf2_sysreg_reset(DeviceState *d)
>>> >> > +{
>>> >> > +    MSF2SysregState *s = MSF2_SYSREG(d);
>>> >> > +
>>> >> > +    DB_PRINT("RESET");
>>> >> > +
>>> >> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
>>> >> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
>>> >> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
>>> >> > +                               msf2_divbits(s->apb1div) << 2;
>>> >> > +}
>>> >> > +
>>> >> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
>>> >> > +    unsigned size)
>>> >> > +{
>>> >> > +    MSF2SysregState *s = opaque;
>>> >> > +    offset /= 4;
>>> >>
>>> >> Probably best to use a bitshift.
>>> >
>>> >
>>> > Ok will change.
>>> >>
>>> >>
>>> >> > +    uint32_t ret = 0;
>>> >> > +
>>> >> > +    if (offset < ARRAY_SIZE(s->regs)) {
>>> >> > +        ret = s->regs[offset];
>>> >> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
>>> >> > +                    offset * 4, ret);
>>> >>
>>> >> Bitshift here as well.
>>> >
>>> >
>>> > Ok will change.
>>> >>
>>> >>
>>> >> > +    } else {
>>> >> > +        qemu_log_mask(LOG_GUEST_ERROR,
>>> >> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n",
>>> __func__,
>>> >> > +                    offset * 4);
>>> >> > +    }
>>> >> > +
>>> >> > +    return ret;
>>> >> > +}
>>> >> > +
>>> >> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
>>> >> > +                          uint64_t val, unsigned size)
>>> >> > +{
>>> >> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
>>> >> > +    uint32_t newval = val;
>>> >> > +    uint32_t oldval;
>>> >> > +
>>> >> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
>>> >> > +            offset, val);
>>> >> > +
>>> >> > +    offset /= 4;
>>> >>
>>> >> Same here
>>> >
>>> >
>>> > Ok will change
>>> >>
>>> >>
>>> >> > +
>>> >> > +    switch (offset) {
>>> >> > +    case MSSDDR_PLL_STATUS:
>>> >> > +        break;
>>> >> > +
>>> >> > +    case ESRAM_CR:
>>> >> > +        oldval = s->regs[ESRAM_CR];
>>> >> > +        if (oldval ^ newval) {
>>> >> > +            qemu_log_mask(LOG_GUEST_ERROR,
>>> >> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
>>> >> > supported\n");
>>> >> > +            abort();
>>> >>
>>> >> The guest should not be able to kill QEMU, a guest error should never
>>> >> result in an abort.
>>> >
>>> >
>>> > Philippe suggested to abort because:
>>> > If guest tries to remap since firmware do a remap, the code flow will
>>> be
>>> > completely wrong.
>>> > Reporting a GUEST_ERROR here is not enough since code flow continuing
>>> would
>>> > be
>>> > pretty hard to understand/debug.
>>>
>>> I don't see how it will be that hard to debug as QEMU will tell you
>>> that the guest is doing something wrong.
>>>
>>> You can't allow the guest to abort or exit QEMU. It's a security
>>> liability issue and specifically mentioned as not allowed:
>>> https://github.com/qemu/qemu/blob/master/HACKING#L230
>>>
>>> Ok. Lets hear from Philippe. Philippe?
>>
>> Thanks,
>> Sundeep
>>
>>
>>> Thanks,
>>> Alistair
>>>
>>> > We decided to abort for now.
>>>
>>
>>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-07-21  9:20             ` sundeep subbaraya
@ 2017-08-01  6:08               ` sundeep subbaraya
  2017-08-22  6:08                 ` sundeep subbaraya
  0 siblings, 1 reply; 22+ messages in thread
From: sundeep subbaraya @ 2017-08-01  6:08 UTC (permalink / raw)
  To: Alistair Francis, Philippe Mathieu-Daudé
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite

Hi Philippe,

Ping again :)

Thanks,
Sundeep

On Fri, Jul 21, 2017 at 2:50 PM, sundeep subbaraya <sundeep.lkml@gmail.com>
wrote:

> Hi,
>
> Ping
>
> On Thu, Jul 13, 2017 at 7:51 AM, sundeep subbaraya <sundeep.lkml@gmail.com
> > wrote:
>
>> Hi Phiiippe,
>>
>> Gentle reminder.
>>
>> Thanks,
>> Sundeep
>>
>>
>> On Mon, Jul 10, 2017 at 1:55 PM, sundeep subbaraya <
>> sundeep.lkml@gmail.com> wrote:
>>
>>> Hi Alistair,
>>>
>>> On Fri, Jul 7, 2017 at 10:03 PM, Alistair Francis <alistair23@gmail.com>
>>> wrote:
>>>
>>>> On Fri, Jul 7, 2017 at 12:08 AM, sundeep subbaraya
>>>> <sundeep.lkml@gmail.com> wrote:
>>>> > Hi Alistair,
>>>> >
>>>> > On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <
>>>> alistair23@gmail.com>
>>>> > wrote:
>>>> >>
>>>> >> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200
>>>> >> > ++++++++++++++++++++++++++++++++++++++++++
>>>> >> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
>>>> >> >  3 files changed, 283 insertions(+)
>>>> >> >  create mode 100644 hw/misc/msf2-sysreg.c
>>>> >> >  create mode 100644 include/hw/misc/msf2-sysreg.h
>>>> >> >
>>>> >> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>>> >> > index c8b4893..0f52354 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..64ee141
>>>> >> > --- /dev/null
>>>> >> > +++ b/hw/misc/msf2-sysreg.c
>>>> >> > @@ -0,0 +1,200 @@
>>>> >> > +/*
>>>> >> > + * 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 "hw/misc/msf2-sysreg.h"
>>>> >>
>>>> >> Same #include comment from patch 1.
>>>> >
>>>> >
>>>> > Ok will change.
>>>> >>
>>>> >>
>>>> >> > +
>>>> >> > +#ifndef MSF2_SYSREG_ERR_DEBUG
>>>> >> > +#define MSF2_SYSREG_ERR_DEBUG  0
>>>> >> > +#endif
>>>> >> > +
>>>> >> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
>>>> >> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
>>>> >> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
>>>> >> > +    } \
>>>> >> > +} while (0);
>>>> >> > +
>>>> >> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>>>> >> > +
>>>> >> > +static inline int msf2_divbits(uint32_t div)
>>>> >> > +{
>>>> >> > +    int ret = 0;
>>>> >> > +
>>>> >> > +    switch (div) {
>>>> >> > +    case 1:
>>>> >> > +        ret = 0;
>>>> >> > +        break;
>>>> >> > +    case 2:
>>>> >> > +        ret = 1;
>>>> >> > +        break;
>>>> >> > +    case 4:
>>>> >> > +        ret = 2;
>>>> >> > +        break;
>>>> >> > +    case 8:
>>>> >> > +        ret = 4;
>>>> >> > +        break;
>>>> >> > +    case 16:
>>>> >> > +        ret = 5;
>>>> >> > +        break;
>>>> >> > +    case 32:
>>>> >> > +        ret = 6;
>>>> >> > +        break;
>>>> >> > +    default:
>>>> >> > +        break;
>>>> >> > +    }
>>>> >> > +
>>>> >> > +    return ret;
>>>> >> > +}
>>>> >> > +
>>>> >> > +static void msf2_sysreg_reset(DeviceState *d)
>>>> >> > +{
>>>> >> > +    MSF2SysregState *s = MSF2_SYSREG(d);
>>>> >> > +
>>>> >> > +    DB_PRINT("RESET");
>>>> >> > +
>>>> >> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
>>>> >> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
>>>> >> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
>>>> >> > +                               msf2_divbits(s->apb1div) << 2;
>>>> >> > +}
>>>> >> > +
>>>> >> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
>>>> >> > +    unsigned size)
>>>> >> > +{
>>>> >> > +    MSF2SysregState *s = opaque;
>>>> >> > +    offset /= 4;
>>>> >>
>>>> >> Probably best to use a bitshift.
>>>> >
>>>> >
>>>> > Ok will change.
>>>> >>
>>>> >>
>>>> >> > +    uint32_t ret = 0;
>>>> >> > +
>>>> >> > +    if (offset < ARRAY_SIZE(s->regs)) {
>>>> >> > +        ret = s->regs[offset];
>>>> >> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
>>>> >> > +                    offset * 4, ret);
>>>> >>
>>>> >> Bitshift here as well.
>>>> >
>>>> >
>>>> > Ok will change.
>>>> >>
>>>> >>
>>>> >> > +    } else {
>>>> >> > +        qemu_log_mask(LOG_GUEST_ERROR,
>>>> >> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n",
>>>> __func__,
>>>> >> > +                    offset * 4);
>>>> >> > +    }
>>>> >> > +
>>>> >> > +    return ret;
>>>> >> > +}
>>>> >> > +
>>>> >> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
>>>> >> > +                          uint64_t val, unsigned size)
>>>> >> > +{
>>>> >> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
>>>> >> > +    uint32_t newval = val;
>>>> >> > +    uint32_t oldval;
>>>> >> > +
>>>> >> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
>>>> >> > +            offset, val);
>>>> >> > +
>>>> >> > +    offset /= 4;
>>>> >>
>>>> >> Same here
>>>> >
>>>> >
>>>> > Ok will change
>>>> >>
>>>> >>
>>>> >> > +
>>>> >> > +    switch (offset) {
>>>> >> > +    case MSSDDR_PLL_STATUS:
>>>> >> > +        break;
>>>> >> > +
>>>> >> > +    case ESRAM_CR:
>>>> >> > +        oldval = s->regs[ESRAM_CR];
>>>> >> > +        if (oldval ^ newval) {
>>>> >> > +            qemu_log_mask(LOG_GUEST_ERROR,
>>>> >> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
>>>> >> > supported\n");
>>>> >> > +            abort();
>>>> >>
>>>> >> The guest should not be able to kill QEMU, a guest error should never
>>>> >> result in an abort.
>>>> >
>>>> >
>>>> > Philippe suggested to abort because:
>>>> > If guest tries to remap since firmware do a remap, the code flow will
>>>> be
>>>> > completely wrong.
>>>> > Reporting a GUEST_ERROR here is not enough since code flow continuing
>>>> would
>>>> > be
>>>> > pretty hard to understand/debug.
>>>>
>>>> I don't see how it will be that hard to debug as QEMU will tell you
>>>> that the guest is doing something wrong.
>>>>
>>>> You can't allow the guest to abort or exit QEMU. It's a security
>>>> liability issue and specifically mentioned as not allowed:
>>>> https://github.com/qemu/qemu/blob/master/HACKING#L230
>>>>
>>>> Ok. Lets hear from Philippe. Philippe?
>>>
>>> Thanks,
>>> Sundeep
>>>
>>>
>>>> Thanks,
>>>> Alistair
>>>>
>>>> > We decided to abort for now.
>>>>
>>>
>>>
>>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.
  2017-08-01  6:08               ` sundeep subbaraya
@ 2017-08-22  6:08                 ` sundeep subbaraya
  0 siblings, 0 replies; 22+ messages in thread
From: sundeep subbaraya @ 2017-08-22  6:08 UTC (permalink / raw)
  To: Alistair Francis, Philippe Mathieu-Daudé
  Cc: qemu-devel@nongnu.org Developers, qemu-arm, Peter Maydell,
	Peter Crosthwaite

Hi Alistair,

I will remove the abort and send next iteration.

Thanks,
Sundeep

On Tue, Aug 1, 2017 at 11:38 AM, sundeep subbaraya <sundeep.lkml@gmail.com>
wrote:

> Hi Philippe,
>
> Ping again :)
>
> Thanks,
> Sundeep
>
> On Fri, Jul 21, 2017 at 2:50 PM, sundeep subbaraya <sundeep.lkml@gmail.com
> > wrote:
>
>> Hi,
>>
>> Ping
>>
>> On Thu, Jul 13, 2017 at 7:51 AM, sundeep subbaraya <
>> sundeep.lkml@gmail.com> wrote:
>>
>>> Hi Phiiippe,
>>>
>>> Gentle reminder.
>>>
>>> Thanks,
>>> Sundeep
>>>
>>>
>>> On Mon, Jul 10, 2017 at 1:55 PM, sundeep subbaraya <
>>> sundeep.lkml@gmail.com> wrote:
>>>
>>>> Hi Alistair,
>>>>
>>>> On Fri, Jul 7, 2017 at 10:03 PM, Alistair Francis <alistair23@gmail.com
>>>> > wrote:
>>>>
>>>>> On Fri, Jul 7, 2017 at 12:08 AM, sundeep subbaraya
>>>>> <sundeep.lkml@gmail.com> wrote:
>>>>> > Hi Alistair,
>>>>> >
>>>>> > On Wed, Jul 5, 2017 at 11:36 PM, Alistair Francis <
>>>>> alistair23@gmail.com>
>>>>> > wrote:
>>>>> >>
>>>>> >> On Sun, Jul 2, 2017 at 9:45 PM, 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         | 200
>>>>> >> > ++++++++++++++++++++++++++++++++++++++++++
>>>>> >> >  include/hw/misc/msf2-sysreg.h |  82 +++++++++++++++++
>>>>> >> >  3 files changed, 283 insertions(+)
>>>>> >> >  create mode 100644 hw/misc/msf2-sysreg.c
>>>>> >> >  create mode 100644 include/hw/misc/msf2-sysreg.h
>>>>> >> >
>>>>> >> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>>>> >> > index c8b4893..0f52354 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..64ee141
>>>>> >> > --- /dev/null
>>>>> >> > +++ b/hw/misc/msf2-sysreg.c
>>>>> >> > @@ -0,0 +1,200 @@
>>>>> >> > +/*
>>>>> >> > + * 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 "hw/misc/msf2-sysreg.h"
>>>>> >>
>>>>> >> Same #include comment from patch 1.
>>>>> >
>>>>> >
>>>>> > Ok will change.
>>>>> >>
>>>>> >>
>>>>> >> > +
>>>>> >> > +#ifndef MSF2_SYSREG_ERR_DEBUG
>>>>> >> > +#define MSF2_SYSREG_ERR_DEBUG  0
>>>>> >> > +#endif
>>>>> >> > +
>>>>> >> > +#define DB_PRINT_L(lvl, fmt, args...) do { \
>>>>> >> > +    if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
>>>>> >> > +        qemu_log("%s: " fmt "\n", __func__, ## args); \
>>>>> >> > +    } \
>>>>> >> > +} while (0);
>>>>> >> > +
>>>>> >> > +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>>>>> >> > +
>>>>> >> > +static inline int msf2_divbits(uint32_t div)
>>>>> >> > +{
>>>>> >> > +    int ret = 0;
>>>>> >> > +
>>>>> >> > +    switch (div) {
>>>>> >> > +    case 1:
>>>>> >> > +        ret = 0;
>>>>> >> > +        break;
>>>>> >> > +    case 2:
>>>>> >> > +        ret = 1;
>>>>> >> > +        break;
>>>>> >> > +    case 4:
>>>>> >> > +        ret = 2;
>>>>> >> > +        break;
>>>>> >> > +    case 8:
>>>>> >> > +        ret = 4;
>>>>> >> > +        break;
>>>>> >> > +    case 16:
>>>>> >> > +        ret = 5;
>>>>> >> > +        break;
>>>>> >> > +    case 32:
>>>>> >> > +        ret = 6;
>>>>> >> > +        break;
>>>>> >> > +    default:
>>>>> >> > +        break;
>>>>> >> > +    }
>>>>> >> > +
>>>>> >> > +    return ret;
>>>>> >> > +}
>>>>> >> > +
>>>>> >> > +static void msf2_sysreg_reset(DeviceState *d)
>>>>> >> > +{
>>>>> >> > +    MSF2SysregState *s = MSF2_SYSREG(d);
>>>>> >> > +
>>>>> >> > +    DB_PRINT("RESET");
>>>>> >> > +
>>>>> >> > +    s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
>>>>> >> > +    s->regs[MSSDDR_PLL_STATUS] = 0x3;
>>>>> >> > +    s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
>>>>> >> > +                               msf2_divbits(s->apb1div) << 2;
>>>>> >> > +}
>>>>> >> > +
>>>>> >> > +static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
>>>>> >> > +    unsigned size)
>>>>> >> > +{
>>>>> >> > +    MSF2SysregState *s = opaque;
>>>>> >> > +    offset /= 4;
>>>>> >>
>>>>> >> Probably best to use a bitshift.
>>>>> >
>>>>> >
>>>>> > Ok will change.
>>>>> >>
>>>>> >>
>>>>> >> > +    uint32_t ret = 0;
>>>>> >> > +
>>>>> >> > +    if (offset < ARRAY_SIZE(s->regs)) {
>>>>> >> > +        ret = s->regs[offset];
>>>>> >> > +        DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
>>>>> >> > +                    offset * 4, ret);
>>>>> >>
>>>>> >> Bitshift here as well.
>>>>> >
>>>>> >
>>>>> > Ok will change.
>>>>> >>
>>>>> >>
>>>>> >> > +    } else {
>>>>> >> > +        qemu_log_mask(LOG_GUEST_ERROR,
>>>>> >> > +                    "%s: Bad offset 0x%08" HWADDR_PRIx "\n",
>>>>> __func__,
>>>>> >> > +                    offset * 4);
>>>>> >> > +    }
>>>>> >> > +
>>>>> >> > +    return ret;
>>>>> >> > +}
>>>>> >> > +
>>>>> >> > +static void msf2_sysreg_write(void *opaque, hwaddr offset,
>>>>> >> > +                          uint64_t val, unsigned size)
>>>>> >> > +{
>>>>> >> > +    MSF2SysregState *s = (MSF2SysregState *)opaque;
>>>>> >> > +    uint32_t newval = val;
>>>>> >> > +    uint32_t oldval;
>>>>> >> > +
>>>>> >> > +    DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
>>>>> >> > +            offset, val);
>>>>> >> > +
>>>>> >> > +    offset /= 4;
>>>>> >>
>>>>> >> Same here
>>>>> >
>>>>> >
>>>>> > Ok will change
>>>>> >>
>>>>> >>
>>>>> >> > +
>>>>> >> > +    switch (offset) {
>>>>> >> > +    case MSSDDR_PLL_STATUS:
>>>>> >> > +        break;
>>>>> >> > +
>>>>> >> > +    case ESRAM_CR:
>>>>> >> > +        oldval = s->regs[ESRAM_CR];
>>>>> >> > +        if (oldval ^ newval) {
>>>>> >> > +            qemu_log_mask(LOG_GUEST_ERROR,
>>>>> >> > +                       TYPE_MSF2_SYSREG": eSRAM remapping not
>>>>> >> > supported\n");
>>>>> >> > +            abort();
>>>>> >>
>>>>> >> The guest should not be able to kill QEMU, a guest error should
>>>>> never
>>>>> >> result in an abort.
>>>>> >
>>>>> >
>>>>> > Philippe suggested to abort because:
>>>>> > If guest tries to remap since firmware do a remap, the code flow
>>>>> will be
>>>>> > completely wrong.
>>>>> > Reporting a GUEST_ERROR here is not enough since code flow
>>>>> continuing would
>>>>> > be
>>>>> > pretty hard to understand/debug.
>>>>>
>>>>> I don't see how it will be that hard to debug as QEMU will tell you
>>>>> that the guest is doing something wrong.
>>>>>
>>>>> You can't allow the guest to abort or exit QEMU. It's a security
>>>>> liability issue and specifically mentioned as not allowed:
>>>>> https://github.com/qemu/qemu/blob/master/HACKING#L230
>>>>>
>>>>> Ok. Lets hear from Philippe. Philippe?
>>>>
>>>> Thanks,
>>>> Sundeep
>>>>
>>>>
>>>>> Thanks,
>>>>> Alistair
>>>>>
>>>>> > We decided to abort for now.
>>>>>
>>>>
>>>>
>>>
>>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2017-08-22  6:08 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-03  4:45 [Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC Subbaraya Sundeep
2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer Subbaraya Sundeep
2017-07-05 17:56   ` Alistair Francis
2017-07-05 17:58     ` Alistair Francis
2017-07-07  7:00       ` sundeep subbaraya
2017-07-07  6:59     ` sundeep subbaraya
2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block Subbaraya Sundeep
2017-07-05 18:06   ` Alistair Francis
2017-07-07  7:08     ` sundeep subbaraya
2017-07-07 16:33       ` Alistair Francis
2017-07-10  8:25         ` sundeep subbaraya
2017-07-13  2:21           ` sundeep subbaraya
2017-07-21  9:20             ` sundeep subbaraya
2017-08-01  6:08               ` sundeep subbaraya
2017-08-22  6:08                 ` sundeep subbaraya
2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller Subbaraya Sundeep
2017-07-05 18:18   ` Alistair Francis
2017-07-07 12:03     ` sundeep subbaraya
2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC Subbaraya Sundeep
2017-07-05 18:25   ` Alistair Francis
2017-07-07  7:09     ` sundeep subbaraya
2017-07-03  4:45 ` [Qemu-devel] [Qemu devel v6 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit Subbaraya Sundeep

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.