Hi Heiko,
On 18 February 2017 at 07:19, Heiko Stuebner <he...@sntech.de> wrote:
> Hi Simon,
>
> Am Montag, 6. Februar 2017, 07:35:12 CET schrieb Simon Glass:
>> On 3 February 2017 at 08:09, Heiko Stuebner <he...@sntech.de> wrote:
>> > Add a driver for setting up and modifying the various PLLs and peripheral
>> > clocks on the RK3188.
>> >
>> > Signed-off-by: Heiko Stuebner <he...@sntech.de>
>> > ---
>> >
>> > 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 <s...@chromium.org>
>>
>> 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 <he...@sntech.de>
>> > + *
>> > + * 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
