From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39307) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHh3v-0007u3-Ek for qemu-devel@nongnu.org; Thu, 05 Sep 2013 17:26:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VHh3r-0003SF-A0 for qemu-devel@nongnu.org; Thu, 05 Sep 2013 17:25:59 -0400 Received: from mail-la0-x22a.google.com ([2a00:1450:4010:c03::22a]:52595) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VHh3q-0003S5-TV for qemu-devel@nongnu.org; Thu, 05 Sep 2013 17:25:55 -0400 Received: by mail-la0-f42.google.com with SMTP id ep20so2130308lab.1 for ; Thu, 05 Sep 2013 14:25:53 -0700 (PDT) Date: Fri, 6 Sep 2013 01:23:28 +0400 From: Antony Pavlov Message-Id: <20130906012328.fbab3c6df75fb6cc12c52ce5@gmail.com> In-Reply-To: <5228C8A2.7030306@suse.de> References: <1378367579-1099-1-git-send-email-antonynpavlov@gmail.com> <1378367579-1099-2-git-send-email-antonynpavlov@gmail.com> <5228C8A2.7030306@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC v4 1/5] hw/arm: add very initial support for Canon DIGIC SoC List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?ISO-8859-1?Q?F=E4rber?= Cc: qemu-devel@nongnu.org On Thu, 05 Sep 2013 20:08:34 +0200 Andreas F=E4rber 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. > >=20 > > 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. > >=20 > > Signed-off-by: Antony Pavlov > > --- > > 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 > >=20 > > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-soft= mmu.mak > > index ac0815d..0d1d783 100644 > > --- a/default-configs/arm-softmmu.mak > > +++ b/default-configs/arm-softmmu.mak > > @@ -63,6 +63,7 @@ CONFIG_FRAMEBUFFER=3Dy > > CONFIG_XILINX_SPIPS=3Dy > > =20 > > CONFIG_A9SCU=3Dy > > +CONFIG_DIGIC=3Dy > > CONFIG_MARVELL_88W8618=3Dy > > CONFIG_OMAP=3Dy > > CONFIG_TSC210X=3Dy > > 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 +=3D integratorcp.o kzm.o mainstone.o musicpal.o = nseries.o > > obj-y +=3D omap_sx1.o palm.o realview.o spitz.o stellaris.o > > obj-y +=3D tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o > > =20 > > -obj-y +=3D armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o > > +obj-y +=3D armv7m.o digic.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx= _pic.o >=20 > Please place it on a line of its own, using >=20 > obj-$(CONFIG_DIGIC) +=3D digic.o >=20 > > obj-y +=3D 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 > > + * > > + * 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 . > > + * > > + */ > > + > > +#include "hw/sysbus.h" > > +#include "target-arm/cpu-qom.h" > > +#include "hw/arm/digic.h" > > + > > +static void digic_init(Object *obj) > > +{ > > + DigicState *s =3D 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 =3D DIGIC(dev); > > + Error *err =3D NULL; > > + > > + object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err); > > + if (err !=3D NULL) { > > + error_propagate(errp, err); > > + return; > > + } > > +} > > + > > +static void digic_class_init(ObjectClass *oc, void *data) > > +{ > > + DeviceClass *dc =3D DEVICE_CLASS(oc); > > + > > + dc->realize =3D digic_realize; > > +} > > + > > +static const TypeInfo digic_type_info =3D { > > + .name =3D TYPE_DIGIC, > > + .parent =3D TYPE_DEVICE, > > + .instance_size =3D sizeof(DigicState), > > + .instance_init =3D digic_init, > > + .class_init =3D 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 > > + * > > + */ > > + > > +#ifndef __DIGIC_H__ > > +#define __DIGIC_H__ >=20 > Identifiers starting in underscore are reserved. Suggest DIGIC_H, > HW_ARM_DIGIC_H, QEMU_DIGIC_H or something like that. >=20 > > + > > +#include "cpu-qom.h" >=20 > 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 { >=20 > Please add > /*< private >*/ >=20 > > + Object parent_obj; >=20 > /*< private >*/ /*< public >*/ ? >=20 > markers for documentation. >=20 > 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__ */ >=20 > Otherwise looks good. --=A0 Best regards, =A0 Antony Pavlov