From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Tue, 21 Feb 2017 13:35:23 -0700 Subject: [U-Boot] [PATCH v3 10/16] rockchip: rk3188: Add clock driver In-Reply-To: References: <20170203160939.27594-1-heiko@sntech.de> <20170203160939.27594-11-heiko@sntech.de> <2833030.KiCRt674Zv@phil> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 21 February 2017 at 11:07, Simon Glass wrote: > Hi Heiko, > > On 18 February 2017 at 07:19, Heiko Stuebner wrote: >> Hi Simon, >> >> Am Montag, 6. Februar 2017, 07:35:12 CET schrieb Simon Glass: >>> On 3 February 2017 at 08:09, Heiko Stuebner wrote: >>> > Add a driver for setting up and modifying the various PLLs and peripheral >>> > clocks on the RK3188. >>> > >>> > Signed-off-by: Heiko Stuebner >>> > --- >>> > >>> > arch/arm/include/asm/arch-rockchip/cru_rk3188.h | 191 +++++++++ >>> > drivers/clk/rockchip/Makefile | 1 + >>> > drivers/clk/rockchip/clk_rk3188.c | 523 >>> > ++++++++++++++++++++++++ 3 files changed, 715 insertions(+) >>> > create mode 100644 arch/arm/include/asm/arch-rockchip/cru_rk3188.h >>> > create mode 100644 drivers/clk/rockchip/clk_rk3188.c >>> >>> Reviewed-by: Simon Glass >>> >>> With one comment below. >>> >>> > diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3188.h >>> > b/arch/arm/include/asm/arch-rockchip/cru_rk3188.h new file mode 100644 >>> > index 0000000000..74f0fedcc6 >>> > --- /dev/null >>> > +++ b/arch/arm/include/asm/arch-rockchip/cru_rk3188.h >>> > @@ -0,0 +1,191 @@ >>> > +/* >>> > + * (C) Copyright 2016 Heiko Stuebner >>> > + * >>> > + * SPDX-License-Identifier: GPL-2.0+ >>> > + */ >>> > +#ifndef _ASM_ARCH_CRU_RK3188_H >>> > +#define _ASM_ARCH_CRU_RK3188_H >>> > + >>> > +#define OSC_HZ (24 * 1000 * 1000) >>> > + >>> > +#define APLL_HZ (1608 * 1000000) >>> > +#define GPLL_HZ (594 * 1000000) >>> > +#define CPLL_HZ (384 * 1000000) >>> > + >>> > +/* The SRAM is clocked off aclk_cpu, so we want to max it out for boot >>> > speed */ +#define CPU_ACLK_HZ 297000000 >>> > +#define CPU_HCLK_HZ 148500000 >>> > +#define CPU_PCLK_HZ 74250000 >>> > +#define CPU_H2P_HZ 74250000 >>> > + >>> > +#define PERI_ACLK_HZ 148500000 >>> > +#define PERI_HCLK_HZ 148500000 >>> > +#define PERI_PCLK_HZ 74250000 >>> > + >>> > +/* Private data for the clock driver - used by rockchip_get_cru() */ >>> > +struct rk3188_clk_priv { >>> > + struct rk3188_grf *grf; >>> > + struct rk3188_cru *cru; >>> > + ulong rate; >>> > + bool has_bwadj; >>> > +}; >>> > + >>> > +struct rk3188_cru { >>> > + struct rk3188_pll { >>> > + u32 con0; >>> > + u32 con1; >>> > + u32 con2; >>> > + u32 con3; >>> > + } pll[4]; >>> > + u32 cru_mode_con; >>> > + u32 cru_clksel_con[35]; >>> > + u32 cru_clkgate_con[10]; >>> > + u32 reserved1[2]; >>> > + u32 cru_glb_srst_fst_value; >>> > + u32 cru_glb_srst_snd_value; >>> > + u32 reserved2[2]; >>> > + u32 cru_softrst_con[9]; >>> > + u32 cru_misc_con; >>> > + u32 reserved3[2]; >>> > + u32 cru_glb_cnt_th; >>> > +}; >>> > +check_member(rk3188_cru, cru_glb_cnt_th, 0x0140); >>> > + >>> > +/* CRU_CLKSEL0_CON */ >>> > +enum { >>> > + /* a9_core_div: core = core_src / (a9_core_div + 1) */ >>> > + A9_CORE_DIV_SHIFT = 9, >>> > + A9_CORE_DIV_MASK = 0x1f, >>> >>> Can you define >>> >>> A9_CORE_DIV_MASK = 0x1f << A9_CORE_DIV_SHIFT >>> >>> and similarly for other masks. I got this wrong with rk3288, and I >>> think shifting the mask makes the code easier in places. >> >> I'd disagree here. We're using this scheme everywhere on Rockchip platforms. >> For example please look at all the pinmux defines >> GPIO3C1_SHIFT = 2, >> GPIO3C1_MASK = 3, >> GPIO3C1_GPIO = 0, >> GPIO3C1_SDMMC1_DATA0, >> GPIO3C1_RMII_TXD1, >> GPIO3C1_RESERVED, >> >> and numerous other places and I'd think mixing paradigms in one soc and >> between all Rockchip socs would be somewhat unwise. > > The new ones are going to use this approach and I will get around to > converting them at some point. I do think I did it wrong in the first > place. > > Anyway let's not hold this up for this as we have the same issue with rk3399. > > Regards, > Simon Applied to u-boot-rockchip, thanks!