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

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

[Antony Pavlov]

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

 Changes since v3:

  1. fix typos and formatting
  2. digic-timer: drop DPRINTF
  3. digic-timer: fix DIGIC4_TIMER_BASE() macro
  4. digic.c: fix max timer device string

 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 the '-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 v4 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

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

[Andreas Färber]

Am 05.09.2013 09:52, schrieb 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

Please place it on a line of its own, using

obj-$(CONFIG_DIGIC) += digic.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__

Identifiers starting in underscore are reserved. Suggest DIGIC_H,
HW_ARM_DIGIC_H, QEMU_DIGIC_H or something like that.

> +
> +#include "cpu-qom.h"

This looks bogus, cpu-qom.h cannot be included on its own since it
depends in CPUARMState in cpu.h these days.

> +
> +#define TYPE_DIGIC "digic"
> +
> +#define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
> +
> +typedef struct DigicState {

Please add
    /*< private >*/

> +    Object parent_obj;

    /*< private >*/

markers for documentation.

It needs to be DeviceState parent_obj though.

> +
> +    ARMCPU cpu;
> +} DigicState;
> +
> +#endif /* __DIGIC_H__ */

Otherwise looks good.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Antony Pavlov]

On Thu, 05 Sep 2013 20:08:34 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.09.2013 09:52, schrieb 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
> 
> Please place it on a line of its own, using
> 
> obj-$(CONFIG_DIGIC) += digic.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__
> 
> Identifiers starting in underscore are reserved. Suggest DIGIC_H,
> HW_ARM_DIGIC_H, QEMU_DIGIC_H or something like that.
> 
> > +
> > +#include "cpu-qom.h"
> 
> This looks bogus, cpu-qom.h cannot be included on its own since it
> depends in CPUARMState in cpu.h these days.
>
> > +
> > +#define TYPE_DIGIC "digic"
> > +
> > +#define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
> > +
> > +typedef struct DigicState {
> 
> Please add
>     /*< private >*/
> 
> > +    Object parent_obj;
> 
>     /*< private >*/

/*< public >*/ ?

> 
> markers for documentation.
> 
> It needs to be DeviceState parent_obj though.

In your tegra2 support 'Object parent_obj' is used in a similar situation.

http://repo.or.cz/w/qemu/afaerber.git/blob/refs/heads/tegra:/include/hw/arm/tegra2.h#l42

> > +
> > +    ARMCPU cpu;
> > +} DigicState;
> > +
> > +#endif /* __DIGIC_H__ */
> 
> Otherwise looks good.

-- 
Best regards,
  Antony Pavlov

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

[Andreas Färber]

Am 05.09.2013 23:23, schrieb Antony Pavlov:
> On Thu, 05 Sep 2013 20:08:34 +0200
> Andreas Färber <afaerber@suse.de> wrote:
>> Am 05.09.2013 09:52, schrieb Antony Pavlov:
>>> 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
[...]
>>> +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
[...]
>>> +typedef struct DigicState {
>>
>> Please add
>>     /*< private >*/
>>
>>> +    Object parent_obj;
>>
>>     /*< private >*/
> 
> /*< public >*/ ?

Yes, sorry, copy&paste and then noticing Object. ;)
Or just leave the latter out so that all fields are undocumented.

>> markers for documentation.
>>
>> It needs to be DeviceState parent_obj though.
> 
> In your tegra2 support 'Object parent_obj' is used in a similar situation.
> 
> http://repo.or.cz/w/qemu/afaerber.git/blob/refs/heads/tegra:/include/hw/arm/tegra2.h#l42

Thanks for spotting, fixed. (It used to be derived from TYPE_OBJECT, but
we decided to provide QOM realize support only for devices.)

Unfortunately Tegra kernel is still stuck after USB init either way...

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Antony Pavlov]

On Thu, 05 Sep 2013 23:38:26 +0200
Andreas Färber <afaerber@suse.de> wrote:

> Am 05.09.2013 23:23, schrieb Antony Pavlov:
> > On Thu, 05 Sep 2013 20:08:34 +0200
> > Andreas Färber <afaerber@suse.de> wrote:
> >> Am 05.09.2013 09:52, schrieb Antony Pavlov:
> >>> 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
> [...]
> >>> +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
> [...]
> >>> +typedef struct DigicState {
> >>
> >> Please add
> >>     /*< private >*/
> >>
> >>> +    Object parent_obj;
> >>
> >>     /*< private >*/
> > 
> > /*< public >*/ ?
> 
> Yes, sorry, copy&paste and then noticing Object. ;)
> Or just leave the latter out so that all fields are undocumented.

I have grepped the qemu code and found that the "private" and "public" labels
usage is:

=== quote start ===
    typedef struct SomethingState {
       /*< private >*/
       DeviceState parent_obj;
       /*< public >*/

       some_type public_field1;
=== quote end ===

Is there any intention in empty line after the "public" label and no empty line befor it?
Can I wrote something like this:

=== quote start ===
    typedef struct SomethingState {
       /*< private >*/
       DeviceState parent_obj;

       /*< public >*/
       some_type public_field1;
=== quote end ===

?


> >> markers for documentation.
> >>
> >> It needs to be DeviceState parent_obj though.
> > 
> > In your tegra2 support 'Object parent_obj' is used in a similar situation.
> > 
> > http://repo.or.cz/w/qemu/afaerber.git/blob/refs/heads/tegra:/include/hw/arm/tegra2.h#l42
> 
> Thanks for spotting, fixed. (It used to be derived from TYPE_OBJECT, but
> we decided to provide QOM realize support only for devices.)
> 
> Unfortunately Tegra kernel is still stuck after USB init either way...
> 
> Cheers,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-- 
-- 
Best regards,
  Antony Pavlov

[Qemu-devel] [RFC v4 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

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

[Peter Maydell]

On 5 September 2013 08:52, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> 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 @@

Copyright and license summary comment at the top of the
file, please.

> +#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;

This is odd. What happens on real hardware? Is
the firmware actually at zero?

> +}
> +
> +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

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

[Antony Pavlov]

On Thu, 5 Sep 2013 18:54:30 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 5 September 2013 08:52, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > 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 @@
> 
> Copyright and license summary comment at the top of the
> file, please.
> 
> > +#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;
> 
> This is odd. What happens on real hardware? Is
> the firmware actually at zero?

There is no public documentation on Digic chips.
Only Canon's engineers know something about real Digic's bootprocess.

Nowadays Canon produce two families of photocameras:
 * relatevely cheap Point-and-Shot cameras (PowerShot series);
 * more expensive cameras with exchangeable lens (EOS series).

The DIGIC chips has two regions for NOR ROM mapping.
The PowerShot cameras (as rule) have simple firmware and  use only one ROM region.
The EOS cameras have more complex firmware and use both ROM regions.
So the board->start_addr parameter is used just for selection of ROM region.

> > +}
> > +
> > +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


-- 
-- 
Best regards,
  Antony Pavlov

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

[Antony Pavlov]

Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
---
 hw/arm/digic.c         |  26 +++++++++++
 hw/timer/Makefile.objs |   1 +
 hw/timer/digic-timer.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/timer/digic-timer.h |  19 ++++++++
 include/hw/arm/digic.h |   7 +++
 5 files changed, 170 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..5932a6a 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -30,21 +30,47 @@
 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++) {
+#define DIGIC_TIMER_NAME_MLEN    11
+        char name[DIGIC_TIMER_NAME_MLEN];
+
+        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, DIGIC_TIMER_NAME_MLEN, "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..75e2713
--- /dev/null
+++ b/hw/timer/digic-timer.c
@@ -0,0 +1,117 @@
+/*
+ * 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"
+
+# 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 &= 0xffff;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                      "digic-timer: read access to unknown register 0x"
+                      TARGET_FMT_plx, offset);
+    }
+
+    return ret;
+}
+
+static void digic_timer_write(void *opaque, hwaddr offset,
+                              uint64_t value, unsigned size)
+{
+    DigicTimerState *s = opaque;
+
+    /* FIXME: without documentation every write just starts timer */
+    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 it always run at 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..48c9f9c 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 v4 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 5932a6a..d99ffd9 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -46,6 +46,11 @@ static void digic_init(Object *obj)
         snprintf(name, DIGIC_TIMER_NAME_MLEN, "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)
@@ -71,6 +76,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..f41f74a
--- /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 48c9f9c..c587ade 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

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

[Peter Maydell]

On 5 September 2013 08:52, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> +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;

Does this UART really not have a FIFO?

> +}

> 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 @@

Copyright/license header comment at start of all files,
please (ditto below).

> +#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];

So this thing only has five registers, one of
which at least (R_TX) doesn't have state that
you'll be storing in this struct anyway, and you're
not implementing reads-as-written behaviour for the
unknown registers, so I think you should drop the
regs[] array and just have individual uint32_t fields
for the registers you implement.

> +} DigicUartState;
> +
> +#endif /* HW_CHAR_DIGIC_UART_H */
> diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> index 48c9f9c..c587ade 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

Does this really need to be in the header file?
It seems like private implementation information that
could go in the source file that needs it.

(same is probably true of some of the other macros.)

> +
>  #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
>

thanks
-- PMM

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

[Antony Pavlov]

On Thu, 5 Sep 2013 19:17:50 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 5 September 2013 08:52, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > +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;
> 
> Does this UART really not have a FIFO?

There is no public documentation on Digic chips.
Only Canon's engineers know something about Digic's FIFO (if it exists :).

> > +}
> 
> > 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 @@
> 
> Copyright/license header comment at start of all files,
> please (ditto below).
> 
> > +#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];
> 
> So this thing only has five registers, one of
> which at least (R_TX) doesn't have state that
> you'll be storing in this struct anyway, and you're
> not implementing reads-as-written behaviour for the
> unknown registers, so I think you should drop the
> regs[] array and just have individual uint32_t fields
> for the registers you implement.
> 
> > +} DigicUartState;
> > +
> > +#endif /* HW_CHAR_DIGIC_UART_H */
> > diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> > index 48c9f9c..c587ade 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
> 
> Does this really need to be in the header file?
> It seems like private implementation information that
> could go in the source file that needs it.
> 
> (same is probably true of some of the other macros.)
> 
> > +
> >  #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
> >
> 
> thanks
> -- PMM


-- 
-- 
Best regards,
  Antony Pavlov

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

[Peter Maydell]

On 6 September 2013 07:54, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Thu, 5 Sep 2013 19:17:50 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> Does this UART really not have a FIFO?
>
> There is no public documentation on Digic chips.
> Only Canon's engineers know something about Digic's FIFO (if it exists :).

You can deduce its existence though -- does the
UART let you feed two or three characters to it
at faster than whatever the serial line speed is
before it sets the "stop sending me bits" status
bit, or does it stop after the first?
If the real firmware uses this for anything remotely
serious (ie for more than trivial and default-disabled
debug info) it probably does have a FIFO. However,
let's assume it doesn't for now.

-- PMM

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

[Antony Pavlov]

On Fri, 6 Sep 2013 08:25:20 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 6 September 2013 07:54, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > On Thu, 5 Sep 2013 19:17:50 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >> Does this UART really not have a FIFO?
> >
> > There is no public documentation on Digic chips.
> > Only Canon's engineers know something about Digic's FIFO (if it exists :).
> 
> You can deduce its existence though -- does the
> UART let you feed two or three characters to it
> at faster than whatever the serial line speed is
> before it sets the "stop sending me bits" status
> bit, or does it stop after the first?

No, I can't :)

How can I check this mysterious "stop sending me bits" status bit?
Do you mean the UART hardware flow control pin?
I can't use hardware flow control as I have only 3-wire UART on the camera.
Moreover I have to enable hardware flow control in software, but I have
no information how to do it.

> If the real firmware uses this for anything remotely
> serious (ie for more than trivial and default-disabled
> debug info) it probably does have a FIFO. However,
> let's assume it doesn't for now.
> 
> -- PMM


-- 
-- 
Best regards,
  Antony Pavlov

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

[Peter Maydell]

On 6 September 2013 14:00, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Fri, 6 Sep 2013 08:25:20 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>> You can deduce its existence though -- does the
>> UART let you feed two or three characters to it
>> at faster than whatever the serial line speed is
>> before it sets the "stop sending me bits" status
>> bit, or does it stop after the first?
>
> No, I can't :)
>
> How can I check this mysterious "stop sending me bits" status bit?
> Do you mean the UART hardware flow control pin?

I meant ST_TX_RDY, but looking again at the
docs perhaps I misinterpreted it. Do you really
have to do:
 * write byte to TX register
 * set TX_RDY bit in status register

to get it to send out a byte?

I was expecting a more standard UART interface
which is typically:
 * write byte to TX register
 * if the UART is unable to accept any more
   bytes (ie FIFO full, or just immediately if
   no FIFO present) it clears TX_RDY
 * software has to wait for TX_RDY to be set
   before writing another byte to TX

-- PMM

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

[Antony Pavlov]

On Fri, 6 Sep 2013 14:40:14 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 6 September 2013 14:00, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > On Fri, 6 Sep 2013 08:25:20 +0100
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >> You can deduce its existence though -- does the
> >> UART let you feed two or three characters to it
> >> at faster than whatever the serial line speed is
> >> before it sets the "stop sending me bits" status
> >> bit, or does it stop after the first?
> >
> > No, I can't :)
> >
> > How can I check this mysterious "stop sending me bits" status bit?
> > Do you mean the UART hardware flow control pin?
> 
> I meant ST_TX_RDY, but looking again at the
> docs perhaps I misinterpreted it. Do you really
> have to do:
>  * write byte to TX register
>  * set TX_RDY bit in status register
> 
> to get it to send out a byte?

I don't want publish Canon firmware disassembled listings and discuss it
in the maillist for legal reasons.

But Canon A1100 firmware can use this routine:
 1. wait TX_RDY bit in status register;
 2. write TX_RDY and some other (unknown behaviour) bit to status register;
 3. write byte to TX register.

IMHO the step 2 is bizarre.

You can easely check me: the disassembly instructions and firmware images can
be found via CHDK site. A puts()-like routine can be found at 0xffff18f0
in the Canon A1100IS firmware image.

> I was expecting a more standard UART interface
> which is typically:
>  * write byte to TX register
>  * if the UART is unable to accept any more
>    bytes (ie FIFO full, or just immediately if
>    no FIFO present) it clears TX_RDY
>  * software has to wait for TX_RDY to be set
>    before writing another byte to TX

IMHO the more standart UART outc() routune is:
   * wait for UART "transmitter is ready to send next char" flag;
   * write char to TX register.

E.g. see
https://github.com/frantony/barebox/blob/master/arch/mips/include/asm/debug_ll_ns16550.h#L60
or
https://github.com/frantony/barebox/blob/master/arch/arm/mach-imx/include/mach/debug_ll.h#L49

-- 
Best regards,
  Antony Pavlov

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

[Peter Maydell]

On 7 September 2013 06:45, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> But Canon A1100 firmware can use this routine:
>  1. wait TX_RDY bit in status register;
>  2. write TX_RDY and some other (unknown behaviour) bit to status register;
>  3. write byte to TX register.
>
> IMHO the step 2 is bizarre.

Maybe the TX_RDY bit is write-1-to-clear? Some devices
use that kind of thing for a "conditions that happened that
could trigger an interrupt" (and if you're using them for
polling you can just poll the register).

> IMHO the more standart UART outc() routune is:
>    * wait for UART "transmitter is ready to send next char" flag;
>    * write char to TX register.

Yes, you're right. In either case you can see if there's
a uart via:
 * wait for TX_RDY to be set
 * clear TX_RDY
 * write byte to TX
 * immediately look at TX_RDY -- if it's set straight away
   there's almost certainly a FIFO involved; otherwise it
   would have to wait for the char to go out over serial
   before setting it again

This is kind of a sidetrack though, we can model it without a
UART for the moment.

-- PMM

[Qemu-devel] [RFC v4 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..850e320 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 v4 5/5] hw/arm/digic: add NOR ROM support

[Peter Maydell]

On 5 September 2013 08:52, Antony Pavlov <antonynpavlov@gmail.com> wrote:

> +    /*
> +     * 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.
> +     */

This comment would be more useful if it explained what the
practical consequences of this inaccuracy in emulation were...

thanks
-- PMM