[Qemu-devel] [RFC v3 0/5] hw/arm: add initial support for Canon DIGIC SoC

[Qemu-devel] [RFC v3 0/5] hw/arm: add initial support for Canon DIGIC SoC

[Antony Pavlov]

[RFC v3 1/5] hw/arm: add very initial support for Canon DIGIC SoC
[RFC v3 2/5] hw/arm/digic: prepare DIGIC-based boards support
[RFC v3 3/5] hw/arm/digic: add timer support
[RFC v3 4/5] hw/arm/digic: add UART support
[RFC v3 5/5] hw/arm/digic: add NOR ROM support

 Changes since v2:

 1. rebase over latest master;
   * pass available size to object_initialize().
 2. digic-uart: qemu_log: use LOG_UNIMP instead LOG_GUEST_ERROR;
 3. digic-boards: update rom image load code: introduce digic_load_rom().

 Changes since v1:

 0. drop the "add ARM946E-S CPU" patch;
 1. convert to QOM, split DIGIC SoC code and board code
 (thanks to Andreas Fa:rber, Peter Maydell and Peter Crosthwaite);
 2. fix digic-uart (many thanks to Peter Crosthwaite for his comments);
 3. digic-boards: digic4_add_k8p3215uqb_rom(): update rom image load code: use -bios option.


 DIGIC is Canon Inc.'s name for a family of SoC
 for digital cameras and camcorders.

 See http://en.wikipedia.org/wiki/DIGIC for details.

 There is no publicly available specification for
 DIGIC chips. All information about DIGIC chip
 internals is based on reverse engineering efforts
 made by CHDK (http://chdk.wikia.com) and
 Magic Lantern (http://www.magiclantern.fm) projects
 contributors.

 Also this patch series adds initial support for Canon
 PowerShot A1100 IS compact camera (it is my only camera
 with connected UART interface). As the DIGIC-based cameras
 differences mostly are unsignificant (e.g. RAM-size,
 ROM type and size, GPIO usage) the other compact
 and DSLR cameras support can be easely added.

 This DIGIC support patch series is inspired
 by EOS QEMU from Magic Lantern project.
 The main differences:
  * EOS QEMU uses home-brew all-in-one monolith design;
  this patch series uses conventional qemu object-centric design;
  * EOS QEMU tries provide simplest emulation for most
  controllers inside SoC to run Magic Lantern firmware;
  this patch series provide more complete support
  only for core devices to run barebox bootloader.
   ** EOS QEMU does not support timer counting
   (this patch series emulate 1 MHz counting);
   ** EOS QEMU support DIGIC UART only for output
   character to stderr; (this patch series emulate
   introduces full blown UART interface);
   ** EOS QEMU has incomplete ROM support;
   (this patch series uses conventional qemu pflash).

 This initial DIGIC support can't be used to run
 the original camera firmware, but it can successfully
 run experimental version of barebox bootloader
 (see http://www.barebox.org).

 The last sources of barebox for PowerShot A1100 can be
 obtained here:
   https://github.com/frantony/barebox/tree/next.digic.20130829

 The precompiled ROM image usable with qemu can be
 obtained here:

 https://github.com/frantony/barebox/blob/next.digic.20130829/canon-a1100-rom1.bin

 This ROM image (after "dancing bit" encoding) can be run on
 real Canon A1100 camera.

 The short build instruction for __previous__ DIGIC barebox
 version (it can be used with more recent sources too) can
 be obtained here:
   http://lists.infradead.org/pipermail/barebox/2013-August/016007.html

[Qemu-devel] [RFC v3 1/5] hw/arm: add very initial support for Canon DIGIC SoC

[Antony Pavlov]

DIGIC is Canon Inc.'s name for a family of SoC
for digital cameras and camcorders.

There is no publicly available specification for
DIGIC chips. All information about DIGIC chip
internals is based on reverse engineering efforts
made by CHDK (http://chdk.wikia.com) and
Magic Lantern (http://www.magiclantern.fm) projects
contributors.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 default-configs/arm-softmmu.mak |  1 +
 hw/arm/Makefile.objs            |  2 +-
 hw/arm/digic.c                  | 70 +++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/digic.h          | 23 ++++++++++++++
 4 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/digic.c
 create mode 100644 include/hw/arm/digic.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ac0815d..0d1d783 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -63,6 +63,7 @@ CONFIG_FRAMEBUFFER=y
 CONFIG_XILINX_SPIPS=y
 
 CONFIG_A9SCU=y
+CONFIG_DIGIC=y
 CONFIG_MARVELL_88W8618=y
 CONFIG_OMAP=y
 CONFIG_TSC210X=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 3671b42..e140485 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -3,5 +3,5 @@ obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
 
-obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
+obj-y += armv7m.o digic.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-y += omap1.o omap2.o strongarm.o
diff --git a/hw/arm/digic.c b/hw/arm/digic.c
new file mode 100644
index 0000000..95a9fcd
--- /dev/null
+++ b/hw/arm/digic.c
@@ -0,0 +1,70 @@
+/*
+ * QEMU model of the Canon DIGIC SoC.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "target-arm/cpu-qom.h"
+#include "hw/arm/digic.h"
+
+static void digic_init(Object *obj)
+{
+    DigicState *s = DIGIC(obj);
+
+    object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
+    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+}
+
+static void digic_realize(DeviceState *dev, Error **errp)
+{
+    DigicState *s = DIGIC(dev);
+    Error *err = NULL;
+
+    object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+}
+
+static void digic_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = digic_realize;
+}
+
+static const TypeInfo digic_type_info = {
+    .name = TYPE_DIGIC,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(DigicState),
+    .instance_init = digic_init,
+    .class_init = digic_class_init,
+};
+
+static void digic_register_types(void)
+{
+    type_register_static(&digic_type_info);
+}
+
+type_init(digic_register_types)
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
new file mode 100644
index 0000000..0ef4723
--- /dev/null
+++ b/include/hw/arm/digic.h
@@ -0,0 +1,23 @@
+/*
+ * Misc DIGIC declarations
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ */
+
+#ifndef __DIGIC_H__
+#define __DIGIC_H__
+
+#include "cpu-qom.h"
+
+#define TYPE_DIGIC "digic"
+
+#define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
+
+typedef struct DigicState {
+    Object parent_obj;
+
+    ARMCPU cpu;
+} DigicState;
+
+#endif /* __DIGIC_H__ */
-- 
1.8.4.rc3

[Qemu-devel] [RFC v3 2/5] hw/arm/digic: prepare DIGIC-based boards support

[Antony Pavlov]

Also this patch adds initial support for Canon
PowerShot A1100 IS compact camera.

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/Makefile.objs  |  2 +-
 hw/arm/digic_boards.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/digic_boards.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index e140485..f6e9533 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -1,4 +1,4 @@
-obj-y += boot.o collie.o exynos4_boards.o gumstix.o highbank.o
+obj-y += boot.o collie.o digic_boards.o exynos4_boards.o gumstix.o highbank.o
 obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o
 obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
new file mode 100644
index 0000000..0b99227
--- /dev/null
+++ b/hw/arm/digic_boards.c
@@ -0,0 +1,63 @@
+#include "hw/boards.h"
+#include "exec/address-spaces.h"
+#include "hw/arm/digic.h"
+
+typedef struct DigicBoardState {
+    DigicState *digic;
+    MemoryRegion ram;
+} DigicBoardState;
+
+typedef struct DigicBoard {
+    hwaddr ram_size;
+    hwaddr start_addr;
+} DigicBoard;
+
+static void digic4_board_setup_ram(DigicBoardState *s, hwaddr ram_size)
+{
+    memory_region_init_ram(&s->ram, NULL, "ram", ram_size);
+    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
+    vmstate_register_ram_global(&s->ram);
+}
+
+static void digic4_board_init(DigicBoard *board)
+{
+    Error *err = NULL;
+
+    DigicBoardState *s = g_new(DigicBoardState, 1);
+
+    s->digic = DIGIC(object_new(TYPE_DIGIC));
+    object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
+    if (err != NULL) {
+        fprintf(stderr, "Couldn't realize DIGIC SoC: %s\n",
+                error_get_pretty(err));
+        exit(1);
+    }
+
+    digic4_board_setup_ram(s, board->ram_size);
+
+    s->digic->cpu.env.regs[15] = board->start_addr;
+}
+
+static DigicBoard digic4_board_canon_a1100 = {
+    .ram_size = 64 * 1024 * 1024,
+    /* CHDK recommends this address for ROM disassembly */
+    .start_addr = 0xffc00000,
+};
+
+static void canon_a1100_init(QEMUMachineInitArgs *args)
+{
+    digic4_board_init(&digic4_board_canon_a1100);
+}
+
+static QEMUMachine canon_a1100 = {
+    .name = "canon-a1100",
+    .desc = "Canon PowerShot A1100 IS",
+    .init = &canon_a1100_init,
+};
+
+static void digic_register_machines(void)
+{
+    qemu_register_machine(&canon_a1100);
+}
+
+machine_init(digic_register_machines)
-- 
1.8.4.rc3

[Qemu-devel] [RFC v3 3/5] hw/arm/digic: add timer support

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic.c         |  25 ++++++++++
 hw/timer/Makefile.objs |   1 +
 hw/timer/digic-timer.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/timer/digic-timer.h |  19 ++++++++
 include/hw/arm/digic.h |   7 +++
 5 files changed, 174 insertions(+)
 create mode 100644 hw/timer/digic-timer.c
 create mode 100644 hw/timer/digic-timer.h

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 95a9fcd..a71364b 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -30,21 +30,46 @@
 static void digic_init(Object *obj)
 {
     DigicState *s = DIGIC(obj);
+    DeviceState *dev;
+    int i;
 
     object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
     object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+
+    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
+        char name[9];
+
+        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
+        dev = DEVICE(&s->timer[i]);
+        qdev_set_parent_bus(dev, sysbus_get_default());
+        snprintf(name, 9, "timer[%d]", i);
+        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
+    }
 }
 
 static void digic_realize(DeviceState *dev, Error **errp)
 {
     DigicState *s = DIGIC(dev);
     Error *err = NULL;
+    SysBusDevice *sbd;
+    int i;
 
     object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
     }
+
+    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
+        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+
+        sbd = SYS_BUS_DEVICE(&s->timer[i]);
+        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
+    }
 }
 
 static void digic_class_init(ObjectClass *oc, void *data)
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index eca5905..5479aee 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -25,5 +25,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
 obj-$(CONFIG_SH4) += sh_timer.o
 obj-$(CONFIG_TUSB6010) += tusb6010.o
+obj-$(CONFIG_DIGIC) += digic-timer.o
 
 obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
new file mode 100644
index 0000000..c6cf7ee
--- /dev/null
+++ b/hw/timer/digic-timer.c
@@ -0,0 +1,122 @@
+/*
+ * QEMU model of the Canon Digic timer block.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * See "Timer/Clock Module" docs here:
+ *   http://magiclantern.wikia.com/wiki/Register_Map
+ *
+ * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
+ * is used as a template.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "qemu/main-loop.h"
+
+#include "hw/timer/digic-timer.h"
+
+#ifdef DEBUG_DIGIC_TIMER
+#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+# define DIGIC_TIMER_CONTROL 0x00
+# define DIGIC_TIMER_VALUE 0x0c
+
+static uint64_t digic_timer_read(void *opaque, hwaddr offset,
+        unsigned size)
+{
+    DigicTimerState *s = opaque;
+    uint32_t ret = 0;
+
+    switch (offset) {
+    case DIGIC_TIMER_VALUE:
+        ret = (uint32_t)ptimer_get_count(s->ptimer);
+        ret = ret & 0xffff;
+        break;
+    default:
+        DPRINTF("Bad offset %x\n", (int)offset);
+    }
+
+    DPRINTF("offset 0x%x, value 0x%x\n", offset, ret);
+    return ret;
+}
+
+static void digic_timer_write(void *opaque, hwaddr offset,
+        uint64_t value, unsigned size)
+{
+    DigicTimerState *s = opaque;
+
+    /* FIXME: just now we ignore timer enable bit */
+    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
+    ptimer_run(s->ptimer, 1);
+}
+
+static const MemoryRegionOps digic_timer_ops = {
+    .read = digic_timer_read,
+    .write = digic_timer_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void digic_timer_tick(void *opaque)
+{
+    DigicTimerState *s = opaque;
+
+    ptimer_run(s->ptimer, 1);
+}
+
+static void digic_timer_init(Object *obj)
+{
+    DigicTimerState *s = DIGIC_TIMER(obj);
+
+    s->bh = qemu_bh_new(digic_timer_tick, s);
+    s->ptimer = ptimer_init(s->bh);
+
+    /* FIXME: there is no documentation on Digic timer
+     * frequency setup so let's it always run on 1 MHz
+     * */
+    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
+            TYPE_DIGIC_TIMER, 0x100);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static const TypeInfo digic_timer_info = {
+    .name = TYPE_DIGIC_TIMER,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(DigicTimerState),
+    .instance_init = digic_timer_init,
+};
+
+static void digic_timer_register_type(void)
+{
+    type_register_static(&digic_timer_info);
+}
+
+type_init(digic_timer_register_type)
diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
new file mode 100644
index 0000000..6483516
--- /dev/null
+++ b/hw/timer/digic-timer.h
@@ -0,0 +1,19 @@
+#ifndef HW_TIMER_DIGIC_TIMER_H
+#define HW_TIMER_DIGIC_TIMER_H
+
+#include "hw/sysbus.h"
+#include "qemu/typedefs.h"
+#include "hw/ptimer.h"
+
+#define TYPE_DIGIC_TIMER "digic-timer"
+#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
+
+typedef struct DigicTimerState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    QEMUBH *bh;
+    ptimer_state *ptimer;
+} DigicTimerState;
+
+#endif /* HW_TIMER_DIGIC_TIMER_H */
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
index 0ef4723..472d7d7 100644
--- a/include/hw/arm/digic.h
+++ b/include/hw/arm/digic.h
@@ -10,6 +10,11 @@
 
 #include "cpu-qom.h"
 
+#include "hw/timer/digic-timer.h"
+
+#define DIGIC4_NB_TIMERS 3
+#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + n * 0x100)
+
 #define TYPE_DIGIC "digic"
 
 #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
@@ -18,6 +23,8 @@ typedef struct DigicState {
     Object parent_obj;
 
     ARMCPU cpu;
+
+    DigicTimerState timer[DIGIC4_NB_TIMERS];
 } DigicState;
 
 #endif /* __DIGIC_H__ */
-- 
1.8.4.rc3

[Qemu-devel] [RFC v3 4/5] hw/arm/digic: add UART support

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic.c         |  14 ++++
 hw/char/Makefile.objs  |   1 +
 hw/char/digic-uart.c   | 197 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/char/digic-uart.h   |  27 +++++++
 include/hw/arm/digic.h |   4 +
 5 files changed, 243 insertions(+)
 create mode 100644 hw/char/digic-uart.c
 create mode 100644 hw/char/digic-uart.h

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index a71364b..89ab61c 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -45,6 +45,11 @@ static void digic_init(Object *obj)
         snprintf(name, 9, "timer[%d]", i);
         object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
     }
+
+    object_initialize(&s->uart, sizeof(s->uart), TYPE_DIGIC_UART);
+    dev = DEVICE(&s->uart);
+    qdev_set_parent_bus(dev, sysbus_get_default());
+    object_property_add_child(obj, "uart", OBJECT(&s->uart), NULL);
 }
 
 static void digic_realize(DeviceState *dev, Error **errp)
@@ -70,6 +75,15 @@ static void digic_realize(DeviceState *dev, Error **errp)
         sbd = SYS_BUS_DEVICE(&s->timer[i]);
         sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
     }
+
+    object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
+    if (err != NULL) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    sbd = SYS_BUS_DEVICE(&s->uart);
+    sysbus_mmio_map(sbd, 0, DIGIC_UART_BASE);
 }
 
 static void digic_class_init(ObjectClass *oc, void *data)
diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index f8f3dbc..00d37ac 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -14,6 +14,7 @@ obj-$(CONFIG_COLDFIRE) += mcf_uart.o
 obj-$(CONFIG_OMAP) += omap_uart.o
 obj-$(CONFIG_SH4) += sh_serial.o
 obj-$(CONFIG_PSERIES) += spapr_vty.o
+obj-$(CONFIG_DIGIC) += digic-uart.o
 
 common-obj-$(CONFIG_ETRAXFS) += etraxfs_ser.o
 common-obj-$(CONFIG_ISA_DEBUG) += debugcon.o
diff --git a/hw/char/digic-uart.c b/hw/char/digic-uart.c
new file mode 100644
index 0000000..bb581f0
--- /dev/null
+++ b/hw/char/digic-uart.c
@@ -0,0 +1,197 @@
+/*
+ * QEMU model of the Canon Digic UART block.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * See "Serial terminal" docs here:
+ *   http://magiclantern.wikia.com/wiki/Register_Map#Misc_Registers
+ *
+ * The QEMU model of the Milkymist UART block by Michael Walle
+ * is used as a template.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "hw/hw.h"
+#include "hw/sysbus.h"
+#include "sysemu/char.h"
+
+#include "hw/char/digic-uart.h"
+
+enum {
+    ST_RX_RDY = (1 << 0),
+    ST_TX_RDY = (1 << 1),
+};
+
+static uint64_t digic_uart_read(void *opaque, hwaddr addr,
+                          unsigned size)
+{
+    DigicUartState *s = opaque;
+
+    addr >>= 2;
+
+    switch (addr) {
+    case R_RX:
+        s->regs[R_ST] &= ~(ST_RX_RDY);
+        break;
+
+    case R_ST:
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP,
+            "digic_uart: read access to unknown register 0x"
+            TARGET_FMT_plx, addr << 2);
+    }
+
+    return s->regs[addr];
+}
+
+static void digic_uart_write(void *opaque, hwaddr addr, uint64_t value,
+                       unsigned size)
+{
+    DigicUartState *s = opaque;
+    unsigned char ch = value;
+
+    addr >>= 2;
+
+    switch (addr) {
+    case R_TX:
+        if (s->chr) {
+            qemu_chr_fe_write_all(s->chr, &ch, 1);
+        }
+        break;
+
+    case R_ST:
+        /*
+         * Ignore write to R_ST.
+         *
+         * The point is that this register is actively used
+         * during receiving and transmitting symbols,
+         * but we don't know the function of most of bits.
+         *
+         * Ignoring writes to R_ST is only a simplification
+         * of the model. It has no perceptible side effects
+         * for existing guests.
+         */
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP,
+            "digic_uart: write access to unknown register 0x"
+            TARGET_FMT_plx, addr << 2);
+    }
+}
+
+static const MemoryRegionOps uart_mmio_ops = {
+    .read = digic_uart_read,
+    .write = digic_uart_write,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static int uart_can_rx(void *opaque)
+{
+    DigicUartState *s = opaque;
+
+    return !(s->regs[R_ST] & ST_RX_RDY);
+}
+
+static void uart_rx(void *opaque, const uint8_t *buf, int size)
+{
+    DigicUartState *s = opaque;
+
+    assert(uart_can_rx(opaque));
+
+    s->regs[R_ST] |= ST_RX_RDY;
+    s->regs[R_RX] = *buf;
+}
+
+static void uart_event(void *opaque, int event)
+{
+}
+
+static void digic_uart_reset(DeviceState *d)
+{
+    DigicUartState *s = DIGIC_UART(d);
+    int i;
+
+    for (i = 0; i < R_MAX; i++) {
+        s->regs[i] = 0;
+    }
+    s->regs[R_ST] = ST_TX_RDY;
+}
+
+static void digic_uart_realize(DeviceState *dev, Error **errp)
+{
+    DigicUartState *s = DIGIC_UART(dev);
+
+    s->chr = qemu_char_get_next_serial();
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
+    }
+}
+
+static void digic_uart_init(Object *obj)
+{
+    DigicUartState *s = DIGIC_UART(obj);
+
+    memory_region_init_io(&s->regs_region, OBJECT(s), &uart_mmio_ops, s,
+                          "digic-uart", R_MAX * 4);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->regs_region);
+}
+
+static const VMStateDescription vmstate_digic_uart = {
+    .name = "digic-uart",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, DigicUartState, R_MAX),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void digic_uart_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = digic_uart_realize;
+    dc->reset = digic_uart_reset;
+    dc->vmsd = &vmstate_digic_uart;
+}
+
+static const TypeInfo digic_uart_info = {
+    .name          = TYPE_DIGIC_UART,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(DigicUartState),
+    .instance_init = digic_uart_init,
+    .class_init    = digic_uart_class_init,
+};
+
+static void digic_uart_register_types(void)
+{
+    type_register_static(&digic_uart_info);
+}
+
+type_init(digic_uart_register_types)
diff --git a/hw/char/digic-uart.h b/hw/char/digic-uart.h
new file mode 100644
index 0000000..ca48f4e
--- /dev/null
+++ b/hw/char/digic-uart.h
@@ -0,0 +1,27 @@
+#ifndef HW_CHAR_DIGIC_UART_H
+#define HW_CHAR_DIGIC_UART_H
+
+#include "hw/sysbus.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_DIGIC_UART "digic-uart"
+#define DIGIC_UART(obj) \
+    OBJECT_CHECK(DigicUartState, (obj), TYPE_DIGIC_UART)
+
+enum {
+    R_TX = 0x00,
+    R_RX,
+    R_ST = (0x14 >> 2),
+    R_MAX
+};
+
+typedef struct DigicUartState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion regs_region;
+    CharDriverState *chr;
+
+    uint32_t regs[R_MAX];
+} DigicUartState;
+
+#endif /* HW_CHAR_DIGIC_UART_H */
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
index 472d7d7..e4d664a 100644
--- a/include/hw/arm/digic.h
+++ b/include/hw/arm/digic.h
@@ -11,10 +11,13 @@
 #include "cpu-qom.h"
 
 #include "hw/timer/digic-timer.h"
+#include "hw/char/digic-uart.h"
 
 #define DIGIC4_NB_TIMERS 3
 #define DIGIC4_TIMER_BASE(n)    (0xc0210000 + n * 0x100)
 
+#define DIGIC_UART_BASE      0xc0800000
+
 #define TYPE_DIGIC "digic"
 
 #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
@@ -25,6 +28,7 @@ typedef struct DigicState {
     ARMCPU cpu;
 
     DigicTimerState timer[DIGIC4_NB_TIMERS];
+    DigicUartState uart;
 } DigicState;
 
 #endif /* __DIGIC_H__ */
-- 
1.8.4.rc3

[Qemu-devel] [RFC v3 5/5] hw/arm/digic: add NOR ROM support

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic_boards.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 0b99227..b5a9e1a 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -1,6 +1,13 @@
 #include "hw/boards.h"
 #include "exec/address-spaces.h"
 #include "hw/arm/digic.h"
+#include "hw/block/flash.h"
+#include "hw/loader.h"
+#include "sysemu/sysemu.h"
+
+#define DIGIC4_ROM0_BASE      0xf0000000
+#define DIGIC4_ROM1_BASE      0xf8000000
+# define DIGIC4_ROM_MAX_SIZE      0x08000000
 
 typedef struct DigicBoardState {
     DigicState *digic;
@@ -9,6 +16,10 @@ typedef struct DigicBoardState {
 
 typedef struct DigicBoard {
     hwaddr ram_size;
+    void (*add_rom0)(DigicBoardState *, hwaddr, const char *);
+    const char *rom0_def_filename;
+    void (*add_rom1)(DigicBoardState *, hwaddr, const char *);
+    const char *rom1_def_filename;
     hwaddr start_addr;
 } DigicBoard;
 
@@ -35,11 +46,74 @@ static void digic4_board_init(DigicBoard *board)
 
     digic4_board_setup_ram(s, board->ram_size);
 
+    if (board->add_rom0) {
+        board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
+    }
+
+    if (board->add_rom1) {
+        board->add_rom1(s, DIGIC4_ROM1_BASE, board->rom1_def_filename);
+    }
+
     s->digic->cpu.env.regs[15] = board->start_addr;
 }
 
+static void digic_load_rom(DigicBoardState *s, hwaddr addr,
+        hwaddr max_size, const char *def_filename)
+{
+
+    target_long rom_size;
+    const char *filename;
+
+    if (bios_name) {
+        filename = bios_name;
+    } else {
+        filename = def_filename;
+    }
+
+    if (filename) {
+        char *fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, filename);
+
+        if (!fn) {
+            fprintf(stderr, "Couldn't find rom image '%s'.\n", filename);
+            exit(1);
+        }
+
+        rom_size = load_image_targphys(fn, addr, max_size);
+        if (rom_size < 0 || rom_size > max_size) {
+            fprintf(stderr, "Couldn't load rom image '%s'\n", filename);
+            exit(1);
+        }
+    }
+}
+
+static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, hwaddr addr,
+        const char *def_filename)
+{
+#define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
+#define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024)
+
+    /*
+     * Samsung K8P3215UQB:
+     *  * AMD command set;
+     *  * multiple sector size: some sectors are 8K the other ones are 64K.
+     * Alas! The pflash_cfi02_register() function creates a flash
+     * device with unified sector size.
+     */
+    pflash_cfi02_register(addr, NULL, "pflash", FLASH_K8P3215UQB_SIZE,
+            NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
+            FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE,
+            DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
+            4,
+            0x00EC, 0x007E, 0x0003, 0x0001,
+            0x0555, 0x2aa, 0);
+
+    digic_load_rom(s, addr, FLASH_K8P3215UQB_SIZE, def_filename);
+}
+
 static DigicBoard digic4_board_canon_a1100 = {
     .ram_size = 64 * 1024 * 1024,
+    .add_rom1 = digic4_add_k8p3215uqb_rom,
+    .rom1_def_filename = "canon-a1100-rom1.bin",
     /* CHDK recommends this address for ROM disassembly */
     .start_addr = 0xffc00000,
 };
-- 
1.8.4.rc3

Re: [Qemu-devel] [RFC v3 3/5] hw/arm/digic: add timer support

[Peter Crosthwaite]

On Wed, Sep 4, 2013 at 3:21 PM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  hw/arm/digic.c         |  25 ++++++++++
>  hw/timer/Makefile.objs |   1 +
>  hw/timer/digic-timer.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/timer/digic-timer.h |  19 ++++++++
>  include/hw/arm/digic.h |   7 +++
>  5 files changed, 174 insertions(+)
>  create mode 100644 hw/timer/digic-timer.c
>  create mode 100644 hw/timer/digic-timer.h
>
> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 95a9fcd..a71364b 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -30,21 +30,46 @@
>  static void digic_init(Object *obj)
>  {
>      DigicState *s = DIGIC(obj);
> +    DeviceState *dev;
> +    int i;
>
>      object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
>      object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +
> +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> +        char name[9];
> +

Bit of a trap if theres every more than 10 timers as the name string
will run off the end if %d is below with 10. Its ok to round up a
little just for defensiveness.

> +        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
> +        dev = DEVICE(&s->timer[i]);
> +        qdev_set_parent_bus(dev, sysbus_get_default());
> +        snprintf(name, 9, "timer[%d]", i);
> +        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
> +    }
>  }
>
>  static void digic_realize(DeviceState *dev, Error **errp)
>  {
>      DigicState *s = DIGIC(dev);
>      Error *err = NULL;
> +    SysBusDevice *sbd;
> +    int i;
>
>      object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
>      if (err != NULL) {
>          error_propagate(errp, err);
>          return;
>      }
> +
> +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sbd = SYS_BUS_DEVICE(&s->timer[i]);
> +        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
> +    }
>  }
>
>  static void digic_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index eca5905..5479aee 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -25,5 +25,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
>  obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
>  obj-$(CONFIG_SH4) += sh_timer.o
>  obj-$(CONFIG_TUSB6010) += tusb6010.o
> +obj-$(CONFIG_DIGIC) += digic-timer.o
>
>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
> new file mode 100644
> index 0000000..c6cf7ee
> --- /dev/null
> +++ b/hw/timer/digic-timer.c
> @@ -0,0 +1,122 @@
> +/*
> + * QEMU model of the Canon Digic timer block.
> + *
> + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> + *
> + * This model is based on reverse engineering efforts
> + * made by CHDK (http://chdk.wikia.com) and
> + * Magic Lantern (http://www.magiclantern.fm) projects
> + * contributors.
> + *
> + * See "Timer/Clock Module" docs here:
> + *   http://magiclantern.wikia.com/wiki/Register_Map
> + *
> + * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
> + * is used as a template.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "qemu/main-loop.h"
> +
> +#include "hw/timer/digic-timer.h"
> +
> +#ifdef DEBUG_DIGIC_TIMER
> +#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif

I think we were encouraging unconditional compilation of error printfs
rather than stripping them. This means bugs in maybe change patterns
involving types which affect debug printfs can be caught in developer
compile testing.

Something like this would work, although Andreas played with this
recently and may have more convincing ideas.

#ifndef XILINX_SPIPS_ERR_DEBUG
#define XILINX_SPIPS_ERR_DEBUG 0
#endif

#define DB_PRINT_L(level, ...) do { \
    if (XILINX_SPIPS_ERR_DEBUG > (level)) { \
        fprintf(stderr,  ": %s: ", __func__); \
        fprintf(stderr, ## __VA_ARGS__); \
    } \
} while (0);

> +
> +# define DIGIC_TIMER_CONTROL 0x00
> +# define DIGIC_TIMER_VALUE 0x0c
> +
> +static uint64_t digic_timer_read(void *opaque, hwaddr offset,
> +        unsigned size)
> +{
> +    DigicTimerState *s = opaque;
> +    uint32_t ret = 0;
> +
> +    switch (offset) {
> +    case DIGIC_TIMER_VALUE:
> +        ret = (uint32_t)ptimer_get_count(s->ptimer);
> +        ret = ret & 0xffff;
> +        break;
> +    default:
> +        DPRINTF("Bad offset %x\n", (int)offset);

Guest error or Unimp?

> +    }
> +
> +    DPRINTF("offset 0x%x, value 0x%x\n", offset, ret);
> +    return ret;
> +}
> +
> +static void digic_timer_write(void *opaque, hwaddr offset,
> +        uint64_t value, unsigned size)
> +{
> +    DigicTimerState *s = opaque;
> +
> +    /* FIXME: just now we ignore timer enable bit */
> +    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
> +    ptimer_run(s->ptimer, 1);
> +}
> +
> +static const MemoryRegionOps digic_timer_ops = {
> +    .read = digic_timer_read,
> +    .write = digic_timer_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void digic_timer_tick(void *opaque)
> +{
> +    DigicTimerState *s = opaque;
> +
> +    ptimer_run(s->ptimer, 1);
> +}
> +
> +static void digic_timer_init(Object *obj)
> +{
> +    DigicTimerState *s = DIGIC_TIMER(obj);
> +
> +    s->bh = qemu_bh_new(digic_timer_tick, s);
> +    s->ptimer = ptimer_init(s->bh);
> +
> +    /* FIXME: there is no documentation on Digic timer
> +     * frequency setup so let's it always run on 1 MHz

"let" (no "'s"). s/on/at

> +     * */

Extra * unneeded.

> +    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
> +            TYPE_DIGIC_TIMER, 0x100);

Line continued function patameters should line up just past the
opening ( on the next line:

function(foo,
         bar

> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
> +
> +static const TypeInfo digic_timer_info = {
> +    .name = TYPE_DIGIC_TIMER,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(DigicTimerState),
> +    .instance_init = digic_timer_init,
> +};
> +
> +static void digic_timer_register_type(void)
> +{
> +    type_register_static(&digic_timer_info);
> +}
> +
> +type_init(digic_timer_register_type)
> diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
> new file mode 100644
> index 0000000..6483516
> --- /dev/null
> +++ b/hw/timer/digic-timer.h
> @@ -0,0 +1,19 @@
> +#ifndef HW_TIMER_DIGIC_TIMER_H
> +#define HW_TIMER_DIGIC_TIMER_H
> +
> +#include "hw/sysbus.h"
> +#include "qemu/typedefs.h"
> +#include "hw/ptimer.h"
> +
> +#define TYPE_DIGIC_TIMER "digic-timer"
> +#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
> +
> +typedef struct DigicTimerState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    QEMUBH *bh;
> +    ptimer_state *ptimer;
> +} DigicTimerState;
> +
> +#endif /* HW_TIMER_DIGIC_TIMER_H */
> diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> index 0ef4723..472d7d7 100644
> --- a/include/hw/arm/digic.h
> +++ b/include/hw/arm/digic.h
> @@ -10,6 +10,11 @@
>
>  #include "cpu-qom.h"
>
> +#include "hw/timer/digic-timer.h"
> +
> +#define DIGIC4_NB_TIMERS 3
> +#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + n * 0x100)

(n) in its usage to guard against order of operations confusion incase
macro is ever used with an arithmetic expression.

> +
>  #define TYPE_DIGIC "digic"
>
>  #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
> @@ -18,6 +23,8 @@ typedef struct DigicState {
>      Object parent_obj;
>
>      ARMCPU cpu;
> +
> +    DigicTimerState timer[DIGIC4_NB_TIMERS];
>  } DigicState;
>
>  #endif /* __DIGIC_H__ */
> --
> 1.8.4.rc3
>
>

Re: [Qemu-devel] [RFC v3 3/5] hw/arm/digic: add timer support

[Antony Pavlov]

On Wed, 4 Sep 2013 16:18:37 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> On Wed, Sep 4, 2013 at 3:21 PM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  hw/arm/digic.c         |  25 ++++++++++
> >  hw/timer/Makefile.objs |   1 +
> >  hw/timer/digic-timer.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/timer/digic-timer.h |  19 ++++++++
> >  include/hw/arm/digic.h |   7 +++
> >  5 files changed, 174 insertions(+)
> >  create mode 100644 hw/timer/digic-timer.c
> >  create mode 100644 hw/timer/digic-timer.h
> >
> > diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> > index 95a9fcd..a71364b 100644
> > --- a/hw/arm/digic.c
> > +++ b/hw/arm/digic.c
> > @@ -30,21 +30,46 @@
> >  static void digic_init(Object *obj)
> >  {
> >      DigicState *s = DIGIC(obj);
> > +    DeviceState *dev;
> > +    int i;
> >
> >      object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
> >      object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> > +
> > +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> > +        char name[9];
> > +
> 
> Bit of a trap if theres every more than 10 timers as the name string
> will run off the end if %d is below with 10. Its ok to round up a
> little just for defensiveness.
> 
> > +        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
> > +        dev = DEVICE(&s->timer[i]);
> > +        qdev_set_parent_bus(dev, sysbus_get_default());
> > +        snprintf(name, 9, "timer[%d]", i);
> > +        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
> > +    }
> >  }
> >
> >  static void digic_realize(DeviceState *dev, Error **errp)
> >  {
> >      DigicState *s = DIGIC(dev);
> >      Error *err = NULL;
> > +    SysBusDevice *sbd;
> > +    int i;
> >
> >      object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
> >      if (err != NULL) {
> >          error_propagate(errp, err);
> >          return;
> >      }
> > +
> > +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> > +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
> > +        if (err != NULL) {
> > +            error_propagate(errp, err);
> > +            return;
> > +        }
> > +
> > +        sbd = SYS_BUS_DEVICE(&s->timer[i]);
> > +        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
> > +    }
> >  }
> >
> >  static void digic_class_init(ObjectClass *oc, void *data)
> > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> > index eca5905..5479aee 100644
> > --- a/hw/timer/Makefile.objs
> > +++ b/hw/timer/Makefile.objs
> > @@ -25,5 +25,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
> >  obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
> >  obj-$(CONFIG_SH4) += sh_timer.o
> >  obj-$(CONFIG_TUSB6010) += tusb6010.o
> > +obj-$(CONFIG_DIGIC) += digic-timer.o
> >
> >  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> > diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
> > new file mode 100644
> > index 0000000..c6cf7ee
> > --- /dev/null
> > +++ b/hw/timer/digic-timer.c
> > @@ -0,0 +1,122 @@
> > +/*
> > + * QEMU model of the Canon Digic timer block.
> > + *
> > + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> > + *
> > + * This model is based on reverse engineering efforts
> > + * made by CHDK (http://chdk.wikia.com) and
> > + * Magic Lantern (http://www.magiclantern.fm) projects
> > + * contributors.
> > + *
> > + * See "Timer/Clock Module" docs here:
> > + *   http://magiclantern.wikia.com/wiki/Register_Map
> > + *
> > + * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
> > + * is used as a template.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + *
> > + */
> > +
> > +#include "hw/sysbus.h"
> > +#include "hw/ptimer.h"
> > +#include "qemu/main-loop.h"
> > +
> > +#include "hw/timer/digic-timer.h"
> > +
> > +#ifdef DEBUG_DIGIC_TIMER
> > +#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
> > +#else
> > +#define DPRINTF(fmt, ...) do {} while (0)
> > +#endif
> 
> I think we were encouraging unconditional compilation of error printfs
> rather than stripping them. This means bugs in maybe change patterns
> involving types which affect debug printfs can be caught in developer
> compile testing.

  Yes, barebox project have same the same tendency. 

> Something like this would work, although Andreas played with this
> recently and may have more convincing ideas.
> 
> #ifndef XILINX_SPIPS_ERR_DEBUG
> #define XILINX_SPIPS_ERR_DEBUG 0
> #endif
> 
> #define DB_PRINT_L(level, ...) do { \
>     if (XILINX_SPIPS_ERR_DEBUG > (level)) { \
>         fprintf(stderr,  ": %s: ", __func__); \
>         fprintf(stderr, ## __VA_ARGS__); \
>     } \
> } while (0);

  Do we really need a preprocessor macro here?
  IMHO we can use a static inline function.

> > +
> > +# define DIGIC_TIMER_CONTROL 0x00
> > +# define DIGIC_TIMER_VALUE 0x0c
> > +
> > +static uint64_t digic_timer_read(void *opaque, hwaddr offset,
> > +        unsigned size)
> > +{
> > +    DigicTimerState *s = opaque;
> > +    uint32_t ret = 0;
> > +
> > +    switch (offset) {
> > +    case DIGIC_TIMER_VALUE:
> > +        ret = (uint32_t)ptimer_get_count(s->ptimer);
> > +        ret = ret & 0xffff;
> > +        break;
> > +    default:
> > +        DPRINTF("Bad offset %x\n", (int)offset);
> 
> Guest error or Unimp?
> 
> > +    }
> > +
> > +    DPRINTF("offset 0x%x, value 0x%x\n", offset, ret);
> > +    return ret;
> > +}
> > +
> > +static void digic_timer_write(void *opaque, hwaddr offset,
> > +        uint64_t value, unsigned size)
> > +{
> > +    DigicTimerState *s = opaque;
> > +
> > +    /* FIXME: just now we ignore timer enable bit */
> > +    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
> > +    ptimer_run(s->ptimer, 1);
> > +}
> > +
> > +static const MemoryRegionOps digic_timer_ops = {
> > +    .read = digic_timer_read,
> > +    .write = digic_timer_write,
> > +    .impl = {
> > +        .min_access_size = 4,
> > +        .max_access_size = 4,
> > +    },
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static void digic_timer_tick(void *opaque)
> > +{
> > +    DigicTimerState *s = opaque;
> > +
> > +    ptimer_run(s->ptimer, 1);
> > +}
> > +
> > +static void digic_timer_init(Object *obj)
> > +{
> > +    DigicTimerState *s = DIGIC_TIMER(obj);
> > +
> > +    s->bh = qemu_bh_new(digic_timer_tick, s);
> > +    s->ptimer = ptimer_init(s->bh);
> > +
> > +    /* FIXME: there is no documentation on Digic timer
> > +     * frequency setup so let's it always run on 1 MHz
> 
> "let" (no "'s"). s/on/at
> 
> > +     * */
> 
> Extra * unneeded.
> 
> > +    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
> > +
> > +    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
> > +            TYPE_DIGIC_TIMER, 0x100);
> 
> Line continued function patameters should line up just past the
> opening ( on the next line:
> 
> function(foo,
>          bar
> 
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> > +}
> > +
> > +static const TypeInfo digic_timer_info = {
> > +    .name = TYPE_DIGIC_TIMER,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(DigicTimerState),
> > +    .instance_init = digic_timer_init,
> > +};
> > +
> > +static void digic_timer_register_type(void)
> > +{
> > +    type_register_static(&digic_timer_info);
> > +}
> > +
> > +type_init(digic_timer_register_type)
> > diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
> > new file mode 100644
> > index 0000000..6483516
> > --- /dev/null
> > +++ b/hw/timer/digic-timer.h
> > @@ -0,0 +1,19 @@
> > +#ifndef HW_TIMER_DIGIC_TIMER_H
> > +#define HW_TIMER_DIGIC_TIMER_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "qemu/typedefs.h"
> > +#include "hw/ptimer.h"
> > +
> > +#define TYPE_DIGIC_TIMER "digic-timer"
> > +#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
> > +
> > +typedef struct DigicTimerState {
> > +    SysBusDevice parent_obj;
> > +
> > +    MemoryRegion iomem;
> > +    QEMUBH *bh;
> > +    ptimer_state *ptimer;
> > +} DigicTimerState;
> > +
> > +#endif /* HW_TIMER_DIGIC_TIMER_H */
> > diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> > index 0ef4723..472d7d7 100644
> > --- a/include/hw/arm/digic.h
> > +++ b/include/hw/arm/digic.h
> > @@ -10,6 +10,11 @@
> >
> >  #include "cpu-qom.h"
> >
> > +#include "hw/timer/digic-timer.h"
> > +
> > +#define DIGIC4_NB_TIMERS 3
> > +#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + n * 0x100)
> 
> (n) in its usage to guard against order of operations confusion incase
> macro is ever used with an arithmetic expression.
> 
> > +
> >  #define TYPE_DIGIC "digic"
> >
> >  #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
> > @@ -18,6 +23,8 @@ typedef struct DigicState {
> >      Object parent_obj;
> >
> >      ARMCPU cpu;
> > +
> > +    DigicTimerState timer[DIGIC4_NB_TIMERS];
> >  } DigicState;
> >
> >  #endif /* __DIGIC_H__ */
> > --
> > 1.8.4.rc3
> >
> >


-- 
-- 
Best regards,
  Antony Pavlov

Re: [Qemu-devel] [RFC v3 3/5] hw/arm/digic: add timer support

[Peter Maydell]

On 4 September 2013 06:21, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> ---
>  hw/arm/digic.c         |  25 ++++++++++
>  hw/timer/Makefile.objs |   1 +
>  hw/timer/digic-timer.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/timer/digic-timer.h |  19 ++++++++
>  include/hw/arm/digic.h |   7 +++
>  5 files changed, 174 insertions(+)
>  create mode 100644 hw/timer/digic-timer.c
>  create mode 100644 hw/timer/digic-timer.h
>
> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 95a9fcd..a71364b 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -30,21 +30,46 @@
>  static void digic_init(Object *obj)
>  {
>      DigicState *s = DIGIC(obj);
> +    DeviceState *dev;
> +    int i;
>
>      object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
>      object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +
> +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> +        char name[9];
> +
> +        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
> +        dev = DEVICE(&s->timer[i]);
> +        qdev_set_parent_bus(dev, sysbus_get_default());
> +        snprintf(name, 9, "timer[%d]", i);

Just use g_strdup_printf() and g_free() it afterwards;
that avoids having to care about array sizes at all,
and this is runs-once code so performance isn't
critical.

> +        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
> +    }
>  }
>
>  static void digic_realize(DeviceState *dev, Error **errp)
>  {
>      DigicState *s = DIGIC(dev);
>      Error *err = NULL;
> +    SysBusDevice *sbd;
> +    int i;
>
>      object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
>      if (err != NULL) {
>          error_propagate(errp, err);
>          return;
>      }
> +
> +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sbd = SYS_BUS_DEVICE(&s->timer[i]);
> +        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
> +    }
>  }
>
>  static void digic_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index eca5905..5479aee 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -25,5 +25,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
>  obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
>  obj-$(CONFIG_SH4) += sh_timer.o
>  obj-$(CONFIG_TUSB6010) += tusb6010.o
> +obj-$(CONFIG_DIGIC) += digic-timer.o
>
>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
> new file mode 100644
> index 0000000..c6cf7ee
> --- /dev/null
> +++ b/hw/timer/digic-timer.c
> @@ -0,0 +1,122 @@
> +/*
> + * QEMU model of the Canon Digic timer block.
> + *
> + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> + *
> + * This model is based on reverse engineering efforts
> + * made by CHDK (http://chdk.wikia.com) and
> + * Magic Lantern (http://www.magiclantern.fm) projects
> + * contributors.
> + *
> + * See "Timer/Clock Module" docs here:
> + *   http://magiclantern.wikia.com/wiki/Register_Map
> + *
> + * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
> + * is used as a template.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "qemu/main-loop.h"
> +
> +#include "hw/timer/digic-timer.h"
> +
> +#ifdef DEBUG_DIGIC_TIMER
> +#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif
> +
> +# define DIGIC_TIMER_CONTROL 0x00
> +# define DIGIC_TIMER_VALUE 0x0c
> +
> +static uint64_t digic_timer_read(void *opaque, hwaddr offset,
> +        unsigned size)
> +{
> +    DigicTimerState *s = opaque;
> +    uint32_t ret = 0;
> +
> +    switch (offset) {
> +    case DIGIC_TIMER_VALUE:
> +        ret = (uint32_t)ptimer_get_count(s->ptimer);
> +        ret = ret & 0xffff;
> +        break;
> +    default:
> +        DPRINTF("Bad offset %x\n", (int)offset);
> +    }
> +
> +    DPRINTF("offset 0x%x, value 0x%x\n", offset, ret);
> +    return ret;
> +}
> +
> +static void digic_timer_write(void *opaque, hwaddr offset,
> +        uint64_t value, unsigned size)
> +{
> +    DigicTimerState *s = opaque;
> +
> +    /* FIXME: just now we ignore timer enable bit */
> +    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
> +    ptimer_run(s->ptimer, 1);
> +}
> +
> +static const MemoryRegionOps digic_timer_ops = {
> +    .read = digic_timer_read,
> +    .write = digic_timer_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void digic_timer_tick(void *opaque)
> +{
> +    DigicTimerState *s = opaque;
> +
> +    ptimer_run(s->ptimer, 1);

This isn't doing anything interesting. Surely it
should trigger an interrupt or something?

> +}
> +
> +static void digic_timer_init(Object *obj)
> +{
> +    DigicTimerState *s = DIGIC_TIMER(obj);
> +
> +    s->bh = qemu_bh_new(digic_timer_tick, s);
> +    s->ptimer = ptimer_init(s->bh);
> +
> +    /* FIXME: there is no documentation on Digic timer
> +     * frequency setup so let's it always run on 1 MHz
> +     * */

this line should be "*/", not "* */".

> +    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
> +            TYPE_DIGIC_TIMER, 0x100);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
> +
> +static const TypeInfo digic_timer_info = {
> +    .name = TYPE_DIGIC_TIMER,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(DigicTimerState),
> +    .instance_init = digic_timer_init,
> +};

You need a DeviceClass::reset function (which
among other things should reset the ptimer
you're using); this will require a class init
function so you have somewhere to set it up.

You also need a VMStateDescription so migration
works.

> +
> +static void digic_timer_register_type(void)
> +{
> +    type_register_static(&digic_timer_info);
> +}
> +
> +type_init(digic_timer_register_type)
> diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
> new file mode 100644
> index 0000000..6483516
> --- /dev/null
> +++ b/hw/timer/digic-timer.h
> @@ -0,0 +1,19 @@
> +#ifndef HW_TIMER_DIGIC_TIMER_H
> +#define HW_TIMER_DIGIC_TIMER_H
> +
> +#include "hw/sysbus.h"
> +#include "qemu/typedefs.h"
> +#include "hw/ptimer.h"
> +
> +#define TYPE_DIGIC_TIMER "digic-timer"
> +#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
> +
> +typedef struct DigicTimerState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    QEMUBH *bh;
> +    ptimer_state *ptimer;
> +} DigicTimerState;
> +
> +#endif /* HW_TIMER_DIGIC_TIMER_H */
> diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> index 0ef4723..472d7d7 100644
> --- a/include/hw/arm/digic.h
> +++ b/include/hw/arm/digic.h
> @@ -10,6 +10,11 @@
>
>  #include "cpu-qom.h"
>
> +#include "hw/timer/digic-timer.h"
> +
> +#define DIGIC4_NB_TIMERS 3
> +#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + n * 0x100)

Brackets round the (n) on right hand side of a macro
definition, please.
> +
>  #define TYPE_DIGIC "digic"
>
>  #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
> @@ -18,6 +23,8 @@ typedef struct DigicState {
>      Object parent_obj;
>
>      ARMCPU cpu;
> +
> +    DigicTimerState timer[DIGIC4_NB_TIMERS];
>  } DigicState;
>
>  #endif /* __DIGIC_H__ */
> --
> 1.8.4.rc3


thanks
-- PMM