Re: [Qemu-devel] [PATCH v2 6/9] i.MX: Add i.MX6 System Reset Controller device.

[Peter Maydell]

On 8 February 2016 at 22:08, Jean-Christophe Dubois <address@hidden> wrote:
> This controller is also present in i.MX5X devices but they are not
> yet emulated by QEMU.
>
> Signed-off-by: Jean-Christophe Dubois <address@hidden>

> @@ -0,0 +1,353 @@
> +/*
> + * IMX6 System Reset Controller
> + *
> + * Copyright (c) 2015 Jean-Christophe Dubois <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/misc/imx6_src.h"
> +#include "sysemu/sysemu.h"
> +#include "qemu/bitops.h"

#include "qemu/osdep.h" as first #include line.

> +static void imx6_src_reset(DeviceState *dev)
> +{
> +    IMX6SRCState *s = IMX6_SRC(dev);
> +
> +    DPRINTF("\n");
> +
> +    /*
> +     * We only clear the first registers as all GPR registers are preserved
> +     * over resets
> +     */
> +    memset(s->regs, 0, SRC_MAX * sizeof(uint32_t));

Comment doesn't seem to match code?

> +    /* Set reset values */
> +    s->regs[SRC_SCR] = 0x521;
> +    s->regs[SRC_SRSR] = 0x1;
> +    s->regs[SRC_SIMR] = 0x1F;
> +}
> +
> +static CPUState *imx6_src_get_cpu_by_id(uint32_t id)
> +{
> +    CPUState *cpu;
> +
> +    DPRINTF("cpu %d\n", id);
> +
> +    CPU_FOREACH(cpu) {
> +        ARMCPU *armcpu = ARM_CPU(cpu);
> +
> +        if (armcpu->mp_affinity == id) {
> +            return cpu;
> +        }
> +    }
> +
> +    qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: Resquesting unknown CPU %d\n",
> +                  TYPE_IMX6_SRC, __func__, id);
> +
> +    return NULL;
> +}
> +
> +static void imx6_src_cpu_on(uint32_t cpuid, uint32_t entry, uint32_t 
> context_id)
> +{
> +    CPUState *target_cpu_state;
> +    ARMCPU *target_cpu;
> +    CPUClass *target_cpu_class;
> +
> +    DPRINTF("cpu %d @ 0x%08x with R0 = 0x%08x\n", cpuid, entry, context_id);
> +
> +    /* change to the cpu we are powering up */
> +    target_cpu_state = imx6_src_get_cpu_by_id(cpuid);
> +    if (!target_cpu_state) {
> +        return;
> +    }
> +    target_cpu = ARM_CPU(target_cpu_state);
> +    if (!target_cpu->powered_off) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: CPU %d is already running\n",
> +                      TYPE_IMX6_SRC, __func__, cpuid);
> +        return;
> +    }
> +    target_cpu_class = CPU_GET_CLASS(target_cpu);
> +
> +    /* Initialize the cpu we are turning on */
> +    cpu_reset(target_cpu_state);
> +    target_cpu->powered_off = false;
> +    target_cpu_state->halted = 0;
> +
> +    target_cpu->env.regs[0] = context_id;
> +    target_cpu->env.thumb = entry & 1;
> +
> +    target_cpu_class->set_pc(target_cpu_state, entry);
> +}
> +
> +static void imx6_src_cpu_off(uint32_t cpuid)
> +{
> +    CPUState *target_cpu_state;
> +    ARMCPU *target_cpu;
> +
> +    DPRINTF("cpu %d\n", cpuid);
> +
> +    /* change to the cpu we are powering up */
> +    target_cpu_state = imx6_src_get_cpu_by_id(cpuid);
> +    if (!target_cpu_state) {
> +        return;
> +    }
> +    target_cpu = ARM_CPU(target_cpu_state);
> +    if (target_cpu->powered_off) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: CPU %d is already off\n",
> +                      TYPE_IMX6_SRC, __func__, cpuid);
> +        return;
> +    }
> +
> +    target_cpu->powered_off = true;
> +    target_cpu_state->halted = 1;
> +    target_cpu_state->exception_index = EXCP_HLT;
> +    cpu_loop_exit(target_cpu_state);
> +}
>
> +static void imx6_src_cpu_reset(uint32_t cpuid)
> +{
> +    CPUState *target_cpu_state;
> +    ARMCPU *target_cpu;
> +
> +    DPRINTF("cpu %d\n", cpuid);
> +
> +    /* change to the cpu we are powering up */
> +    target_cpu_state = imx6_src_get_cpu_by_id(cpuid);
> +    if (!target_cpu_state) {
> +        return;
> +    }
> +    target_cpu = ARM_CPU(target_cpu_state);
> +    if (target_cpu->powered_off) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: CPU %d is off\n",
> +                      TYPE_IMX6_SRC, __func__, cpuid);
> +        return;
> +    }
> +
> +    /* Reset the cpu we are turning on */
> +    cpu_reset(target_cpu_state);
> +}

The code that is messing about with target CPUs to power them up
and down needs to be abstracted out into a public API in target-arm/,
so it can be used both by your device and by target-arm/psci.c.
I don't want variations on this code to duplicate into various
devices, because chances are good it will need to change for
multithreaded TCG.

> +static void imx6_src_realize(DeviceState *dev, Error **errp)
> +{
> +    IMX6SRCState *s = IMX6_SRC(dev);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(dev), &imx6_src_ops, s,
> +                          TYPE_IMX6_SRC, 0x1000);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> +}
> +
> +static void imx6_src_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = imx6_src_realize;
> +    dc->reset = imx6_src_reset;
> +    dc->vmsd = &vmstate_imx6_src;
> +    dc->desc = "i.MX6 System Reset Controller";
> +}

> +/* SRC_SCR */
> +#define CORE3_ENABLE_SHIFT     (24)
> +#define CORE3_ENABLE_LENGTH    (1)
> +#define CORE2_ENABLE_SHIFT     (23)
> +#define CORE2_ENABLE_LENGTH    (1)
> +#define CORE1_ENABLE_SHIFT     (22)
> +#define CORE1_ENABLE_LENGTH    (1)
> +#define CORE3_RST_SHIFT        (16)
> +#define CORE3_RST_LENGTH       (1)
> +#define CORE2_RST_SHIFT        (15)
> +#define CORE2_RST_LENGTH       (1)
> +#define CORE1_RST_SHIFT        (14)
> +#define CORE1_RST_LENGTH       (1)
> +#define CORE0_RST_SHIFT        (13)
> +#define CORE0_RST_LENGTH       (1)
> +#define SW_IPU1_RST_SHIFT      (3)
> +#define SW_IPU1_RST_LENGTH     (1)
> +#define SW_IPU2_RST_SHIFT      (12)
> +#define SW_IPU2_RST_LENGTH     (1)
> +#define WARM_RST_ENABLE_SHIFT  (0)
> +#define WARM_RST_ENABLE_LENGTH (1)

The brackets here are unnecessary.

thanks
-- PMM