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 <[email protected]> wrote: > Signed-off-by: Jean-Christophe Dubois <[email protected]> > --- > > 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 <[email protected]> > * > * 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 <[email protected]> > + * > + * 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 <[email protected]> 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 > >
