From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48092) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqsTr-0004hZ-TB for qemu-devel@nongnu.org; Mon, 16 Jul 2012 17:05:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SqsTm-0007Q3-KC for qemu-devel@nongnu.org; Mon, 16 Jul 2012 17:05:23 -0400 Received: from mail-bk0-f45.google.com ([209.85.214.45]:35705) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SqsTm-0007Po-1y for qemu-devel@nongnu.org; Mon, 16 Jul 2012 17:05:18 -0400 Received: by bkcji1 with SMTP id ji1so4206608bkc.4 for ; Mon, 16 Jul 2012 14:05:17 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1341419277-19674-2-git-send-email-m.kozlov@samsung.com> References: <1341419277-19674-1-git-send-email-m.kozlov@samsung.com> <1341419277-19674-2-git-send-email-m.kozlov@samsung.com> Date: Mon, 16 Jul 2012 22:05:16 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v4 1/2] ARM: exynos4210: CMU support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Maksim Kozlov Cc: kyungmin.park@samsung.com, qemu-devel@nongnu.org On 4 July 2012 17:27, Maksim Kozlov wrote: > Add exynos4210 Clock Management Units emulation General comment, which I suspect you're not going to like... Underlying the exynos-specific code here, there's a general mechanism of "I'm a device, I have a clock input, I want to be able to find out when that clock is reprogrammed so I can adjust my behaviour, and I want to do things like ask what its rate is", and there's a provider of clock signals which has some kind of clock tree, possibly rearrangable at runtime by reprogramming the clock management unit. This is basically the same thing the OMAP clock code in hw/omap_clk.c also provides (though again the commonality is obscured under a heap of OMAP specific stuff). I think we need a generic interface for providing clocks and plumbing them together, which can then be used by both OMAP and Exynos and whatever later platforms come along with clock trees. I'd also really like to see this use QOM links rather than being a set of ad-hoc C functions for register/get rate/etc. You're platform number two, which I'm afraid means you get the pain of writing a generic mechanism... (There is a conceptual parallel with the kernel efforts to move to a common struct_clk infrastructure, I think; I'd much rather we did this while we have two clock tree models rather than waiting until we have half a dozen.) I've added some specific code review comments below, but this is the biggie. > Signed-off-by: Maksim Kozlov > --- > hw/arm/Makefile.objs | 1 + > hw/exynos4210.c | 16 + > hw/exynos4210.h | 42 ++ > hw/exynos4210_cmu.c | 1484 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 1543 insertions(+), 0 deletions(-) > create mode 100644 hw/exynos4210_cmu.c > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > index 88ff47d..20d19f2 100644 > --- a/hw/arm/Makefile.objs > +++ b/hw/arm/Makefile.objs > @@ -11,6 +11,7 @@ obj-y += realview_gic.o realview.o arm_sysctl.o arm11mpcore.o a9mpcore.o > obj-y += exynos4210_gic.o exynos4210_combiner.o exynos4210.o > obj-y += exynos4_boards.o exynos4210_uart.o exynos4210_pwm.o > obj-y += exynos4210_pmu.o exynos4210_mct.o exynos4210_fimd.o > +obj-y += exynos4210_cmu.o > obj-y += arm_l2x0.o > obj-y += arm_mptimer.o a15mpcore.o > obj-y += armv7m.o armv7m_nvic.o stellaris.o pl022.o stellaris_enet.o > diff --git a/hw/exynos4210.c b/hw/exynos4210.c > index 9c20b3f..55b8e24 100644 > --- a/hw/exynos4210.c > +++ b/hw/exynos4210.c > @@ -30,6 +30,13 @@ > > #define EXYNOS4210_CHIPID_ADDR 0x10000000 > > +/* CMUs */ > +#define EXYNOS4210_CMU_LEFTBUS_BASE_ADDR 0x10034000 > +#define EXYNOS4210_CMU_RIGHTBUS_BASE_ADDR 0x10038000 > +#define EXYNOS4210_CMU_TOP_BASE_ADDR 0x1003C000 > +#define EXYNOS4210_CMU_DMC_BASE_ADDR 0x10040000 > +#define EXYNOS4210_CMU_CPU_BASE_ADDR 0x10044000 > + > /* PWM */ > #define EXYNOS4210_PWM_BASE_ADDR 0x139D0000 > > @@ -250,6 +257,15 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, > */ > sysbus_create_simple("exynos4210.pmu", EXYNOS4210_PMU_BASE_ADDR, NULL); > > + /* CMUs */ > + exynos4210_cmu_create(EXYNOS4210_CMU_LEFTBUS_BASE_ADDR, > + EXYNOS4210_CMU_LEFTBUS); > + exynos4210_cmu_create(EXYNOS4210_CMU_RIGHTBUS_BASE_ADDR, > + EXYNOS4210_CMU_RIGHTBUS); > + exynos4210_cmu_create(EXYNOS4210_CMU_TOP_BASE_ADDR, EXYNOS4210_CMU_TOP); > + exynos4210_cmu_create(EXYNOS4210_CMU_DMC_BASE_ADDR, EXYNOS4210_CMU_DMC); > + exynos4210_cmu_create(EXYNOS4210_CMU_CPU_BASE_ADDR, EXYNOS4210_CMU_CPU); > + > /* PWM */ > sysbus_create_varargs("exynos4210.pwm", EXYNOS4210_PWM_BASE_ADDR, > s->irq_table[exynos4210_get_irq(22, 0)], > diff --git a/hw/exynos4210.h b/hw/exynos4210.h > index 9b1ae4c..fbdff7a 100644 > --- a/hw/exynos4210.h > +++ b/hw/exynos4210.h > @@ -123,6 +123,48 @@ void exynos4210_combiner_get_gpioin(Exynos4210Irq *irqs, DeviceState *dev, > int ext); > > /* > + * Interface for exynos4210 Clock Management Units (CMUs) > + */ > +typedef enum { > + UNSPECIFIED_CMU = -1, > + EXYNOS4210_CMU_LEFTBUS, > + EXYNOS4210_CMU_RIGHTBUS, > + EXYNOS4210_CMU_TOP, > + EXYNOS4210_CMU_DMC, > + EXYNOS4210_CMU_CPU, > + EXYNOS4210_CMU_NUMBER > +} Exynos4210Cmu; > + > +typedef enum { > + UNSPECIFIED_CLOCK, > + EXYNOS4210_XXTI, > + EXYNOS4210_XUSBXTI, > + EXYNOS4210_APLL, > + EXYNOS4210_MPLL, > + EXYNOS4210_SCLK_HDMI24M, > + EXYNOS4210_SCLK_USBPHY0, > + EXYNOS4210_SCLK_USBPHY1, > + EXYNOS4210_SCLK_HDMIPHY, > + EXYNOS4210_SCLK_APLL, > + EXYNOS4210_SCLK_MPLL, > + EXYNOS4210_ACLK_100, > + EXYNOS4210_SCLK_UART0, > + EXYNOS4210_SCLK_UART1, > + EXYNOS4210_SCLK_UART2, > + EXYNOS4210_SCLK_UART3, > + EXYNOS4210_SCLK_UART4, So how many UARTs are there on the Exynos4210? This enum defines 5 clocks, but your patch 2 in this series adds code to exynos4210_uart.c which switches on s->channel and only has cases for 0 to 3... > + EXYNOS4210_CLOCKS_NUMBER > +} Exynos4210Clock; > + > +typedef void ClockChangeHandler(void *opaque); > + > +DeviceState *exynos4210_cmu_create(target_phys_addr_t addr, Exynos4210Cmu cmu); > +uint64_t exynos4210_cmu_get_rate(Exynos4210Clock clock_id); > +void exynos4210_register_clock_handler(ClockChangeHandler *func, > + Exynos4210Clock clock_id, > + void *opaque); > + > +/* > * exynos4210 UART > */ > DeviceState *exynos4210_uart_create(target_phys_addr_t addr, > diff --git a/hw/exynos4210_cmu.c b/hw/exynos4210_cmu.c > new file mode 100644 > index 0000000..058f9dd > --- /dev/null > +++ b/hw/exynos4210_cmu.c > @@ -0,0 +1,1484 @@ > +/* > + * exynos4210 Clock Management Units (CMUs) Emulation > + * > + * Copyright (C) 2011 Samsung Electronics Co Ltd. > + * Maksim Kozlov, > + * > + * 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 . > + * > + */ > + > +#include "sysbus.h" > + > +#include "exynos4210.h" > + > + > +#define DEBUG_CMU 0 > +#define DEBUG_CMU_EXTEND 0 > + > +#if DEBUG_CMU || DEBUG_CMU_EXTEND > + > + #define PRINT_DEBUG(fmt, args...) \ > + do { \ > + fprintf(stderr, " [%s:%d] "fmt, __func__, __LINE__, ##args); \ > + } while (0) > + > + #define PRINT_DEBUG_SIMPLE(fmt, args...) \ > + do { \ > + fprintf(stderr, fmt, ## args); \ > + } while (0) > + > +#if DEBUG_CMU_EXTEND > + > + #define PRINT_DEBUG_EXTEND(fmt, args...) \ > + do { \ > + fprintf(stderr, " [%s:%d] "fmt, __func__, __LINE__, ##args); \ > + } while (0) > +#else > + #define PRINT_DEBUG_EXTEND(fmt, args...) \ > + do {} while (0) > +#endif /* EXTEND */ > + > +#else > + #define PRINT_DEBUG(fmt, args...) \ > + do {} while (0) > + #define PRINT_DEBUG_SIMPLE(fmt, args...) \ > + do {} while (0) > + #define PRINT_DEBUG_EXTEND(fmt, args...) \ > + do {} while (0) > +#endif > + > +#define PRINT_ERROR(fmt, args...) \ > + do { \ > + fprintf(stderr, " [%s:%d] "fmt, __func__, __LINE__, ##args); \ > + } while (0) > + > + > + > +/* function blocks */ > +#define LEFTBUS_BLK 0x0 > +#define RIGHTBUS_BLK 0x0 > +#define TOP_BLK 0x10 > +#define TOP0_BLK 0x10 > +#define TOP1_BLK 0x14 > +#define DMC_BLK 0x0 > +#define DMC0_BLK 0x0 > +#define DMC1_BLK 0x4 > +#define CPU_BLK 0x0 > +#define CPU0_BLK 0x0 > +#define CPU1_BLK 0x4 > + > +#define CAM_BLK 0x20 > +#define TV_BLK 0x24 > +#define MFC_BLK 0x28 > +#define G3D_BLK 0x2C > +#define IMAGE_BLK 0x30 > +#define LCD0_BLK 0x34 > +#define LCD1_BLK 0x38 > +#define MAUDIO_BLK 0x3C > +#define FSYS_BLK 0x40 > +#define FSYS0_BLK 0x40 > +#define FSYS1_BLK 0x44 > +#define FSYS2_BLK 0x48 > +#define FSYS3_BLK 0x4C > +#define GPS_BLK 0x4C /* CLK_GATE_IP_GPS in CMU_TOP */ > +#define PERIL_BLK 0x50 > +#define PERIL0_BLK 0x50 > +#define PERIL1_BLK 0x54 > +#define PERIL2_BLK 0x58 > +#define PERIL3_BLK 0x5C > +#define PERIL4_BLK 0x60 > +#define PERIR_BLK 0x60 /* CLK_GATE_IP_PERIR in CMU_TOP */ > +#define PERIL5_BLK 0x64 > + > +#define BLOCK_MASK 0xFF > + > +/* PLLs */ > +/* located in CMU_CPU block */ > +#define APLL 0x00 > +#define MPLL 0x08 > +/* located in CMU_TOP block */ > +#define EPLL 0x10 > +#define VPLL 0x20 > + > +/* groups of registers */ > +#define PLL_LOCK 0x000 > +#define PLL_CON 0x100 > +#define CLK_SRC 0x200 > +#define CLK_SRC_MASK 0x300 > +#define CLK_MUX_STAT 0x400 > +#define CLK_DIV 0x500 > +#define CLK_DIV_STAT 0x600 > +#define CLK_GATE_SCLK 0x800 > +#define CLK_GATE_IP 0x900 > + > +#define GROUP_MASK 0xF00 > + > +#define PLL_LOCK_(pll) (PLL_LOCK + pll) > +#define PLL_CON0_(pll) (PLL_CON + pll) > +#define PLL_CON1_(pll) (PLL_CON + pll + 4) > + > +#define CLK_SRC_(block) (CLK_SRC + block) > +#define CLK_SRC_MASK_(block) (CLK_SRC_MASK + block) > +#define CLK_MUX_STAT_(block) (CLK_MUX_STAT + block) > +#define CLK_DIV_(block) (CLK_DIV + block) > +#define CLKDIV2_RATIO 0x580 /* described for CMU_TOP only */ > +#define CLK_DIV_STAT_(block) (CLK_DIV_STAT + block) > +#define CLKDIV2_STAT 0x680 /* described for CMU_TOP only */ > +#define CLK_GATE_SCLK_(block) (CLK_GATE_SCLK + block) > +/* For CMU_LEFTBUS and CMU_RIGHTBUS, CLK_GATE_IP_XXX > + registers are located at 0x800. */ > +#define CLK_GATE_IP_LR_BUS 0x800 > +#define CLK_GATE_IP_(block) (CLK_GATE_IP + block) > +#define CLK_GATE_BLOCK 0x970 /* described for CMU_TOP only */ > +#define CLKOUT_CMU 0xA00 > +#define CLKOUT_CMU_DIV_STAT 0xA04 > + > +/* > + * registers which are located outside of 0xAFF region > + */ > +/* CMU_DMC */ > +#define DCGIDX_MAP0 0x01000 > +#define DCGIDX_MAP1 0x01004 > +#define DCGIDX_MAP2 0x01008 > +#define DCGPERF_MAP0 0x01020 > +#define DCGPERF_MAP1 0x01024 > +#define DVCIDX_MAP 0x01040 > +#define FREQ_CPU 0x01060 > +#define FREQ_DPM 0x01064 > +#define DVSEMCLK_EN 0x01080 > +#define MAXPERF 0x01084 > +/* CMU_CPU */ > +#define ARMCLK_STOPCTRL 0x01000 > +#define ATCLK_STOPCTRL 0x01004 > +#define PARITYFAIL_STATUS 0x01010 > +#define PARITYFAIL_CLEAR 0x01014 > +#define PWR_CTRL 0x01020 > +#define APLL_CON0_L8 0x01100 > +#define APLL_CON0_L7 0x01104 > +#define APLL_CON0_L6 0x01108 > +#define APLL_CON0_L5 0x0110C > +#define APLL_CON0_L4 0x01110 > +#define APLL_CON0_L3 0x01114 > +#define APLL_CON0_L2 0x01118 > +#define APLL_CON0_L1 0x0111C > +#define IEM_CONTROL 0x01120 > +#define APLL_CON1_L8 0x01200 > +#define APLL_CON1_L7 0x01204 > +#define APLL_CON1_L6 0x01208 > +#define APLL_CON1_L5 0x0120C > +#define APLL_CON1_L4 0x01210 > +#define APLL_CON1_L3 0x01214 > +#define APLL_CON1_L2 0x01218 > +#define APLL_CON1_L1 0x0121C > +#define CLKDIV_IEM_L8 0x01300 > +#define CLKDIV_IEM_L7 0x01304 > +#define CLKDIV_IEM_L6 0x01308 > +#define CLKDIV_IEM_L5 0x0130C > +#define CLKDIV_IEM_L4 0x01310 > +#define CLKDIV_IEM_L3 0x01314 > +#define CLKDIV_IEM_L2 0x01318 > +#define CLKDIV_IEM_L1 0x0131C > + > +#define EXTENDED_REGION_MASK 0x1000 > +#define EXTENDED_REGISTER_MASK 0xFFF > + > +typedef struct { > + const char *name; /* for debugging */ > + uint32_t offset; > + uint32_t reset_value; > +} Exynos4210CmuReg; > + > + > +static const Exynos4210CmuReg exynos4210_cmu_leftbus_regs[] = { > + /* CMU_LEFTBUS registers */ > + { "CLK_SRC_LEFTBUS", CLK_SRC_(LEFTBUS_BLK), 0x00000000 }, > + { "CLK_MUX_STAT_LEFTBUS", CLK_MUX_STAT_(LEFTBUS_BLK), 0x00000001 }, > + { "CLK_DIV_LEFTBUS", CLK_DIV_(LEFTBUS_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_LEFTBUS", CLK_DIV_STAT_(LEFTBUS_BLK), 0x00000000 }, > + { "CLK_GATE_IP_LEFTBUS", CLK_GATE_IP_LR_BUS, 0xFFFFFFFF }, > + { "CLKOUT_CMU_LEFTBUS", CLKOUT_CMU, 0x00010000 }, > + { "CLKOUT_CMU_LEFTBUS_DIV_STAT", CLKOUT_CMU_DIV_STAT, 0x00000000 }, > +}; > + > +static const Exynos4210CmuReg exynos4210_cmu_rightbus_regs[] = { > + /* CMU_RIGHTBUS registers */ > + { "CLK_SRC_RIGHTBUS", CLK_SRC_(RIGHTBUS_BLK), 0x00000000 }, > + { "CLK_MUX_STAT_RIGHTBUS", CLK_MUX_STAT_(RIGHTBUS_BLK), 0x00000001 }, > + { "CLK_DIV_RIGHTBUS", CLK_DIV_(RIGHTBUS_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_RIGHTBUS", CLK_DIV_STAT_(RIGHTBUS_BLK), 0x00000000 }, > + { "CLK_GATE_IP_RIGHTBUS", CLK_GATE_IP_LR_BUS, 0xFFFFFFFF }, > + { "CLKOUT_CMU_RIGHTBUS", CLKOUT_CMU, 0x00010000 }, > + { "CLKOUT_CMU_RIGHTBUS_DIV_STAT", CLKOUT_CMU_DIV_STAT, 0x00000000 }, > +}; > + > +static const Exynos4210CmuReg exynos4210_cmu_top_regs[] = { > + /* CMU_TOP registers */ > + { "EPLL_LOCK", PLL_LOCK_(EPLL), 0x00000FFF }, > + { "VPLL_LOCK", PLL_LOCK_(VPLL), 0x00000FFF }, > + { "EPLL_CON0", PLL_CON0_(EPLL), 0x00300301 }, > + { "EPLL_CON1", PLL_CON1_(EPLL), 0x00000000 }, > + { "VPLL_CON0", PLL_CON0_(VPLL), 0x00240201 }, > + { "VPLL_CON1", PLL_CON1_(VPLL), 0x66010464 }, > + { "CLK_SRC_TOP0", CLK_SRC_(TOP0_BLK), 0x00000000 }, > + { "CLK_SRC_TOP1", CLK_SRC_(TOP1_BLK), 0x00000000 }, > + { "CLK_SRC_CAM", CLK_SRC_(CAM_BLK), 0x11111111 }, > + { "CLK_SRC_TV", CLK_SRC_(TV_BLK), 0x00000000 }, > + { "CLK_SRC_MFC", CLK_SRC_(MFC_BLK), 0x00000000 }, > + { "CLK_SRC_G3D", CLK_SRC_(G3D_BLK), 0x00000000 }, > + { "CLK_SRC_IMAGE", CLK_SRC_(IMAGE_BLK), 0x00000000 }, > + { "CLK_SRC_LCD0", CLK_SRC_(LCD0_BLK), 0x00001111 }, > + { "CLK_SRC_LCD1", CLK_SRC_(LCD1_BLK), 0x00001111 }, > + { "CLK_SRC_MAUDIO", CLK_SRC_(MAUDIO_BLK), 0x00000005 }, > + { "CLK_SRC_FSYS", CLK_SRC_(FSYS_BLK), 0x00011111 }, > + { "CLK_SRC_PERIL0", CLK_SRC_(PERIL0_BLK), 0x00011111 }, > + { "CLK_SRC_PERIL1", CLK_SRC_(PERIL1_BLK), 0x01110055 }, > + { "CLK_SRC_MASK_TOP", CLK_SRC_MASK_(TOP_BLK), 0x00000001 }, > + { "CLK_SRC_MASK_CAM", CLK_SRC_MASK_(CAM_BLK), 0x11111111 }, > + { "CLK_SRC_MASK_TV", CLK_SRC_MASK_(TV_BLK), 0x00000111 }, > + { "CLK_SRC_MASK_LCD0", CLK_SRC_MASK_(LCD0_BLK), 0x00001111 }, > + { "CLK_SRC_MASK_LCD1", CLK_SRC_MASK_(LCD1_BLK), 0x00001111 }, > + { "CLK_SRC_MASK_MAUDIO", CLK_SRC_MASK_(MAUDIO_BLK), 0x00000001 }, > + { "CLK_SRC_MASK_FSYS", CLK_SRC_MASK_(FSYS_BLK), 0x01011111 }, > + { "CLK_SRC_MASK_PERIL0", CLK_SRC_MASK_(PERIL0_BLK), 0x00011111 }, > + { "CLK_SRC_MASK_PERIL1", CLK_SRC_MASK_(PERIL1_BLK), 0x01110111 }, > + { "CLK_MUX_STAT_TOP", CLK_MUX_STAT_(TOP_BLK), 0x11111111 }, > + { "CLK_MUX_STAT_MFC", CLK_MUX_STAT_(MFC_BLK), 0x00000111 }, > + { "CLK_MUX_STAT_G3D", CLK_MUX_STAT_(G3D_BLK), 0x00000111 }, > + { "CLK_MUX_STAT_IMAGE", CLK_MUX_STAT_(IMAGE_BLK), 0x00000111 }, > + { "CLK_DIV_TOP", CLK_DIV_(TOP_BLK), 0x00000000 }, > + { "CLK_DIV_CAM", CLK_DIV_(CAM_BLK), 0x00000000 }, > + { "CLK_DIV_TV", CLK_DIV_(TV_BLK), 0x00000000 }, > + { "CLK_DIV_MFC", CLK_DIV_(MFC_BLK), 0x00000000 }, > + { "CLK_DIV_G3D", CLK_DIV_(G3D_BLK), 0x00000000 }, > + { "CLK_DIV_IMAGE", CLK_DIV_(IMAGE_BLK), 0x00000000 }, > + { "CLK_DIV_LCD0", CLK_DIV_(LCD0_BLK), 0x00700000 }, > + { "CLK_DIV_LCD1", CLK_DIV_(LCD1_BLK), 0x00700000 }, > + { "CLK_DIV_MAUDIO", CLK_DIV_(MAUDIO_BLK), 0x00000000 }, > + { "CLK_DIV_FSYS0", CLK_DIV_(FSYS0_BLK), 0x00B00000 }, > + { "CLK_DIV_FSYS1", CLK_DIV_(FSYS1_BLK), 0x00000000 }, > + { "CLK_DIV_FSYS2", CLK_DIV_(FSYS2_BLK), 0x00000000 }, > + { "CLK_DIV_FSYS3", CLK_DIV_(FSYS3_BLK), 0x00000000 }, > + { "CLK_DIV_PERIL0", CLK_DIV_(PERIL0_BLK), 0x00000000 }, > + { "CLK_DIV_PERIL1", CLK_DIV_(PERIL1_BLK), 0x00000000 }, > + { "CLK_DIV_PERIL2", CLK_DIV_(PERIL2_BLK), 0x00000000 }, > + { "CLK_DIV_PERIL3", CLK_DIV_(PERIL3_BLK), 0x00000000 }, > + { "CLK_DIV_PERIL4", CLK_DIV_(PERIL4_BLK), 0x00000000 }, > + { "CLK_DIV_PERIL5", CLK_DIV_(PERIL5_BLK), 0x00000000 }, > + { "CLKDIV2_RATIO", CLKDIV2_RATIO, 0x11111111 }, > + { "CLK_DIV_STAT_TOP", CLK_DIV_STAT_(TOP_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_CAM", CLK_DIV_STAT_(CAM_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_TV", CLK_DIV_STAT_(TV_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_MFC", CLK_DIV_STAT_(MFC_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_G3D", CLK_DIV_STAT_(G3D_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_IMAGE", CLK_DIV_STAT_(IMAGE_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_LCD0", CLK_DIV_STAT_(LCD0_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_LCD1", CLK_DIV_STAT_(LCD1_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_MAUDIO", CLK_DIV_STAT_(MAUDIO_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_FSYS0", CLK_DIV_STAT_(FSYS0_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_FSYS1", CLK_DIV_STAT_(FSYS1_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_FSYS2", CLK_DIV_STAT_(FSYS2_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_FSYS3", CLK_DIV_STAT_(FSYS3_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_PERIL0", CLK_DIV_STAT_(PERIL0_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_PERIL1", CLK_DIV_STAT_(PERIL1_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_PERIL2", CLK_DIV_STAT_(PERIL2_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_PERIL3", CLK_DIV_STAT_(PERIL3_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_PERIL4", CLK_DIV_STAT_(PERIL4_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_PERIL5", CLK_DIV_STAT_(PERIL5_BLK), 0x00000000 }, > + { "CLKDIV2_STAT", CLKDIV2_STAT, 0x00000000 }, > + { "CLK_GATE_SCLK_CAM", CLK_GATE_SCLK_(CAM_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_CAM", CLK_GATE_IP_(CAM_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_TV", CLK_GATE_IP_(TV_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_MFC", CLK_GATE_IP_(MFC_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_G3D", CLK_GATE_IP_(G3D_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_IMAGE", CLK_GATE_IP_(IMAGE_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_LCD0", CLK_GATE_IP_(LCD0_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_LCD1", CLK_GATE_IP_(LCD1_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_FSYS", CLK_GATE_IP_(FSYS_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_GPS", CLK_GATE_IP_(GPS_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_PERIL", CLK_GATE_IP_(PERIL_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_PERIR", CLK_GATE_IP_(PERIR_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_BLOCK", CLK_GATE_BLOCK, 0xFFFFFFFF }, > + { "CLKOUT_CMU_TOP", CLKOUT_CMU, 0x00010000 }, > + { "CLKOUT_CMU_TOP_DIV_STAT", CLKOUT_CMU_DIV_STAT, 0x00000000 }, > +}; > + > +static const Exynos4210CmuReg exynos4210_cmu_dmc_regs[] = { > + /* CMU_DMC registers */ > + { "CLK_SRC_DMC", CLK_SRC_(DMC_BLK), 0x00010000 }, > + { "CLK_SRC_MASK_DMC", CLK_SRC_MASK_(DMC_BLK), 0x00010000 }, > + { "CLK_MUX_STAT_DMC", CLK_MUX_STAT_(DMC_BLK), 0x11100110 }, > + { "CLK_DIV_DMC0", CLK_DIV_(DMC0_BLK), 0x00000000 }, > + { "CLK_DIV_DMC1", CLK_DIV_(DMC1_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_DMC0", CLK_DIV_STAT_(DMC0_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_DMC1", CLK_DIV_STAT_(DMC1_BLK), 0x00000000 }, > + { "CLK_GATE_IP_DMC", CLK_GATE_IP_(DMC_BLK), 0xFFFFFFFF }, > + { "CLKOUT_CMU_DMC", CLKOUT_CMU, 0x00010000 }, > + { "CLKOUT_CMU_DMC_DIV_STAT", CLKOUT_CMU_DIV_STAT, 0x00000000 }, > + { "DCGIDX_MAP0", DCGIDX_MAP0, 0xFFFFFFFF }, > + { "DCGIDX_MAP1", DCGIDX_MAP1, 0xFFFFFFFF }, > + { "DCGIDX_MAP2", DCGIDX_MAP2, 0xFFFFFFFF }, > + { "DCGPERF_MAP0", DCGPERF_MAP0, 0xFFFFFFFF }, > + { "DCGPERF_MAP1", DCGPERF_MAP1, 0xFFFFFFFF }, > + { "DVCIDX_MAP", DVCIDX_MAP, 0xFFFFFFFF }, > + { "FREQ_CPU", FREQ_CPU, 0x00000000 }, > + { "FREQ_DPM", FREQ_DPM, 0x00000000 }, > + { "DVSEMCLK_EN", DVSEMCLK_EN, 0x00000000 }, > + { "MAXPERF", MAXPERF, 0x00000000 }, > +}; > + > +static const Exynos4210CmuReg exynos4210_cmu_cpu_regs[] = { > + /* CMU_CPU registers */ > + { "APLL_LOCK", PLL_LOCK_(APLL), 0x00000FFF }, > + { "MPLL_LOCK", PLL_LOCK_(MPLL), 0x00000FFF }, > + { "APLL_CON0", PLL_CON0_(APLL), 0x00C80601 }, > + { "APLL_CON1", PLL_CON1_(APLL), 0x0000001C }, > + { "MPLL_CON0", PLL_CON0_(MPLL), 0x00C80601 }, > + { "MPLL_CON1", PLL_CON1_(MPLL), 0x0000001C }, > + { "CLK_SRC_CPU", CLK_SRC_(CPU_BLK), 0x00000000 }, > + { "CLK_MUX_STAT_CPU", CLK_MUX_STAT_(CPU_BLK), 0x00110101 }, > + { "CLK_DIV_CPU0", CLK_DIV_(CPU0_BLK), 0x00000000 }, > + { "CLK_DIV_CPU1", CLK_DIV_(CPU1_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_CPU0", CLK_DIV_STAT_(CPU0_BLK), 0x00000000 }, > + { "CLK_DIV_STAT_CPU1", CLK_DIV_STAT_(CPU1_BLK), 0x00000000 }, > + { "CLK_GATE_SCLK_CPU", CLK_GATE_SCLK_(CPU_BLK), 0xFFFFFFFF }, > + { "CLK_GATE_IP_CPU", CLK_GATE_IP_(CPU_BLK), 0xFFFFFFFF }, > + { "CLKOUT_CMU_CPU", CLKOUT_CMU, 0x00010000 }, > + { "CLKOUT_CMU_CPU_DIV_STAT", CLKOUT_CMU_DIV_STAT, 0x00000000 }, > + { "ARMCLK_STOPCTRL", ARMCLK_STOPCTRL, 0x00000044 }, > + { "ATCLK_STOPCTRL", ATCLK_STOPCTRL, 0x00000044 }, > + { "PARITYFAIL_STATUS", PARITYFAIL_STATUS, 0x00000000 }, > + { "PARITYFAIL_CLEAR", PARITYFAIL_CLEAR, 0x00000000 }, > + { "PWR_CTRL", PWR_CTRL, 0x00000033 }, > + { "APLL_CON0_L8", APLL_CON0_L8, 0x00C80601 }, > + { "APLL_CON0_L7", APLL_CON0_L7, 0x00C80601 }, > + { "APLL_CON0_L6", APLL_CON0_L6, 0x00C80601 }, > + { "APLL_CON0_L5", APLL_CON0_L5, 0x00C80601 }, > + { "APLL_CON0_L4", APLL_CON0_L4, 0x00C80601 }, > + { "APLL_CON0_L3", APLL_CON0_L3, 0x00C80601 }, > + { "APLL_CON0_L2", APLL_CON0_L2, 0x00C80601 }, > + { "APLL_CON0_L1", APLL_CON0_L1, 0x00C80601 }, > + { "IEM_CONTROL", IEM_CONTROL, 0x00000000 }, > + { "APLL_CON1_L8", APLL_CON1_L8, 0x00000000 }, > + { "APLL_CON1_L7", APLL_CON1_L7, 0x00000000 }, > + { "APLL_CON1_L6", APLL_CON1_L6, 0x00000000 }, > + { "APLL_CON1_L5", APLL_CON1_L5, 0x00000000 }, > + { "APLL_CON1_L4", APLL_CON1_L4, 0x00000000 }, > + { "APLL_CON1_L3", APLL_CON1_L3, 0x00000000 }, > + { "APLL_CON1_L2", APLL_CON1_L2, 0x00000000 }, > + { "APLL_CON1_L1", APLL_CON1_L1, 0x00000000 }, > + { "CLKDIV_IEM_L8", CLKDIV_IEM_L8, 0x00000000 }, > + { "CLKDIV_IEM_L7", CLKDIV_IEM_L7, 0x00000000 }, > + { "CLKDIV_IEM_L6", CLKDIV_IEM_L6, 0x00000000 }, > + { "CLKDIV_IEM_L5", CLKDIV_IEM_L5, 0x00000000 }, > + { "CLKDIV_IEM_L4", CLKDIV_IEM_L4, 0x00000000 }, > + { "CLKDIV_IEM_L3", CLKDIV_IEM_L3, 0x00000000 }, > + { "CLKDIV_IEM_L2", CLKDIV_IEM_L2, 0x00000000 }, > + { "CLKDIV_IEM_L1", CLKDIV_IEM_L1, 0x00000000 }, > +}; > + > +#define EXYNOS4210_CMU_REGS_MEM_SIZE 0x4000 > + > +/* > + * for indexing register in the uint32_t array > + * > + * 'reg' - register offset (see offsets definitions above) > + * > + */ > +#define I_(reg) (reg / sizeof(uint32_t)) > + > +#define XOM_0 1 /* Select XXTI (0) or XUSBXTI (1) base clock source */ > + > +/* > + * Offsets in CLK_SRC_CPU register > + * for control MUXMPLL and MUXAPLL > + * > + * 0 = FINPLL, 1 = MOUTM(A)PLLFOUT > + */ > +#define MUX_APLL_SEL_SHIFT 0 > +#define MUX_MPLL_SEL_SHIFT 8 > +#define MUX_CORE_SEL_SHIFT 16 > +#define MUX_HPM_SEL_SHIFT 20 > + > +#define MUX_APLL_SEL (1 << MUX_APLL_SEL_SHIFT) > +#define MUX_MPLL_SEL (1 << MUX_MPLL_SEL_SHIFT) > +#define MUX_CORE_SEL (1 << MUX_CORE_SEL_SHIFT) > +#define MUX_HPM_SEL (1 << MUX_HPM_SEL_SHIFT) > + > +/* Offsets for fields in CLK_MUX_STAT_CPU register */ > +#define APLL_SEL_SHIFT 0 > +#define APLL_SEL_MASK 0x00000007 > +#define MPLL_SEL_SHIFT 8 > +#define MPLL_SEL_MASK 0x00000700 > +#define CORE_SEL_SHIFT 16 > +#define CORE_SEL_MASK 0x00070000 > +#define HPM_SEL_SHIFT 20 > +#define HPM_SEL_MASK 0x00700000 > + > + > +/* Offsets for fields in _CON0 register */ > +#define PLL_ENABLE_SHIFT 31 > +#define PLL_ENABLE_MASK 0x80000000 /* [31] bit */ > +#define PLL_LOCKED_MASK 0x20000000 /* [29] bit */ > +#define PLL_MDIV_SHIFT 16 > +#define PLL_MDIV_MASK 0x03FF0000 /* [25:16] bits */ > +#define PLL_PDIV_SHIFT 8 > +#define PLL_PDIV_MASK 0x00003F00 /* [13:8] bits */ > +#define PLL_SDIV_SHIFT 0 > +#define PLL_SDIV_MASK 0x00000007 /* [2:0] bits */ > + > +/* > + * Offset in CLK_DIV_CPU0 register > + * for DIVAPLL clock divider ratio > + */ > +#define APLL_RATIO_SHIFT 24 > +#define APLL_RATIO_MASK 0x07000000 /* [26:24] bits */ > + > +/* > + * Offset in CLK_DIV_TOP register > + * for DIVACLK_100 clock divider ratio > + */ > +#define ACLK_100_RATIO_SHIFT 4 > +#define ACLK_100_RATIO_MASK 0x000000f0 /* [7:4] bits */ > + > +/* Offset in CLK_SRC_TOP0 register */ > +#define MUX_ACLK_100_SEL_SHIFT 16 > + > +/* > + * Offsets in CLK_SRC_PERIL0 register > + * for clock sources of UARTs > + */ > +#define UART0_SEL_SHIFT 0 > +#define UART1_SEL_SHIFT 4 > +#define UART2_SEL_SHIFT 8 > +#define UART3_SEL_SHIFT 12 > +#define UART4_SEL_SHIFT 16 > +/* > + * Offsets in CLK_DIV_PERIL0 register > + * for clock divider of UARTs > + */ > +#define UART0_DIV_SHIFT 0 > +#define UART1_DIV_SHIFT 4 > +#define UART2_DIV_SHIFT 8 > +#define UART3_DIV_SHIFT 12 > +#define UART4_DIV_SHIFT 16 > + > +#define SOURCES_NUMBER 9 > + > +typedef struct ClockChangeEntry { > + QTAILQ_ENTRY(ClockChangeEntry) entry; > + ClockChangeHandler *func; > + void *opaque; > +} ClockChangeEntry; > + > +#define TYPE_EXYNOS4210_CMU "exynos4210.cmu" > +#define TYPE_EXYNOS4210_CLOCK "exynos4210.clock" > + > +typedef struct { > + const char *name; > + int32_t id; > + uint64_t rate; > + > + /* Current source clock */ > + int32_t src_id; > + /* > + * Available sources. Their order must correspond to CLK_SRC_ register > + */ > + int32_t src_ids[SOURCES_NUMBER]; > + > + uint32_t src_reg; /* Offset of CLK_SRC_<*> register */ > + uint32_t div_reg; /* Offset of CLK_DIV_<*> register */ > + > + /* > + * Shift for MUX__SEL value which is stored > + * in appropriate CLK_MUX_STAT_ register > + */ > + uint8_t mux_shift; > + > + /* > + * Shift for _RATIO value which is stored > + * in appropriate CLK_DIV_ register > + */ > + uint8_t div_shift; > + > + /* Which CMU controls this clock */ > + int32_t cmu_id; > + > + QTAILQ_HEAD(, ClockChangeEntry) clock_change_handler; > +} Exynos4210ClockState; > + > +typedef struct { > + SysBusDevice busdev; > + MemoryRegion iomem; > + > + /* registers values */ > + uint32_t reg[EXYNOS4210_CMU_REGS_MEM_SIZE / sizeof(uint32_t)]; > + > + /* which CMU it is */ > + Exynos4210Cmu cmu_id; > + > + /* registers information for debugging and resetting */ > + const Exynos4210CmuReg *regs; > + int regs_number; > + > + Exynos4210ClockState *clock; > + int clock_number; /* how many clocks are controlled by given CMU */ > +} Exynos4210CmuState; > + > + > +/* Clocks from Clock Pads */ > +/* > + * Two following clocks aren't controlled by any CMUs. These structures are > + * used directly from global space and their fields shouldn't be modified. > + */ > + > +/* It should be used only for testing purposes. XOM_0 is 0 */ Testing of what? Testing QEMU?? > +static const Exynos4210ClockState xxti = { > + .name = "XXTI", > + .id = EXYNOS4210_XXTI, > + .rate = 24000000, > + .cmu_id = UNSPECIFIED_CMU, > +}; > + > +/* Main source. XOM_0 is 1 */ > +static const Exynos4210ClockState xusbxti = { > + .name = "XUSBXTI", > + .id = EXYNOS4210_XUSBXTI, > + .rate = 24000000, > + .cmu_id = UNSPECIFIED_CMU, > +}; > + > +/* PLLs */ > + > +static const Exynos4210ClockState mpll = { > + .name = "MPLL", > + .id = EXYNOS4210_MPLL, > + .src_id = (XOM_0 ? EXYNOS4210_XUSBXTI : EXYNOS4210_XXTI), > + .div_reg = PLL_CON0_(MPLL), > + .cmu_id = EXYNOS4210_CMU_CPU, > +}; > + > +static const Exynos4210ClockState apll = { > + .name = "APLL", > + .id = EXYNOS4210_APLL, > + .src_id = (XOM_0 ? EXYNOS4210_XUSBXTI : EXYNOS4210_XXTI), > + .div_reg = PLL_CON0_(APLL), > + .cmu_id = EXYNOS4210_CMU_CPU, > +}; > + > + > +/**/ > + > +static const Exynos4210ClockState sclk_hdmi24m = { > + .name = "SCLK_HDMI24M", > + .id = EXYNOS4210_SCLK_HDMI24M, > + .rate = 24000000, > + .cmu_id = UNSPECIFIED_CMU, > +}; > + > +static const Exynos4210ClockState sclk_usbphy0 = { > + .name = "SCLK_USBPHY0", > + .id = EXYNOS4210_SCLK_USBPHY0, > + .rate = 24000000, > + .cmu_id = UNSPECIFIED_CMU, > +}; > + > +static const Exynos4210ClockState sclk_usbphy1 = { > + .name = "SCLK_USBPHY1", > + .id = EXYNOS4210_SCLK_USBPHY1, > + .rate = 24000000, > + .cmu_id = UNSPECIFIED_CMU, > +}; > + > +static Exynos4210ClockState sclk_hdmiphy = { > + .name = "SCLK_HDMIPHY", > + .id = EXYNOS4210_SCLK_HDMIPHY, > + .rate = 24000000, > + .cmu_id = UNSPECIFIED_CMU, > +}; > + > +static const Exynos4210ClockState sclk_mpll = { > + .name = "SCLK_MPLL", > + .id = EXYNOS4210_SCLK_MPLL, > + .src_ids = {XOM_0 ? EXYNOS4210_XUSBXTI : EXYNOS4210_XXTI, > + EXYNOS4210_MPLL}, > + .src_reg = CLK_SRC_(CPU_BLK), > + .mux_shift = MUX_MPLL_SEL_SHIFT, > + .cmu_id = EXYNOS4210_CMU_CPU, > +}; > + > +static const Exynos4210ClockState sclk_apll = { > + .name = "SCLK_APLL", > + .id = EXYNOS4210_SCLK_APLL, > + .src_ids = {XOM_0 ? EXYNOS4210_XUSBXTI : EXYNOS4210_XXTI, > + EXYNOS4210_APLL}, > + .src_reg = CLK_SRC_(CPU_BLK), > + .div_reg = CLK_DIV_(CPU0_BLK), > + .mux_shift = MUX_APLL_SEL_SHIFT, > + .div_shift = APLL_RATIO_SHIFT, > + .cmu_id = EXYNOS4210_CMU_CPU, > +}; > + > +static const Exynos4210ClockState aclk_100 = { > + .name = "ACLK_100", > + .id = EXYNOS4210_ACLK_100, > + .src_ids = {EXYNOS4210_SCLK_MPLL, EXYNOS4210_SCLK_APLL}, > + .src_reg = CLK_SRC_(TOP0_BLK), > + .div_reg = CLK_DIV_(TOP_BLK), > + .mux_shift = MUX_ACLK_100_SEL_SHIFT, > + .div_shift = ACLK_100_RATIO_SHIFT, > + .cmu_id = EXYNOS4210_CMU_TOP, > +}; > + > +static const Exynos4210ClockState sclk_uart0 = { > + .name = "SCLK_UART0", > + .id = EXYNOS4210_SCLK_UART0, > + .src_ids = {EXYNOS4210_XXTI, > + EXYNOS4210_XUSBXTI, > + EXYNOS4210_SCLK_HDMI24M, > + EXYNOS4210_SCLK_USBPHY0, > + EXYNOS4210_SCLK_USBPHY1, > + EXYNOS4210_SCLK_HDMIPHY, > + EXYNOS4210_SCLK_MPLL}, > + .src_reg = CLK_SRC_(PERIL0_BLK), > + .div_reg = CLK_DIV_(PERIL0_BLK), > + .mux_shift = UART0_SEL_SHIFT, > + .div_shift = UART0_DIV_SHIFT, > + .cmu_id = EXYNOS4210_CMU_TOP, > +}; > + > +static const Exynos4210ClockState sclk_uart1 = { > + .name = "SCLK_UART1", > + .id = EXYNOS4210_SCLK_UART1, > + .src_ids = {EXYNOS4210_XXTI, > + EXYNOS4210_XUSBXTI, > + EXYNOS4210_SCLK_HDMI24M, > + EXYNOS4210_SCLK_USBPHY0, > + EXYNOS4210_SCLK_USBPHY1, > + EXYNOS4210_SCLK_HDMIPHY, > + EXYNOS4210_SCLK_MPLL}, > + .src_reg = CLK_SRC_(PERIL0_BLK), > + .div_reg = CLK_DIV_(PERIL0_BLK), > + .mux_shift = UART1_SEL_SHIFT, > + .div_shift = UART1_DIV_SHIFT, > + .cmu_id = EXYNOS4210_CMU_TOP, > +}; > + > +static const Exynos4210ClockState sclk_uart2 = { > + .name = "SCLK_UART2", > + .id = EXYNOS4210_SCLK_UART2, > + .src_ids = {EXYNOS4210_XXTI, > + EXYNOS4210_XUSBXTI, > + EXYNOS4210_SCLK_HDMI24M, > + EXYNOS4210_SCLK_USBPHY0, > + EXYNOS4210_SCLK_USBPHY1, > + EXYNOS4210_SCLK_HDMIPHY, > + EXYNOS4210_SCLK_MPLL}, > + .src_reg = CLK_SRC_(PERIL0_BLK), > + .div_reg = CLK_DIV_(PERIL0_BLK), > + .mux_shift = UART2_SEL_SHIFT, > + .div_shift = UART2_DIV_SHIFT, > + .cmu_id = EXYNOS4210_CMU_TOP, > +}; > + > +static const Exynos4210ClockState sclk_uart3 = { > + .name = "SCLK_UART3", > + .id = EXYNOS4210_SCLK_UART3, > + .src_ids = {EXYNOS4210_XXTI, > + EXYNOS4210_XUSBXTI, > + EXYNOS4210_SCLK_HDMI24M, > + EXYNOS4210_SCLK_USBPHY0, > + EXYNOS4210_SCLK_USBPHY1, > + EXYNOS4210_SCLK_HDMIPHY, > + EXYNOS4210_SCLK_MPLL}, > + .src_reg = CLK_SRC_(PERIL0_BLK), > + .div_reg = CLK_DIV_(PERIL0_BLK), > + .mux_shift = UART3_SEL_SHIFT, > + .div_shift = UART3_DIV_SHIFT, > + .cmu_id = EXYNOS4210_CMU_TOP, > +}; > + > +static const Exynos4210ClockState sclk_uart4 = { > + .name = "SCLK_UART4", > + .id = EXYNOS4210_SCLK_UART4, > + .src_ids = {EXYNOS4210_XXTI, > + EXYNOS4210_XUSBXTI, > + EXYNOS4210_SCLK_HDMI24M, > + EXYNOS4210_SCLK_USBPHY0, > + EXYNOS4210_SCLK_USBPHY1, > + EXYNOS4210_SCLK_HDMIPHY, > + EXYNOS4210_SCLK_MPLL}, > + .src_reg = CLK_SRC_(PERIL0_BLK), > + .div_reg = CLK_DIV_(PERIL0_BLK), > + .mux_shift = UART4_SEL_SHIFT, > + .div_shift = UART4_DIV_SHIFT, > + .cmu_id = EXYNOS4210_CMU_TOP, > +}; > + > +/* > + * This array must correspond to Exynos4210Clock enumerator > + * which is defined in exynos4210.h file > + * > + */ You could avoid this requirement by using the syntax [EXYNOS4210_XXTI] = &xxti, [EXYNOS4210_XUSBXTI] = &xusbxti, (and then you could drop the NULLs as the array zero initialises where it's not explicitly initialised) > +static const Exynos4210ClockState *exynos4210_clock[] = { > + NULL, > + &xxti, > + &xusbxti, > + &apll, > + &mpll, > + &sclk_hdmi24m, > + &sclk_usbphy0, > + &sclk_usbphy1, > + &sclk_hdmiphy, > + &sclk_apll, > + &sclk_mpll, > + &aclk_100, > + &sclk_uart0, > + &sclk_uart1, > + &sclk_uart2, > + &sclk_uart3, > + &sclk_uart4, > + NULL, > +}; > + > +/* > + * This array must correspond to Exynos4210Cmu enumerator > + * which is defined in exynos4210.h file > + * > + */ > +static const char exynos4210_cmu_path[][13] = { You should just make this an array of const char* rather than having that hardcoded 13 in there (ie "static const char * const exynos4210_cmu_path[] = {") See above re initialiser syntax. > + "cmu_leftbus", > + "cmu_rightbus", > + "cmu_top", > + "cmu_dmc", > + "cmu_cpu", > +}; > + > +#if DEBUG_CMU_EXTEND > +/* The only meaning of life - debugging. This function should be only used > + * inside PRINT_DEBUG_EXTEND macros > + */ > +static const char *exynos4210_cmu_regname(Exynos4210CmuState *s, > + target_phys_addr_t offset) > +{ > + int i; > + > + for (i = 0; i < s->regs_number; i++) { > + if (offset == s->regs[i].offset) { > + return s->regs[i].name; > + } > + } > + > + return NULL; > +} > +#endif > + > + > +static Exynos4210ClockState *exynos4210_clock_find(Exynos4210Clock clock_id) > +{ > + int i; > + int cmu_id; > + Object *cmu; > + Exynos4210CmuState *s; > + > + cmu_id = exynos4210_clock[clock_id]->cmu_id; > + > + if (cmu_id == UNSPECIFIED_CMU) { > + for (i = 1; i < EXYNOS4210_CLOCKS_NUMBER; i++) { > + if (exynos4210_clock[i]->id == clock_id) { > + > + PRINT_DEBUG("Clock %s [%p] in CMU %d have been found\n", "has been found" > + exynos4210_clock[i]->name, > + exynos4210_clock[i], > + cmu_id); > + > + return (Exynos4210ClockState *)exynos4210_clock[i]; > + } > + } > + } > + > + cmu = object_resolve_path(exynos4210_cmu_path[cmu_id], NULL); > + s = OBJECT_CHECK(Exynos4210CmuState, cmu, TYPE_EXYNOS4210_CMU); > + > + for (i = 0; i < s->clock_number; i++) { > + if (s->clock[i].id == clock_id) { > + > + PRINT_DEBUG("Clock %s [%p] in CMU %d have been found\n", "has been found" > + s->clock[i].name, > + &s->clock[i], > + s->clock[i].cmu_id); > + return &s->clock[i]; > + } > + } > + > + PRINT_ERROR("Clock %d not found\n", clock_id); > + > + return NULL; > +} > + > + > +void exynos4210_register_clock_handler(ClockChangeHandler *func, > + Exynos4210Clock clock_id, void *opaque) > +{ > + ClockChangeEntry *cce = g_malloc0(sizeof(ClockChangeEntry)); > + Exynos4210ClockState *clock = exynos4210_clock_find(clock_id); > + > + if (clock == NULL) { > + hw_error("We aren't be able to find clock %d\n", clock_id); "Could not find clock %d" > + } else if (clock->cmu_id == UNSPECIFIED_CMU) { > + > + PRINT_DEBUG("Clock %s never are changed. Handler won't be set.", "is never changed". > + exynos4210_clock[clock_id]->name); Is this a programming error by whoever calls this function, or is this expected to happen sometimes? > + > + return; > + } > + > + cce->func = func; > + cce->opaque = opaque; > + > + QTAILQ_INSERT_TAIL(&clock->clock_change_handler, cce, entry); > + > + PRINT_DEBUG("For %s have been set handler [%p]\n", clock->name, cce->func); "Set handler [%p] for clock %s\n" > +} > + > +uint64_t exynos4210_cmu_get_rate(Exynos4210Clock clock_id) > +{ > + Exynos4210ClockState *clock = exynos4210_clock_find(clock_id); > + > + if (clock == NULL) { > + hw_error("We aren't be able to find clock %d\n", clock_id); "Could not find clock %d\n". > + } > + > + return clock->rate; > +} > + > +static void exynos4210_cmu_set_pll(void *opaque, Exynos4210ClockState *pll) > +{ > + Exynos4210CmuState *s = opaque; > + Exynos4210ClockState *source; > + target_phys_addr_t offset = pll->div_reg; > + ClockChangeEntry *cce; > + uint32_t pdiv, mdiv, sdiv, enable; > + > + source = exynos4210_clock_find(pll->src_id); > + > + if (source == NULL) { > + hw_error("We haven't find source clock %d (requested for %s)\n", > + pll->src_id, pll->name); "Could not find" (also in similar messages below). > + } > + > + /* > + * FOUT = MDIV * FIN / (PDIV * 2^(SDIV-1)) > + */ > + > + enable = (s->reg[I_(offset)] & PLL_ENABLE_MASK) >> PLL_ENABLE_SHIFT; > + mdiv = (s->reg[I_(offset)] & PLL_MDIV_MASK) >> PLL_MDIV_SHIFT; > + pdiv = (s->reg[I_(offset)] & PLL_PDIV_MASK) >> PLL_PDIV_SHIFT; > + sdiv = (s->reg[I_(offset)] & PLL_SDIV_MASK) >> PLL_SDIV_SHIFT; > + > + if (source) { > + if (enable) { > + pll->rate = mdiv * source->rate / (pdiv * (1 << (sdiv-1))); > + } else { > + pll->rate = 0; > + } > + } else { > + hw_error("%s: Source undefined for %s\n", __func__, pll->name); > + } > + > + QTAILQ_FOREACH(cce, &pll->clock_change_handler, entry) { > + cce->func(cce->opaque); > + } > + > + PRINT_DEBUG("%s rate: %" PRIu64 "\n", pll->name, pll->rate); > + > + s->reg[I_(offset)] |= PLL_LOCKED_MASK; > +} > + > + > +static void exynos4210_cmu_set_rate(void *opaque, Exynos4210Clock clock_id) > +{ > + Exynos4210CmuState *s = opaque; > + Exynos4210ClockState *clock = exynos4210_clock_find(clock_id); > + ClockChangeEntry *cce; > + > + if (clock == NULL) { > + hw_error("We haven't find source clock %d ", clock_id); > + } > + > + if ((clock->id == EXYNOS4210_MPLL) || (clock->id == EXYNOS4210_APLL)) { > + > + exynos4210_cmu_set_pll(s, clock); > + > + } else if ((clock->cmu_id != UNSPECIFIED_CMU)) { > + > + Exynos4210ClockState *source; > + > + uint32_t src_index = I_(clock->src_reg); > + uint32_t div_index = I_(clock->div_reg); > + > + clock->src_id = clock->src_ids[(s->reg[src_index] >> > + clock->mux_shift) & 0xf]; > + > + source = exynos4210_clock_find(clock->src_id); > + if (source == NULL) { > + hw_error("We haven't find source clock %d (requested for %s)\n", > + clock->src_id, clock->name); > + } > + > + clock->rate = muldiv64(source->rate, 1, > + ((((clock->div_reg ? s->reg[div_index] : 0) >> > + clock->div_shift) & 0xf) + 1)); > + > + QTAILQ_FOREACH(cce, &clock->clock_change_handler, entry) { > + cce->func(cce->opaque); > + } > + > + PRINT_DEBUG_EXTEND("SRC: <0x%05x> %s, SHIFT: %d\n", > + clock->src_reg, > + exynos4210_cmu_regname(s, clock->src_reg), > + clock->mux_shift); > + > + PRINT_DEBUG("%s [%s:%" PRIu64 "]: %" PRIu64 "\n", > + clock->name, source->name, source->rate, clock->rate); > + } > +} > + > + > +static uint64_t exynos4210_cmu_read(void *opaque, target_phys_addr_t offset, > + unsigned size) > +{ > + Exynos4210CmuState *s = opaque; > + uint64_t retval; > + > + if (offset > (EXYNOS4210_CMU_REGS_MEM_SIZE - sizeof(uint32_t))) { > + PRINT_ERROR("Bad offset: 0x%x\n", (int)offset); > + return 0; > + } > + > + if (offset & EXTENDED_REGION_MASK) { > + if (s->cmu_id == EXYNOS4210_CMU_DMC) { Don't switch based on cmu_id in the read/write functions, just register suitable memory regions in the right places at init depending on cmu_id. These big sections of code that go "if $this then registers-like-this else if $that then registers-like-that should be a flag that they should be handled with their own read/write memory regions. > + switch (offset & 0xFFF) { > + case DCGIDX_MAP0: > + case DCGIDX_MAP1: > + case DCGIDX_MAP2: > + case DCGPERF_MAP0: > + case DCGPERF_MAP1: > + case DVCIDX_MAP: > + case FREQ_CPU: > + case FREQ_DPM: > + case DVSEMCLK_EN: > + case MAXPERF: > + retval = s->reg[I_(offset)]; > + break; > + default: > + PRINT_ERROR("Bad offset: 0x%x\n", (int)offset); > + retval = 0; > + break; > + } > + > + PRINT_DEBUG_EXTEND("<0x%05x> %s -> %08x\n", offset, > + exynos4210_cmu_regname(s, offset), retval); > + > + return retval; > + } > + > + if (s->cmu_id == EXYNOS4210_CMU_CPU) { > + switch (offset & 0xFFF) { > + case ARMCLK_STOPCTRL: > + case ATCLK_STOPCTRL: > + case PARITYFAIL_STATUS: > + case PARITYFAIL_CLEAR: > + case PWR_CTRL: > + case APLL_CON0_L8: > + case APLL_CON0_L7: > + case APLL_CON0_L6: > + case APLL_CON0_L5: > + case APLL_CON0_L4: > + case APLL_CON0_L3: > + case APLL_CON0_L2: > + case APLL_CON0_L1: > + case IEM_CONTROL: > + case APLL_CON1_L8: > + case APLL_CON1_L7: > + case APLL_CON1_L6: > + case APLL_CON1_L5: > + case APLL_CON1_L4: > + case APLL_CON1_L3: > + case APLL_CON1_L2: > + case APLL_CON1_L1: > + case CLKDIV_IEM_L8: > + case CLKDIV_IEM_L7: > + case CLKDIV_IEM_L6: > + case CLKDIV_IEM_L5: > + case CLKDIV_IEM_L4: > + case CLKDIV_IEM_L3: > + case CLKDIV_IEM_L2: > + case CLKDIV_IEM_L1: > + retval = s->reg[I_(offset)]; > + break; > + default: > + PRINT_ERROR("Bad offset: 0x%x\n", (int)offset); > + retval = 0; > + break; > + } > + > + PRINT_DEBUG_EXTEND("<0x%05x> %s -> %08x\n", offset, > + exynos4210_cmu_regname(s, offset), retval); > + > + return retval; > + } > + } > + > + switch (offset & GROUP_MASK) { > + case PLL_LOCK: > + case PLL_CON: > + case CLK_SRC: > + case CLK_SRC_MASK: > + case CLK_MUX_STAT: > + case CLK_DIV: > + case CLK_DIV_STAT: > + case 0x700: /* Reserved */ > + case CLK_GATE_SCLK: /* reserved? */ > + case CLK_GATE_IP: > + case CLKOUT_CMU: > + retval = s->reg[I_(offset)]; > + break; > + default: > + PRINT_ERROR("Bad offset: 0x%x\n", (int)offset); > + retval = 0; > + break; > + } > + > + PRINT_DEBUG_EXTEND("<0x%05x> %s -> %08x\n", offset, > + exynos4210_cmu_regname(s, offset), retval); > + > + return retval; > +} > + > + > +static void exynos4210_cmu_write(void *opaque, target_phys_addr_t offset, > + uint64_t val, unsigned size) > +{ > + Exynos4210CmuState *s = opaque; > + uint32_t group, block; > + > + group = offset & GROUP_MASK; > + block = offset & BLOCK_MASK; > + > + switch (group) { > + case PLL_LOCK: > + /* it's not necessary at this moment > + * TODO: do it > + */ You either need to implement whatever this is or provide a better explanation of why it's OK to have a no-op implementation. Remember that readers of the code don't necessarily know much about the hardware being modelled. > + break; > + case PLL_CON: > + switch (block) { > + case APLL: > + { > + uint32_t pre_val = s->reg[I_(offset)]; > + s->reg[I_(offset)] = val; > + val = (val & ~PLL_LOCKED_MASK) | (pre_val & PLL_LOCKED_MASK); > + s->reg[I_(offset)] = val; > + exynos4210_cmu_set_rate(s, EXYNOS4210_APLL); > + break; > + } > + case MPLL: > + { > + uint32_t pre_val = s->reg[I_(offset)]; > + s->reg[I_(offset)] = val; > + val = (val & ~PLL_LOCKED_MASK) | (pre_val & PLL_LOCKED_MASK); > + s->reg[I_(offset)] = val; > + exynos4210_cmu_set_rate(s, EXYNOS4210_MPLL); > + break; > + } > + } > + break; > + case CLK_SRC: > + switch (block) { > + case CPU_BLK: > + { > + uint32_t pre_val = s->reg[I_(offset)]; > + s->reg[I_(offset)] = val; > + > + if (val & MUX_APLL_SEL) { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(APLL_SEL_MASK)) | > + (2 << APLL_SEL_SHIFT); > + > + if ((pre_val & MUX_APLL_SEL) != > + (s->reg[I_(offset)] & MUX_APLL_SEL)) { > + exynos4210_cmu_set_rate(s, EXYNOS4210_APLL); > + } > + > + } else { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(APLL_SEL_MASK)) | > + (1 << APLL_SEL_SHIFT); > + > + if ((pre_val & MUX_APLL_SEL) != > + (s->reg[I_(offset)] & MUX_APLL_SEL)) { > + exynos4210_cmu_set_rate(s, XOM_0 ? EXYNOS4210_XUSBXTI : > + EXYNOS4210_XXTI); > + } > + } I'm sure there must be a less ugly way to write this (maybe try the deposit32() function??) but I can't think of it right now. > + > + > + if (val & MUX_MPLL_SEL) { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(MPLL_SEL_MASK)) | > + (2 << MPLL_SEL_SHIFT); > + > + if ((pre_val & MUX_MPLL_SEL) != > + (s->reg[I_(offset)] & MUX_MPLL_SEL)) { > + exynos4210_cmu_set_rate(s, EXYNOS4210_MPLL); > + } > + > + } else { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(MPLL_SEL_MASK)) | > + (1 << MPLL_SEL_SHIFT); > + > + if ((pre_val & MUX_MPLL_SEL) != > + (s->reg[I_(offset)] & MUX_MPLL_SEL)) { > + exynos4210_cmu_set_rate(s, XOM_0 ? EXYNOS4210_XUSBXTI : > + EXYNOS4210_XXTI); > + } > + } > + > + if (val & MUX_CORE_SEL) { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(CORE_SEL_MASK)) | > + (2 << CORE_SEL_SHIFT); > + > + if ((pre_val & MUX_CORE_SEL) != > + (s->reg[I_(offset)] & MUX_CORE_SEL)) { > + exynos4210_cmu_set_rate(s, EXYNOS4210_SCLK_MPLL); > + } > + > + } else { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(CORE_SEL_MASK)) | > + (1 << CORE_SEL_SHIFT); > + > + if ((pre_val & MUX_CORE_SEL) != > + (s->reg[I_(offset)] & MUX_CORE_SEL)) { > + exynos4210_cmu_set_rate(s, XOM_0 ? EXYNOS4210_XUSBXTI : > + EXYNOS4210_XXTI); > + } > + } > + > + if (val & MUX_HPM_SEL) { > + exynos4210_cmu_set_rate(s, EXYNOS4210_SCLK_MPLL); > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(HPM_SEL_MASK)) | > + (2 << HPM_SEL_SHIFT); > + > + if ((pre_val & MUX_HPM_SEL) != > + (s->reg[I_(offset)] & MUX_HPM_SEL)) { > + exynos4210_cmu_set_rate(s, EXYNOS4210_SCLK_MPLL); > + } > + > + } else { > + s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] = > + (s->reg[I_(CLK_MUX_STAT_(CPU_BLK))] & ~(HPM_SEL_MASK)) | > + (1 << HPM_SEL_SHIFT); > + > + if ((pre_val & MUX_HPM_SEL) != > + (s->reg[I_(offset)] & MUX_HPM_SEL)) { > + exynos4210_cmu_set_rate(s, XOM_0 ? EXYNOS4210_XUSBXTI : > + EXYNOS4210_XXTI); > + } > + } > + } > + break; > + case TOP0_BLK: > + s->reg[I_(offset)] = val; > + exynos4210_cmu_set_rate(s, EXYNOS4210_ACLK_100); > + break; > + default: > + PRINT_ERROR("Unknown functional block: 0x%x\n", (int)block); > + break; > + } > + break; > + case CLK_SRC_MASK: > + break; > + case CLK_MUX_STAT: > + break; > + case CLK_DIV: > + switch (block) { > + case TOP_BLK: > + s->reg[I_(offset)] = val; > + exynos4210_cmu_set_rate(s, EXYNOS4210_ACLK_100); > + break; > + case CPU0_BLK: > + s->reg[I_(offset)] = val; > + exynos4210_cmu_set_rate(s, EXYNOS4210_SCLK_APLL); > + exynos4210_cmu_set_rate(s, EXYNOS4210_SCLK_MPLL); > + break; > + default: > + PRINT_ERROR("Unknown functional block: 0x%x\n", (int)block); > + break; > + } > + break; > + case CLK_DIV_STAT: /* CLK_DIV_STAT */ > + case 0x700: /* Reserved */ > + case CLK_GATE_SCLK: /* reserved? */ > + case CLK_GATE_IP: > + case CLKOUT_CMU: > + break; > + default: > + PRINT_ERROR("Bad offset: 0x%x\n", (int)offset); > + break; > + } > +} > + > +static const MemoryRegionOps exynos4210_cmu_ops = { > + .read = exynos4210_cmu_read, > + .write = exynos4210_cmu_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static void clock_rate_changed(void *opaque) > +{ > + Exynos4210ClockState *cs = (Exynos4210ClockState *)opaque; > + Object *cmu = object_resolve_path(exynos4210_cmu_path[cs->cmu_id], NULL); > + Exynos4210CmuState *s = OBJECT_CHECK(Exynos4210CmuState, cmu, > + TYPE_EXYNOS4210_CMU); > + > + PRINT_DEBUG("Clock %s was changed\n", cs->name); > + > + exynos4210_cmu_set_rate(s, cs->id); > + > +} > + > +static void exynos4210_cmu_reset(DeviceState *dev) > +{ > + Exynos4210CmuState *s = OBJECT_CHECK(Exynos4210CmuState, OBJECT(dev), > + TYPE_EXYNOS4210_CMU); > + int i, j; > + uint32_t index = 0; > + > + for (i = 0; i < s->regs_number; i++) { > + index = (s->regs[i].offset) / sizeof(uint32_t); > + s->reg[index] = s->regs[i].reset_value; > + } > + The code in this next set of nested loops is triggering my "ugly in some way I can't put my finger on" detector. Please try to rephrase. Some hints: if (thing) { big long code block; } else { break; } is less readable than if (!thing) { break; } big long code block; Can you avoid having quite so many levels of nested for/if/if/if? Also, too many blank lines. > + for (i = 0; i < s->clock_number; i++) { > + > + for (j = 0; j < SOURCES_NUMBER; j++) { > + > + if (s->clock[i].src_ids[j] == UNSPECIFIED_CLOCK) { > + > + if (j == 0) { > + /* > + * we have empty '.sources[]' array > + */ > + if (s->clock[i].src_id != UNSPECIFIED_CLOCK) { > + > + s->clock[i].src_ids[j] = s->clock[i].src_id; > + > + } else { > + > + if (s->clock[i].cmu_id != UNSPECIFIED_CMU) { > + /* > + * We haven't any defined sources for this clock. > + * Error during definition of appropriate clock > + * structure > + */ > + hw_error("exynos4210_cmu_reset:" > + "There aren't any sources for %s clock!\n", > + s->clock[i].name); > + } else { > + /* > + * we don't need any sources for this clock > + * because it's a root clock > + */ > + break; > + } > + } > + } else { > + break; /* leave because there are no more sources */ > + } > + } /* src_ids[j] == UNSPECIFIED_CLOCK */ > + > + Exynos4210ClockState *source = > + exynos4210_clock_find(s->clock[i].src_ids[j]); > + > + if (source == NULL) { > + hw_error("We aren't be able to find source clock %d " > + "(requested for %s)\n", > + s->clock[i].src_ids[j], s->clock[i].name); > + } > + > + if (source->cmu_id != UNSPECIFIED_CMU) { > + > + exynos4210_register_clock_handler(clock_rate_changed, > + s->clock[i].src_ids[j], &s->clock[i]); > + } > + } /* SOURCES_NUMBER */ > + > + exynos4210_cmu_set_rate(s, s->clock[i].id); > + } > + > + PRINT_DEBUG("CMU %d reset completed\n", s->cmu_id); > +} > + > +static const VMStateDescription vmstate_exynos4210_clock = { > + .name = TYPE_EXYNOS4210_CLOCK, > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT64(rate, Exynos4210ClockState), > + VMSTATE_INT32(src_id, Exynos4210ClockState), > + VMSTATE_INT32_ARRAY(src_ids, Exynos4210ClockState, SOURCES_NUMBER), > + VMSTATE_UINT32(src_reg, Exynos4210ClockState), > + VMSTATE_UINT32(div_reg, Exynos4210ClockState), > + VMSTATE_UINT8(mux_shift, Exynos4210ClockState), > + VMSTATE_UINT8(div_shift, Exynos4210ClockState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_exynos4210_cmu = { > + .name = TYPE_EXYNOS4210_CMU, > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32_ARRAY(reg, Exynos4210CmuState, > + EXYNOS4210_CMU_REGS_MEM_SIZE / sizeof(uint32_t)), > + VMSTATE_STRUCT_VARRAY_INT32(clock, Exynos4210CmuState, > + clock_number, 0, > + vmstate_exynos4210_clock, > + Exynos4210ClockState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +DeviceState *exynos4210_cmu_create(target_phys_addr_t addr, > + Exynos4210Cmu cmu_id) Really weird indentation on that line. > +{ > + DeviceState *dev; > + SysBusDevice *bus; > + > + dev = qdev_create(NULL, TYPE_EXYNOS4210_CMU); > + > + qdev_prop_set_int32(dev, "cmu_id", cmu_id); > + > + bus = sysbus_from_qdev(dev); > + qdev_init_nofail(dev); > + if (addr != (target_phys_addr_t)-1) { > + sysbus_mmio_map(bus, 0, addr); > + } > + > + return dev; > +} > + > +static int exynos4210_cmu_init(SysBusDevice *dev) > +{ > + Exynos4210CmuState *s = FROM_SYSBUS(Exynos4210CmuState, dev); > + int i, n; > + > + memory_region_init_io(&s->iomem, &exynos4210_cmu_ops, s, > + TYPE_EXYNOS4210_CMU, EXYNOS4210_CMU_REGS_MEM_SIZE); > + sysbus_init_mmio(dev, &s->iomem); > + > + switch (s->cmu_id) { > + case EXYNOS4210_CMU_LEFTBUS: > + s->regs = exynos4210_cmu_leftbus_regs; > + s->regs_number = ARRAY_SIZE(exynos4210_cmu_leftbus_regs); Consider having these in arrays indexed by cmu_id rather than using a switch statement (especially if you're also going to be registering different memory regions depending on the cmu_id). > + break; > + case EXYNOS4210_CMU_RIGHTBUS: > + s->regs = exynos4210_cmu_rightbus_regs; > + s->regs_number = ARRAY_SIZE(exynos4210_cmu_rightbus_regs); > + break; > + case EXYNOS4210_CMU_TOP: > + s->regs = exynos4210_cmu_top_regs; > + s->regs_number = ARRAY_SIZE(exynos4210_cmu_top_regs); > + break; > + case EXYNOS4210_CMU_DMC: > + s->regs = exynos4210_cmu_dmc_regs; > + s->regs_number = ARRAY_SIZE(exynos4210_cmu_dmc_regs); > + break; > + case EXYNOS4210_CMU_CPU: > + s->regs = exynos4210_cmu_cpu_regs; > + s->regs_number = ARRAY_SIZE(exynos4210_cmu_cpu_regs); > + break; > + default: > + hw_error("Wrong CMU: %d\n", s->cmu_id); > + } > + > + for (i = 1, n = 0; i < EXYNOS4210_CLOCKS_NUMBER; i++) { > + if (s->cmu_id == exynos4210_clock[i]->cmu_id) { > + n++; > + } > + } > + > + s->clock = > + (Exynos4210ClockState *)g_malloc0(n * sizeof(Exynos4210ClockState)); Don't need to cast return values from g_malloc0. Also, use g_new0 rather than manual multiply by sizeof. > + > + for (i = 1, s->clock_number = 0; i < EXYNOS4210_CLOCKS_NUMBER; i++) { I wouldn't put 's->clock_number = 0' in the for(;;)'s init section. > + > + if (s->cmu_id == exynos4210_clock[i]->cmu_id) { > + > + memcpy(&s->clock[s->clock_number], exynos4210_clock[i], > + sizeof(Exynos4210ClockState)); Can't we just use structure copy here rather than memcpy? > + > + QTAILQ_INIT(&s->clock[s->clock_number].clock_change_handler); > + > + PRINT_DEBUG("Clock %s was added to \"%s\"\n", > + s->clock[s->clock_number].name, > + exynos4210_cmu_path[s->cmu_id]); > + > + s->clock_number++; > + } > + } > + > + object_property_add_child(object_get_root(), exynos4210_cmu_path[s->cmu_id], > + OBJECT(dev), NULL); > + > + return 0; > +} > + > +static Property exynos4210_cmu_properties[] = { > + DEFINE_PROP_INT32("cmu_id", Exynos4210CmuState, cmu_id, UNSPECIFIED_CMU), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void exynos4210_cmu_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + > + k->init = exynos4210_cmu_init; > + dc->reset = exynos4210_cmu_reset; > + dc->props = exynos4210_cmu_properties; > + dc->vmsd = &vmstate_exynos4210_cmu; > +} > + > +static const TypeInfo exynos4210_cmu_info = { > + .name = TYPE_EXYNOS4210_CMU, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(Exynos4210CmuState), > + .class_init = exynos4210_cmu_class_init, > +}; > + > +static void exynos4210_cmu_register_types(void) > +{ > + type_register_static(&exynos4210_cmu_info); > +} > + > +type_init(exynos4210_cmu_register_types) > -- > 1.7.5.4 > -- PMM