[v2,2/2] i.MX: Add i.MX25 CCM driver

The i.MX25 CCM device is different from the i.MX31 one.

So for now the emulation was not correct even if linux was working OK
on top of the i.MX25 emulation.

We add an i.MX25 specific CCM driver and use it in the i.MX25 SOC
emulation.

Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---

Changes since v1:
 * rework loging to match other i.MX drivers

 hw/arm/fsl-imx25.c          |   2 +-
 hw/misc/Makefile.objs       |   1 +
 hw/misc/imx25_ccm.c         | 228 ++++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/fsl-imx25.h  |   4 +-
 include/hw/misc/imx25_ccm.h |  61 ++++++++++++
 5 files changed, 293 insertions(+), 3 deletions(-)
 create mode 100644 hw/misc/imx25_ccm.c
 create mode 100644 include/hw/misc/imx25_ccm.h

On Wed, Oct 28, 2015 at 4:02 PM, Jean-Christophe Dubois
<jcd@tribudubois.net> wrote:
> The i.MX25 CCM device is different from the i.MX31 one.
>
> So for now the emulation was not correct even if linux was working OK
> on top of the i.MX25 emulation.
>
> We add an i.MX25 specific CCM driver and use it in the i.MX25 SOC
> emulation.
>

s/driver/device here and in subject.

> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
> ---
>
> Changes since v1:
>  * rework loging to match other i.MX drivers
>
>  hw/arm/fsl-imx25.c          |   2 +-
>  hw/misc/Makefile.objs       |   1 +
>  hw/misc/imx25_ccm.c         | 228 ++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/fsl-imx25.h  |   4 +-
>  include/hw/misc/imx25_ccm.h |  61 ++++++++++++
>  5 files changed, 293 insertions(+), 3 deletions(-)
>  create mode 100644 hw/misc/imx25_ccm.c
>  create mode 100644 include/hw/misc/imx25_ccm.h
>
> diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
> index 620c5c6..5301c1c 100644
> --- a/hw/arm/fsl-imx25.c
> +++ b/hw/arm/fsl-imx25.c
> @@ -38,7 +38,7 @@ static void fsl_imx25_init(Object *obj)
>      object_initialize(&s->avic, sizeof(s->avic), TYPE_IMX_AVIC);
>      qdev_set_parent_bus(DEVICE(&s->avic), sysbus_get_default());
>
> -    object_initialize(&s->ccm, sizeof(s->ccm), TYPE_IMX31_CCM);
> +    object_initialize(&s->ccm, sizeof(s->ccm), TYPE_IMX25_CCM);
>      qdev_set_parent_bus(DEVICE(&s->ccm), sysbus_get_default());
>
>      for (i = 0; i < FSL_IMX25_NUM_UARTS; i++) {
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 79b3487..e8dc22d 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -27,6 +27,7 @@ obj-$(CONFIG_ECCMEMCTL) += eccmemctl.o
>  obj-$(CONFIG_EXYNOS4) += exynos4210_pmu.o
>  obj-$(CONFIG_IMX) += imx_ccm.o
>  obj-$(CONFIG_IMX) += imx31_ccm.o
> +obj-$(CONFIG_IMX) += imx25_ccm.o
>  obj-$(CONFIG_MILKYMIST) += milkymist-hpdmc.o
>  obj-$(CONFIG_MILKYMIST) += milkymist-pfpu.o
>  obj-$(CONFIG_MAINSTONE) += mst_fpga.o
> diff --git a/hw/misc/imx25_ccm.c b/hw/misc/imx25_ccm.c
> new file mode 100644
> index 0000000..e76292d
> --- /dev/null
> +++ b/hw/misc/imx25_ccm.c
> @@ -0,0 +1,228 @@
> +/*
> + * IMX25 Clock Control Module
> + *
> + * Copyright (C) 2012 NICTA
> + * Updated by Jean-Christophe Dubois <jcd@tribudubois.net>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + * To get the timer frequencies right, we need to emulate at least part of
> + * the CCM.
> + */
> +
> +#include "hw/misc/imx25_ccm.h"
> +
> +#ifndef DEBUG_IMX25_CCM
> +#define DEBUG_IMX25_CCM 0
> +#endif
> +
> +#define DPRINTF(fmt, args...) \
> +    do { \
> +        if (DEBUG_IMX25_CCM) { \
> +            fprintf(stderr, "[%s]%s: " fmt , TYPE_IMX25_CCM, \
> +                                             __func__, ##args); \
> +        } \
> +    } while (0)
> +
> +#define CKIH_FREQ 24000000 /* 24MHz crystal input */
> +

Is the crystal freq really decided by the controller? Should this be
on the board level instead? Same applies to prev patch.

> +static int imx25_ccm_post_load(void *opaque, int version_id);
> +

Can you just reorder to remove the forward reference?

> +static const VMStateDescription vmstate_imx25_ccm = {
> +    .name = TYPE_IMX25_CCM,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(mpctl, IMX25CCMState),
> +        VMSTATE_UINT32(upctl, IMX25CCMState),
> +        VMSTATE_UINT32(cctl, IMX25CCMState),
> +        VMSTATE_UINT32_ARRAY(cgcr, IMX25CCMState, 3),
> +        VMSTATE_UINT32_ARRAY(pcdr, IMX25CCMState, 4),
> +        VMSTATE_UINT32(rcsr, IMX25CCMState),
> +        VMSTATE_UINT32(crdr, IMX25CCMState),
> +        VMSTATE_UINT32_ARRAY(dcvr, IMX25CCMState, 4),
> +        VMSTATE_UINT32_ARRAY(ltr, IMX25CCMState, 4),
> +        VMSTATE_UINT32_ARRAY(ltbr, IMX25CCMState, 2),
> +        VMSTATE_UINT32_ARRAY(pmcr, IMX25CCMState, 3),
> +        VMSTATE_UINT32(mcr, IMX25CCMState),
> +        VMSTATE_UINT32_ARRAY(lpimr, IMX25CCMState, 2),
> +        VMSTATE_UINT32(pll_refclk_freq, IMX25CCMState),
> +        VMSTATE_END_OF_LIST()
> +    },
> +    .post_load = imx25_ccm_post_load,
> +};
> +
> +static void imx25_ccm_update_clocks(IMX25CCMState *s)
> +{
> +    DPRINTF("\n");
> +    /*
> +     * If we ever emulate more clocks, this should switch to a data-driven
> +     * approach
> +     */
> +
> +    /* Our input CLK */
> +    s->pll_refclk_freq = CKIH_FREQ;
> +
> +    /* Set MCU PLL CLK */
> +    if (EXTRACT(s->cctl, MPLL_BYPASS)) {
> +        imx_ccm_set_clock_frequency(CLK_MPLL, s->pll_refclk_freq);
> +    } else {
> +        imx_ccm_set_clock_frequency(CLK_MPLL,
> +                                    imx_ccm_calc_pll(s->mpctl,
> +                                                     s->pll_refclk_freq));
> +    }
> +
> +    /* Set USB PLL CLK */
> +    imx_ccm_set_clock_frequency(CLK_UPLL,
> +                                imx_ccm_calc_pll(s->upctl,
> +                                                 s->pll_refclk_freq));
> +
> +    /* Set Processor clock */
> +    if (EXTRACT(s->cctl, ARM_SRC)) {
> +        imx_ccm_set_clock_frequency(CLK_MCU,
> +                                    (imx_ccm_get_clock_frequency(CLK_MPLL) * 3
> +                                     / 4)
> +                                     / (1 + EXTRACT(s->cctl, ARM_CLK_DIV)));
> +    } else {
> +        imx_ccm_set_clock_frequency(CLK_MCU,
> +                                    imx_ccm_get_clock_frequency(CLK_MPLL) /
> +                                    (1 + EXTRACT(s->cctl, ARM_CLK_DIV)));
> +    }
> +
> +    /* Set AHB CLK */
> +    imx_ccm_set_clock_frequency(CLK_AHB, imx_ccm_get_clock_frequency(CLK_MCU) /
> +                                         (1 + EXTRACT(s->cctl, AHB_CLK_DIV)));
> +
> +    /* Set IPG CLK */
> +    imx_ccm_set_clock_frequency(CLK_IPG, imx_ccm_get_clock_frequency(CLK_AHB) /
> +                                         2);
> +
> +    /* Set 32K cristal CLK */
> +    imx_ccm_set_clock_frequency(CLK_32k, CKIL_FREQ);
> +
> +}
> +
> +static void imx25_ccm_reset(DeviceState *dev)
> +{
> +    IMX25CCMState *s = IMX25_CCM(dev);
> +
> +    DPRINTF("\n");
> +
> +    s->mpctl = 0x800b2c01;
> +    s->upctl = 0x84002800;
> +    s->cctl = 0x40030000;
> +    s->cgcr[0] = 0x028A0100;
> +    s->cgcr[1] = 0x04008100;
> +    s->cgcr[2] = 0x00000438;
> +    s->pcdr[0] = 0x01010101;
> +    s->pcdr[1] = 0x01010101;
> +    s->pcdr[2] = 0x01010101;
> +    s->pcdr[3] = 0x01010101;
> +    s->rcsr = 0;
> +    s->crdr = 0;
> +    s->dcvr[0] = 0;
> +    s->dcvr[1] = 0;
> +    s->dcvr[2] = 0;
> +    s->dcvr[3] = 0;
> +    s->ltr[0] = 0;
> +    s->ltr[1] = 0;
> +    s->ltr[2] = 0;
> +    s->ltr[3] = 0;
> +    s->ltbr[0] = 0;
> +    s->ltbr[1] = 0;
> +    s->pmcr[0] = 0x00A00000;
> +    s->pmcr[1] = 0x0000A030;
> +    s->pmcr[2] = 0x0000A030;
> +    s->mcr = 0x43000000;
> +    s->lpimr[0] = 0;
> +    s->lpimr[1] = 0;
> +
> +    /* default ROM Boot will change the reset values */

"boot"

> +    s->cctl |= INSERT(1, ARM_SRC);
> +    s->cctl |= INSERT(1, AHB_CLK_DIV);
> +
> +    imx25_ccm_update_clocks(s);
> +}
> +
> +static uint64_t imx25_ccm_read(void *opaque, hwaddr offset,
> +                                unsigned size)
> +{
> +    IMX25CCMState *s = (IMX25CCMState *)opaque;
> +    uint32_t *reg = &s->mpctl;

I think it is just easier to just make it an array in state, and index
into it with macros for the non-array uses. Then you can also memset-0
in reset and just set the non-zeros afterwards.

> +
> +    DPRINTF("(offset=0x%" HWADDR_PRIx ")\n", offset);
> +
> +    if (offset >= 0x70) {
> +        return 0;
> +    } else {
> +        return reg[offset >> 2];
> +    }
> +}
> +
> +static void imx25_ccm_write(void *opaque, hwaddr offset,
> +                          uint64_t value, unsigned size)
> +{
> +    IMX25CCMState *s = (IMX25CCMState *)opaque;
> +    uint32_t *reg = &s->mpctl;
> +
> +    DPRINTF("(offset=0x%" HWADDR_PRIx ", value = %x)\n",
> +            offset, (unsigned int)value);
> +
> +    if (offset < 0x70) {
> +        reg[offset >> 2] = value;
> +    }
> +
> +    imx25_ccm_update_clocks(s);
> +}
> +
> +static const struct MemoryRegionOps imx25_ccm_ops = {
> +    .read = imx25_ccm_read,
> +    .write = imx25_ccm_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static int imx25_ccm_init(SysBusDevice *dev)
> +{
> +    IMX25CCMState *s = IMX25_CCM(dev);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(dev), &imx25_ccm_ops, s,
> +                          TYPE_IMX25_CCM, 0x1000);
> +    sysbus_init_mmio(dev, &s->iomem);
> +
> +    return 0;
> +}
> +
> +static int imx25_ccm_post_load(void *opaque, int version_id)
> +{
> +    IMX25CCMState *s = (IMX25CCMState *)opaque;
> +
> +    imx25_ccm_update_clocks(s);
> +
> +    return 0;
> +}
> +
> +static void imx25_ccm_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SysBusDeviceClass *sbc = SYS_BUS_DEVICE_CLASS(klass);
> +
> +    sbc->init = imx25_ccm_init;

Sysbus device init is deprecated, use object instance_init and device
realize instead. I think this is hangover from the i.MX31 that is
unconverted. A possible follow up patch is to do the conversion for 31
based on this change. You have just mr_init_io and sb_init_mmio with
constant sizes, so that can be done with just an instance_init I
think.

> +    dc->reset = imx25_ccm_reset;
> +    dc->vmsd = &vmstate_imx25_ccm;
> +    dc->desc = "i.MX25 Clock Control Module";
> +}
> +
> +static const TypeInfo imx25_ccm_info = {
> +    .name = TYPE_IMX25_CCM,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(IMX25CCMState),
> +    .class_init = imx25_ccm_class_init,
> +};
> +
> +static void imx25_ccm_register_types(void)
> +{
> +    type_register_static(&imx25_ccm_info);
> +}
> +
> +type_init(imx25_ccm_register_types)
> diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h
> index 5c62fde..d0e8e9d 100644
> --- a/include/hw/arm/fsl-imx25.h
> +++ b/include/hw/arm/fsl-imx25.h
> @@ -19,7 +19,7 @@
>
>  #include "hw/arm/arm.h"
>  #include "hw/intc/imx_avic.h"
> -#include "hw/misc/imx31_ccm.h"
> +#include "hw/misc/imx25_ccm.h"
>  #include "hw/char/imx_serial.h"
>  #include "hw/timer/imx_gpt.h"
>  #include "hw/timer/imx_epit.h"
> @@ -44,7 +44,7 @@ typedef struct FslIMX25State {
>      /*< public >*/
>      ARMCPU         cpu;
>      IMXAVICState   avic;
> -    IMX31CCMState  ccm;
> +    IMX25CCMState  ccm;
>      IMXSerialState uart[FSL_IMX25_NUM_UARTS];
>      IMXGPTState    gpt[FSL_IMX25_NUM_GPTS];
>      IMXEPITState   epit[FSL_IMX25_NUM_EPITS];
> diff --git a/include/hw/misc/imx25_ccm.h b/include/hw/misc/imx25_ccm.h
> new file mode 100644
> index 0000000..20100ca
> --- /dev/null
> +++ b/include/hw/misc/imx25_ccm.h
> @@ -0,0 +1,61 @@
> +/*
> + * IMX25 Clock Control Module
> + *
> + * Copyright (C) 2012 NICTA
> + * Updated by Jean-Christophe Dubois <jcd@tribudubois.net>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef IMX25_CCM_H
> +#define IMX25_CCM_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/misc/imx_ccm.h"
> +
> +/* CCTL */
> +#define CCTL_ARM_CLK_DIV_SHIFT (30)
> +#define CCTL_ARM_CLK_DIV_MASK  (0x3)
> +#define CCTL_AHB_CLK_DIV_SHIFT (28)
> +#define CCTL_AHB_CLK_DIV_MASK  (0x3)
> +#define CCTL_MPLL_BYPASS_SHIFT (22)
> +#define CCTL_MPLL_BYPASS_MASK  (0x1)
> +#define CCTL_USB_DIV_SHIFT (16)
> +#define CCTL_USB_DIV_MASK  (0x3F)
> +#define CCTL_ARM_SRC_SHIFT (13)
> +#define CCTL_ARM_SRC_MASK  (0x1)
> +
> +#define EXTRACT(value, name) (((value) >> CCTL_##name##_SHIFT) \
> +                              & CCTL_##name##_MASK)
> +#define INSERT(value, name) (((value) & CCTL_##name##_MASK) << \
> +                             CCTL_##name##_SHIFT)
> +

I have used this approach before, but wanted to make it globally
available, this might be relevant:

https://lists.nongnu.org/archive/html/qemu-devel/2015-04/msg03614.html

We could split off just the macros from that patch into a new
register.h (basically make that P4 a patch in its own right).

Regards,
Peter

> +#define TYPE_IMX25_CCM "imx25.ccm"
> +#define IMX25_CCM(obj) OBJECT_CHECK(IMX25CCMState, (obj), TYPE_IMX25_CCM)
> +
> +typedef struct IMX25CCMState {
> +    /* <private> */
> +    SysBusDevice parent_obj;
> +
> +    /* <public> */
> +    MemoryRegion iomem;
> +
> +    uint32_t mpctl;
> +    uint32_t upctl;
> +    uint32_t cctl;
> +    uint32_t cgcr[3];
> +    uint32_t pcdr[4];
> +    uint32_t rcsr;
> +    uint32_t crdr;
> +    uint32_t dcvr[4];
> +    uint32_t ltr[4];
> +    uint32_t ltbr[2];
> +    uint32_t pmcr[3];
> +    uint32_t mcr;
> +    uint32_t lpimr[2];
> +
> +    uint32_t pll_refclk_freq;
> +} IMX25CCMState;
> +
> +#endif /* IMX25_CCM_H */
> --
> 2.5.0
>