On Tue, Aug 5, 2014 at 7:32 PM, Fabian Aggeler <aggel...@ethz.ch> wrote: > This adds a device model for the PrimeXsys System Controller (SP810) > which is present in the Versatile Express motherboards. It is > so far read-only but allows to set the SCCTRL register. > > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch> > --- > default-configs/arm-softmmu.mak | 1 + > hw/misc/Makefile.objs | 1 + > hw/misc/arm_sp810.c | 98 > +++++++++++++++++++++++++++++++++++++++++ > include/hw/misc/arm_sp810.h | 85 +++++++++++++++++++++++++++++++++++ > 4 files changed, 185 insertions(+) > create mode 100644 hw/misc/arm_sp810.c > create mode 100644 include/hw/misc/arm_sp810.h > > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak > index f3513fa..67ba99a 100644 > --- a/default-configs/arm-softmmu.mak > +++ b/default-configs/arm-softmmu.mak > @@ -55,6 +55,7 @@ CONFIG_PL181=y > CONFIG_PL190=y > CONFIG_PL310=y > CONFIG_PL330=y > +CONFIG_SP810=y > CONFIG_CADENCE=y > CONFIG_XGMAC=y > CONFIG_EXYNOS4=y > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs > index 979e532..49d023b 100644 > --- a/hw/misc/Makefile.objs > +++ b/hw/misc/Makefile.objs > @@ -13,6 +13,7 @@ common-obj-$(CONFIG_PL310) += arm_l2x0.o > common-obj-$(CONFIG_INTEGRATOR_DEBUG) += arm_integrator_debug.o > common-obj-$(CONFIG_A9SCU) += a9scu.o > common-obj-$(CONFIG_ARM11SCU) += arm11scu.o > +common-obj-$(CONFIG_SP810) += arm_sp810.o > > # PKUnity SoC devices > common-obj-$(CONFIG_PUV3) += puv3_pm.o > diff --git a/hw/misc/arm_sp810.c b/hw/misc/arm_sp810.c > new file mode 100644 > index 0000000..21eb816 > --- /dev/null > +++ b/hw/misc/arm_sp810.c > @@ -0,0 +1,98 @@ > +/* > + * ARM PrimeXsys System Controller (SP810) > + * > + * Copyright (C) 2014 Fabian Aggeler <aggel...@ethz.ch> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program 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 General Public License for more details. > + * > + */ > + > +#include "hw/misc/arm_sp810.h" > + > +static const VMStateDescription vmstate_arm_sysctl = { > + .name = "arm_sp810", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(sc_ctrl, arm_sp810_state), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static uint64_t arm_sp810_read(void *opaque, hwaddr offset, unsigned size) > +{ > + arm_sp810_state *s = opaque; > + > + switch (offset) { > + case SCCTRL: > + return s->sc_ctrl; > + default: > + qemu_log_mask(LOG_UNIMP, > + "arm_sp810_read: Register not yet implemented: %s\n", > + HWADDR_PRIx); > + return 0; > + } > +} > + > +static void arm_sp810_write(void *opaque, hwaddr offset, > + uint64_t val, unsigned size) > +{ > + switch (offset) { > + default: > + qemu_log_mask(LOG_UNIMP, > + "arm_sp810_write: Register not yet implemented: %s\n", > + HWADDR_PRIx); > + return; > + } > +} > + > +static const MemoryRegionOps arm_sp810_ops = { > + .read = arm_sp810_read, > + .write = arm_sp810_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static void arm_sp810_init(Object *obj) > +{ > + arm_sp810_state *s = ARM_SP810(obj); > + > + memory_region_init_io(&s->iomem, obj, &arm_sp810_ops, s, "arm-sp810", > + 0x1000); > + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); > +} > + > +static Property arm_sp810_properties[] = { > + DEFINE_PROP_UINT32("sc-ctrl", arm_sp810_state, sc_ctrl, 0x000009), > + DEFINE_PROP_END_OF_LIST(), > +}; > +
So it looks to me like the SCCTRL contains multiple register fields for multiple configurable options. Although i'm having trouble getting my head around it as I could not easily find public docco for this core (have a link handy?). Anyways, the thinking is you should define multiple configurable options as invididual QOM properties, rather than have board level code init registers. This would mean that you DEFINE_PROP_UINT8("timeren0-sel", ...) DEFINE_PROP_UINT8("timeren1-sel", ...) Have a look at hw/dma/pl330.c for an example of invidividual configs getting packed into a single sw visible register. However these look like software configurable options. Does the hardware IP provide the option to configure these with a default or must software (probably early stage firmware) configure these to match clocking? > +static void arm_sp810_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->vmsd = &vmstate_arm_sysctl; > + dc->props = arm_sp810_properties; > +} > + > +static const TypeInfo arm_sp810_info = { > + .name = TYPE_ARM_SP810, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(arm_sp810_state), > + .instance_init = arm_sp810_init, > + .class_init = arm_sp810_class_init, > +}; > + > +static void arm_sp810_register_types(void) > +{ > + type_register_static(&arm_sp810_info); > +} > + > +type_init(arm_sp810_register_types) > diff --git a/include/hw/misc/arm_sp810.h b/include/hw/misc/arm_sp810.h > new file mode 100644 > index 0000000..33021d5 > --- /dev/null > +++ b/include/hw/misc/arm_sp810.h > @@ -0,0 +1,85 @@ > +/* > + * ARM PrimeXsys System Controller (SP810) > + * > + * Copyright (C) 2014 Fabian Aggeler <aggel...@ethz.ch> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program 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 General Public License for more details. > + * > + */ > + > +#ifndef HW_MISC_ARM_SP810H > +#define HW_MISC_ARM_SP810H > + > +#include "hw/hw.h" > +#include "hw/sysbus.h" > + > +#define TYPE_ARM_SP810 "arm_sp810" > +#define ARM_SP810(obj) OBJECT_CHECK(arm_sp810_state, (obj), TYPE_ARM_SP810) > + > +/* Register Offsets */ > +#define SCCTRL 0x000 > +#define SCSYSSTAT 0x004 > +#define SCIMCTRL 0x008 > +#define SCIMSTAT 0x00C > +#define SCXTALCTRL 0x010 > +#define SCPLLCTRL 0x014 > +#define SCPLLFCTRL 0x018 > +#define SCPERCTRL0 0x01C > +#define SCPERCTRL1 0x020 > +#define SCPEREN 0x024 > +#define SCPERDIS 0x028 > +#define SCPERCLKEN 0x02C > +#define SCPERSTAT 0x030 > +#define SCSYSID0 0xEE0 > +#define SCSYSID1 0xEE4 > +#define SCSYSID2 0xEE8 > +#define SCSYSID3 0xEEC > +#define SCITCR 0xF00 > +#define SCITIR0 0xF04 > +#define SCITIR1 0xF08 > +#define SCITOR 0xF0C > +#define SCCNTCTRL 0xF10 > +#define SCCNTDATA 0xF14 > +#define SCCNTSTEP 0xF18 > +#define SCPERIPHID0 0xFE0 > +#define SCPERIPHID1 0xFE4 > +#define SCPERIPHID2 0xFE8 > +#define SCPERIPHID3 0xFEC > +#define SCPCELLID0 0xFF0 > +#define SCPCELLID1 0xFF4 > +#define SCPCELLID2 0xFF8 > +#define SCPCELLID3 0xFFC > + > +/* System Control Register bits */ > +#define SCCTRL_TIMEREN0SEL (1 << 15) > +#define SCCTRL_TIMEREN1SEL (1 << 17) > +#define SCCTRL_TIMEREN2SEL (1 << 19) > +#define SCCTRL_TIMEREN3SEL (1 << 21) > + > +static inline DeviceState *sp810_init(hwaddr base, uint32_t scctrl) > +{ > + DeviceState *sp810; > + > + sp810 = qdev_create(NULL, "arm_sp810"); > + qdev_prop_set_uint32(sp810, "sc-ctrl", scctrl); > + qdev_init_nofail(sp810); > + sysbus_mmio_map(SYS_BUS_DEVICE(sp810), 0, base); > + return sp810; > +} > + _init helpers are progressively phasing out. Just inline this straight into vexpress - its expected for boards code to do its own qdev_foo sysbus_foo etc. > +typedef struct { > + SysBusDevice parent_obj; > + MemoryRegion iomem; > + > + uint32_t sc_ctrl; /* SCCTRL */ Redundant comment. Regards, Peter > +} arm_sp810_state; > + > +#endif > -- > 1.8.3.2 > >