On 19 Aug 2014, at 16:03, Peter Maydell <peter.mayd...@linaro.org> wrote:

> On 17 August 2014 15:24, 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 read 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             | 109 
>> ++++++++++++++++++++++++++++++++++++++++
>> include/hw/misc/arm_sp810.h     |  80 +++++++++++++++++++++++++++++
>> 4 files changed, 191 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
> 
> We mostly don't use the 'arm' prefix (compare pl330, pl011,
> and other examples for other architectures), so you can
> remove it from filenames and variable names here.

Ok, will remove it then.
> 
>> 
>> # 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..7aff9bd
>> --- /dev/null
>> +++ b/hw/misc/arm_sp810.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * 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 = {
> 
> ^^^ should be vmstate_sp810.

Thanks for catching this. I will change it.
> 
>> +    .name = "arm_sp810",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT8(timeren0_sel, arm_sp810_state),
>> +        VMSTATE_UINT8(timeren1_sel, arm_sp810_state),
>> +        VMSTATE_UINT8(timeren2_sel, arm_sp810_state),
>> +        VMSTATE_UINT8(timeren3_sel, arm_sp810_state),
> 
> These are read only properties, so you don't need to save them.

Okay.

> 
>> +        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->timeren3_sel << 21) | (s->timeren2_sel << 19)
>> +                | (s->timeren1_sel << 17) | (s->timeren0_sel << 15);
>> +    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_UINT8("timeren0-sel", arm_sp810_state, timeren0_sel,
>> +                                                       SCCTRL_TIMER_REFCLK),
>> +    DEFINE_PROP_UINT8("timeren1-sel", arm_sp810_state, timeren1_sel,
>> +                                                       SCCTRL_TIMER_REFCLK),
>> +    DEFINE_PROP_UINT8("timeren2-sel", arm_sp810_state, timeren2_sel,
>> +                                                       SCCTRL_TIMER_REFCLK),
>> +    DEFINE_PROP_UINT8("timeren3-sel", arm_sp810_state, timeren3_sel,
>> +                                                       SCCTRL_TIMER_REFCLK),
> 
> Which input signals in the hardware do these properties
> correspond to? I can't figure out what the mapping is.
> As far as I can tell from the documentation the bits
> in SCCTRL are just always reset to 0 and are not
> controlled by external signals (which is what qdev
> properties should typically correspond to).

Actually I don’t know how it is done in hardware. My goal was
to reflect the default speed of the SP804 timer in QEMU’s vexpress
emulation in the SCCTRL. What do you suggest in this case?
I could remove the QOM properties and set the reset value of the
TimerEnXSel bits in SCCTRL to 1 (TIMCLK) to match the SCCTRL 
with the speed of the SP804. 

Thanks,
Fabian

> 
> thanks
> -- PMM


Reply via email to