[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 06/20] i.MX: Split CCM emulator in a header f
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v9 06/20] i.MX: Split CCM emulator in a header file and a source file |
Date: |
Sat, 4 Jul 2015 21:17:13 -0700 |
You need a commit message body. In trival ones like this I usally just
repeat the what with some why.
"Split CCM state struct and register defs into a header file to
prepare support for use in a SoC device"
On Sat, Jul 4, 2015 at 7:34 AM, Jean-Christophe Dubois
<address@hidden> wrote:
> Signed-off-by: Jean-Christophe Dubois <address@hidden>
> ---
>
> Changes since v1:
> * not present on v1
>
> Changes since v2:
> * not present on v2
>
> Changes since v3:
> * not present on v3
>
> Changes since v4:
> * not present on v4
>
> Changes since v5:
> * not present on v5
>
> Changes since v6:
> * not present on v6
>
> Changes since v7:
> * Splited the i.MX CCM emulator into a header file and a source file
>
> Changes since v8:
> * no change
>
> hw/misc/imx_ccm.c | 70 ++----------------------------------
> include/hw/arm/imx.h | 10 ------
> include/hw/misc/imx_ccm.h | 91
> +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 94 insertions(+), 77 deletions(-)
> create mode 100644 include/hw/misc/imx_ccm.h
>
> diff --git a/hw/misc/imx_ccm.c b/hw/misc/imx_ccm.c
> index 0920288..2e9bd9c 100644
> --- a/hw/misc/imx_ccm.c
> +++ b/hw/misc/imx_ccm.c
> @@ -2,6 +2,7 @@
> * IMX31 Clock Control Module
> *
> * Copyright (C) 2012 NICTA
> + * Updated by 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.
> @@ -10,10 +11,7 @@
> * the CCM.
> */
>
> -#include "hw/hw.h"
> -#include "hw/sysbus.h"
> -#include "sysemu/sysemu.h"
> -#include "hw/arm/imx.h"
> +#include "hw/misc/imx_ccm.h"
>
> #define CKIH_FREQ 26000000 /* 26MHz crystal input */
> #define CKIL_FREQ 32768 /* nominal 32khz clock */
> @@ -29,30 +27,6 @@ do { printf("imx_ccm: " fmt , ##args); } while (0)
>
> static int imx_ccm_post_load(void *opaque, int version_id);
>
> -#define TYPE_IMX_CCM "imx_ccm"
> -#define IMX_CCM(obj) OBJECT_CHECK(IMXCCMState, (obj), TYPE_IMX_CCM)
> -
> -typedef struct IMXCCMState {
> - SysBusDevice parent_obj;
> -
> - MemoryRegion iomem;
> -
> - uint32_t ccmr;
> - uint32_t pdr0;
> - uint32_t pdr1;
> - uint32_t mpctl;
> - uint32_t spctl;
> - uint32_t cgr[3];
> - uint32_t pmcr0;
> - uint32_t pmcr1;
> -
> - /* Frequencies precalculated on register changes */
> - uint32_t pll_refclk_freq;
> - uint32_t mcu_clk_freq;
> - uint32_t hsp_clk_freq;
> - uint32_t ipg_clk_freq;
> -} IMXCCMState;
> -
> static const VMStateDescription vmstate_imx_ccm = {
> .name = "imx-ccm",
> .version_id = 1,
> @@ -72,44 +46,6 @@ static const VMStateDescription vmstate_imx_ccm = {
> .post_load = imx_ccm_post_load,
> };
>
> -/* CCMR */
> -#define CCMR_FPME (1<<0)
> -#define CCMR_MPE (1<<3)
> -#define CCMR_MDS (1<<7)
> -#define CCMR_FPMF (1<<26)
> -#define CCMR_PRCS (3<<1)
> -
> -/* PDR0 */
> -#define PDR0_MCU_PODF_SHIFT (0)
> -#define PDR0_MCU_PODF_MASK (0x7)
> -#define PDR0_MAX_PODF_SHIFT (3)
> -#define PDR0_MAX_PODF_MASK (0x7)
> -#define PDR0_IPG_PODF_SHIFT (6)
> -#define PDR0_IPG_PODF_MASK (0x3)
> -#define PDR0_NFC_PODF_SHIFT (8)
> -#define PDR0_NFC_PODF_MASK (0x7)
> -#define PDR0_HSP_PODF_SHIFT (11)
> -#define PDR0_HSP_PODF_MASK (0x7)
> -#define PDR0_PER_PODF_SHIFT (16)
> -#define PDR0_PER_PODF_MASK (0x1f)
> -#define PDR0_CSI_PODF_SHIFT (23)
> -#define PDR0_CSI_PODF_MASK (0x1ff)
> -
> -#define EXTRACT(value, name) (((value) >> PDR0_##name##_PODF_SHIFT) \
> - & PDR0_##name##_PODF_MASK)
> -#define INSERT(value, name) (((value) & PDR0_##name##_PODF_MASK) << \
> - PDR0_##name##_PODF_SHIFT)
> -/* PLL control registers */
> -#define PD(v) (((v) >> 26) & 0xf)
> -#define MFD(v) (((v) >> 16) & 0x3ff)
> -#define MFI(v) (((v) >> 10) & 0xf);
> -#define MFN(v) ((v) & 0x3ff)
> -
> -#define PLL_PD(x) (((x) & 0xf) << 26)
> -#define PLL_MFD(x) (((x) & 0x3ff) << 16)
> -#define PLL_MFI(x) (((x) & 0xf) << 10)
> -#define PLL_MFN(x) (((x) & 0x3ff) << 0)
> -
> uint32_t imx_clock_frequency(DeviceState *dev, IMXClk clock)
> {
> IMXCCMState *s = IMX_CCM(dev);
> @@ -286,7 +222,7 @@ static int imx_ccm_init(SysBusDevice *dev)
> IMXCCMState *s = IMX_CCM(dev);
>
> memory_region_init_io(&s->iomem, OBJECT(dev), &imx_ccm_ops, s,
> - "imx_ccm", 0x1000);
> + TYPE_IMX_CCM, 0x1000);
> sysbus_init_mmio(dev, &s->iomem);
>
> return 0;
> diff --git a/include/hw/arm/imx.h b/include/hw/arm/imx.h
> index c39f112..6cb90cc 100644
> --- a/include/hw/arm/imx.h
> +++ b/include/hw/arm/imx.h
> @@ -11,16 +11,6 @@
> #ifndef IMX_H
> #define IMX_H
>
> -typedef enum {
> - NOCLK,
> - MCU,
> - HSP,
> - IPG,
> - CLK_32k
> -} IMXClk;
> -
> -uint32_t imx_clock_frequency(DeviceState *s, IMXClk clock);
> -
> void imx_timerp_create(const hwaddr addr,
> qemu_irq irq,
> DeviceState *ccm);
> diff --git a/include/hw/misc/imx_ccm.h b/include/hw/misc/imx_ccm.h
> new file mode 100644
> index 0000000..7febafd
> --- /dev/null
> +++ b/include/hw/misc/imx_ccm.h
> @@ -0,0 +1,91 @@
> +/*
> + * IMX31 Clock Control Module
> + *
> + * Copyright (C) 2012 NICTA
> + * Updated by 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.
> + */
> +
> +#ifndef IMX_CCM_H
> +#define IMX_CCM_H
> +
> +#include "hw/sysbus.h"
> +
> +/* CCMR */
> +#define CCMR_FPME (1<<0)
> +#define CCMR_MPE (1<<3)
> +#define CCMR_MDS (1<<7)
> +#define CCMR_FPMF (1<<26)
> +#define CCMR_PRCS (3<<1)
> +
> +/* PDR0 */
> +#define PDR0_MCU_PODF_SHIFT (0)
> +#define PDR0_MCU_PODF_MASK (0x7)
> +#define PDR0_MAX_PODF_SHIFT (3)
> +#define PDR0_MAX_PODF_MASK (0x7)
> +#define PDR0_IPG_PODF_SHIFT (6)
> +#define PDR0_IPG_PODF_MASK (0x3)
> +#define PDR0_NFC_PODF_SHIFT (8)
> +#define PDR0_NFC_PODF_MASK (0x7)
> +#define PDR0_HSP_PODF_SHIFT (11)
> +#define PDR0_HSP_PODF_MASK (0x7)
> +#define PDR0_PER_PODF_SHIFT (16)
> +#define PDR0_PER_PODF_MASK (0x1f)
> +#define PDR0_CSI_PODF_SHIFT (23)
> +#define PDR0_CSI_PODF_MASK (0x1ff)
> +
> +#define EXTRACT(value, name) (((value) >> PDR0_##name##_PODF_SHIFT) \
> + & PDR0_##name##_PODF_MASK)
> +#define INSERT(value, name) (((value) & PDR0_##name##_PODF_MASK) << \
> + PDR0_##name##_PODF_SHIFT)
> +
> +/* PLL control registers */
> +#define PD(v) (((v) >> 26) & 0xf)
> +#define MFD(v) (((v) >> 16) & 0x3ff)
> +#define MFI(v) (((v) >> 10) & 0xf);
> +#define MFN(v) ((v) & 0x3ff)
> +
> +#define PLL_PD(x) (((x) & 0xf) << 26)
> +#define PLL_MFD(x) (((x) & 0x3ff) << 16)
> +#define PLL_MFI(x) (((x) & 0xf) << 10)
> +#define PLL_MFN(x) (((x) & 0x3ff) << 0)
> +
> +#define TYPE_IMX_CCM "imx.ccm"
You should mention this addition in commit message.
> +#define IMX_CCM(obj) OBJECT_CHECK(IMXCCMState, (obj), TYPE_IMX_CCM)
> +
> +typedef struct {
Keep the IMXCCMState from original code. Most typedef struct defs do
the double define.
"Add missing macro for QOM type name in new header".
Otherwise:
Reviewed-by: Peter Crosthwaite <address@hidden>
Regards,
Peter
> + /* <private> */
> + SysBusDevice parent_obj;
> +
> + /* <public> */
> + MemoryRegion iomem;
> +
> + uint32_t ccmr;
> + uint32_t pdr0;
> + uint32_t pdr1;
> + uint32_t mpctl;
> + uint32_t spctl;
> + uint32_t cgr[3];
> + uint32_t pmcr0;
> + uint32_t pmcr1;
> +
> + /* Frequencies precalculated on register changes */
> + uint32_t pll_refclk_freq;
> + uint32_t mcu_clk_freq;
> + uint32_t hsp_clk_freq;
> + uint32_t ipg_clk_freq;
> +} IMXCCMState;
> +
> +typedef enum {
> + NOCLK,
> + MCU,
> + HSP,
> + IPG,
> + CLK_32k
> +} IMXClk;
> +
> +uint32_t imx_clock_frequency(DeviceState *s, IMXClk clock);
> +
> +#endif /* IMX_CCM_H */
> --
> 2.1.4
>
>