[U-Boot] [PATCH 9/9] rockchip: rk3188: Add clock driver

[Simon Glass]

Hi Heiko,

On 17 July 2016 at 09:48, Simon Glass <sjg at chromium.org> wrote:
> Hi Heiko,
>
> On 17 July 2016 at 09:33, Heiko Stübner <heiko at sntech.de> wrote:
>> Am Sonntag, 17. Juli 2016, 08:13:58 schrieb Simon Glass:
>>> Hi Heiko,
>>>
>>> On 15 July 2016 at 16:17, Heiko Stuebner <heiko at 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 <heiko at sntech.de>
>>> > ---
>>> >
>>> >  arch/arm/include/asm/arch-rockchip/cru_rk3188.h | 186 ++++++++++
>>> >  drivers/clk/Makefile                            |   1 +
>>> >  drivers/clk/clk_rk3188.c                        | 464
>>> >  ++++++++++++++++++++++++ 3 files changed, 651 insertions(+)
>>> >  create mode 100644 arch/arm/include/asm/arch-rockchip/cru_rk3188.h
>>> >  create mode 100644 drivers/clk/clk_rk3188.c
>>>
>>> Could you add a patch to move these files into a drivers/clk/rockchip
>>> directory?
>>
>> ok
>>
>>> > new file mode 100644
>>> > index 0000000..4c28393
>>> > --- /dev/null
>>> > +++ b/drivers/clk/clk_rk3188.c
>>> > @@ -0,0 +1,465 @@
>>> > +/*
>>> > + * (C) Copyright 2015 Google, Inc
>>> > + * (C) Copyright 2016 Heiko Stuebner <heiko at sntech.de>
>>> > + *
>>> > + * SPDX-License-Identifier:    GPL-2.0
>>> > + */
>>> > +
>>> > +#include <common.h>
>>> > +#include <clk-uclass.h>
>>> > +#include <dm.h>
>>> > +#include <errno.h>
>>> > +#include <syscon.h>
>>> > +#include <asm/io.h>
>>> > +#include <asm/arch/clock.h>
>>> > +#include <asm/arch/cru_rk3188.h>
>>> > +#include <asm/arch/grf_rk3188.h>
>>> > +#include <asm/arch/hardware.h>
>>> > +#include <dt-bindings/clock/rk3188-cru.h>
>>> > +#include <dm/device-internal.h>
>>> > +#include <dm/lists.h>
>>> > +#include <dm/uclass-internal.h>
>>> > +
>>> > +DECLARE_GLOBAL_DATA_PTR;
>>> > +
>>> > +struct rk3188_clk_priv {
>>> > +       struct rk3188_grf *grf;
>>> > +       struct rk3188_cru *cru;
>>> > +       ulong rate;
>>> > +       bool has_bwadj;
>>> > +};
>>> > +
>>> > +struct pll_div {
>>> > +       u32 nr;
>>> > +       u32 nf;
>>> > +       u32 no;
>>> > +};
>>> > +
>>> > +enum {
>>> > +       VCO_MAX_HZ      = 2200U * 1000000,
>>> > +       VCO_MIN_HZ      = 440 * 1000000,
>>> > +       OUTPUT_MAX_HZ   = 2200U * 1000000,
>>> > +       OUTPUT_MIN_HZ   = 30 * 1000000,
>>> > +       FREF_MAX_HZ     = 2200U * 1000000,
>>> > +       FREF_MIN_HZ     = 30 * 1000,
>>> > +};
>>> > +
>>> > +enum {
>>> > +       /* PLL CON0 */
>>> > +       PLL_OD_MASK             = 0x0f,
>>> > +
>>> > +       /* PLL CON1 */
>>> > +       PLL_NF_MASK             = 0x1fff,
>>> > +
>>> > +       /* PLL CON2 */
>>> > +       PLL_BWADJ_MASK          = 0x0fff,
>>> > +
>>> > +       /* PLL CON3 */
>>> > +       PLL_RESET_SHIFT         = 5,
>>> > +
>>> > +       /* GRF_SOC_STATUS0 */
>>> > +       SOCSTS_DPLL_LOCK        = 1 << 5,
>>> > +       SOCSTS_APLL_LOCK        = 1 << 6,
>>> > +       SOCSTS_CPLL_LOCK        = 1 << 7,
>>> > +       SOCSTS_GPLL_LOCK        = 1 << 8,
>>> > +};
>>> > +
>>> > +#define RATE_TO_DIV(input_rate, output_rate) \
>>> > +       ((input_rate) / (output_rate) - 1);
>>> > +
>>> > +#define DIV_TO_RATE(input_rate, div)   ((input_rate) / ((div) + 1))
>>> > +
>>> > +#define PLL_DIVISORS(hz, _nr, _no) {\
>>> > +       .nr = _nr, .nf = (u32)((u64)hz * _nr * _no / OSC_HZ), .no = _no};\
>>> > +       _Static_assert(((u64)hz * _nr * _no / OSC_HZ) * OSC_HZ /\
>>> > +                      (_nr * _no) == hz, #hz "Hz cannot be hit with PLL
>>> > "\
>>> > +                      "divisors on line " __stringify(__LINE__));
>>> > +
>>> > +/* Keep divisors as low as possible to reduce jitter and power usage */
>>> > +static const struct pll_div apll_init_cfg = PLL_DIVISORS(APLL_HZ, 1, 1);
>>> > +static const struct pll_div gpll_init_cfg = PLL_DIVISORS(GPLL_HZ, 2, 2);
>>> > +static const struct pll_div cpll_init_cfg = PLL_DIVISORS(CPLL_HZ, 1, 2);
>>> > +
>>> > +void *rockchip_get_cru(void)
>>> > +{
>>> > +       struct rk3188_clk_priv *priv;
>>> > +       struct udevice *dev;
>>> > +       int ret;
>>> > +
>>> > +       ret = uclass_get_device_by_name(UCLASS_CLK,
>>> > +                                       "clock-controller at 20000000",
>>> > &dev);
>>>
>>> This seems odd. Could you use uclass_get_device(UCLASS_CLK, 0, .&dev) ?
>>
>> Index 0 actually gets me the 24MHz oscillator fixed clock :-), which is why I
>> switched to the by-name variant to not depend on some dts or uclass ordering.
>> I'm wondering how that works on the other socs.
>
> I suspect this might have become broken by Stephen's clock changes. I
> had a bit of a look at this but have not resolved it yet. For example
> on firefly, HDMI does not work now.
>
> There is this:
>
> enum rk_clk_id {
> CLK_OSC,
> CLK_ARM,
> CLK_DDR,
> CLK_CODEC,
> CLK_GENERAL,
> CLK_NEW,
>
> CLK_COUNT,
> };
>
> and it used to check against the platform data (see rkclk_get_clk() in
> v2016.05). I'll have a think about it.
>
>>

I found a problem here...please see:

http://patchwork.ozlabs.org/patch/649283/

Regards,
Simon

[...]