| Message ID | 1378367579-1099-2-git-send-email-antonynpavlov@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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__ */
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