Am 10.07.2013 07:08, schrieb peter.crosthwa...@xilinx.com:
> From: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>
> The ARM A9 MPCore has a timer that is global to all CPUs in the mpcore.
> The timer is shared but each CPU has a private independent comparator
> and interrupt.
>
> Original version contributed by Francois LEGAL.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> ---
> Francois, do you want to re-add your SOB? I have changed the device
> a lot since your V2.
>
> hw/timer/Makefile.objs | 2 +-
> hw/timer/a9gtimer.c | 437
> +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 438 insertions(+), 1 deletion(-)
> create mode 100644 hw/timer/a9gtimer.c
>
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 32b5c1a..8ed379e 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -25,5 +25,5 @@ obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
> obj-$(CONFIG_SH4) += sh_timer.o
> obj-$(CONFIG_TUSB6010) += tusb6010.o
>
> -obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o
> +obj-$(CONFIG_ARM_MPTIMER) += arm_mptimer.o a9gtimer.o
> obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
> new file mode 100644
> index 0000000..f996169
> --- /dev/null
> +++ b/hw/timer/a9gtimer.c
> @@ -0,0 +1,437 @@
> +/*
> + * Global peripheral timer block for ARM A9MP
> + *
> + * (C) 2013 Xilinx Inc.
> + *
> + * Written by François LEGAL
> + * Written by Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "qemu/timer.h"
> +#include "qemu/bitops.h"
> +#include "qemu/log.h"
> +
> +#ifndef ARM_GTIMER_ERR_DEBUG
> +#define ARM_GTIMER_ERR_DEBUG 0
> +#endif
> +
> +#define DB_PRINT_L(level, ...) do { \
> + if (ARM_GTIMER_ERR_DEBUG > (level)) { \
> + fprintf(stderr, ": %s: ", __func__); \
> + fprintf(stderr, ## __VA_ARGS__); \
> + } \
> +} while (0);
> +
> +#define DB_PRINT(...) DB_PRINT_L(0, ## __VA_ARGS__)
I only spot DB_PRINT() being used below. Would be easier to grasp if it
were just #ifndef ARM_GTIMER_ERR_DEBUG #define
ARM_GTIMER_ERR_DEBUG_ENABLED 0 and if (ARM_GTIMER_ERR_DEBUG_ENABLED).
Either something is missing before ": %s: " or that's a duplicate.
> +
> +/* This device implements the Cortex-A9MP global timer. */
> +
> +#define MAX_CPUS 4
> +#define TYPE_ARM_GTIMER "arm.cortex-a9-global-timer"
> +#define ARM_GTIMER(obj) OBJECT_CHECK(A9GlobalTimerState, (obj),
> TYPE_ARM_GTIMER)
> +
> +#define R_COUNTER_LO 0x00
> +#define R_COUNTER_HI 0x04
> +
> +#define R_CONTROL 0x08
> +#define R_CONTROL_TIMER_ENABLE (1 << 0)
> +#define R_CONTROL_COMP_ENABLE (1 << 1)
> +#define R_CONTROL_IRQ_ENABLE (1 << 2)
> +#define R_CONTROL_AUTO_INCREMENT (1 << 2)
> +#define R_CONTROL_PRESCALER_SHIFT 8
> +#define R_CONTROL_PRESCALER_LEN 8
> +#define R_CONTROL_PRESCALER_MASK (((1 << R_CONTROL_PRESCALER_LEN) - 1) <<
> \
> + R_CONTROL_PRESCALER_SHIFT)
> +
> +#define R_CONTROL_BANKED (R_CONTROL_COMP_ENABLE | \
> + R_CONTROL_IRQ_ENABLE | \
> + R_CONTROL_AUTO_INCREMENT)
> +#define R_CONTROL_NEEDS_SYNC (R_CONTROL_TIMER_ENABLE | \
> + R_CONTROL_PRESCALER_MASK)
> +
> +#define R_INTERRUPT_STATUS 0x0C
> +#define R_COMPARATOR_LO 0x10
> +#define R_COMPARATOR_HI 0x14
> +#define R_AUTO_INCREMENT 0x18
> +
> +typedef struct A9GlobalTimerPerCPU A9GlobalTimerPerCPU;
> +typedef struct A9GlobalTimerState A9GlobalTimerState;
> +
> +struct A9GlobalTimerPerCPU {
> + A9GlobalTimerState *parent;
> +
> + uint32_t control; /* only per cpu banked bits valid */
> + uint64_t compare;
> + uint32_t status;
> + uint32_t inc;
> +
> + MemoryRegion iomem;
> + qemu_irq irq; /* PPI interrupts */
> +};
> +
> +struct A9GlobalTimerState {
> + SysBusDevice parent_obj;
> +
> + MemoryRegion iomem;
> + /* static props */
> + uint32_t num_cpu;
> + QEMUTimer *timer;
> +
> + uint64_t counter; /* current timer value */
> +
> + uint64_t ref_counter;
> + uint64_t cpu_ref_time; /* the cpu time as of last update of ref_counter
> */
> + uint32_t control; /* only non per cpu banked bits valid */
> +
> + A9GlobalTimerPerCPU per_cpu[MAX_CPUS];
> +};
Can we move these to a new header from the start, while not using them
elsewhere yet? In that case please add the gtk-doc private/public markers.
> +
> +typedef struct A9GTimerUpdate {
> + uint64_t now;
> + uint64_t new;
> +} A9GTimerUpdate;
> +
> +static inline int a9_gtimer_get_current_cpu(A9GlobalTimerState *s)
> +{
> + CPUState *cpu_single_cpu = ENV_GET_CPU(cpu_single_env);
> +
> + if (cpu_single_cpu->cpu_index >= s->num_cpu) {
> + hw_error("arm_mptimer: num-cpu %d but this cpu is %d!\n",
> + s->num_cpu, cpu_single_cpu->cpu_index);
> + }
> + return cpu_single_cpu->cpu_index;
> +}
[snip]
Seeing this, I have sent out my qom-cpu PULL, which replaces
cpu_single_env with current_cpu for your convenience.
Otherwise looks okay from a QOM/style viewpoint.
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
