[U-Boot] [PATCH 3/7] clk: rockchip: add support for rk3328

[Kever Yang <kever.yang@rock-chips.com>]

Hi Simon,

On 02/22/2017 02:06 AM, Simon Glass wrote:
> Hi Kever,
>
> On 17 February 2017 at 01:07, Kever Yang <kever.yang@rock-chips.com> wrote:
>> Add rk3328 clock driver and cru structure definition.
>>
>> Signed-off-by: William Zhang <william.zhang@rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>>   arch/arm/include/asm/arch-rockchip/cru_rk3328.h |  65 +++
>>   drivers/clk/rockchip/Makefile                   |   1 +
>>   drivers/clk/rockchip/clk_rk3328.c               | 607 ++++++++++++++++++++++++
>>   3 files changed, 673 insertions(+)
>>   create mode 100644 arch/arm/include/asm/arch-rockchip/cru_rk3328.h
>>   create mode 100644 drivers/clk/rockchip/clk_rk3328.c
>>
>> diff --git a/arch/arm/include/asm/arch-rockchip/cru_rk3328.h b/arch/arm/include/asm/arch-rockchip/cru_rk3328.h
>> new file mode 100644
>> index 0000000..089c4ca
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-rockchip/cru_rk3328.h
>> @@ -0,0 +1,65 @@
>> +/*
>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#ifndef __ASM_ARCH_CRU_RK3328_H_
>> +#define __ASM_ARCH_CRU_RK3328_H_
>> +
>> +#include <common.h>
>> +
>> +struct rk3328_cru {
>> +       u32 apll_con[5];
>> +       u32 reserved1[3];
>> +       u32 dpll_con[5];
>> +       u32 reserved2[3];
>> +       u32 cpll_con[5];
>> +       u32 reserved3[3];
>> +       u32 gpll_con[5];
>> +       u32 reserved4[3];
>> +       u32 mode_con;
>> +       u32 misc;
>> +       u32 reserved5[2];
>> +       u32 glb_cnt_th;
>> +       u32 glb_rst_st;
>> +       u32 glb_srst_snd_value;
>> +       u32 glb_srst_fst_value;
>> +       u32 npll_con[5];
>> +       u32 reserved6[(0x100-0xb4)/4];
>> +       u32 clksel_con[53];
>> +       u32 reserved7[(0x200-0x1d4)/4];
>> +       u32 clkgate_con[29];
>> +       u32 reserved8[3];
>> +       u32 ssgtbl[32];
>> +       u32 softrst_con[12];
>> +       u32 reserved9[(0x380-0x330)/4];
>> +       u32 sdmmc_con[2];
>> +       u32 sdio_con[2];
>> +       u32 emmc_con[2];
>> +       u32 sdmmc_ext_con[2];
>> +};
>> +check_member(rk3328_cru, sdmmc_ext_con[1], 0x39c);
>> +#define MHz            1000000
>> +#define KHz            1000
>> +#define OSC_HZ         (24 * MHz)
>> +#define APLL_HZ                (600 * MHz)
>> +#define GPLL_HZ                (576 * MHz)
>> +#define CPLL_HZ                (594 * MHz)
>> +
>> +#define CLK_CORE_HZ    (600 * MHz)
>> +#define ACLKM_CORE_HZ  (300 * MHz)
>> +#define PCLK_DBG_HZ    (300 * MHz)
>> +
>> +#define PERIHP_ACLK_HZ (144000 * KHz)
>> +#define PERIHP_HCLK_HZ (72000 * KHz)
>> +#define PERIHP_PCLK_HZ (72000 * KHz)
>> +
>> +#define PWM_CLOCK_HZ    (74 * MHz)
>> +
>> +enum apll_frequencies {
>> +       APLL_816_MHZ,
>> +       APLL_600_MHZ,
>> +};
>> +
>> +#endif /* __ASM_ARCH_CRU_RK3328_H_ */
>> diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile
>> index 1f8e417..c4e81e9 100644
>> --- a/drivers/clk/rockchip/Makefile
>> +++ b/drivers/clk/rockchip/Makefile
>> @@ -6,4 +6,5 @@
>>
>>   obj-$(CONFIG_ROCKCHIP_RK3036) += clk_rk3036.o
>>   obj-$(CONFIG_ROCKCHIP_RK3288) += clk_rk3288.o
>> +obj-$(CONFIG_ROCKCHIP_RK3328) += clk_rk3328.o
>>   obj-$(CONFIG_ROCKCHIP_RK3399) += clk_rk3399.o
>> diff --git a/drivers/clk/rockchip/clk_rk3328.c b/drivers/clk/rockchip/clk_rk3328.c
>> new file mode 100644
>> index 0000000..e075414
>> --- /dev/null
>> +++ b/drivers/clk/rockchip/clk_rk3328.c
>> @@ -0,0 +1,607 @@
>> +/*
>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/arch/clock.h>
>> +#include <asm/arch/cru_rk3328.h>
>> +#include <asm/arch/hardware.h>
>> +#include <asm/io.h>
>> +#include <clk-uclass.h>
>> +#include <dm.h>
>> +#include <dm/lists.h>
>> +#include <dt-bindings/clock/rk3328-cru.h>
>> +#include <errno.h>
>> +#include <syscon.h>
> Can you please tidy up the order here? asm/ should go after the normal
> headers, and dm/lists and dt-bindings/ afer that

OK, will fix.
>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +struct rk3328_clk_priv {
>> +       struct rk3328_cru *cru;
>> +       ulong rate;
>> +};
>> +
> [..]
>
>> +
>> +#define I2C_CLK_REG_MASK(bus) \
>> +                       (CLK_I2C_DIV_CON_MASK << \
>> +                       CLK_I2C ##bus## _DIV_CON_SHIFT | \
>> +                       CLK_I2C_PLL_SEL_MASK << \
>> +                       CLK_I2C ##bus## _PLL_SEL_SHIFT)
>> +
>> +#define (bus, clk_div) \
>> +                        ((clk_div - 1) << \
>> +                        CLK_I2C ##bus## _DIV_CON_SHIFT | \
>> +                        CLK_I2C_PLL_SEL_GPLL << \
>> +                        CLK_I2C ##bus## _PLL_SEL_SHIFT)
>> +
>> +#define I2C_CLK_DIV_VALUE(con, bus) \
>> +                        (con >> CLK_I2C ##bus## _DIV_CON_SHIFT) & \
>> +                        CLK_I2C_DIV_CON_MASK;
> Can we drop these three and instead write them out below?

I don't know why you don't like this kind of MACRO, like the size_mb in 
sdram driver,
we though this help people understand the C source and make the C source 
cold looks much clean,
in some platform, maintainer may ask for this when there are multi 
controller and can reuse
the same MACRO.
Anyway, I will make this fallback to normal shift/mask style in next 
version.

>
>> +
>> +#define VCO_MAX_KHZ    (3200 * (MHz / KHz))
>> +#define VCO_MIN_KHZ    (800 * (MHz / KHz))
>> +#define OUTPUT_MAX_KHZ (3200 * (MHz / KHz))
>> +#define OUTPUT_MIN_KHZ (16 * (MHz / KHz))
>> +
>> +/*
>> + *  the div restructions of pll in integer mode, these are defined in
>> + *  * CRU_*PLL_CON0 or PMUCRU_*PLL_CON0
>> + */
>> +#define PLL_DIV_MIN    16
>> +#define PLL_DIV_MAX    3200
>> +
> [...]
>
>> +static ulong rk3328_mmc_get_clk(struct rk3328_cru *cru, uint clk_id)
>> +{
>> +       u32 div, con, con_id;
>> +
>> +       switch (clk_id) {
>> +       case HCLK_SDMMC:
>> +               con_id = 30;
>> +               break;
>> +       case HCLK_EMMC:
>> +               con_id = 32;
>> +               break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +       con = readl(&cru->clksel_con[con_id]);
>> +       div = (con & CLK_EMMC_DIV_CON_MASK) >> CLK_EMMC_DIV_CON_SHIFT;
>> +
>> +       if ((con & CLK_EMMC_PLL_MASK) >> CLK_EMMC_PLL_SHIFT
>> +           == CLK_EMMC_PLL_SEL_24M)
>> +               return DIV_TO_RATE(24*1024*1024, div);
> Perhaps should have a #define for the 24MHz oscillator?

Already have one, will fix.
>
>> +       else
>> +               return DIV_TO_RATE(GPLL_HZ, div);
>> +}
>> +
>> +static ulong rk3328_mmc_set_clk(struct rk3328_cru *cru,
>> +                               ulong clk_id, ulong set_rate)
>> +{
>> +       int src_clk_div;
>> +       u32 con_id;
>> +
>> +       switch (clk_id) {
>> +       case HCLK_SDMMC:
>> +               con_id = 30;
>> +               break;
>> +       case HCLK_EMMC:
>> +               con_id = 32;
>> +               break;
>> +       default:
>> +               return -EINVAL;
>> +       }
>> +       /* Select clk_sdmmc/emmc source from GPLL by default */
>> +       src_clk_div = GPLL_HZ / set_rate;
>> +
>> +       if (src_clk_div > 127) {
>> +               /* use 24MHz source for 400KHz clock */
>> +               src_clk_div = 24*1024*1024 / set_rate;
>> +               rk_clrsetreg(&cru->clksel_con[con_id],
>> +                            CLK_EMMC_PLL_MASK | CLK_EMMC_DIV_CON_MASK,
>> +                            CLK_EMMC_PLL_SEL_24M << CLK_EMMC_PLL_SHIFT |
>> +                            (src_clk_div - 1) << CLK_EMMC_DIV_CON_SHIFT);
>> +       } else {
>> +               rk_clrsetreg(&cru->clksel_con[con_id],
>> +                            CLK_EMMC_PLL_MASK | CLK_EMMC_DIV_CON_MASK,
>> +                            CLK_EMMC_PLL_SEL_GPLL << CLK_EMMC_PLL_SHIFT |
>> +                            (src_clk_div - 1) << CLK_EMMC_DIV_CON_SHIFT);
>> +       }
>> +
>> +       return rk3328_mmc_get_clk(cru, clk_id);
>> +}
>> +
>> +static ulong rk3328_pwm_get_clk(struct rk3328_cru *cru)
>> +{
>> +       u32 div, con;
>> +
>> +       con = readl(&cru->clksel_con[24]);
>> +       div = (con & CLK_PWM_DIV_CON_MASK) >> CLK_PWM_DIV_CON_SHIFT;
>> +
>> +       return DIV_TO_RATE(GPLL_HZ, div);
>> +}
>> +
>> +static ulong rk3328_pwm_set_clk(struct rk3328_cru *cru, uint hz)
>> +{
>> +       u32 div = GPLL_HZ / hz;
>> +
>> +       rk_clrsetreg(&cru->clksel_con[24],
>> +                    CLK_PWM_PLL_SEL_MASK | CLK_PWM_DIV_CON_MASK,
>> +                    CLK_PWM_PLL_SEL_GPLL << CLK_PWM_PLL_SEL_SHIFT |
>> +                    (div - 1) << CLK_PWM_DIV_CON_SHIFT);
>> +
>> +       return DIV_TO_RATE(GPLL_HZ, div);
>> +}
>> +
>> +static ulong rk3328_clk_get_rate(struct clk *clk)
>> +{
>> +       struct rk3328_clk_priv *priv = dev_get_priv(clk->dev);
>> +       ulong rate = 0;
>> +
>> +       switch (clk->id) {
>> +       case 0 ... 29:
>> +               return 0;
>> +       case HCLK_SDMMC:
>> +       case HCLK_EMMC:
>> +               rate = rk3328_mmc_get_clk(priv->cru, clk->id);
>> +               break;
>> +       case SCLK_I2C0:
>> +       case SCLK_I2C1:
>> +       case SCLK_I2C2:
>> +       case SCLK_I2C3:
>> +               rate = rk3328_i2c_get_clk(priv->cru, clk->id);
>> +               break;
>> +       case SCLK_PWM:
>> +               rate = rk3328_pwm_get_clk(priv->cru);
>> +               break;
>> +       default:
>> +               return -ENOENT;
>> +       }
>> +
>> +       return rate;
>> +}
>> +
>> +static ulong rk3328_clk_set_rate(struct clk *clk, ulong rate)
>> +{
>> +       struct rk3328_clk_priv *priv = dev_get_priv(clk->dev);
>> +       ulong ret = 0;
>> +
>> +       switch (clk->id) {
>> +       case 0 ... 29:
>> +               return 0;
>> +       case HCLK_SDMMC:
>> +       case HCLK_EMMC:
>> +               ret = rk3328_mmc_set_clk(priv->cru, clk->id, rate);
>> +               break;
>> +       case SCLK_I2C0:
>> +       case SCLK_I2C1:
>> +       case SCLK_I2C2:
>> +       case SCLK_I2C3:
>> +               ret = rk3328_i2c_set_clk(priv->cru, clk->id, rate);
>> +               break;
>> +       case SCLK_PWM:
>> +               ret = rk3328_pwm_set_clk(priv->cru, rate);
>> +               break;
>> +       default:
>> +               return -ENOENT;
>> +       }
>> +
>> +       return ret;
>> +}
>> +
>> +static struct clk_ops rk3328_clk_ops = {
>> +       .get_rate = rk3328_clk_get_rate,
>> +       .set_rate = rk3328_clk_set_rate,
>> +};
>> +
>> +void *rockchip_get_cru(void)
>> +{
>> +       struct udevice *dev;
>> +       fdt_addr_t *addr;
>> +       int ret;
>> +
>> +       ret = uclass_get_device_by_name(UCLASS_CLK, "clk_rk3328", &dev);
> Would it be possible to use uclass_get_device_by_driver()? Also, if
> this function is to be exported, it really should be in
> arch/arm/mach-rockchip somewhere.

Will move to arch/arm/mach-rockchip.

Thanks,
- Kever
>
>> +       if (ret)
>> +               return ERR_PTR(ret);
>> +
>> +       addr = dev_get_addr_ptr(dev);
>> +       if ((fdt_addr_t)addr == FDT_ADDR_T_NONE)
>> +               return ERR_PTR(-EINVAL);
>> +
>> +       return addr;
>> +}
>> +
>> +static int rk3328_clk_probe(struct udevice *dev)
>> +{
>> +       struct rk3328_clk_priv *priv = dev_get_priv(dev);
>> +
>> +       rkclk_init(priv->cru);
>> +
>> +       return 0;
>> +}
>> +
>> +static int rk3328_clk_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct rk3328_clk_priv *priv = dev_get_priv(dev);
>> +
>> +       priv->cru = (struct rk3328_cru *)dev_get_addr(dev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int rk3328_clk_bind(struct udevice *dev)
>> +{
>> +       int ret;
>> +
>> +       /* The reset driver does not have a device node, so bind it here */
>> +       ret = device_bind_driver(gd->dm_root, "rk3328_sysreset", "reset", &dev);
>> +       if (ret)
>> +               printf("Warning: No RK3328 reset driver: ret=%d\n", ret);
>> +
>> +       return ret;
>> +}
>> +
>> +static const struct udevice_id rk3328_clk_ids[] = {
>> +       { .compatible = "rockchip,rk3328-cru" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(clk_rk3328) = {
>> +       .name           = "clk_rk3328",
>> +       .id             = UCLASS_CLK,
>> +       .of_match       = rk3328_clk_ids,
>> +       .priv_auto_alloc_size = sizeof(struct rk3328_clk_priv),
>> +       .ofdata_to_platdata = rk3328_clk_ofdata_to_platdata,
>> +       .ops            = &rk3328_clk_ops,
>> +       .bind           = rk3328_clk_bind,
>> +       .probe          = rk3328_clk_probe,
>> +};
>> --
>> 1.9.1
>>
> Regards,
> Simon
>
>
>