diff mbox series

[v2,02/18] clk: intel: Add clock driver for Intel MIPS SoCs

Message ID 20180803030237.3366-3-songjun.wu@linux.intel.com (mailing list archive)
State Changes Requested, archived
Headers show
Series MIPS: intel: add initial support for Intel MIPS SoCs | expand

Commit Message

Wu, Songjun Aug. 3, 2018, 3:02 a.m. UTC
From: Yixin Zhu <yixin.zhu@linux.intel.com>

This driver provides PLL clock registration as well as various clock
branches, e.g. MUX clock, gate clock, divider clock and so on.

PLLs that provide clock to DDR, CPU and peripherals are shown below:

                 +---------+
            |--->| LCPLL3 0|--PCIe clk-->
   XO       |    +---------+
+-----------|
            |    +---------+
            |    |        3|--PAE clk-->
            |--->| PLL0B  2|--GSWIP clk-->
            |    |        1|--DDR clk-->DDR PHY clk-->
            |    |        0|--CPU1 clk--+   +-----+
            |    +---------+            |--->0    |
            |                               | MUX |--CPU clk-->
            |    +---------+            |--->1    |
            |    |        0|--CPU0 clk--+   +-----+
            |--->| PLLOA  1|--SSX4 clk-->
                 |        2|--NGI clk-->
                 |        3|--CBM clk-->
                 +---------+

Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
---

Changes in v2:
- Rewrite clock driver, add platform clock description details in
  clock driver.

 drivers/clk/Kconfig                          |   1 +
 drivers/clk/Makefile                         |   3 +
 drivers/clk/intel/Kconfig                    |  20 ++
 drivers/clk/intel/Makefile                   |   7 +
 drivers/clk/intel/clk-cgu-pll.c              | 166 ++++++++++
 drivers/clk/intel/clk-cgu-pll.h              |  34 ++
 drivers/clk/intel/clk-cgu.c                  | 470 +++++++++++++++++++++++++++
 drivers/clk/intel/clk-cgu.h                  | 259 +++++++++++++++
 drivers/clk/intel/clk-grx500.c               | 168 ++++++++++
 include/dt-bindings/clock/intel,grx500-clk.h |  69 ++++
 10 files changed, 1197 insertions(+)
 create mode 100644 drivers/clk/intel/Kconfig
 create mode 100644 drivers/clk/intel/Makefile
 create mode 100644 drivers/clk/intel/clk-cgu-pll.c
 create mode 100644 drivers/clk/intel/clk-cgu-pll.h
 create mode 100644 drivers/clk/intel/clk-cgu.c
 create mode 100644 drivers/clk/intel/clk-cgu.h
 create mode 100644 drivers/clk/intel/clk-grx500.c
 create mode 100644 include/dt-bindings/clock/intel,grx500-clk.h

Comments

Rob Herring Aug. 6, 2018, 3:19 p.m. UTC | #1
On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>
> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>
> This driver provides PLL clock registration as well as various clock
> branches, e.g. MUX clock, gate clock, divider clock and so on.
>
> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>
>                  +---------+
>             |--->| LCPLL3 0|--PCIe clk-->
>    XO       |    +---------+
> +-----------|
>             |    +---------+
>             |    |        3|--PAE clk-->
>             |--->| PLL0B  2|--GSWIP clk-->
>             |    |        1|--DDR clk-->DDR PHY clk-->
>             |    |        0|--CPU1 clk--+   +-----+
>             |    +---------+            |--->0    |
>             |                               | MUX |--CPU clk-->
>             |    +---------+            |--->1    |
>             |    |        0|--CPU0 clk--+   +-----+
>             |--->| PLLOA  1|--SSX4 clk-->
>                  |        2|--NGI clk-->
>                  |        3|--CBM clk-->
>                  +---------+
>
> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> ---
>
> Changes in v2:
> - Rewrite clock driver, add platform clock description details in
>   clock driver.
>
>  drivers/clk/Kconfig                          |   1 +
>  drivers/clk/Makefile                         |   3 +
>  drivers/clk/intel/Kconfig                    |  20 ++
>  drivers/clk/intel/Makefile                   |   7 +
>  drivers/clk/intel/clk-cgu-pll.c              | 166 ++++++++++
>  drivers/clk/intel/clk-cgu-pll.h              |  34 ++
>  drivers/clk/intel/clk-cgu.c                  | 470 +++++++++++++++++++++++++++
>  drivers/clk/intel/clk-cgu.h                  | 259 +++++++++++++++
>  drivers/clk/intel/clk-grx500.c               | 168 ++++++++++

>  include/dt-bindings/clock/intel,grx500-clk.h |  69 ++++

This belongs with the clk binding patch.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu, Yi Xin Aug. 8, 2018, 2:51 a.m. UTC | #2
On 8/6/2018 11:19 PM, Rob Herring wrote:
> On Thu, Aug 2, 2018 at 9:03 PM Songjun Wu <songjun.wu@linux.intel.com> wrote:
>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> This driver provides PLL clock registration as well as various clock
>> branches, e.g. MUX clock, gate clock, divider clock and so on.
>>
>> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>>
>>                   +---------+
>>              |--->| LCPLL3 0|--PCIe clk-->
>>     XO       |    +---------+
>> +-----------|
>>              |    +---------+
>>              |    |        3|--PAE clk-->
>>              |--->| PLL0B  2|--GSWIP clk-->
>>              |    |        1|--DDR clk-->DDR PHY clk-->
>>              |    |        0|--CPU1 clk--+   +-----+
>>              |    +---------+            |--->0    |
>>              |                               | MUX |--CPU clk-->
>>              |    +---------+            |--->1    |
>>              |    |        0|--CPU0 clk--+   +-----+
>>              |--->| PLLOA  1|--SSX4 clk-->
>>                   |        2|--NGI clk-->
>>                   |        3|--CBM clk-->
>>                   +---------+
>>
>> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>> ---
>>
>> Changes in v2:
>> - Rewrite clock driver, add platform clock description details in
>>    clock driver.
>>
>>   drivers/clk/Kconfig                          |   1 +
>>   drivers/clk/Makefile                         |   3 +
>>   drivers/clk/intel/Kconfig                    |  20 ++
>>   drivers/clk/intel/Makefile                   |   7 +
>>   drivers/clk/intel/clk-cgu-pll.c              | 166 ++++++++++
>>   drivers/clk/intel/clk-cgu-pll.h              |  34 ++
>>   drivers/clk/intel/clk-cgu.c                  | 470 +++++++++++++++++++++++++++
>>   drivers/clk/intel/clk-cgu.h                  | 259 +++++++++++++++
>>   drivers/clk/intel/clk-grx500.c               | 168 ++++++++++
>>   include/dt-bindings/clock/intel,grx500-clk.h |  69 ++++
> This belongs with the clk binding patch.
>
> Rob
Thanks for review.
Will move it to clock binding patch.

>

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Aug. 8, 2018, 5:50 a.m. UTC | #3
Quoting Songjun Wu (2018-08-02 20:02:21)
> From: Yixin Zhu <yixin.zhu@linux.intel.com>
> 
> This driver provides PLL clock registration as well as various clock
> branches, e.g. MUX clock, gate clock, divider clock and so on.
> 
> PLLs that provide clock to DDR, CPU and peripherals are shown below:
> 
>                  +---------+
>             |--->| LCPLL3 0|--PCIe clk-->
>    XO       |    +---------+
> +-----------|
>             |    +---------+
>             |    |        3|--PAE clk-->
>             |--->| PLL0B  2|--GSWIP clk-->
>             |    |        1|--DDR clk-->DDR PHY clk-->
>             |    |        0|--CPU1 clk--+   +-----+
>             |    +---------+            |--->0    |
>             |                               | MUX |--CPU clk-->
>             |    +---------+            |--->1    |
>             |    |        0|--CPU0 clk--+   +-----+
>             |--->| PLLOA  1|--SSX4 clk-->
>                  |        2|--NGI clk-->
>                  |        3|--CBM clk-->
>                  +---------+

Thanks for the picture!

> 
> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 0bb25dd009d1..d929ca4607cf 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI)                       += hisilicon/
>  obj-y                                  += imgtec/
>  obj-$(CONFIG_ARCH_MXC)                 += imx/
>  obj-$(CONFIG_MACH_INGENIC)             += ingenic/
> +ifeq ($(CONFIG_COMMON_CLK), y)
> +obj-y                                                  +=intel/
> +endif

Why not obj-$(CONFIG_INTEL_CCF) or something like that?

>  obj-$(CONFIG_ARCH_KEYSTONE)            += keystone/
>  obj-$(CONFIG_MACH_LOONGSON32)          += loongson1/
>  obj-y                                  += mediatek/
> diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
> new file mode 100644
> index 000000000000..c7d3fb1721fa
> --- /dev/null
> +++ b/drivers/clk/intel/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0
> +config INTEL_CGU_CLK
> +       depends on COMMON_CLK
> +       depends on INTEL_MIPS || COMPILE_TEST
> +       select MFD_SYSCON
> +       bool "Intel clock controller support"
> +       help
> +         This driver support Intel CGU (Clock Generation Unit).

Is it really called a clock generation unit? Or that's just copied from
sunxi driver?

> +
> +choice
> +       prompt "SoC platform selection"
> +       depends on INTEL_CGU_CLK
> +       default INTEL_GRX500_CGU_CLK
> +
> +config INTEL_GRX500_CGU_CLK
> +       bool "GRX500 CLK"
> +       help
> +         Clock driver of GRX500 platform.
> +
> +endchoice
> diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
> new file mode 100644
> index 000000000000..16a0138e52c2
> --- /dev/null
> +++ b/drivers/clk/intel/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Makefile for intel specific clk
> +
> +obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
> +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
> +       obj-y += clk-grx500.o
> +endif
> diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
> new file mode 100644
> index 000000000000..20759bc27e95
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu-pll.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2018 Intel Corporation.
> + *  Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#include <linux/clk.h>

Is this include used?

> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include "clk-cgu-pll.h"
> +#include "clk-cgu.h"
> +
> +#define to_intel_clk_pll(_hw)  container_of(_hw, struct intel_clk_pll, hw)
> +
> +/*
> + * Calculate formula:
> + * rate = (prate * mult + (prate * frac) / frac_div) / div
> + */
> +static unsigned long
> +intel_pll_calc_rate(unsigned long prate, unsigned int mult,
> +                   unsigned int div, unsigned int frac,
> +                   unsigned int frac_div)
> +{
> +       u64 crate, frate, rate64;
> +
> +       rate64 = prate;
> +       crate = rate64 * mult;
> +
> +       if (frac) {
> +               frate = rate64 * frac;
> +               do_div(frate, frac_div);
> +               crate += frate;
> +       }
> +       do_div(crate, div);
> +
> +       return (unsigned long)crate;
> +}
> +
> +static void
> +grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
> +                     unsigned int *frac)
> +{
> +       *mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
> +       *frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
> +}
> +
> +static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
> +{
> +       unsigned int val;
> +
> +       return regmap_read_poll_timeout(pll->map, pll->reg, val,
> +                                       val & BIT(bit_idx), 10, 1000);
> +}
> +
> +static unsigned long
> +intel_grx500_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
> +{
> +       struct intel_clk_pll *pll = to_intel_clk_pll(hw);
> +       unsigned int mult, frac;
> +
> +       grx500_pll_get_params(pll, &mult, &frac);
> +
> +       return intel_pll_calc_rate(prate, mult, 1, frac, BIT(20));
> +}
> +
> +static int intel_grx500_pll_is_enabled(struct clk_hw *hw)
> +{
> +       struct intel_clk_pll *pll = to_intel_clk_pll(hw);
> +
> +       if (intel_wait_pll_lock(pll, 1)) {
> +               pr_err("%s: pll: %s is not locked!\n",
> +                      __func__, clk_hw_get_name(hw));
> +               return 0;
> +       }
> +
> +       return intel_get_clk_val(pll->map, pll->reg, 1, 1);
> +}
> +
> +const static struct clk_ops intel_grx500_pll_ops = {

Should be static const struct ...

> +       .recalc_rate = intel_grx500_pll_recalc_rate,
> +       .is_enabled = intel_grx500_pll_is_enabled,
> +};
> +
> +static struct clk
> +*intel_clk_register_pll(struct intel_clk_provider *ctx,
> +                       enum intel_pll_type type, const char *cname,
> +                       const char *const *pname, u8 num_parents,
> +                       unsigned long flags, unsigned int reg,
> +                       const struct intel_pll_rate_table *table,
> +                       unsigned int mult, unsigned int div, unsigned int frac)
> +{
> +       struct clk_init_data init;
> +       struct intel_clk_pll *pll;
> +       struct clk_hw *hw;
> +       int ret, i;
> +
> +       if (type != pll_grx500) {
> +               pr_err("%s: pll type %d not supported!\n",
> +                      __func__, type);
> +               return ERR_PTR(-EINVAL);
> +       }
> +       init.name = cname;
> +       init.ops = &intel_grx500_pll_ops;
> +       init.flags = CLK_IS_BASIC;

Don't use this flag unless you have some reason to need it.

> +       init.parent_names = pname;
> +       init.num_parents = num_parents;
> +
> +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +       if (!pll)
> +               return ERR_PTR(-ENOMEM);
> +       pll->map = ctx->map;
> +       pll->reg = reg;
> +       pll->flags = flags;
> +       pll->mult = mult;
> +       pll->div = div;
> +       pll->frac = frac;
> +       pll->hw.init = &init;
> +       if (table) {
> +               for (i = 0; table[i].rate != 0; i++)
> +                       ;
> +               pll->table_sz = i;
> +               pll->rate_table = kmemdup(table, i * sizeof(table[0]),
> +                                         GFP_KERNEL);
> +               if (!pll->rate_table) {
> +                       ret = -ENOMEM;
> +                       goto err_free_pll;
> +               }
> +       }
> +       hw = &pll->hw;
> +       ret = clk_hw_register(NULL, hw);
> +       if (ret)
> +               goto err_free_pll;
> +
> +       return hw->clk;
> +
> +err_free_pll:
> +       kfree(pll);
> +       return ERR_PTR(ret);
> +}
> +
> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
> +                            struct intel_pll_clk *list, unsigned int nr_clk)
> +{
> +       struct clk *clk;
> +       int i;
> +
> +       for (i = 0; i < nr_clk; i++, list++) {
> +               clk = intel_clk_register_pll(ctx, list->type, list->name,
> +                               list->parent_names, list->num_parents,
> +                               list->flags, list->reg, list->rate_table,
> +                               list->mult, list->div, list->frac);
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: failed to register pll: %s\n",
> +                              __func__, list->name);
> +                       continue;
> +               }
> +
> +               intel_clk_add_lookup(ctx, clk, list->id);
> +       }
> +}
> diff --git a/drivers/clk/intel/clk-cgu-pll.h b/drivers/clk/intel/clk-cgu-pll.h
> new file mode 100644
> index 000000000000..3e7cff1d5e16
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu-pll.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *  Copyright(c) 2018 Intel Corporation.
> + *  Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#ifndef __INTEL_CLK_PLL_H
> +#define __INTEL_CLK_PLL_H
> +
> +enum intel_pll_type {
> +       pll_grx500,
> +};
> +
> +struct intel_pll_rate_table {
> +       unsigned long   prate;
> +       unsigned long   rate;
> +       unsigned int    mult;
> +       unsigned int    div;
> +       unsigned int    frac;
> +};
> +
> +struct intel_clk_pll {
> +       struct clk_hw   hw;
> +       struct regmap   *map;
> +       unsigned int    reg;
> +       unsigned long   flags;
> +       unsigned int    mult;
> +       unsigned int    div;
> +       unsigned int    frac;
> +       unsigned int    table_sz;
> +       const struct intel_pll_rate_table *rate_table;
> +};
> +
> +#endif /* __INTEL_CLK_PLL_H */
> diff --git a/drivers/clk/intel/clk-cgu.c b/drivers/clk/intel/clk-cgu.c
> new file mode 100644
> index 000000000000..10cacbe0fbcd
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu.c
> @@ -0,0 +1,470 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2018 Intel Corporation.
> + *  Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +
> +#include "clk-cgu-pll.h"
> +#include "clk-cgu.h"
> +
> +#define GATE_HW_REG_STAT(reg)  (reg)
> +#define GATE_HW_REG_EN(reg)    ((reg) + 0x4)
> +#define GATE_HW_REG_DIS(reg)   ((reg) + 0x8)
> +
> +#define to_intel_clk_mux(_hw) container_of(_hw, struct intel_clk_mux, hw)
> +#define to_intel_clk_divider(_hw) \
> +               container_of(_hw, struct intel_clk_divider, hw)
> +#define to_intel_clk_gate(_hw) container_of(_hw, struct intel_clk_gate, hw)
> +
> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
> +                      u8 width, u32 set_val)
> +{
> +       u32 mask = GENMASK(width + shift, shift);
> +
> +       regmap_update_bits(map, reg, mask, set_val << shift);
> +}
> +
> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift,
> +                     u8 width)
> +{
> +       u32 val;
> +
> +       if (regmap_read(map, reg, &val)) {
> +               WARN_ONCE(1, "Failed to read clk reg: 0x%x\n", reg);
> +               return 0;
> +       }
> +       val >>= shift;
> +       val &= BIT(width) - 1;
> +
> +       return val;
> +}
> +
> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
> +                         struct clk *clk, unsigned int id)
> +{
> +       pr_debug("Add clk: %s, id: %u\n", __clk_get_name(clk), id);
> +       if (ctx->clk_data.clks && id)
> +               ctx->clk_data.clks[id] = clk;
> +}
> +
> +static struct clk
> +*intel_clk_register_fixed(struct intel_clk_provider *ctx,
> +                         struct intel_clk_branch *list)
> +{
> +       if (list->div_flags & CLOCK_FLAG_VAL_INIT)
> +               intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
> +                                 list->div_width, list->div_val);
> +
> +       return clk_register_fixed_rate(NULL, list->name, list->parent_names[0],
> +                                      list->flags, list->mux_flags);
> +}
> +
> +static u8 intel_clk_mux_get_parent(struct clk_hw *hw)
> +{
> +       struct intel_clk_mux *mux = to_intel_clk_mux(hw);
> +       u32 val;
> +
> +       val = intel_get_clk_val(mux->map, mux->reg, mux->shift, mux->width);
> +       return clk_mux_val_to_index(hw, NULL, mux->flags, val);
> +}
> +
> +static int intel_clk_mux_set_parent(struct clk_hw *hw, u8 index)
> +{
> +       struct intel_clk_mux *mux = to_intel_clk_mux(hw);
> +       u32 val;
> +
> +       val = clk_mux_index_to_val(NULL, mux->flags, index);
> +       intel_set_clk_val(mux->map, mux->reg, mux->shift, mux->width, val);
> +
> +       return 0;
> +}
> +
> +static int intel_clk_mux_determine_rate(struct clk_hw *hw,
> +                                       struct clk_rate_request *req)
> +{
> +       struct intel_clk_mux *mux = to_intel_clk_mux(hw);
> +
> +       return clk_mux_determine_rate_flags(hw, req, mux->flags);
> +}
> +
> +const static struct clk_ops intel_clk_mux_ops = {
> +       .get_parent = intel_clk_mux_get_parent,
> +       .set_parent = intel_clk_mux_set_parent,
> +       .determine_rate = intel_clk_mux_determine_rate,
> +};
> +
> +static struct clk
> +*intel_clk_register_mux(struct intel_clk_provider *ctx,
> +                       struct intel_clk_branch *list)
> +{
> +       struct clk_init_data init;
> +       struct clk_hw *hw;
> +       struct intel_clk_mux *mux;
> +       u32 reg = list->mux_off;
> +       u8 shift = list->mux_shift;
> +       u8 width = list->mux_width;
> +       unsigned long cflags = list->mux_flags;
> +       int ret;
> +
> +       mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +       if (!mux)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = list->name;
> +       init.ops = &intel_clk_mux_ops;
> +       init.flags = list->flags | CLK_IS_BASIC;
> +       init.parent_names = list->parent_names;
> +       init.num_parents = list->num_parents;
> +
> +       mux->map = ctx->map;
> +       mux->reg = reg;
> +       mux->shift = shift;
> +       mux->width = width;
> +       mux->flags = cflags;
> +       mux->hw.init = &init;
> +
> +       hw = &mux->hw;
> +       ret = clk_hw_register(NULL, hw);
> +       if (ret) {
> +               kfree(mux);
> +               return ERR_PTR(ret);
> +       }
> +
> +       if (cflags & CLOCK_FLAG_VAL_INIT)
> +               intel_set_clk_val(ctx->map, reg, shift, width, list->mux_val);
> +
> +       return hw->clk;
> +}
> +
> +static unsigned long
> +intel_clk_divider_recalc_rate(struct clk_hw *hw,
> +                             unsigned long parent_rate)
> +{
> +       struct intel_clk_divider *divider = to_intel_clk_divider(hw);
> +       unsigned int val;
> +
> +       val = intel_get_clk_val(divider->map, divider->reg,
> +                               divider->shift, divider->width);
> +       return divider_recalc_rate(hw, parent_rate, val, divider->table,
> +                                  divider->flags, divider->width);
> +}
> +
> +static long
> +intel_clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> +                            unsigned long *prate)
> +{
> +       struct intel_clk_divider *divider = to_intel_clk_divider(hw);
> +
> +       return divider_round_rate(hw, rate, prate, divider->table,
> +                                 divider->width, divider->flags);
> +}
> +
> +static int
> +intel_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
> +                          unsigned long prate)
> +{
> +       struct intel_clk_divider *divider = to_intel_clk_divider(hw);
> +       int value;
> +
> +       value = divider_get_val(rate, prate, divider->table,
> +                               divider->width, divider->flags);
> +       if (value < 0)
> +               return value;
> +
> +       intel_set_clk_val(divider->map, divider->reg,
> +                         divider->shift, divider->width, value);
> +
> +       return 0;
> +}
> +
> +const static struct clk_ops intel_clk_divider_ops = {
> +       .recalc_rate = intel_clk_divider_recalc_rate,
> +       .round_rate = intel_clk_divider_round_rate,
> +       .set_rate = intel_clk_divider_set_rate,
> +};
> +
> +static struct clk
> +*intel_clk_register_divider(struct intel_clk_provider *ctx,
> +                           struct intel_clk_branch *list)
> +{
> +       struct clk_init_data init;
> +       struct clk_hw *hw;
> +       struct intel_clk_divider *div;
> +       u32 reg = list->div_off;
> +       u8 shift = list->div_shift;
> +       u8 width = list->div_width;
> +       unsigned long cflags = list->div_flags;
> +       int ret;
> +
> +       div = kzalloc(sizeof(*div), GFP_KERNEL);
> +       if (!div)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = list->name;
> +       init.ops = &intel_clk_divider_ops;
> +       init.flags = list->flags | CLK_IS_BASIC;
> +       init.parent_names = &list->parent_names[0];
> +       init.num_parents = 1;
> +
> +       div->map = ctx->map;
> +       div->reg = reg;
> +       div->shift = shift;
> +       div->width = width;
> +       div->flags = cflags;
> +       div->table = list->div_table;
> +       div->hw.init = &init;
> +
> +       hw = &div->hw;
> +       ret = clk_hw_register(NULL, hw);
> +       if (ret) {
> +               pr_err("%s: register clk: %s failed!\n",
> +                      __func__, list->name);
> +               kfree(div);
> +               return ERR_PTR(ret);
> +       }
> +
> +       if (cflags & CLOCK_FLAG_VAL_INIT)
> +               intel_set_clk_val(ctx->map, reg, shift, width, list->div_val);
> +
> +       return hw->clk;
> +}
> +
> +static struct clk
> +*intel_clk_register_fixed_factor(struct intel_clk_provider *ctx,
> +                                struct intel_clk_branch *list)
> +{
> +       struct clk_hw *hw;
> +
> +       hw = clk_hw_register_fixed_factor(NULL, list->name,
> +                                         list->parent_names[0], list->flags,
> +                                         list->mult, list->div);
> +       if (IS_ERR(hw))
> +               return ERR_CAST(hw);
> +
> +       if (list->div_flags & CLOCK_FLAG_VAL_INIT)
> +               intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
> +                                 list->div_width, list->div_val);
> +
> +       return hw->clk;
> +}
> +
> +static int
> +intel_clk_gate_enable(struct clk_hw *hw)
> +{
> +       struct intel_clk_gate *gate = to_intel_clk_gate(hw);
> +       unsigned int reg;
> +
> +       if (gate->flags & GATE_CLK_VT) {
> +               gate->reg = 1;
> +               return 0;
> +       }
> +
> +       if (gate->flags & GATE_CLK_HW) {
> +               reg = GATE_HW_REG_EN(gate->reg);
> +       } else if (gate->flags & GATE_CLK_SW) {
> +               reg = gate->reg;
> +       } else {
> +               pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
> +                      __func__, clk_hw_get_name(hw), gate->flags);
> +               return 0;
> +       }
> +
> +       intel_set_clk_val(gate->map, reg, gate->shift, 1, 1);
> +
> +       return 0;
> +}
> +
> +static void
> +intel_clk_gate_disable(struct clk_hw *hw)
> +{
> +       struct intel_clk_gate *gate = to_intel_clk_gate(hw);
> +       unsigned int reg;
> +       unsigned int set;
> +
> +       if (gate->flags & GATE_CLK_VT) {
> +               gate->reg = 0;
> +               return;
> +       }
> +
> +       if (gate->flags & GATE_CLK_HW) {
> +               reg = GATE_HW_REG_DIS(gate->reg);
> +               set = 1;
> +       } else if (gate->flags & GATE_CLK_SW) {
> +               reg = gate->reg;
> +               set = 0;
> +       } else {
> +               pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
> +                      __func__, clk_hw_get_name(hw), gate->flags);
> +               return;
> +       }
> +
> +       intel_set_clk_val(gate->map, reg, gate->shift, 1, set);
> +}
> +
> +static int
> +intel_clk_gate_is_enabled(struct clk_hw *hw)
> +{
> +       struct intel_clk_gate *gate = to_intel_clk_gate(hw);
> +       unsigned int reg;
> +
> +       if (gate->flags & GATE_CLK_VT)
> +               return gate->reg;
> +
> +       if (gate->flags & GATE_CLK_HW) {
> +               reg = GATE_HW_REG_STAT(gate->reg);
> +       } else if (gate->flags & GATE_CLK_SW) {
> +               reg = gate->reg;
> +       } else {
> +               pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
> +                      __func__, clk_hw_get_name(hw), gate->flags);
> +               return 0;
> +       }
> +
> +       return intel_get_clk_val(gate->map, reg, gate->shift, 1);
> +}
> +
> +const static struct clk_ops intel_clk_gate_ops = {
> +       .enable = intel_clk_gate_enable,
> +       .disable = intel_clk_gate_disable,
> +       .is_enabled = intel_clk_gate_is_enabled,
> +};
> +
> +static struct clk
> +*intel_clk_register_gate(struct intel_clk_provider *ctx,
> +                        struct intel_clk_branch *list)
> +{
> +       struct clk_init_data init;

Please init the init struct with { } so that future possible additions
to the structure don't require us to hunt this silent corruption down
later.

> +       struct clk_hw *hw;
> +       struct intel_clk_gate *gate;
> +       u32 reg = list->gate_off;
> +       u8 shift = list->gate_shift;
> +       unsigned long cflags = list->gate_flags;
> +       const char *pname = list->parent_names[0];
> +       int ret;
> +
> +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +       if (!gate)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = list->name;
> +       init.ops = &intel_clk_gate_ops;
> +       init.flags = list->flags | CLK_IS_BASIC;
> +       init.parent_names = pname ? &pname : NULL;
> +       init.num_parents = pname ? 1 : 0;
> +
> +       gate->map       = ctx->map;
> +       gate->reg       = reg;
> +       gate->shift     = shift;
> +       gate->flags     = cflags;
> +       gate->hw.init   = &init;
> +
> +       hw = &gate->hw;
> +       ret = clk_hw_register(NULL, hw);
> +       if (ret) {
> +               kfree(gate);
> +               return ERR_PTR(ret);
> +       }
> +
> +       if (cflags & CLOCK_FLAG_VAL_INIT)
> +               intel_set_clk_val(ctx->map, reg, shift, 1, list->gate_val);
> +
> +       return hw->clk;
> +}
> +
> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
> +                                struct intel_clk_branch *list,
> +                                unsigned int nr_clk)
> +{
> +       struct clk *clk;
> +       unsigned int idx;
> +
> +       for (idx = 0; idx < nr_clk; idx++, list++) {
> +               switch (list->type) {
> +               case intel_clk_fixed:

Please use uppercase for enums.

> +                       clk = intel_clk_register_fixed(ctx, list);
> +                       break;
> +               case intel_clk_mux:
> +                       clk = intel_clk_register_mux(ctx, list);
> +                       break;
> +               case intel_clk_divider:
> +                       clk = intel_clk_register_divider(ctx, list);
> +                       break;
> +               case intel_clk_fixed_factor:
> +                       clk = intel_clk_register_fixed_factor(ctx, list);
> +                       break;
> +               case intel_clk_gate:
> +                       clk = intel_clk_register_gate(ctx, list);
> +                       break;
> +               default:
> +                       pr_err("%s: type: %u not supported!\n",
> +                              __func__, list->type);
> +                       return;
> +               }
> +
> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: register clk: %s, type: %u failed!\n",
> +                              __func__, list->name, list->type);
> +                       return;
> +               }
> +
> +               intel_clk_add_lookup(ctx, clk, list->id);
> +       }
> +}
> +
> +struct intel_clk_provider * __init
> +intel_clk_init(struct device_node *np, struct regmap *map, unsigned int nr_clks)
> +{
> +       struct intel_clk_provider *ctx;
> +       struct clk **clks;
> +
> +       ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +       if (!ctx)
> +               return ERR_PTR(-ENOMEM);
> +
> +       clks = kcalloc(nr_clks, sizeof(*clks), GFP_KERNEL);
> +       if (!clks) {
> +               kfree(ctx);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       memset_p((void **)clks, ERR_PTR(-ENOENT), nr_clks);
> +       ctx->map = map;
> +       ctx->clk_data.clks = clks;
> +       ctx->clk_data.clk_num = nr_clks;
> +       ctx->np = np;
> +
> +       return ctx;
> +}
> +
> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
> +                                  struct intel_osc_clk *osc,
> +                                  unsigned int nr_clks)
> +{
> +       u32 freq;
> +       struct clk *clk;
> +       int idx;
> +
> +       for (idx = 0; idx < nr_clks; idx++, osc++) {
> +               if (!osc->dt_freq ||
> +                   of_property_read_u32(ctx->np, osc->dt_freq, &freq))
> +                       freq = osc->def_rate;
> +
> +               clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);

Should come from DT itself.

> +               if (IS_ERR(clk)) {
> +                       pr_err("%s: Failed to register clock: %s\n",
> +                              __func__, osc->name);
> +                       return;
> +               }
> +
> +               intel_clk_add_lookup(ctx, clk, osc->id);
> +       }
> +}
> diff --git a/drivers/clk/intel/clk-cgu.h b/drivers/clk/intel/clk-cgu.h
> new file mode 100644
> index 000000000000..6dc4e45fc499
> --- /dev/null
> +++ b/drivers/clk/intel/clk-cgu.h
> @@ -0,0 +1,259 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + *  Copyright(c) 2018 Intel Corporation.
> + *  Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#ifndef __INTEL_CLK_H
> +#define __INTEL_CLK_H
> +
> +#define PNAME(x) static const char *const x[] __initconst
> +
> +struct intel_clk_mux {
> +       struct clk_hw   hw;
> +       struct regmap   *map;
> +       unsigned int    reg;
> +       u8              shift;
> +       u8              width;
> +       unsigned long   flags;
> +};
> +
> +struct intel_clk_divider {
> +       struct clk_hw   hw;
> +       struct regmap   *map;
> +       unsigned int    reg;
> +       u8              shift;
> +       u8              width;
> +       unsigned long   flags;
> +       const struct clk_div_table      *table;
> +};
> +
> +struct intel_clk_gate {
> +       struct clk_hw   hw;
> +       struct regmap   *map;
> +       unsigned int    reg;
> +       u8              shift;
> +       unsigned long   flags;
> +};
> +
> +enum intel_clk_type {
> +       intel_clk_fixed,
> +       intel_clk_mux,
> +       intel_clk_divider,
> +       intel_clk_fixed_factor,
> +       intel_clk_gate,
> +};
> +
> +/**
> + * struct intel_clk_provider
> + * @map: regmap type base address for register.
> + * @np: device node
> + * @clk_data: array of hw clocks and clk number.
> + */
> +struct intel_clk_provider {
> +       struct regmap           *map;
> +       struct device_node      *np;
> +       struct clk_onecell_data clk_data;

Please register clk_hw pointers instead of clk pointers with the of
provider APIs.

> +};
> +
> +/**
> + * struct intel_pll_clk
> + * @id: plaform specific id of the clock.
> + * @name: name of this pll clock.
> + * @parent_names: name of the parent clock.
> + * @num_parents: number of parents.
> + * @flags: optional flags for basic clock.
> + * @type: platform type of pll.
> + * @reg: offset of the register.
> + * @mult: init value of mulitplier.
> + * @div: init value of divider.
> + * @frac: init value of fraction.
> + * @rate_table: table of pll clock rate.

Please drop the full-stop on kernel doc one-liners like this.

> + */
> +struct intel_pll_clk {
> +       unsigned int            id;
> +       const char              *name;
> +       const char              *const *parent_names;
> +       u8                      num_parents;

Can the PLL have multiple parents?

> +       unsigned long           flags;
> +       enum intel_pll_type     type;
> +       int                     reg;
> +       unsigned int            mult;
> +       unsigned int            div;
> +       unsigned int            frac;
> +       const struct intel_pll_rate_table *rate_table;
> +};
> +
> +#define INTEL_PLL(_id, _type, _name, _pnames, _flags,  \
> +           _reg, _rtable, _mult, _div, _frac)          \
> +       {                                               \
> +               .id             = _id,                  \
> +               .type           = _type,                \
> +               .name           = _name,                \
> +               .parent_names   = _pnames,              \
> +               .num_parents    = ARRAY_SIZE(_pnames),  \
> +               .flags          = _flags,               \
> +               .reg            = _reg,                 \
> +               .rate_table     = _rtable,              \
> +               .mult           = _mult,                \
> +               .div            = _div,                 \
> +               .frac           = _frac                 \
> +       }
> +
> +/**
> + * struct intel_osc_clk
> + * @id: platform specific id of the clock.
> + * @name: name of the osc clock.
> + * @dt_freq: frequency node name in device tree.
> + * @def_rate: default rate of the osc clock.
> + * @flags: optional flags for basic clock.

There aren't flags though. I'm very confused by this kernel-doc too.
Looks like something that should be done with a fixed rate clk in DT.

> + */
> +struct intel_osc_clk {
> +       unsigned int            id;
> +       const char              *name;
> +       const char              *dt_freq;
> +       const u32               def_rate;
> +};
> +
> +#define INTEL_OSC(_id, _name, _freq, _rate)                    \
> +       {                                               \
> +               .id             = _id,                  \
> +               .name           = _name,                \
> +               .dt_freq        = _freq,                \
> +               .def_rate       = _rate,                \
> +       }
> +
> +struct intel_clk_branch {

Seems to be more like intel_clk instead of intel_clk_branch because it
does lots of stuff.

> +       unsigned int                    id;
> +       enum intel_clk_type             type;
> +       const char                      *name;
> +       const char                      *const *parent_names;
> +       u8                              num_parents;
> +       unsigned long                   flags;
> +       unsigned int                    mux_off;
> +       u8                              mux_shift;
> +       u8                              mux_width;
> +       unsigned long                   mux_flags;
> +       unsigned int                    mux_val;
> +       unsigned int                    div_off;
> +       u8                              div_shift;
> +       u8                              div_width;
> +       unsigned long                   div_flags;
> +       unsigned int                    div_val;
> +       const struct clk_div_table      *div_table;
> +       unsigned int                    gate_off;
> +       u8                              gate_shift;
> +       unsigned long                   gate_flags;
> +       unsigned int                    gate_val;
> +       unsigned int                    mult;
> +       unsigned int                    div;
> +};
> +
> +/* clock flags definition */
> +#define CLOCK_FLAG_VAL_INIT    BIT(16)
> +#define GATE_CLK_HW            BIT(17)
> +#define GATE_CLK_SW            BIT(18)
> +#define GATE_CLK_VT            BIT(19)

What does VT mean? Virtual?

> +
> +#define INTEL_MUX(_id, _name, _pname, _f, _reg,                        \
> +           _shift, _width, _cf, _v)                            \
> +       {                                                       \
> +               .id             = _id,                          \
> +               .type           = intel_clk_mux,                \
> +               .name           = _name,                        \
> +               .parent_names   = _pname,                       \
> +               .num_parents    = ARRAY_SIZE(_pname),           \
> +               .flags          = _f,                           \
> +               .mux_off        = _reg,                         \
> +               .mux_shift      = _shift,                       \
> +               .mux_width      = _width,                       \
> +               .mux_flags      = _cf,                          \
> +               .mux_val        = _v,                           \
> +       }
> +
> +#define INTEL_DIV(_id, _name, _pname, _f, _reg,                        \
> +           _shift, _width, _cf, _v, _dtable)                   \
> +       {                                                       \
> +               .id             = _id,                          \
> +               .type           = intel_clk_divider,            \
> +               .name           = _name,                        \
> +               .parent_names   = (const char *[]) { _pname },  \
> +               .num_parents    = 1,                            \
> +               .flags          = _f,                           \
> +               .div_off        = _reg,                         \
> +               .div_shift      = _shift,                       \
> +               .div_width      = _width,                       \
> +               .div_flags      = _cf,                          \
> +               .div_val        = _v,                           \
> +               .div_table      = _dtable,                      \
> +       }
> +
> +#define INTEL_GATE(_id, _name, _pname, _f, _reg,               \
> +            _shift, _cf, _v)                                   \
> +       {                                                       \
> +               .id             = _id,                          \
> +               .type           = intel_clk_gate,               \
> +               .name           = _name,                        \
> +               .parent_names   = (const char *[]) { _pname },  \
> +               .num_parents    = !_pname ? 0 : 1,              \
> +               .flags          = _f,                           \
> +               .gate_off       = _reg,                         \
> +               .gate_shift     = _shift,                       \
> +               .gate_flags     = _cf,                          \
> +               .gate_val       = _v,                           \
> +       }
> +
> +#define INTEL_FIXED(_id, _name, _pname, _f, _reg,              \
> +             _shift, _width, _cf, _freq, _v)                   \
> +       {                                                       \
> +               .id             = _id,                          \
> +               .type           = intel_clk_fixed,              \
> +               .name           = _name,                        \
> +               .parent_names   = (const char *[]) { _pname },  \
> +               .num_parents    = !_pname ? 0 : 1,              \
> +               .flags          = _f,                           \
> +               .div_off        = _reg,                         \
> +               .div_shift      = _shift,                       \
> +               .div_width      = _width,                       \
> +               .div_flags      = _cf,                          \
> +               .div_val        = _v,                           \
> +               .mux_flags      = _freq,                        \
> +       }
> +
> +#define INTEL_FIXED_FACTOR(_id, _name, _pname, _f, _reg,       \
> +              _shift, _width, _cf, _v, _m, _d)                 \
> +       {                                                       \
> +               .id             = _id,                          \
> +               .type           = intel_clk_fixed_factor,       \
> +               .name           = _name,                        \
> +               .parent_names   = (const char *[]) { _pname },  \
> +               .num_parents    = 1,                            \
> +               .flags          = _f,                           \
> +               .div_off        = _reg,                         \
> +               .div_shift      = _shift,                       \
> +               .div_width      = _width,                       \
> +               .div_flags      = _cf,                          \
> +               .div_val        = _v,                           \
> +               .mult           = _m,                           \
> +               .div            = _d,                           \
> +       }
> +
> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
> +                      u8 width, u32 set_val);
> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift, u8 width);
> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
> +                         struct clk *clk, unsigned int id);
> +void __init intel_clk_of_add_provider(struct device_node *np,
> +                                     struct intel_clk_provider *ctx);
> +struct intel_clk_provider * __init
> +intel_clk_init(struct device_node *np, struct regmap *map,
> +              unsigned int nr_clks);
> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
> +                                  struct intel_osc_clk *osc,
> +                                  unsigned int nr_clks);

Remove __init from headers files. It does nothing.

> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
> +                                struct intel_clk_branch *list,
> +                                unsigned int nr_clk);
> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
> +                            struct intel_pll_clk *list, unsigned int nr_clk);
> +#endif /* __INTEL_CLK_H */
> diff --git a/drivers/clk/intel/clk-grx500.c b/drivers/clk/intel/clk-grx500.c
> new file mode 100644
> index 000000000000..5c2546f82579
> --- /dev/null
> +++ b/drivers/clk/intel/clk-grx500.c
> @@ -0,0 +1,168 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Copyright (C) 2018 Intel Corporation.
> + *  Zhu YiXin <Yixin.zhu@intel.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/spinlock.h>

Used?

> +#include <dt-bindings/clock/intel,grx500-clk.h>
> +
> +#include "clk-cgu-pll.h"
> +#include "clk-cgu.h"
> +
> +#define PLL_DIV_WIDTH          4
> +
> +/* Gate1 clock shift */
> +#define G_VCODEC_SHIFT         2
> +#define G_DMA0_SHIFT           5
> +#define G_USB0_SHIFT           6
> +#define G_SPI1_SHIFT           7
> +#define G_SPI0_SHIFT           8
> +#define G_CBM_SHIFT            9
> +#define G_EBU_SHIFT            10
> +#define G_SSO_SHIFT            11
> +#define G_GPTC0_SHIFT          12
> +#define G_GPTC1_SHIFT          13
> +#define G_GPTC2_SHIFT          14
> +#define G_UART_SHIFT           17
> +#define G_CPYTO_SHIFT          20
> +#define G_SECPT_SHIFT          21
> +#define G_TOE_SHIFT            22
> +#define G_MPE_SHIFT            23
> +#define G_TDM_SHIFT            25
> +#define G_PAE_SHIFT            26
> +#define G_USB1_SHIFT           27
> +#define G_SWITCH_SHIFT         28
> +
> +/* Gate2 clock shift */
> +#define G_PCIE0_SHIFT          1
> +#define G_PCIE1_SHIFT          17
> +#define G_PCIE2_SHIFT          25
> +
> +/* Register definition */
> +#define GRX500_PLL0A_CFG0      0x0004
> +#define GRX500_PLL0A_CFG1      0x0008
> +#define GRX500_PLL0B_CFG0      0x0034
> +#define GRX500_PLL0B_CFG1      0x0038
> +#define GRX500_LCPLL_CFG0      0x0094
> +#define GRX500_LCPLL_CFG1      0x0098
> +#define GRX500_IF_CLK          0x00c4
> +#define GRX500_CLK_GSR1                0x0120
> +#define GRX500_CLK_GSR2                0x0130
> +
> +static const struct clk_div_table pll_div[] = {
> +       {1,     2},

Please write it like

	  { 1,    2 },

instead.

> +       {2,     3},
> +       {3,     4},
> +       {4,     5},
> +       {5,     6},
> +       {6,     8},
> +       {7,     10},
> +       {8,     12},
> +       {9,     16},
> +       {10,    20},
> +       {11,    24},
> +       {12,    32},
> +       {13,    40},
> +       {14,    48},
> +       {15,    64}
> +};
> +
> +enum grx500_plls {
> +       pll0a, pll0b, pll3,
> +};

What's the point of the enum?

> +
> +PNAME(pll_p)   = { "osc" };
> +PNAME(cpu_p)   = { "cpu0", "cpu1" };
> +
> +static struct intel_osc_clk grx500_osc_clks[] __initdata = {
> +       INTEL_OSC(CLK_OSC, "osc", "intel,osc-frequency", 40000000),
> +};
> +
> +static struct intel_pll_clk grx500_pll_clks[] __initdata = {
> +       [pll0a] = INTEL_PLL(CLK_PLL0A, pll_grx500, "pll0a",
> +                     pll_p, 0, GRX500_PLL0A_CFG0, NULL, 0, 0, 0),
> +       [pll0b] = INTEL_PLL(CLK_PLL0B, pll_grx500, "pll0b",
> +                     pll_p, 0, GRX500_PLL0B_CFG0, NULL, 0, 0, 0),
> +       [pll3] = INTEL_PLL(CLK_PLL3, pll_grx500, "pll3",
> +                    pll_p, 0, GRX500_LCPLL_CFG0, NULL, 0, 0, 0),
> +};
> +
> +static struct intel_clk_branch grx500_branch_clks[] __initdata = {
> +       INTEL_DIV(CLK_CBM, "cbm", "pll0a", 0, GRX500_PLL0A_CFG1,
> +                 0, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_NGI, "ngi", "pll0a", 0, GRX500_PLL0A_CFG1,
> +                 4, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_SSX4, "ssx4", "pll0a", 0, GRX500_PLL0A_CFG1,
> +                 8, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_CPU0, "cpu0", "pll0a", 0, GRX500_PLL0A_CFG1,
> +                 12, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_PAE, "pae", "pll0b", 0, GRX500_PLL0B_CFG1,
> +                 0, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_GSWIP, "gswip", "pll0b", 0, GRX500_PLL0B_CFG1,
> +                 4, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_DDR, "ddr", "pll0b", 0, GRX500_PLL0B_CFG1,
> +                 8, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_DIV(CLK_CPU1, "cpu1", "pll0b", 0, GRX500_PLL0B_CFG1,
> +                 12, PLL_DIV_WIDTH, 0, 0, pll_div),
> +       INTEL_MUX(CLK_CPU, "cpu", cpu_p, CLK_SET_RATE_PARENT,
> +                 GRX500_PLL0A_CFG1, 29, 1, 0, 0),
> +       INTEL_GATE(GCLK_DMA0, "g_dma0", NULL, 0, GRX500_CLK_GSR1,
> +                  G_DMA0_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_USB0, "g_usb0", NULL, 0, GRX500_CLK_GSR1,
> +                  G_USB0_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_GPTC0, "g_gptc0", NULL, 0, GRX500_CLK_GSR1,
> +                  G_GPTC0_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_GPTC1, "g_gptc1", NULL, 0, GRX500_CLK_GSR1,
> +                  G_GPTC1_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_GPTC2, "g_gptc2", NULL, 0, GRX500_CLK_GSR1,
> +                  G_GPTC2_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_UART, "g_uart", NULL, 0, GRX500_CLK_GSR1,
> +                  G_UART_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_PCIE0, "g_pcie0", NULL, 0, GRX500_CLK_GSR2,
> +                  G_PCIE0_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_PCIE1, "g_pcie1", NULL, 0, GRX500_CLK_GSR2,
> +                  G_PCIE1_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_PCIE2, "g_pcie2", NULL, 0, GRX500_CLK_GSR2,
> +                  G_PCIE2_SHIFT, GATE_CLK_HW, 0),
> +       INTEL_GATE(GCLK_I2C, "g_i2c", NULL, 0, 0, 0, GATE_CLK_VT, 0),
> +       INTEL_FIXED(CLK_VOICE, "voice", NULL, 0, GRX500_IF_CLK, 14, 2,
> +                   CLOCK_FLAG_VAL_INIT, 8192000, 2),
> +       INTEL_FIXED_FACTOR(CLK_DDRPHY, "ddrphy", "ddr", 0, 0, 0,
> +                          0, 0, 0, 2, 1),
> +       INTEL_FIXED_FACTOR(CLK_PCIE, "pcie", "pll3", 0, 0, 0,
> +                          0, 0, 0, 1, 40),
> +};
> +
> +static void __init grx500_clk_init(struct device_node *np)
> +{
> +       struct intel_clk_provider *ctx;
> +       struct regmap *map;
> +
> +       map = syscon_node_to_regmap(np);
> +       if (IS_ERR(map))
> +               return;
> +
> +       ctx = intel_clk_init(np, map, CLK_NR_CLKS);
> +       if (IS_ERR(ctx)) {
> +               regmap_exit(map);
> +               return;
> +       }
> +
> +       intel_clk_register_osc(ctx, grx500_osc_clks,
> +                              ARRAY_SIZE(grx500_osc_clks));
> +       intel_clk_register_plls(ctx, grx500_pll_clks,
> +                               ARRAY_SIZE(grx500_pll_clks));
> +       intel_clk_register_branches(ctx, grx500_branch_clks,
> +                                   ARRAY_SIZE(grx500_branch_clks));
> +       of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
> +
> +       pr_debug("%s clk init done!\n", __func__);

Yay!!!

> +}
> +
> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);

Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu, Yi Xin Aug. 8, 2018, 8:52 a.m. UTC | #4
On 8/8/2018 1:50 PM, Stephen Boyd wrote:
> Quoting Songjun Wu (2018-08-02 20:02:21)
>> From: Yixin Zhu <yixin.zhu@linux.intel.com>
>>
>> This driver provides PLL clock registration as well as various clock
>> branches, e.g. MUX clock, gate clock, divider clock and so on.
>>
>> PLLs that provide clock to DDR, CPU and peripherals are shown below:
>>
>>                   +---------+
>>              |--->| LCPLL3 0|--PCIe clk-->
>>     XO       |    +---------+
>> +-----------|
>>              |    +---------+
>>              |    |        3|--PAE clk-->
>>              |--->| PLL0B  2|--GSWIP clk-->
>>              |    |        1|--DDR clk-->DDR PHY clk-->
>>              |    |        0|--CPU1 clk--+   +-----+
>>              |    +---------+            |--->0    |
>>              |                               | MUX |--CPU clk-->
>>              |    +---------+            |--->1    |
>>              |    |        0|--CPU0 clk--+   +-----+
>>              |--->| PLLOA  1|--SSX4 clk-->
>>                   |        2|--NGI clk-->
>>                   |        3|--CBM clk-->
>>                   +---------+
> Thanks for the picture!
Thanks for the review.

>
>> Signed-off-by: Yixin Zhu <yixin.zhu@linux.intel.com>
>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index 0bb25dd009d1..d929ca4607cf 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI)                       += hisilicon/
>>   obj-y                                  += imgtec/
>>   obj-$(CONFIG_ARCH_MXC)                 += imx/
>>   obj-$(CONFIG_MACH_INGENIC)             += ingenic/
>> +ifeq ($(CONFIG_COMMON_CLK), y)
>> +obj-y                                                  +=intel/
>> +endif
> Why not obj-$(CONFIG_INTEL_CCF) or something like that?
Will use obj-$(CONFIG_INTEL_CCF)

>>   obj-$(CONFIG_ARCH_KEYSTONE)            += keystone/
>>   obj-$(CONFIG_MACH_LOONGSON32)          += loongson1/
>>   obj-y                                  += mediatek/
>> diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
>> new file mode 100644
>> index 000000000000..c7d3fb1721fa
>> --- /dev/null
>> +++ b/drivers/clk/intel/Kconfig
>> @@ -0,0 +1,20 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +config INTEL_CGU_CLK
>> +       depends on COMMON_CLK
>> +       depends on INTEL_MIPS || COMPILE_TEST
>> +       select MFD_SYSCON
>> +       bool "Intel clock controller support"
>> +       help
>> +         This driver support Intel CGU (Clock Generation Unit).
> Is it really called a clock generation unit? Or that's just copied from
> sunxi driver?
Yes,  It's called clock generation unit(CGU) in our HW chip spec.

>> +
>> +choice
>> +       prompt "SoC platform selection"
>> +       depends on INTEL_CGU_CLK
>> +       default INTEL_GRX500_CGU_CLK
>> +
>> +config INTEL_GRX500_CGU_CLK
>> +       bool "GRX500 CLK"
>> +       help
>> +         Clock driver of GRX500 platform.
>> +
>> +endchoice
>> diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
>> new file mode 100644
>> index 000000000000..16a0138e52c2
>> --- /dev/null
>> +++ b/drivers/clk/intel/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Makefile for intel specific clk
>> +
>> +obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
>> +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
>> +       obj-y += clk-grx500.o
>> +endif
>> diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
>> new file mode 100644
>> index 000000000000..20759bc27e95
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu-pll.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2018 Intel Corporation.
>> + *  Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#include <linux/clk.h>
> Is this include used?
Not used. Will remove it.

>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk-cgu-pll.h"
>> +#include "clk-cgu.h"
>> +
>> +#define to_intel_clk_pll(_hw)  container_of(_hw, struct intel_clk_pll, hw)
>> +
>> +/*
>> + * Calculate formula:
>> + * rate = (prate * mult + (prate * frac) / frac_div) / div
>> + */
>> +static unsigned long
>> +intel_pll_calc_rate(unsigned long prate, unsigned int mult,
>> +                   unsigned int div, unsigned int frac,
>> +                   unsigned int frac_div)
>> +{
>> +       u64 crate, frate, rate64;
>> +
>> +       rate64 = prate;
>> +       crate = rate64 * mult;
>> +
>> +       if (frac) {
>> +               frate = rate64 * frac;
>> +               do_div(frate, frac_div);
>> +               crate += frate;
>> +       }
>> +       do_div(crate, div);
>> +
>> +       return (unsigned long)crate;
>> +}
>> +
>> +static void
>> +grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
>> +                     unsigned int *frac)
>> +{
>> +       *mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
>> +       *frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
>> +}
>> +
>> +static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
>> +{
>> +       unsigned int val;
>> +
>> +       return regmap_read_poll_timeout(pll->map, pll->reg, val,
>> +                                       val & BIT(bit_idx), 10, 1000);
>> +}
>> +
>> +static unsigned long
>> +intel_grx500_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
>> +{
>> +       struct intel_clk_pll *pll = to_intel_clk_pll(hw);
>> +       unsigned int mult, frac;
>> +
>> +       grx500_pll_get_params(pll, &mult, &frac);
>> +
>> +       return intel_pll_calc_rate(prate, mult, 1, frac, BIT(20));
>> +}
>> +
>> +static int intel_grx500_pll_is_enabled(struct clk_hw *hw)
>> +{
>> +       struct intel_clk_pll *pll = to_intel_clk_pll(hw);
>> +
>> +       if (intel_wait_pll_lock(pll, 1)) {
>> +               pr_err("%s: pll: %s is not locked!\n",
>> +                      __func__, clk_hw_get_name(hw));
>> +               return 0;
>> +       }
>> +
>> +       return intel_get_clk_val(pll->map, pll->reg, 1, 1);
>> +}
>> +
>> +const static struct clk_ops intel_grx500_pll_ops = {
> Should be static const struct ...
Will update it.

>> +       .recalc_rate = intel_grx500_pll_recalc_rate,
>> +       .is_enabled = intel_grx500_pll_is_enabled,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_pll(struct intel_clk_provider *ctx,
>> +                       enum intel_pll_type type, const char *cname,
>> +                       const char *const *pname, u8 num_parents,
>> +                       unsigned long flags, unsigned int reg,
>> +                       const struct intel_pll_rate_table *table,
>> +                       unsigned int mult, unsigned int div, unsigned int frac)
>> +{
>> +       struct clk_init_data init;
>> +       struct intel_clk_pll *pll;
>> +       struct clk_hw *hw;
>> +       int ret, i;
>> +
>> +       if (type != pll_grx500) {
>> +               pr_err("%s: pll type %d not supported!\n",
>> +                      __func__, type);
>> +               return ERR_PTR(-EINVAL);
>> +       }
>> +       init.name = cname;
>> +       init.ops = &intel_grx500_pll_ops;
>> +       init.flags = CLK_IS_BASIC;
> Don't use this flag unless you have some reason to need it.
Will remove it.

>> +       init.parent_names = pname;
>> +       init.num_parents = num_parents;
>> +
>> +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
>> +       if (!pll)
>> +               return ERR_PTR(-ENOMEM);
>> +       pll->map = ctx->map;
>> +       pll->reg = reg;
>> +       pll->flags = flags;
>> +       pll->mult = mult;
>> +       pll->div = div;
>> +       pll->frac = frac;
>> +       pll->hw.init = &init;
>> +       if (table) {
>> +               for (i = 0; table[i].rate != 0; i++)
>> +                       ;
>> +               pll->table_sz = i;
>> +               pll->rate_table = kmemdup(table, i * sizeof(table[0]),
>> +                                         GFP_KERNEL);
>> +               if (!pll->rate_table) {
>> +                       ret = -ENOMEM;
>> +                       goto err_free_pll;
>> +               }
>> +       }
>> +       hw = &pll->hw;
>> +       ret = clk_hw_register(NULL, hw);
>> +       if (ret)
>> +               goto err_free_pll;
>> +
>> +       return hw->clk;
>> +
>> +err_free_pll:
>> +       kfree(pll);
>> +       return ERR_PTR(ret);
>> +}
>> +
>> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
>> +                            struct intel_pll_clk *list, unsigned int nr_clk)
>> +{
>> +       struct clk *clk;
>> +       int i;
>> +
>> +       for (i = 0; i < nr_clk; i++, list++) {
>> +               clk = intel_clk_register_pll(ctx, list->type, list->name,
>> +                               list->parent_names, list->num_parents,
>> +                               list->flags, list->reg, list->rate_table,
>> +                               list->mult, list->div, list->frac);
>> +               if (IS_ERR(clk)) {
>> +                       pr_err("%s: failed to register pll: %s\n",
>> +                              __func__, list->name);
>> +                       continue;
>> +               }
>> +
>> +               intel_clk_add_lookup(ctx, clk, list->id);
>> +       }
>> +}
>> diff --git a/drivers/clk/intel/clk-cgu-pll.h b/drivers/clk/intel/clk-cgu-pll.h
>> new file mode 100644
>> index 000000000000..3e7cff1d5e16
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu-pll.h
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + *  Copyright(c) 2018 Intel Corporation.
>> + *  Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#ifndef __INTEL_CLK_PLL_H
>> +#define __INTEL_CLK_PLL_H
>> +
>> +enum intel_pll_type {
>> +       pll_grx500,
>> +};
>> +
>> +struct intel_pll_rate_table {
>> +       unsigned long   prate;
>> +       unsigned long   rate;
>> +       unsigned int    mult;
>> +       unsigned int    div;
>> +       unsigned int    frac;
>> +};
>> +
>> +struct intel_clk_pll {
>> +       struct clk_hw   hw;
>> +       struct regmap   *map;
>> +       unsigned int    reg;
>> +       unsigned long   flags;
>> +       unsigned int    mult;
>> +       unsigned int    div;
>> +       unsigned int    frac;
>> +       unsigned int    table_sz;
>> +       const struct intel_pll_rate_table *rate_table;
>> +};
>> +
>> +#endif /* __INTEL_CLK_PLL_H */
>> diff --git a/drivers/clk/intel/clk-cgu.c b/drivers/clk/intel/clk-cgu.c
>> new file mode 100644
>> index 000000000000..10cacbe0fbcd
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu.c
>> @@ -0,0 +1,470 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2018 Intel Corporation.
>> + *  Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/regmap.h>
>> +#include <linux/slab.h>
>> +
>> +#include "clk-cgu-pll.h"
>> +#include "clk-cgu.h"
>> +
>> +#define GATE_HW_REG_STAT(reg)  (reg)
>> +#define GATE_HW_REG_EN(reg)    ((reg) + 0x4)
>> +#define GATE_HW_REG_DIS(reg)   ((reg) + 0x8)
>> +
>> +#define to_intel_clk_mux(_hw) container_of(_hw, struct intel_clk_mux, hw)
>> +#define to_intel_clk_divider(_hw) \
>> +               container_of(_hw, struct intel_clk_divider, hw)
>> +#define to_intel_clk_gate(_hw) container_of(_hw, struct intel_clk_gate, hw)
>> +
>> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
>> +                      u8 width, u32 set_val)
>> +{
>> +       u32 mask = GENMASK(width + shift, shift);
>> +
>> +       regmap_update_bits(map, reg, mask, set_val << shift);
>> +}
>> +
>> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift,
>> +                     u8 width)
>> +{
>> +       u32 val;
>> +
>> +       if (regmap_read(map, reg, &val)) {
>> +               WARN_ONCE(1, "Failed to read clk reg: 0x%x\n", reg);
>> +               return 0;
>> +       }
>> +       val >>= shift;
>> +       val &= BIT(width) - 1;
>> +
>> +       return val;
>> +}
>> +
>> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
>> +                         struct clk *clk, unsigned int id)
>> +{
>> +       pr_debug("Add clk: %s, id: %u\n", __clk_get_name(clk), id);
>> +       if (ctx->clk_data.clks && id)
>> +               ctx->clk_data.clks[id] = clk;
>> +}
>> +
>> +static struct clk
>> +*intel_clk_register_fixed(struct intel_clk_provider *ctx,
>> +                         struct intel_clk_branch *list)
>> +{
>> +       if (list->div_flags & CLOCK_FLAG_VAL_INIT)
>> +               intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
>> +                                 list->div_width, list->div_val);
>> +
>> +       return clk_register_fixed_rate(NULL, list->name, list->parent_names[0],
>> +                                      list->flags, list->mux_flags);
>> +}
>> +
>> +static u8 intel_clk_mux_get_parent(struct clk_hw *hw)
>> +{
>> +       struct intel_clk_mux *mux = to_intel_clk_mux(hw);
>> +       u32 val;
>> +
>> +       val = intel_get_clk_val(mux->map, mux->reg, mux->shift, mux->width);
>> +       return clk_mux_val_to_index(hw, NULL, mux->flags, val);
>> +}
>> +
>> +static int intel_clk_mux_set_parent(struct clk_hw *hw, u8 index)
>> +{
>> +       struct intel_clk_mux *mux = to_intel_clk_mux(hw);
>> +       u32 val;
>> +
>> +       val = clk_mux_index_to_val(NULL, mux->flags, index);
>> +       intel_set_clk_val(mux->map, mux->reg, mux->shift, mux->width, val);
>> +
>> +       return 0;
>> +}
>> +
>> +static int intel_clk_mux_determine_rate(struct clk_hw *hw,
>> +                                       struct clk_rate_request *req)
>> +{
>> +       struct intel_clk_mux *mux = to_intel_clk_mux(hw);
>> +
>> +       return clk_mux_determine_rate_flags(hw, req, mux->flags);
>> +}
>> +
>> +const static struct clk_ops intel_clk_mux_ops = {
>> +       .get_parent = intel_clk_mux_get_parent,
>> +       .set_parent = intel_clk_mux_set_parent,
>> +       .determine_rate = intel_clk_mux_determine_rate,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_mux(struct intel_clk_provider *ctx,
>> +                       struct intel_clk_branch *list)
>> +{
>> +       struct clk_init_data init;
>> +       struct clk_hw *hw;
>> +       struct intel_clk_mux *mux;
>> +       u32 reg = list->mux_off;
>> +       u8 shift = list->mux_shift;
>> +       u8 width = list->mux_width;
>> +       unsigned long cflags = list->mux_flags;
>> +       int ret;
>> +
>> +       mux = kzalloc(sizeof(*mux), GFP_KERNEL);
>> +       if (!mux)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       init.name = list->name;
>> +       init.ops = &intel_clk_mux_ops;
>> +       init.flags = list->flags | CLK_IS_BASIC;
>> +       init.parent_names = list->parent_names;
>> +       init.num_parents = list->num_parents;
>> +
>> +       mux->map = ctx->map;
>> +       mux->reg = reg;
>> +       mux->shift = shift;
>> +       mux->width = width;
>> +       mux->flags = cflags;
>> +       mux->hw.init = &init;
>> +
>> +       hw = &mux->hw;
>> +       ret = clk_hw_register(NULL, hw);
>> +       if (ret) {
>> +               kfree(mux);
>> +               return ERR_PTR(ret);
>> +       }
>> +
>> +       if (cflags & CLOCK_FLAG_VAL_INIT)
>> +               intel_set_clk_val(ctx->map, reg, shift, width, list->mux_val);
>> +
>> +       return hw->clk;
>> +}
>> +
>> +static unsigned long
>> +intel_clk_divider_recalc_rate(struct clk_hw *hw,
>> +                             unsigned long parent_rate)
>> +{
>> +       struct intel_clk_divider *divider = to_intel_clk_divider(hw);
>> +       unsigned int val;
>> +
>> +       val = intel_get_clk_val(divider->map, divider->reg,
>> +                               divider->shift, divider->width);
>> +       return divider_recalc_rate(hw, parent_rate, val, divider->table,
>> +                                  divider->flags, divider->width);
>> +}
>> +
>> +static long
>> +intel_clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
>> +                            unsigned long *prate)
>> +{
>> +       struct intel_clk_divider *divider = to_intel_clk_divider(hw);
>> +
>> +       return divider_round_rate(hw, rate, prate, divider->table,
>> +                                 divider->width, divider->flags);
>> +}
>> +
>> +static int
>> +intel_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>> +                          unsigned long prate)
>> +{
>> +       struct intel_clk_divider *divider = to_intel_clk_divider(hw);
>> +       int value;
>> +
>> +       value = divider_get_val(rate, prate, divider->table,
>> +                               divider->width, divider->flags);
>> +       if (value < 0)
>> +               return value;
>> +
>> +       intel_set_clk_val(divider->map, divider->reg,
>> +                         divider->shift, divider->width, value);
>> +
>> +       return 0;
>> +}
>> +
>> +const static struct clk_ops intel_clk_divider_ops = {
>> +       .recalc_rate = intel_clk_divider_recalc_rate,
>> +       .round_rate = intel_clk_divider_round_rate,
>> +       .set_rate = intel_clk_divider_set_rate,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_divider(struct intel_clk_provider *ctx,
>> +                           struct intel_clk_branch *list)
>> +{
>> +       struct clk_init_data init;
>> +       struct clk_hw *hw;
>> +       struct intel_clk_divider *div;
>> +       u32 reg = list->div_off;
>> +       u8 shift = list->div_shift;
>> +       u8 width = list->div_width;
>> +       unsigned long cflags = list->div_flags;
>> +       int ret;
>> +
>> +       div = kzalloc(sizeof(*div), GFP_KERNEL);
>> +       if (!div)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       init.name = list->name;
>> +       init.ops = &intel_clk_divider_ops;
>> +       init.flags = list->flags | CLK_IS_BASIC;
>> +       init.parent_names = &list->parent_names[0];
>> +       init.num_parents = 1;
>> +
>> +       div->map = ctx->map;
>> +       div->reg = reg;
>> +       div->shift = shift;
>> +       div->width = width;
>> +       div->flags = cflags;
>> +       div->table = list->div_table;
>> +       div->hw.init = &init;
>> +
>> +       hw = &div->hw;
>> +       ret = clk_hw_register(NULL, hw);
>> +       if (ret) {
>> +               pr_err("%s: register clk: %s failed!\n",
>> +                      __func__, list->name);
>> +               kfree(div);
>> +               return ERR_PTR(ret);
>> +       }
>> +
>> +       if (cflags & CLOCK_FLAG_VAL_INIT)
>> +               intel_set_clk_val(ctx->map, reg, shift, width, list->div_val);
>> +
>> +       return hw->clk;
>> +}
>> +
>> +static struct clk
>> +*intel_clk_register_fixed_factor(struct intel_clk_provider *ctx,
>> +                                struct intel_clk_branch *list)
>> +{
>> +       struct clk_hw *hw;
>> +
>> +       hw = clk_hw_register_fixed_factor(NULL, list->name,
>> +                                         list->parent_names[0], list->flags,
>> +                                         list->mult, list->div);
>> +       if (IS_ERR(hw))
>> +               return ERR_CAST(hw);
>> +
>> +       if (list->div_flags & CLOCK_FLAG_VAL_INIT)
>> +               intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
>> +                                 list->div_width, list->div_val);
>> +
>> +       return hw->clk;
>> +}
>> +
>> +static int
>> +intel_clk_gate_enable(struct clk_hw *hw)
>> +{
>> +       struct intel_clk_gate *gate = to_intel_clk_gate(hw);
>> +       unsigned int reg;
>> +
>> +       if (gate->flags & GATE_CLK_VT) {
>> +               gate->reg = 1;
>> +               return 0;
>> +       }
>> +
>> +       if (gate->flags & GATE_CLK_HW) {
>> +               reg = GATE_HW_REG_EN(gate->reg);
>> +       } else if (gate->flags & GATE_CLK_SW) {
>> +               reg = gate->reg;
>> +       } else {
>> +               pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
>> +                      __func__, clk_hw_get_name(hw), gate->flags);
>> +               return 0;
>> +       }
>> +
>> +       intel_set_clk_val(gate->map, reg, gate->shift, 1, 1);
>> +
>> +       return 0;
>> +}
>> +
>> +static void
>> +intel_clk_gate_disable(struct clk_hw *hw)
>> +{
>> +       struct intel_clk_gate *gate = to_intel_clk_gate(hw);
>> +       unsigned int reg;
>> +       unsigned int set;
>> +
>> +       if (gate->flags & GATE_CLK_VT) {
>> +               gate->reg = 0;
>> +               return;
>> +       }
>> +
>> +       if (gate->flags & GATE_CLK_HW) {
>> +               reg = GATE_HW_REG_DIS(gate->reg);
>> +               set = 1;
>> +       } else if (gate->flags & GATE_CLK_SW) {
>> +               reg = gate->reg;
>> +               set = 0;
>> +       } else {
>> +               pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
>> +                      __func__, clk_hw_get_name(hw), gate->flags);
>> +               return;
>> +       }
>> +
>> +       intel_set_clk_val(gate->map, reg, gate->shift, 1, set);
>> +}
>> +
>> +static int
>> +intel_clk_gate_is_enabled(struct clk_hw *hw)
>> +{
>> +       struct intel_clk_gate *gate = to_intel_clk_gate(hw);
>> +       unsigned int reg;
>> +
>> +       if (gate->flags & GATE_CLK_VT)
>> +               return gate->reg;
>> +
>> +       if (gate->flags & GATE_CLK_HW) {
>> +               reg = GATE_HW_REG_STAT(gate->reg);
>> +       } else if (gate->flags & GATE_CLK_SW) {
>> +               reg = gate->reg;
>> +       } else {
>> +               pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
>> +                      __func__, clk_hw_get_name(hw), gate->flags);
>> +               return 0;
>> +       }
>> +
>> +       return intel_get_clk_val(gate->map, reg, gate->shift, 1);
>> +}
>> +
>> +const static struct clk_ops intel_clk_gate_ops = {
>> +       .enable = intel_clk_gate_enable,
>> +       .disable = intel_clk_gate_disable,
>> +       .is_enabled = intel_clk_gate_is_enabled,
>> +};
>> +
>> +static struct clk
>> +*intel_clk_register_gate(struct intel_clk_provider *ctx,
>> +                        struct intel_clk_branch *list)
>> +{
>> +       struct clk_init_data init;
> Please init the init struct with { } so that future possible additions
> to the structure don't require us to hunt this silent corruption down
> later.
Will update it.

>> +       struct clk_hw *hw;
>> +       struct intel_clk_gate *gate;
>> +       u32 reg = list->gate_off;
>> +       u8 shift = list->gate_shift;
>> +       unsigned long cflags = list->gate_flags;
>> +       const char *pname = list->parent_names[0];
>> +       int ret;
>> +
>> +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> +       if (!gate)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       init.name = list->name;
>> +       init.ops = &intel_clk_gate_ops;
>> +       init.flags = list->flags | CLK_IS_BASIC;
>> +       init.parent_names = pname ? &pname : NULL;
>> +       init.num_parents = pname ? 1 : 0;
>> +
>> +       gate->map       = ctx->map;
>> +       gate->reg       = reg;
>> +       gate->shift     = shift;
>> +       gate->flags     = cflags;
>> +       gate->hw.init   = &init;
>> +
>> +       hw = &gate->hw;
>> +       ret = clk_hw_register(NULL, hw);
>> +       if (ret) {
>> +               kfree(gate);
>> +               return ERR_PTR(ret);
>> +       }
>> +
>> +       if (cflags & CLOCK_FLAG_VAL_INIT)
>> +               intel_set_clk_val(ctx->map, reg, shift, 1, list->gate_val);
>> +
>> +       return hw->clk;
>> +}
>> +
>> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
>> +                                struct intel_clk_branch *list,
>> +                                unsigned int nr_clk)
>> +{
>> +       struct clk *clk;
>> +       unsigned int idx;
>> +
>> +       for (idx = 0; idx < nr_clk; idx++, list++) {
>> +               switch (list->type) {
>> +               case intel_clk_fixed:
> Please use uppercase for enums.
Will update.

>
>> +                       clk = intel_clk_register_fixed(ctx, list);
>> +                       break;
>> +               case intel_clk_mux:
>> +                       clk = intel_clk_register_mux(ctx, list);
>> +                       break;
>> +               case intel_clk_divider:
>> +                       clk = intel_clk_register_divider(ctx, list);
>> +                       break;
>> +               case intel_clk_fixed_factor:
>> +                       clk = intel_clk_register_fixed_factor(ctx, list);
>> +                       break;
>> +               case intel_clk_gate:
>> +                       clk = intel_clk_register_gate(ctx, list);
>> +                       break;
>> +               default:
>> +                       pr_err("%s: type: %u not supported!\n",
>> +                              __func__, list->type);
>> +                       return;
>> +               }
>> +
>> +               if (IS_ERR(clk)) {
>> +                       pr_err("%s: register clk: %s, type: %u failed!\n",
>> +                              __func__, list->name, list->type);
>> +                       return;
>> +               }
>> +
>> +               intel_clk_add_lookup(ctx, clk, list->id);
>> +       }
>> +}
>> +
>> +struct intel_clk_provider * __init
>> +intel_clk_init(struct device_node *np, struct regmap *map, unsigned int nr_clks)
>> +{
>> +       struct intel_clk_provider *ctx;
>> +       struct clk **clks;
>> +
>> +       ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>> +       if (!ctx)
>> +               return ERR_PTR(-ENOMEM);
>> +
>> +       clks = kcalloc(nr_clks, sizeof(*clks), GFP_KERNEL);
>> +       if (!clks) {
>> +               kfree(ctx);
>> +               return ERR_PTR(-ENOMEM);
>> +       }
>> +
>> +       memset_p((void **)clks, ERR_PTR(-ENOENT), nr_clks);
>> +       ctx->map = map;
>> +       ctx->clk_data.clks = clks;
>> +       ctx->clk_data.clk_num = nr_clks;
>> +       ctx->np = np;
>> +
>> +       return ctx;
>> +}
>> +
>> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
>> +                                  struct intel_osc_clk *osc,
>> +                                  unsigned int nr_clks)
>> +{
>> +       u32 freq;
>> +       struct clk *clk;
>> +       int idx;
>> +
>> +       for (idx = 0; idx < nr_clks; idx++, osc++) {
>> +               if (!osc->dt_freq ||
>> +                   of_property_read_u32(ctx->np, osc->dt_freq, &freq))
>> +                       freq = osc->def_rate;
>> +
>> +               clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
> Should come from DT itself.
Yes. It can be defined as fixed-clock node in device tree.
Do you mean it should be defined in device tree and driver reference it 
via device tree?

>> +               iS_ERR(clk)) {
>> +                       pr_err("%s: Failed to register clock: %s\n",
>> +                              __func__, osc->name);
>> +                       return;
>> +               }
>> +
>> +               intel_clk_add_lookup(ctx, clk, osc->id);
>> +       }
>> +}
>> diff --git a/drivers/clk/intel/clk-cgu.h b/drivers/clk/intel/clk-cgu.h
>> new file mode 100644
>> index 000000000000..6dc4e45fc499
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-cgu.h
>> @@ -0,0 +1,259 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + *  Copyright(c) 2018 Intel Corporation.
>> + *  Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#ifndef __INTEL_CLK_H
>> +#define __INTEL_CLK_H
>> +
>> +#define PNAME(x) static const char *const x[] __initconst
>> +
>> +struct intel_clk_mux {
>> +       struct clk_hw   hw;
>> +       struct regmap   *map;
>> +       unsigned int    reg;
>> +       u8              shift;
>> +       u8              width;
>> +       unsigned long   flags;
>> +};
>> +
>> +struct intel_clk_divider {
>> +       struct clk_hw   hw;
>> +       struct regmap   *map;
>> +       unsigned int    reg;
>> +       u8              shift;
>> +       u8              width;
>> +       unsigned long   flags;
>> +       const struct clk_div_table      *table;
>> +};
>> +
>> +struct intel_clk_gate {
>> +       struct clk_hw   hw;
>> +       struct regmap   *map;
>> +       unsigned int    reg;
>> +       u8              shift;
>> +       unsigned long   flags;
>> +};
>> +
>> +enum intel_clk_type {
>> +       intel_clk_fixed,
>> +       intel_clk_mux,
>> +       intel_clk_divider,
>> +       intel_clk_fixed_factor,
>> +       intel_clk_gate,
>> +};
>> +
>> +/**
>> + * struct intel_clk_provider
>> + * @map: regmap type base address for register.
>> + * @np: device node
>> + * @clk_data: array of hw clocks and clk number.
>> + */
>> +struct intel_clk_provider {
>> +       struct regmap           *map;
>> +       struct device_node      *np;
>> +       struct clk_onecell_data clk_data;
> Please register clk_hw pointers instead of clk pointers with the of
> provider APIs.
Sorry.  I'm not sure I understand you correctly.
If only registering clk_hw pointer,  not registering of_provider API, then
how to reference it in the user drivers ?
Could you please give me more hints ?

>> +};
>> +
>> +/**
>> + * struct intel_pll_clk
>> + * @id: plaform specific id of the clock.
>> + * @name: name of this pll clock.
>> + * @parent_names: name of the parent clock.
>> + * @num_parents: number of parents.
>> + * @flags: optional flags for basic clock.
>> + * @type: platform type of pll.
>> + * @reg: offset of the register.
>> + * @mult: init value of mulitplier.
>> + * @div: init value of divider.
>> + * @frac: init value of fraction.
>> + * @rate_table: table of pll clock rate.
> Please drop the full-stop on kernel doc one-liners like this.
Will update it.

>
>> + */
>> +struct intel_pll_clk {
>> +       unsigned int            id;
>> +       const char              *name;
>> +       const char              *const *parent_names;
>> +       u8                      num_parents;
> Can the PLL have multiple parents?
Yes. But not in this platform.
The define here make it easy to expand to support new platform.

>> +       unsigned long           flags;
>> +       enum intel_pll_type     type;
>> +       int                     reg;
>> +       unsigned int            mult;
>> +       unsigned int            div;
>> +       unsigned int            frac;
>> +       const struct intel_pll_rate_table *rate_table;
>> +};
>> +
>> +#define INTEL_PLL(_id, _type, _name, _pnames, _flags,  \
>> +           _reg, _rtable, _mult, _div, _frac)          \
>> +       {                                               \
>> +               .id             = _id,                  \
>> +               .type           = _type,                \
>> +               .name           = _name,                \
>> +               .parent_names   = _pnames,              \
>> +               .num_parents    = ARRAY_SIZE(_pnames),  \
>> +               .flags          = _flags,               \
>> +               .reg            = _reg,                 \
>> +               .rate_table     = _rtable,              \
>> +               .mult           = _mult,                \
>> +               .div            = _div,                 \
>> +               .frac           = _frac                 \
>> +       }
>> +
>> +/**
>> + * struct intel_osc_clk
>> + * @id: platform specific id of the clock.
>> + * @name: name of the osc clock.
>> + * @dt_freq: frequency node name in device tree.
>> + * @def_rate: default rate of the osc clock.
>> + * @flags: optional flags for basic clock.
> There aren't flags though. I'm very confused by this kernel-doc too.
> Looks like something that should be done with a fixed rate clk in DT.
Will remove the flags comments.

>> + */
>> +struct intel_osc_clk {
>> +       unsigned int            id;
>> +       const char              *name;
>> +       const char              *dt_freq;
>> +       const u32               def_rate;
>> +};
>> +
>> +#define INTEL_OSC(_id, _name, _freq, _rate)                    \
>> +       {                                               \
>> +               .id             = _id,                  \
>> +               .name           = _name,                \
>> +               .dt_freq        = _freq,                \
>> +               .def_rate       = _rate,                \
>> +       }
>> +
>> +struct intel_clk_branch {
> Seems to be more like intel_clk instead of intel_clk_branch because it
> does lots of stuff.
Will update.

>> +       unsigned int                    id;
>> +       enum intel_clk_type             type;
>> +       const char                      *name;
>> +       const char                      *const *parent_names;
>> +       u8                              num_parents;
>> +       unsigned long                   flags;
>> +       unsigned int                    mux_off;
>> +       u8                              mux_shift;
>> +       u8                              mux_width;
>> +       unsigned long                   mux_flags;
>> +       unsigned int                    mux_val;
>> +       unsigned int                    div_off;
>> +       u8                              div_shift;
>> +       u8                              div_width;
>> +       unsigned long                   div_flags;
>> +       unsigned int                    div_val;
>> +       const struct clk_div_table      *div_table;
>> +       unsigned int                    gate_off;
>> +       u8                              gate_shift;
>> +       unsigned long                   gate_flags;
>> +       unsigned int                    gate_val;
>> +       unsigned int                    mult;
>> +       unsigned int                    div;
>> +};
>> +
>> +/* clock flags definition */
>> +#define CLOCK_FLAG_VAL_INIT    BIT(16)
>> +#define GATE_CLK_HW            BIT(17)
>> +#define GATE_CLK_SW            BIT(18)
>> +#define GATE_CLK_VT            BIT(19)
> What does VT mean? Virtual?
Yes. VT means virtual here.
Will change to GATE_CLK_VIRT.

>> +
>> +#define INTEL_MUX(_id, _name, _pname, _f, _reg,                        \
>> +           _shift, _width, _cf, _v)                            \
>> +       {                                                       \
>> +               .id             = _id,                          \
>> +               .type           = intel_clk_mux,                \
>> +               .name           = _name,                        \
>> +               .parent_names   = _pname,                       \
>> +               .num_parents    = ARRAY_SIZE(_pname),           \
>> +               .flags          = _f,                           \
>> +               .mux_off        = _reg,                         \
>> +               .mux_shift      = _shift,                       \
>> +               .mux_width      = _width,                       \
>> +               .mux_flags      = _cf,                          \
>> +               .mux_val        = _v,                           \
>> +       }
>> +
>> +#define INTEL_DIV(_id, _name, _pname, _f, _reg,                        \
>> +           _shift, _width, _cf, _v, _dtable)                   \
>> +       {                                                       \
>> +               .id             = _id,                          \
>> +               .type           = intel_clk_divider,            \
>> +               .name           = _name,                        \
>> +               .parent_names   = (const char *[]) { _pname },  \
>> +               .num_parents    = 1,                            \
>> +               .flags          = _f,                           \
>> +               .div_off        = _reg,                         \
>> +               .div_shift      = _shift,                       \
>> +               .div_width      = _width,                       \
>> +               .div_flags      = _cf,                          \
>> +               .div_val        = _v,                           \
>> +               .div_table      = _dtable,                      \
>> +       }
>> +
>> +#define INTEL_GATE(_id, _name, _pname, _f, _reg,               \
>> +            _shift, _cf, _v)                                   \
>> +       {                                                       \
>> +               .id             = _id,                          \
>> +               .type           = intel_clk_gate,               \
>> +               .name           = _name,                        \
>> +               .parent_names   = (const char *[]) { _pname },  \
>> +               .num_parents    = !_pname ? 0 : 1,              \
>> +               .flags          = _f,                           \
>> +               .gate_off       = _reg,                         \
>> +               .gate_shift     = _shift,                       \
>> +               .gate_flags     = _cf,                          \
>> +               .gate_val       = _v,                           \
>> +       }
>> +
>> +#define INTEL_FIXED(_id, _name, _pname, _f, _reg,              \
>> +             _shift, _width, _cf, _freq, _v)                   \
>> +       {                                                       \
>> +               .id             = _id,                          \
>> +               .type           = intel_clk_fixed,              \
>> +               .name           = _name,                        \
>> +               .parent_names   = (const char *[]) { _pname },  \
>> +               .num_parents    = !_pname ? 0 : 1,              \
>> +               .flags          = _f,                           \
>> +               .div_off        = _reg,                         \
>> +               .div_shift      = _shift,                       \
>> +               .div_width      = _width,                       \
>> +               .div_flags      = _cf,                          \
>> +               .div_val        = _v,                           \
>> +               .mux_flags      = _freq,                        \
>> +       }
>> +
>> +#define INTEL_FIXED_FACTOR(_id, _name, _pname, _f, _reg,       \
>> +              _shift, _width, _cf, _v, _m, _d)                 \
>> +       {                                                       \
>> +               .id             = _id,                          \
>> +               .type           = intel_clk_fixed_factor,       \
>> +               .name           = _name,                        \
>> +               .parent_names   = (const char *[]) { _pname },  \
>> +               .num_parents    = 1,                            \
>> +               .flags          = _f,                           \
>> +               .div_off        = _reg,                         \
>> +               .div_shift      = _shift,                       \
>> +               .div_width      = _width,                       \
>> +               .div_flags      = _cf,                          \
>> +               .div_val        = _v,                           \
>> +               .mult           = _m,                           \
>> +               .div            = _d,                           \
>> +       }
>> +
>> +void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
>> +                      u8 width, u32 set_val);
>> +u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift, u8 width);
>> +void intel_clk_add_lookup(struct intel_clk_provider *ctx,
>> +                         struct clk *clk, unsigned int id);
>> +void __init intel_clk_of_add_provider(struct device_node *np,
>> +                                     struct intel_clk_provider *ctx);
>> +struct intel_clk_provider * __init
>> +intel_clk_init(struct device_node *np, struct regmap *map,
>> +              unsigned int nr_clks);
>> +void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
>> +                                  struct intel_osc_clk *osc,
>> +                                  unsigned int nr_clks);
> Remove __init from headers files. It does nothing.
Will remove it.

>
>> +void intel_clk_register_branches(struct intel_clk_provider *ctx,
>> +                                struct intel_clk_branch *list,
>> +                                unsigned int nr_clk);
>> +void intel_clk_register_plls(struct intel_clk_provider *ctx,
>> +                            struct intel_pll_clk *list, unsigned int nr_clk);
>> +#endif /* __INTEL_CLK_H */
>> diff --git a/drivers/clk/intel/clk-grx500.c b/drivers/clk/intel/clk-grx500.c
>> new file mode 100644
>> index 000000000000..5c2546f82579
>> --- /dev/null
>> +++ b/drivers/clk/intel/clk-grx500.c
>> @@ -0,0 +1,168 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + *  Copyright (C) 2018 Intel Corporation.
>> + *  Zhu YiXin <Yixin.zhu@intel.com>
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spinlock.h>
> Used?
Will remove it

>> +#include <dt-bindings/clock/intel,grx500-clk.h>
>> +
>> +#include "clk-cgu-pll.h"
>> +#include "clk-cgu.h"
>> +
>> +#define PLL_DIV_WIDTH          4
>> +
>> +/* Gate1 clock shift */
>> +#define G_VCODEC_SHIFT         2
>> +#define G_DMA0_SHIFT           5
>> +#define G_USB0_SHIFT           6
>> +#define G_SPI1_SHIFT           7
>> +#define G_SPI0_SHIFT           8
>> +#define G_CBM_SHIFT            9
>> +#define G_EBU_SHIFT            10
>> +#define G_SSO_SHIFT            11
>> +#define G_GPTC0_SHIFT          12
>> +#define G_GPTC1_SHIFT          13
>> +#define G_GPTC2_SHIFT          14
>> +#define G_UART_SHIFT           17
>> +#define G_CPYTO_SHIFT          20
>> +#define G_SECPT_SHIFT          21
>> +#define G_TOE_SHIFT            22
>> +#define G_MPE_SHIFT            23
>> +#define G_TDM_SHIFT            25
>> +#define G_PAE_SHIFT            26
>> +#define G_USB1_SHIFT           27
>> +#define G_SWITCH_SHIFT         28
>> +
>> +/* Gate2 clock shift */
>> +#define G_PCIE0_SHIFT          1
>> +#define G_PCIE1_SHIFT          17
>> +#define G_PCIE2_SHIFT          25
>> +
>> +/* Register definition */
>> +#define GRX500_PLL0A_CFG0      0x0004
>> +#define GRX500_PLL0A_CFG1      0x0008
>> +#define GRX500_PLL0B_CFG0      0x0034
>> +#define GRX500_PLL0B_CFG1      0x0038
>> +#define GRX500_LCPLL_CFG0      0x0094
>> +#define GRX500_LCPLL_CFG1      0x0098
>> +#define GRX500_IF_CLK          0x00c4
>> +#define GRX500_CLK_GSR1                0x0120
>> +#define GRX500_CLK_GSR2                0x0130
>> +
>> +static const struct clk_div_table pll_div[] = {
>> +       {1,     2},
> Please write it like
>
> 	  { 1,    2 },
>
> instead.
Will update.

>> +       {2,     3},
>> +       {3,     4},
>> +       {4,     5},
>> +       {5,     6},
>> +       {6,     8},
>> +       {7,     10},
>> +       {8,     12},
>> +       {9,     16},
>> +       {10,    20},
>> +       {11,    24},
>> +       {12,    32},
>> +       {13,    40},
>> +       {14,    48},
>> +       {15,    64}
>> +};
>> +
>> +enum grx500_plls {
>> +       pll0a, pll0b, pll3,
>> +};
> What's the point of the enum?
Will remove it.

>> +
>> +PNAME(pll_p)   = { "osc" };
>> +PNAME(cpu_p)   = { "cpu0", "cpu1" };
>> +
>> +static struct intel_osc_clk grx500_osc_clks[] __initdata = {
>> +       INTEL_OSC(CLK_OSC, "osc", "intel,osc-frequency", 40000000),
>> +};
>> +
>> +static struct intel_pll_clk grx500_pll_clks[] __initdata = {
>> +       [pll0a] = INTEL_PLL(CLK_PLL0A, pll_grx500, "pll0a",
>> +                     pll_p, 0, GRX500_PLL0A_CFG0, NULL, 0, 0, 0),
>> +       [pll0b] = INTEL_PLL(CLK_PLL0B, pll_grx500, "pll0b",
>> +                     pll_p, 0, GRX500_PLL0B_CFG0, NULL, 0, 0, 0),
>> +       [pll3] = INTEL_PLL(CLK_PLL3, pll_grx500, "pll3",
>> +                    pll_p, 0, GRX500_LCPLL_CFG0, NULL, 0, 0, 0),
>> +};
>> +
>> +static struct intel_clk_branch grx500_branch_clks[] __initdata = {
>> +       INTEL_DIV(CLK_CBM, "cbm", "pll0a", 0, GRX500_PLL0A_CFG1,
>> +                 0, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_NGI, "ngi", "pll0a", 0, GRX500_PLL0A_CFG1,
>> +                 4, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_SSX4, "ssx4", "pll0a", 0, GRX500_PLL0A_CFG1,
>> +                 8, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_CPU0, "cpu0", "pll0a", 0, GRX500_PLL0A_CFG1,
>> +                 12, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_PAE, "pae", "pll0b", 0, GRX500_PLL0B_CFG1,
>> +                 0, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_GSWIP, "gswip", "pll0b", 0, GRX500_PLL0B_CFG1,
>> +                 4, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_DDR, "ddr", "pll0b", 0, GRX500_PLL0B_CFG1,
>> +                 8, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_DIV(CLK_CPU1, "cpu1", "pll0b", 0, GRX500_PLL0B_CFG1,
>> +                 12, PLL_DIV_WIDTH, 0, 0, pll_div),
>> +       INTEL_MUX(CLK_CPU, "cpu", cpu_p, CLK_SET_RATE_PARENT,
>> +                 GRX500_PLL0A_CFG1, 29, 1, 0, 0),
>> +       INTEL_GATE(GCLK_DMA0, "g_dma0", NULL, 0, GRX500_CLK_GSR1,
>> +                  G_DMA0_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_USB0, "g_usb0", NULL, 0, GRX500_CLK_GSR1,
>> +                  G_USB0_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_GPTC0, "g_gptc0", NULL, 0, GRX500_CLK_GSR1,
>> +                  G_GPTC0_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_GPTC1, "g_gptc1", NULL, 0, GRX500_CLK_GSR1,
>> +                  G_GPTC1_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_GPTC2, "g_gptc2", NULL, 0, GRX500_CLK_GSR1,
>> +                  G_GPTC2_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_UART, "g_uart", NULL, 0, GRX500_CLK_GSR1,
>> +                  G_UART_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_PCIE0, "g_pcie0", NULL, 0, GRX500_CLK_GSR2,
>> +                  G_PCIE0_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_PCIE1, "g_pcie1", NULL, 0, GRX500_CLK_GSR2,
>> +                  G_PCIE1_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_PCIE2, "g_pcie2", NULL, 0, GRX500_CLK_GSR2,
>> +                  G_PCIE2_SHIFT, GATE_CLK_HW, 0),
>> +       INTEL_GATE(GCLK_I2C, "g_i2c", NULL, 0, 0, 0, GATE_CLK_VT, 0),
>> +       INTEL_FIXED(CLK_VOICE, "voice", NULL, 0, GRX500_IF_CLK, 14, 2,
>> +                   CLOCK_FLAG_VAL_INIT, 8192000, 2),
>> +       INTEL_FIXED_FACTOR(CLK_DDRPHY, "ddrphy", "ddr", 0, 0, 0,
>> +                          0, 0, 0, 2, 1),
>> +       INTEL_FIXED_FACTOR(CLK_PCIE, "pcie", "pll3", 0, 0, 0,
>> +                          0, 0, 0, 1, 40),
>> +};
>> +
>> +static void __init grx500_clk_init(struct device_node *np)
>> +{
>> +       struct intel_clk_provider *ctx;
>> +       struct regmap *map;
>> +
>> +       map = syscon_node_to_regmap(np);
>> +       if (IS_ERR(map))
>> +               return;
>> +
>> +       ctx = intel_clk_init(np, map, CLK_NR_CLKS);
>> +       if (IS_ERR(ctx)) {
>> +               regmap_exit(map);
>> +               return;
>> +       }
>> +
>> +       intel_clk_register_osc(ctx, grx500_osc_clks,
>> +                              ARRAY_SIZE(grx500_osc_clks));
>> +       intel_clk_register_plls(ctx, grx500_pll_clks,
>> +                               ARRAY_SIZE(grx500_pll_clks));
>> +       intel_clk_register_branches(ctx, grx500_branch_clks,
>> +                                   ARRAY_SIZE(grx500_branch_clks));
>> +       of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
>> +
>> +       pr_debug("%s clk init done!\n", __func__);
> Yay!!!
>
>> +}
>> +
>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
It provides CPU clock which is used in early boot stage.

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Aug. 27, 2018, 7:09 p.m. UTC | #5
Quoting yixin zhu (2018-08-08 01:52:20)
> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
> > Quoting Songjun Wu (2018-08-02 20:02:21)
> >> +       struct clk *clk;
> >> +       int idx;
> >> +
> >> +       for (idx = 0; idx < nr_clks; idx++, osc++) {
> >> +               if (!osc->dt_freq ||
> >> +                   of_property_read_u32(ctx->np, osc->dt_freq, &freq))
> >> +                       freq = osc->def_rate;
> >> +
> >> +               clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
> > Should come from DT itself.
> Yes. It can be defined as fixed-clock node in device tree.
> Do you mean it should be defined in device tree and driver reference it 
> via device tree?

Yes the oscillator should be in DT and then the DT node here can call
clk_get() or just hardcode the parent name to be what it knows it is.
Eventually we'd like to be able to move away from string names for
hierarchy descriptions but that's far off. To get there, we would need
DT nodes for clock controllers to indicate their clk parents with the
clocks and clock-names properties. So for the oscillator, DT would
define it and then the driver would eventually have a way to specify
that some parent is index 5 or clock name "foo" and then the clk core
could figure out the linkage. I haven't written that code yet, but I'll
probably do it soon if nobody beats me to it.

> >> +/**
> >> + * struct intel_clk_provider
> >> + * @map: regmap type base address for register.
> >> + * @np: device node
> >> + * @clk_data: array of hw clocks and clk number.
> >> + */
> >> +struct intel_clk_provider {
> >> +       struct regmap           *map;
> >> +       struct device_node      *np;
> >> +       struct clk_onecell_data clk_data;
> > Please register clk_hw pointers instead of clk pointers with the of
> > provider APIs.
> Sorry.  I'm not sure I understand you correctly.
> If only registering clk_hw pointer,  not registering of_provider API, then
> how to reference it in the user drivers ?
> Could you please give me more hints ?

Clk provider drivers shouldn't be using clk pointers directly. Usually
when that happens something is wrong. So new clk drivers should register
clk_hw pointers and pretty much only deal with clk_hw pointers instead
of struct clk pointers. You still register an of_provider, but that
provider hands out clk_hw pointers so that clk provider drivers aren't
tempted to use struct clk pointers.

> 
> 
> >
> >> + */
> >> +struct intel_pll_clk {
> >> +       unsigned int            id;
> >> +       const char              *name;
> >> +       const char              *const *parent_names;
> >> +       u8                      num_parents;
> > Can the PLL have multiple parents?
> Yes. But not in this platform.
> The define here make it easy to expand to support new platform.
> 

Ok, so it has a mux inside.

> 
> >> +       unsigned int                    id;
> >> +       enum intel_clk_type             type;
> >> +       const char                      *name;
> >> +       const char                      *const *parent_names;
> >> +       u8                              num_parents;
> >> +       unsigned long                   flags;
> >> +       unsigned int                    mux_off;
> >> +       u8                              mux_shift;
> >> +       u8                              mux_width;
> >> +       unsigned long                   mux_flags;
> >> +       unsigned int                    mux_val;
> >> +       unsigned int                    div_off;
> >> +       u8                              div_shift;
> >> +       u8                              div_width;
> >> +       unsigned long                   div_flags;
> >> +       unsigned int                    div_val;
> >> +       const struct clk_div_table      *div_table;
> >> +       unsigned int                    gate_off;
> >> +       u8                              gate_shift;
> >> +       unsigned long                   gate_flags;
> >> +       unsigned int                    gate_val;
> >> +       unsigned int                    mult;
> >> +       unsigned int                    div;
> >> +};
> >> +
> >> +/* clock flags definition */
> >> +#define CLOCK_FLAG_VAL_INIT    BIT(16)
> >> +#define GATE_CLK_HW            BIT(17)
> >> +#define GATE_CLK_SW            BIT(18)
> >> +#define GATE_CLK_VT            BIT(19)
> > What does VT mean? Virtual?
> Yes. VT means virtual here.
> Will change to GATE_CLK_VIRT.
> 

Is it a hardware concept? Or virtualization with hypervisor?

> >
> >> +}
> >> +
> >> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> > Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
> It provides CPU clock which is used in early boot stage.
> 

Ok. What is the CPU clock doing in early boot stage? Some sort of timer
frequency? If the driver can be split into two pieces, one to handle the
really early stuff that must be in place to get timers up and running
and the other to register the rest of the clks that aren't critical from
a regular platform driver it would be good. That's preferred model if
something is super critical.
Zhu, Yi Xin Aug. 29, 2018, 6:56 a.m. UTC | #6
On 8/28/2018 3:09 AM, Stephen Boyd wrote:
> Quoting yixin zhu (2018-08-08 01:52:20)
>> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
>>> Quoting Songjun Wu (2018-08-02 20:02:21)
>>>> +       struct clk *clk;
>>>> +       int idx;
>>>> +
>>>> +       for (idx = 0; idx < nr_clks; idx++, osc++) {
>>>> +               if (!osc->dt_freq ||
>>>> +                   of_property_read_u32(ctx->np, osc->dt_freq, &freq))
>>>> +                       freq = osc->def_rate;
>>>> +
>>>> +               clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
>>> Should come from DT itself.
>> Yes. It can be defined as fixed-clock node in device tree.
>> Do you mean it should be defined in device tree and driver reference it
>> via device tree?
> Yes the oscillator should be in DT and then the DT node here can call
> clk_get() or just hardcode the parent name to be what it knows it is.
> Eventually we'd like to be able to move away from string names for
> hierarchy descriptions but that's far off. To get there, we would need
> DT nodes for clock controllers to indicate their clk parents with the
> clocks and clock-names properties. So for the oscillator, DT would
> define it and then the driver would eventually have a way to specify
> that some parent is index 5 or clock name "foo" and then the clk core
> could figure out the linkage. I haven't written that code yet, but I'll
> probably do it soon if nobody beats me to it.

Thanks.  Will update.


>
>>>> +/**
>>>> + * struct intel_clk_provider
>>>> + * @map: regmap type base address for register.
>>>> + * @np: device node
>>>> + * @clk_data: array of hw clocks and clk number.
>>>> + */
>>>> +struct intel_clk_provider {
>>>> +       struct regmap           *map;
>>>> +       struct device_node      *np;
>>>> +       struct clk_onecell_data clk_data;
>>> Please register clk_hw pointers instead of clk pointers with the of
>>> provider APIs.
>> Sorry.  I'm not sure I understand you correctly.
>> If only registering clk_hw pointer,  not registering of_provider API, then
>> how to reference it in the user drivers ?
>> Could you please give me more hints ?
> Clk provider drivers shouldn't be using clk pointers directly. Usually
> when that happens something is wrong. So new clk drivers should register
> clk_hw pointers and pretty much only deal with clk_hw pointers instead
> of struct clk pointers. You still register an of_provider, but that
> provider hands out clk_hw pointers so that clk provider drivers aren't
> tempted to use struct clk pointers.

Understood.  Will update to use clk_hw_onecell_data and change the 
registration accordingly.


>>
>>>> + */
>>>> +struct intel_pll_clk {
>>>> +       unsigned int            id;
>>>> +       const char              *name;
>>>> +       const char              *const *parent_names;
>>>> +       u8                      num_parents;
>>> Can the PLL have multiple parents?
>> Yes. But not in this platform.
>> The define here make it easy to expand to support new platform.
>>
> Ok, so it has a mux inside.
>
>>>> +       unsigned int                    id;
>>>> +       enum intel_clk_type             type;
>>>> +       const char                      *name;
>>>> +       const char                      *const *parent_names;
>>>> +       u8                              num_parents;
>>>> +       unsigned long                   flags;
>>>> +       unsigned int                    mux_off;
>>>> +       u8                              mux_shift;
>>>> +       u8                              mux_width;
>>>> +       unsigned long                   mux_flags;
>>>> +       unsigned int                    mux_val;
>>>> +       unsigned int                    div_off;
>>>> +       u8                              div_shift;
>>>> +       u8                              div_width;
>>>> +       unsigned long                   div_flags;
>>>> +       unsigned int                    div_val;
>>>> +       const struct clk_div_table      *div_table;
>>>> +       unsigned int                    gate_off;
>>>> +       u8                              gate_shift;
>>>> +       unsigned long                   gate_flags;
>>>> +       unsigned int                    gate_val;
>>>> +       unsigned int                    mult;
>>>> +       unsigned int                    div;
>>>> +};
>>>> +
>>>> +/* clock flags definition */
>>>> +#define CLOCK_FLAG_VAL_INIT    BIT(16)
>>>> +#define GATE_CLK_HW            BIT(17)
>>>> +#define GATE_CLK_SW            BIT(18)
>>>> +#define GATE_CLK_VT            BIT(19)
>>> What does VT mean? Virtual?
>> Yes. VT means virtual here.
>> Will change to GATE_CLK_VIRT.
>>
> Is it a hardware concept? Or virtualization with hypervisor?

Some peripheral drivers want to use same code cross platforms.

But not all platforms provide HW gate clock.  So in this case, clock 
driver creates

a virtual gate clock to make it work if no HW gate clock in the SoC.


>
>>>> +}
>>>> +
>>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
>>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
>> It provides CPU clock which is used in early boot stage.
>>
> Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> frequency? If the driver can be split into two pieces, one to handle the
> really early stuff that must be in place to get timers up and running
> and the other to register the rest of the clks that aren't critical from
> a regular platform driver it would be good. That's preferred model if
> something is super critical.

Yes, CPU clock is providing CPU frequency in the early boot stage.

Will put the non-critical clocks in the platform driver.


>
Zhu, Yi Xin Aug. 29, 2018, 10:34 a.m. UTC | #7
>>>> +}
>>>> +
>>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
>>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
>> It provides CPU clock which is used in early boot stage.
>>
> Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> frequency? If the driver can be split into two pieces, one to handle the
> really early stuff that must be in place to get timers up and running
> and the other to register the rest of the clks that aren't critical from
> a regular platform driver it would be good. That's preferred model if
> something is super critical.
>
Just to make sure my approach is same as you think.

In the driver, there's two clock registrations.

- One through CLK_OF_DECLARE for early stage clocks.

- The other via platform driver for the non-critical clocks.

In the device tree,  two clock device nodes are required.

e.g. device tree:

cgu: cgu@16200000 {
                 compatible = "intel,grx500-clk", "syscon";
                 reg = <0x16200000 0x200>;
                 #clock-cells = <1>;
};

clk: clk {
                 compatible = "intel,grx500-cgu";
                 #clock-cells = <1>;
                 intel,cgu-syscon = <&cgu>;
};

source code:

CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);

static const struct of_device_id of_intel_grx500_cgu_match[] = {
         { .compatible = "intel,grx500-clk" },
         {}
};

static struct platform_driver intel_grx500_clk_driver = {
         .probe  = intel_grx500_clk_probe,
         .driver = {
                 .name = "grx500-cgu",
                 .of_match_table = of_match_ptr(of_intel_grx500_cgu_match),
         },
};

static int __init intel_grx500_cgu_init(void)
{
         return platform_driver_register(&intel_grx500_clk_driver);
}
arch_initcall(intel_grx500_cgu_init);
Stephen Boyd Aug. 31, 2018, 5:10 p.m. UTC | #8
Quoting Zhu, Yi Xin (2018-08-28 23:56:22)
> 
> On 8/28/2018 3:09 AM, Stephen Boyd wrote:
> > Quoting yixin zhu (2018-08-08 01:52:20)
> >> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
> >>>> +/* clock flags definition */
> >>>> +#define CLOCK_FLAG_VAL_INIT    BIT(16)
> >>>> +#define GATE_CLK_HW            BIT(17)
> >>>> +#define GATE_CLK_SW            BIT(18)
> >>>> +#define GATE_CLK_VT            BIT(19)
> >>> What does VT mean? Virtual?
> >> Yes. VT means virtual here.
> >> Will change to GATE_CLK_VIRT.
> >>
> > Is it a hardware concept? Or virtualization with hypervisor?
> 
> Some peripheral drivers want to use same code cross platforms.
> 
> But not all platforms provide HW gate clock.  So in this case, clock 
> driver creates
> 
> a virtual gate clock to make it work if no HW gate clock in the SoC.

That's not how things are supposed to work. If a clk isn't there in the
hardware we don't make them up in software so that the consumer software
drivers can keep requesting clks on different platforms. On a different
platform, the driver needs to know that the clks aren't there with a
different compatible string.

> 
> 
> >
> >>>> +}
> >>>> +
> >>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> >>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
> >> It provides CPU clock which is used in early boot stage.
> >>
> > Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> > frequency? If the driver can be split into two pieces, one to handle the
> > really early stuff that must be in place to get timers up and running
> > and the other to register the rest of the clks that aren't critical from
> > a regular platform driver it would be good. That's preferred model if
> > something is super critical.
> 
> Yes, CPU clock is providing CPU frequency in the early boot stage.
> 
> Will put the non-critical clocks in the platform driver.
> 
> 

Sure the CPU clock is handling frequency, but does that matter for early
boot to get going? If timers aren't involved here then it doesn't sound
like this needs CLK_OF_DECLARE.
Stephen Boyd Aug. 31, 2018, 5:13 p.m. UTC | #9
Quoting Zhu, Yi Xin (2018-08-29 03:34:26)
> >>>> +}
> >>>> +
> >>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
> >>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
> >> It provides CPU clock which is used in early boot stage.
> >>
> > Ok. What is the CPU clock doing in early boot stage? Some sort of timer
> > frequency? If the driver can be split into two pieces, one to handle the
> > really early stuff that must be in place to get timers up and running
> > and the other to register the rest of the clks that aren't critical from
> > a regular platform driver it would be good. That's preferred model if
> > something is super critical.
> >
> Just to make sure my approach is same as you think.
> 
> In the driver, there's two clock registrations.
> 
> - One through CLK_OF_DECLARE for early stage clocks.
> 
> - The other via platform driver for the non-critical clocks.

You can use the same DT node for both parts, no need to split the node
into two and use syscon here.
Zhu, Yi Xin Sept. 3, 2018, 10:47 a.m. UTC | #10
On 9/1/2018 1:10 AM, Stephen Boyd wrote:
> Quoting Zhu, Yi Xin (2018-08-28 23:56:22)
>> On 8/28/2018 3:09 AM, Stephen Boyd wrote:
>>> Quoting yixin zhu (2018-08-08 01:52:20)
>>>> On 8/8/2018 1:50 PM, Stephen Boyd wrote:
>>>>>> +/* clock flags definition */
>>>>>> +#define CLOCK_FLAG_VAL_INIT    BIT(16)
>>>>>> +#define GATE_CLK_HW            BIT(17)
>>>>>> +#define GATE_CLK_SW            BIT(18)
>>>>>> +#define GATE_CLK_VT            BIT(19)
>>>>> What does VT mean? Virtual?
>>>> Yes. VT means virtual here.
>>>> Will change to GATE_CLK_VIRT.
>>>>
>>> Is it a hardware concept? Or virtualization with hypervisor?
>> Some peripheral drivers want to use same code cross platforms.
>>
>> But not all platforms provide HW gate clock.  So in this case, clock
>> driver creates
>>
>> a virtual gate clock to make it work if no HW gate clock in the SoC.
> That's not how things are supposed to work. If a clk isn't there in the
> hardware we don't make them up in software so that the consumer software
> drivers can keep requesting clks on different platforms. On a different
> platform, the driver needs to know that the clks aren't there with a
> different compatible string.

OK. Will remove virtual gate clock.


>>
>>>>>> +}
>>>>>> +
>>>>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
>>>>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
>>>> It provides CPU clock which is used in early boot stage.
>>>>
>>> Ok. What is the CPU clock doing in early boot stage? Some sort of timer
>>> frequency? If the driver can be split into two pieces, one to handle the
>>> really early stuff that must be in place to get timers up and running
>>> and the other to register the rest of the clks that aren't critical from
>>> a regular platform driver it would be good. That's preferred model if
>>> something is super critical.
>> Yes, CPU clock is providing CPU frequency in the early boot stage.
>>
>> Will put the non-critical clocks in the platform driver.
>>
>>
> Sure the CPU clock is handling frequency, but does that matter for early
> boot to get going? If timers aren't involved here then it doesn't sound
> like this needs CLK_OF_DECLARE.

Yes, timer is involved here.

CPU frequency get by early stage platform code used in clockevent 
registration.


>
Zhu, Yi Xin Sept. 3, 2018, 10:52 a.m. UTC | #11
On 9/1/2018 1:13 AM, Stephen Boyd wrote:
> Quoting Zhu, Yi Xin (2018-08-29 03:34:26)
>>>>>> +}
>>>>>> +
>>>>>> +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
>>>>> Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
>>>> It provides CPU clock which is used in early boot stage.
>>>>
>>> Ok. What is the CPU clock doing in early boot stage? Some sort of timer
>>> frequency? If the driver can be split into two pieces, one to handle the
>>> really early stuff that must be in place to get timers up and running
>>> and the other to register the rest of the clks that aren't critical from
>>> a regular platform driver it would be good. That's preferred model if
>>> something is super critical.
>>>
>> Just to make sure my approach is same as you think.
>>
>> In the driver, there's two clock registrations.
>>
>> - One through CLK_OF_DECLARE for early stage clocks.
>>
>> - The other via platform driver for the non-critical clocks.
> You can use the same DT node for both parts, no need to split the node
> into two and use syscon here.
>
Thank you.

Will use CLK_OF_DECLARE_DRIVER to use the same DT node.
diff mbox series

Patch

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 721572a8c429..5e0c1597b0d3 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -281,6 +281,7 @@  source "drivers/clk/actions/Kconfig"
 source "drivers/clk/bcm/Kconfig"
 source "drivers/clk/hisilicon/Kconfig"
 source "drivers/clk/imgtec/Kconfig"
+source "drivers/clk/intel/Kconfig"
 source "drivers/clk/keystone/Kconfig"
 source "drivers/clk/mediatek/Kconfig"
 source "drivers/clk/meson/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 0bb25dd009d1..d929ca4607cf 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -72,6 +72,9 @@  obj-$(CONFIG_ARCH_HISI)			+= hisilicon/
 obj-y					+= imgtec/
 obj-$(CONFIG_ARCH_MXC)			+= imx/
 obj-$(CONFIG_MACH_INGENIC)		+= ingenic/
+ifeq ($(CONFIG_COMMON_CLK), y)
+obj-y							+=intel/
+endif
 obj-$(CONFIG_ARCH_KEYSTONE)		+= keystone/
 obj-$(CONFIG_MACH_LOONGSON32)		+= loongson1/
 obj-y					+= mediatek/
diff --git a/drivers/clk/intel/Kconfig b/drivers/clk/intel/Kconfig
new file mode 100644
index 000000000000..c7d3fb1721fa
--- /dev/null
+++ b/drivers/clk/intel/Kconfig
@@ -0,0 +1,20 @@ 
+# SPDX-License-Identifier: GPL-2.0
+config INTEL_CGU_CLK
+	depends on COMMON_CLK
+	depends on INTEL_MIPS || COMPILE_TEST
+	select MFD_SYSCON
+	bool "Intel clock controller support"
+	help
+	  This driver support Intel CGU (Clock Generation Unit).
+
+choice
+	prompt "SoC platform selection"
+	depends on INTEL_CGU_CLK
+	default INTEL_GRX500_CGU_CLK
+
+config INTEL_GRX500_CGU_CLK
+	bool "GRX500 CLK"
+	help
+	  Clock driver of GRX500 platform.
+
+endchoice
diff --git a/drivers/clk/intel/Makefile b/drivers/clk/intel/Makefile
new file mode 100644
index 000000000000..16a0138e52c2
--- /dev/null
+++ b/drivers/clk/intel/Makefile
@@ -0,0 +1,7 @@ 
+# SPDX-License-Identifier: GPL-2.0
+# Makefile for intel specific clk
+
+obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o
+ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),)
+	obj-y += clk-grx500.o
+endif
diff --git a/drivers/clk/intel/clk-cgu-pll.c b/drivers/clk/intel/clk-cgu-pll.c
new file mode 100644
index 000000000000..20759bc27e95
--- /dev/null
+++ b/drivers/clk/intel/clk-cgu-pll.c
@@ -0,0 +1,166 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2018 Intel Corporation.
+ *  Zhu YiXin <Yixin.zhu@intel.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "clk-cgu-pll.h"
+#include "clk-cgu.h"
+
+#define to_intel_clk_pll(_hw)	container_of(_hw, struct intel_clk_pll, hw)
+
+/*
+ * Calculate formula:
+ * rate = (prate * mult + (prate * frac) / frac_div) / div
+ */
+static unsigned long
+intel_pll_calc_rate(unsigned long prate, unsigned int mult,
+		    unsigned int div, unsigned int frac,
+		    unsigned int frac_div)
+{
+	u64 crate, frate, rate64;
+
+	rate64 = prate;
+	crate = rate64 * mult;
+
+	if (frac) {
+		frate = rate64 * frac;
+		do_div(frate, frac_div);
+		crate += frate;
+	}
+	do_div(crate, div);
+
+	return (unsigned long)crate;
+}
+
+static void
+grx500_pll_get_params(struct intel_clk_pll *pll, unsigned int *mult,
+		      unsigned int *frac)
+{
+	*mult = intel_get_clk_val(pll->map, pll->reg, 2, 7);
+	*frac = intel_get_clk_val(pll->map, pll->reg, 9, 21);
+}
+
+static int intel_wait_pll_lock(struct intel_clk_pll *pll, int bit_idx)
+{
+	unsigned int val;
+
+	return regmap_read_poll_timeout(pll->map, pll->reg, val,
+					val & BIT(bit_idx), 10, 1000);
+}
+
+static unsigned long
+intel_grx500_pll_recalc_rate(struct clk_hw *hw, unsigned long prate)
+{
+	struct intel_clk_pll *pll = to_intel_clk_pll(hw);
+	unsigned int mult, frac;
+
+	grx500_pll_get_params(pll, &mult, &frac);
+
+	return intel_pll_calc_rate(prate, mult, 1, frac, BIT(20));
+}
+
+static int intel_grx500_pll_is_enabled(struct clk_hw *hw)
+{
+	struct intel_clk_pll *pll = to_intel_clk_pll(hw);
+
+	if (intel_wait_pll_lock(pll, 1)) {
+		pr_err("%s: pll: %s is not locked!\n",
+		       __func__, clk_hw_get_name(hw));
+		return 0;
+	}
+
+	return intel_get_clk_val(pll->map, pll->reg, 1, 1);
+}
+
+const static struct clk_ops intel_grx500_pll_ops = {
+	.recalc_rate = intel_grx500_pll_recalc_rate,
+	.is_enabled = intel_grx500_pll_is_enabled,
+};
+
+static struct clk
+*intel_clk_register_pll(struct intel_clk_provider *ctx,
+			enum intel_pll_type type, const char *cname,
+			const char *const *pname, u8 num_parents,
+			unsigned long flags, unsigned int reg,
+			const struct intel_pll_rate_table *table,
+			unsigned int mult, unsigned int div, unsigned int frac)
+{
+	struct clk_init_data init;
+	struct intel_clk_pll *pll;
+	struct clk_hw *hw;
+	int ret, i;
+
+	if (type != pll_grx500) {
+		pr_err("%s: pll type %d not supported!\n",
+		       __func__, type);
+		return ERR_PTR(-EINVAL);
+	}
+	init.name = cname;
+	init.ops = &intel_grx500_pll_ops;
+	init.flags = CLK_IS_BASIC;
+	init.parent_names = pname;
+	init.num_parents = num_parents;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+	pll->map = ctx->map;
+	pll->reg = reg;
+	pll->flags = flags;
+	pll->mult = mult;
+	pll->div = div;
+	pll->frac = frac;
+	pll->hw.init = &init;
+	if (table) {
+		for (i = 0; table[i].rate != 0; i++)
+			;
+		pll->table_sz = i;
+		pll->rate_table = kmemdup(table, i * sizeof(table[0]),
+					  GFP_KERNEL);
+		if (!pll->rate_table) {
+			ret = -ENOMEM;
+			goto err_free_pll;
+		}
+	}
+	hw = &pll->hw;
+	ret = clk_hw_register(NULL, hw);
+	if (ret)
+		goto err_free_pll;
+
+	return hw->clk;
+
+err_free_pll:
+	kfree(pll);
+	return ERR_PTR(ret);
+}
+
+void intel_clk_register_plls(struct intel_clk_provider *ctx,
+			     struct intel_pll_clk *list, unsigned int nr_clk)
+{
+	struct clk *clk;
+	int i;
+
+	for (i = 0; i < nr_clk; i++, list++) {
+		clk = intel_clk_register_pll(ctx, list->type, list->name,
+				list->parent_names, list->num_parents,
+				list->flags, list->reg, list->rate_table,
+				list->mult, list->div, list->frac);
+		if (IS_ERR(clk)) {
+			pr_err("%s: failed to register pll: %s\n",
+			       __func__, list->name);
+			continue;
+		}
+
+		intel_clk_add_lookup(ctx, clk, list->id);
+	}
+}
diff --git a/drivers/clk/intel/clk-cgu-pll.h b/drivers/clk/intel/clk-cgu-pll.h
new file mode 100644
index 000000000000..3e7cff1d5e16
--- /dev/null
+++ b/drivers/clk/intel/clk-cgu-pll.h
@@ -0,0 +1,34 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Copyright(c) 2018 Intel Corporation.
+ *  Zhu YiXin <Yixin.zhu@intel.com>
+ */
+
+#ifndef __INTEL_CLK_PLL_H
+#define __INTEL_CLK_PLL_H
+
+enum intel_pll_type {
+	pll_grx500,
+};
+
+struct intel_pll_rate_table {
+	unsigned long	prate;
+	unsigned long	rate;
+	unsigned int	mult;
+	unsigned int	div;
+	unsigned int	frac;
+};
+
+struct intel_clk_pll {
+	struct clk_hw	hw;
+	struct regmap	*map;
+	unsigned int	reg;
+	unsigned long	flags;
+	unsigned int	mult;
+	unsigned int	div;
+	unsigned int	frac;
+	unsigned int	table_sz;
+	const struct intel_pll_rate_table *rate_table;
+};
+
+#endif /* __INTEL_CLK_PLL_H */
diff --git a/drivers/clk/intel/clk-cgu.c b/drivers/clk/intel/clk-cgu.c
new file mode 100644
index 000000000000..10cacbe0fbcd
--- /dev/null
+++ b/drivers/clk/intel/clk-cgu.c
@@ -0,0 +1,470 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2018 Intel Corporation.
+ *  Zhu YiXin <Yixin.zhu@intel.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "clk-cgu-pll.h"
+#include "clk-cgu.h"
+
+#define GATE_HW_REG_STAT(reg)	(reg)
+#define GATE_HW_REG_EN(reg)	((reg) + 0x4)
+#define GATE_HW_REG_DIS(reg)	((reg) + 0x8)
+
+#define to_intel_clk_mux(_hw) container_of(_hw, struct intel_clk_mux, hw)
+#define to_intel_clk_divider(_hw) \
+		container_of(_hw, struct intel_clk_divider, hw)
+#define to_intel_clk_gate(_hw) container_of(_hw, struct intel_clk_gate, hw)
+
+void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
+		       u8 width, u32 set_val)
+{
+	u32 mask = GENMASK(width + shift, shift);
+
+	regmap_update_bits(map, reg, mask, set_val << shift);
+}
+
+u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift,
+		      u8 width)
+{
+	u32 val;
+
+	if (regmap_read(map, reg, &val)) {
+		WARN_ONCE(1, "Failed to read clk reg: 0x%x\n", reg);
+		return 0;
+	}
+	val >>= shift;
+	val &= BIT(width) - 1;
+
+	return val;
+}
+
+void intel_clk_add_lookup(struct intel_clk_provider *ctx,
+			  struct clk *clk, unsigned int id)
+{
+	pr_debug("Add clk: %s, id: %u\n", __clk_get_name(clk), id);
+	if (ctx->clk_data.clks && id)
+		ctx->clk_data.clks[id] = clk;
+}
+
+static struct clk
+*intel_clk_register_fixed(struct intel_clk_provider *ctx,
+			  struct intel_clk_branch *list)
+{
+	if (list->div_flags & CLOCK_FLAG_VAL_INIT)
+		intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
+				  list->div_width, list->div_val);
+
+	return clk_register_fixed_rate(NULL, list->name, list->parent_names[0],
+				       list->flags, list->mux_flags);
+}
+
+static u8 intel_clk_mux_get_parent(struct clk_hw *hw)
+{
+	struct intel_clk_mux *mux = to_intel_clk_mux(hw);
+	u32 val;
+
+	val = intel_get_clk_val(mux->map, mux->reg, mux->shift, mux->width);
+	return clk_mux_val_to_index(hw, NULL, mux->flags, val);
+}
+
+static int intel_clk_mux_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct intel_clk_mux *mux = to_intel_clk_mux(hw);
+	u32 val;
+
+	val = clk_mux_index_to_val(NULL, mux->flags, index);
+	intel_set_clk_val(mux->map, mux->reg, mux->shift, mux->width, val);
+
+	return 0;
+}
+
+static int intel_clk_mux_determine_rate(struct clk_hw *hw,
+					struct clk_rate_request *req)
+{
+	struct intel_clk_mux *mux = to_intel_clk_mux(hw);
+
+	return clk_mux_determine_rate_flags(hw, req, mux->flags);
+}
+
+const static struct clk_ops intel_clk_mux_ops = {
+	.get_parent = intel_clk_mux_get_parent,
+	.set_parent = intel_clk_mux_set_parent,
+	.determine_rate = intel_clk_mux_determine_rate,
+};
+
+static struct clk
+*intel_clk_register_mux(struct intel_clk_provider *ctx,
+			struct intel_clk_branch *list)
+{
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	struct intel_clk_mux *mux;
+	u32 reg = list->mux_off;
+	u8 shift = list->mux_shift;
+	u8 width = list->mux_width;
+	unsigned long cflags = list->mux_flags;
+	int ret;
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = list->name;
+	init.ops = &intel_clk_mux_ops;
+	init.flags = list->flags | CLK_IS_BASIC;
+	init.parent_names = list->parent_names;
+	init.num_parents = list->num_parents;
+
+	mux->map = ctx->map;
+	mux->reg = reg;
+	mux->shift = shift;
+	mux->width = width;
+	mux->flags = cflags;
+	mux->hw.init = &init;
+
+	hw = &mux->hw;
+	ret = clk_hw_register(NULL, hw);
+	if (ret) {
+		kfree(mux);
+		return ERR_PTR(ret);
+	}
+
+	if (cflags & CLOCK_FLAG_VAL_INIT)
+		intel_set_clk_val(ctx->map, reg, shift, width, list->mux_val);
+
+	return hw->clk;
+}
+
+static unsigned long
+intel_clk_divider_recalc_rate(struct clk_hw *hw,
+			      unsigned long parent_rate)
+{
+	struct intel_clk_divider *divider = to_intel_clk_divider(hw);
+	unsigned int val;
+
+	val = intel_get_clk_val(divider->map, divider->reg,
+				divider->shift, divider->width);
+	return divider_recalc_rate(hw, parent_rate, val, divider->table,
+				   divider->flags, divider->width);
+}
+
+static long
+intel_clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
+			     unsigned long *prate)
+{
+	struct intel_clk_divider *divider = to_intel_clk_divider(hw);
+
+	return divider_round_rate(hw, rate, prate, divider->table,
+				  divider->width, divider->flags);
+}
+
+static int
+intel_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
+			   unsigned long prate)
+{
+	struct intel_clk_divider *divider = to_intel_clk_divider(hw);
+	int value;
+
+	value = divider_get_val(rate, prate, divider->table,
+				divider->width, divider->flags);
+	if (value < 0)
+		return value;
+
+	intel_set_clk_val(divider->map, divider->reg,
+			  divider->shift, divider->width, value);
+
+	return 0;
+}
+
+const static struct clk_ops intel_clk_divider_ops = {
+	.recalc_rate = intel_clk_divider_recalc_rate,
+	.round_rate = intel_clk_divider_round_rate,
+	.set_rate = intel_clk_divider_set_rate,
+};
+
+static struct clk
+*intel_clk_register_divider(struct intel_clk_provider *ctx,
+			    struct intel_clk_branch *list)
+{
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	struct intel_clk_divider *div;
+	u32 reg = list->div_off;
+	u8 shift = list->div_shift;
+	u8 width = list->div_width;
+	unsigned long cflags = list->div_flags;
+	int ret;
+
+	div = kzalloc(sizeof(*div), GFP_KERNEL);
+	if (!div)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = list->name;
+	init.ops = &intel_clk_divider_ops;
+	init.flags = list->flags | CLK_IS_BASIC;
+	init.parent_names = &list->parent_names[0];
+	init.num_parents = 1;
+
+	div->map = ctx->map;
+	div->reg = reg;
+	div->shift = shift;
+	div->width = width;
+	div->flags = cflags;
+	div->table = list->div_table;
+	div->hw.init = &init;
+
+	hw = &div->hw;
+	ret = clk_hw_register(NULL, hw);
+	if (ret) {
+		pr_err("%s: register clk: %s failed!\n",
+		       __func__, list->name);
+		kfree(div);
+		return ERR_PTR(ret);
+	}
+
+	if (cflags & CLOCK_FLAG_VAL_INIT)
+		intel_set_clk_val(ctx->map, reg, shift, width, list->div_val);
+
+	return hw->clk;
+}
+
+static struct clk
+*intel_clk_register_fixed_factor(struct intel_clk_provider *ctx,
+				 struct intel_clk_branch *list)
+{
+	struct clk_hw *hw;
+
+	hw = clk_hw_register_fixed_factor(NULL, list->name,
+					  list->parent_names[0], list->flags,
+					  list->mult, list->div);
+	if (IS_ERR(hw))
+		return ERR_CAST(hw);
+
+	if (list->div_flags & CLOCK_FLAG_VAL_INIT)
+		intel_set_clk_val(ctx->map, list->div_off, list->div_shift,
+				  list->div_width, list->div_val);
+
+	return hw->clk;
+}
+
+static int
+intel_clk_gate_enable(struct clk_hw *hw)
+{
+	struct intel_clk_gate *gate = to_intel_clk_gate(hw);
+	unsigned int reg;
+
+	if (gate->flags & GATE_CLK_VT) {
+		gate->reg = 1;
+		return 0;
+	}
+
+	if (gate->flags & GATE_CLK_HW) {
+		reg = GATE_HW_REG_EN(gate->reg);
+	} else if (gate->flags & GATE_CLK_SW) {
+		reg = gate->reg;
+	} else {
+		pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
+		       __func__, clk_hw_get_name(hw), gate->flags);
+		return 0;
+	}
+
+	intel_set_clk_val(gate->map, reg, gate->shift, 1, 1);
+
+	return 0;
+}
+
+static void
+intel_clk_gate_disable(struct clk_hw *hw)
+{
+	struct intel_clk_gate *gate = to_intel_clk_gate(hw);
+	unsigned int reg;
+	unsigned int set;
+
+	if (gate->flags & GATE_CLK_VT) {
+		gate->reg = 0;
+		return;
+	}
+
+	if (gate->flags & GATE_CLK_HW) {
+		reg = GATE_HW_REG_DIS(gate->reg);
+		set = 1;
+	} else if (gate->flags & GATE_CLK_SW) {
+		reg = gate->reg;
+		set = 0;
+	} else {
+		pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
+		       __func__, clk_hw_get_name(hw), gate->flags);
+		return;
+	}
+
+	intel_set_clk_val(gate->map, reg, gate->shift, 1, set);
+}
+
+static int
+intel_clk_gate_is_enabled(struct clk_hw *hw)
+{
+	struct intel_clk_gate *gate = to_intel_clk_gate(hw);
+	unsigned int reg;
+
+	if (gate->flags & GATE_CLK_VT)
+		return gate->reg;
+
+	if (gate->flags & GATE_CLK_HW) {
+		reg = GATE_HW_REG_STAT(gate->reg);
+	} else if (gate->flags & GATE_CLK_SW) {
+		reg = gate->reg;
+	} else {
+		pr_err("%s: gate clk: %s: flag 0x%lx not supported!\n",
+		       __func__, clk_hw_get_name(hw), gate->flags);
+		return 0;
+	}
+
+	return intel_get_clk_val(gate->map, reg, gate->shift, 1);
+}
+
+const static struct clk_ops intel_clk_gate_ops = {
+	.enable = intel_clk_gate_enable,
+	.disable = intel_clk_gate_disable,
+	.is_enabled = intel_clk_gate_is_enabled,
+};
+
+static struct clk
+*intel_clk_register_gate(struct intel_clk_provider *ctx,
+			 struct intel_clk_branch *list)
+{
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	struct intel_clk_gate *gate;
+	u32 reg = list->gate_off;
+	u8 shift = list->gate_shift;
+	unsigned long cflags = list->gate_flags;
+	const char *pname = list->parent_names[0];
+	int ret;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = list->name;
+	init.ops = &intel_clk_gate_ops;
+	init.flags = list->flags | CLK_IS_BASIC;
+	init.parent_names = pname ? &pname : NULL;
+	init.num_parents = pname ? 1 : 0;
+
+	gate->map	= ctx->map;
+	gate->reg	= reg;
+	gate->shift	= shift;
+	gate->flags	= cflags;
+	gate->hw.init	= &init;
+
+	hw = &gate->hw;
+	ret = clk_hw_register(NULL, hw);
+	if (ret) {
+		kfree(gate);
+		return ERR_PTR(ret);
+	}
+
+	if (cflags & CLOCK_FLAG_VAL_INIT)
+		intel_set_clk_val(ctx->map, reg, shift, 1, list->gate_val);
+
+	return hw->clk;
+}
+
+void intel_clk_register_branches(struct intel_clk_provider *ctx,
+				 struct intel_clk_branch *list,
+				 unsigned int nr_clk)
+{
+	struct clk *clk;
+	unsigned int idx;
+
+	for (idx = 0; idx < nr_clk; idx++, list++) {
+		switch (list->type) {
+		case intel_clk_fixed:
+			clk = intel_clk_register_fixed(ctx, list);
+			break;
+		case intel_clk_mux:
+			clk = intel_clk_register_mux(ctx, list);
+			break;
+		case intel_clk_divider:
+			clk = intel_clk_register_divider(ctx, list);
+			break;
+		case intel_clk_fixed_factor:
+			clk = intel_clk_register_fixed_factor(ctx, list);
+			break;
+		case intel_clk_gate:
+			clk = intel_clk_register_gate(ctx, list);
+			break;
+		default:
+			pr_err("%s: type: %u not supported!\n",
+			       __func__, list->type);
+			return;
+		}
+
+		if (IS_ERR(clk)) {
+			pr_err("%s: register clk: %s, type: %u failed!\n",
+			       __func__, list->name, list->type);
+			return;
+		}
+
+		intel_clk_add_lookup(ctx, clk, list->id);
+	}
+}
+
+struct intel_clk_provider * __init
+intel_clk_init(struct device_node *np, struct regmap *map, unsigned int nr_clks)
+{
+	struct intel_clk_provider *ctx;
+	struct clk **clks;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	clks = kcalloc(nr_clks, sizeof(*clks), GFP_KERNEL);
+	if (!clks) {
+		kfree(ctx);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	memset_p((void **)clks, ERR_PTR(-ENOENT), nr_clks);
+	ctx->map = map;
+	ctx->clk_data.clks = clks;
+	ctx->clk_data.clk_num = nr_clks;
+	ctx->np = np;
+
+	return ctx;
+}
+
+void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
+				   struct intel_osc_clk *osc,
+				   unsigned int nr_clks)
+{
+	u32 freq;
+	struct clk *clk;
+	int idx;
+
+	for (idx = 0; idx < nr_clks; idx++, osc++) {
+		if (!osc->dt_freq ||
+		    of_property_read_u32(ctx->np, osc->dt_freq, &freq))
+			freq = osc->def_rate;
+
+		clk = clk_register_fixed_rate(NULL, osc->name, NULL, 0, freq);
+		if (IS_ERR(clk)) {
+			pr_err("%s: Failed to register clock: %s\n",
+			       __func__, osc->name);
+			return;
+		}
+
+		intel_clk_add_lookup(ctx, clk, osc->id);
+	}
+}
diff --git a/drivers/clk/intel/clk-cgu.h b/drivers/clk/intel/clk-cgu.h
new file mode 100644
index 000000000000..6dc4e45fc499
--- /dev/null
+++ b/drivers/clk/intel/clk-cgu.h
@@ -0,0 +1,259 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Copyright(c) 2018 Intel Corporation.
+ *  Zhu YiXin <Yixin.zhu@intel.com>
+ */
+
+#ifndef __INTEL_CLK_H
+#define __INTEL_CLK_H
+
+#define PNAME(x) static const char *const x[] __initconst
+
+struct intel_clk_mux {
+	struct clk_hw	hw;
+	struct regmap	*map;
+	unsigned int	reg;
+	u8		shift;
+	u8		width;
+	unsigned long	flags;
+};
+
+struct intel_clk_divider {
+	struct clk_hw	hw;
+	struct regmap	*map;
+	unsigned int	reg;
+	u8		shift;
+	u8		width;
+	unsigned long	flags;
+	const struct clk_div_table	*table;
+};
+
+struct intel_clk_gate {
+	struct clk_hw	hw;
+	struct regmap	*map;
+	unsigned int	reg;
+	u8		shift;
+	unsigned long	flags;
+};
+
+enum intel_clk_type {
+	intel_clk_fixed,
+	intel_clk_mux,
+	intel_clk_divider,
+	intel_clk_fixed_factor,
+	intel_clk_gate,
+};
+
+/**
+ * struct intel_clk_provider
+ * @map: regmap type base address for register.
+ * @np: device node
+ * @clk_data: array of hw clocks and clk number.
+ */
+struct intel_clk_provider {
+	struct regmap		*map;
+	struct device_node	*np;
+	struct clk_onecell_data	clk_data;
+};
+
+/**
+ * struct intel_pll_clk
+ * @id: plaform specific id of the clock.
+ * @name: name of this pll clock.
+ * @parent_names: name of the parent clock.
+ * @num_parents: number of parents.
+ * @flags: optional flags for basic clock.
+ * @type: platform type of pll.
+ * @reg: offset of the register.
+ * @mult: init value of mulitplier.
+ * @div: init value of divider.
+ * @frac: init value of fraction.
+ * @rate_table: table of pll clock rate.
+ */
+struct intel_pll_clk {
+	unsigned int		id;
+	const char		*name;
+	const char		*const *parent_names;
+	u8			num_parents;
+	unsigned long		flags;
+	enum intel_pll_type	type;
+	int			reg;
+	unsigned int		mult;
+	unsigned int		div;
+	unsigned int		frac;
+	const struct intel_pll_rate_table *rate_table;
+};
+
+#define INTEL_PLL(_id, _type, _name, _pnames, _flags,	\
+	    _reg, _rtable, _mult, _div, _frac)		\
+	{						\
+		.id		= _id,			\
+		.type		= _type,		\
+		.name		= _name,		\
+		.parent_names	= _pnames,		\
+		.num_parents	= ARRAY_SIZE(_pnames),	\
+		.flags		= _flags,		\
+		.reg		= _reg,			\
+		.rate_table	= _rtable,		\
+		.mult		= _mult,		\
+		.div		= _div,			\
+		.frac		= _frac			\
+	}
+
+/**
+ * struct intel_osc_clk
+ * @id: platform specific id of the clock.
+ * @name: name of the osc clock.
+ * @dt_freq: frequency node name in device tree.
+ * @def_rate: default rate of the osc clock.
+ * @flags: optional flags for basic clock.
+ */
+struct intel_osc_clk {
+	unsigned int		id;
+	const char		*name;
+	const char		*dt_freq;
+	const u32		def_rate;
+};
+
+#define INTEL_OSC(_id, _name, _freq, _rate)			\
+	{						\
+		.id		= _id,			\
+		.name		= _name,		\
+		.dt_freq	= _freq,		\
+		.def_rate	= _rate,		\
+	}
+
+struct intel_clk_branch {
+	unsigned int			id;
+	enum intel_clk_type		type;
+	const char			*name;
+	const char			*const *parent_names;
+	u8				num_parents;
+	unsigned long			flags;
+	unsigned int			mux_off;
+	u8				mux_shift;
+	u8				mux_width;
+	unsigned long			mux_flags;
+	unsigned int			mux_val;
+	unsigned int			div_off;
+	u8				div_shift;
+	u8				div_width;
+	unsigned long			div_flags;
+	unsigned int			div_val;
+	const struct clk_div_table	*div_table;
+	unsigned int			gate_off;
+	u8				gate_shift;
+	unsigned long			gate_flags;
+	unsigned int			gate_val;
+	unsigned int			mult;
+	unsigned int			div;
+};
+
+/* clock flags definition */
+#define CLOCK_FLAG_VAL_INIT	BIT(16)
+#define GATE_CLK_HW		BIT(17)
+#define GATE_CLK_SW		BIT(18)
+#define GATE_CLK_VT		BIT(19)
+
+#define INTEL_MUX(_id, _name, _pname, _f, _reg,			\
+	    _shift, _width, _cf, _v)				\
+	{							\
+		.id		= _id,				\
+		.type		= intel_clk_mux,		\
+		.name		= _name,			\
+		.parent_names	= _pname,			\
+		.num_parents	= ARRAY_SIZE(_pname),		\
+		.flags		= _f,				\
+		.mux_off	= _reg,				\
+		.mux_shift	= _shift,			\
+		.mux_width	= _width,			\
+		.mux_flags	= _cf,				\
+		.mux_val	= _v,				\
+	}
+
+#define INTEL_DIV(_id, _name, _pname, _f, _reg,			\
+	    _shift, _width, _cf, _v, _dtable)			\
+	{							\
+		.id		= _id,				\
+		.type		= intel_clk_divider,		\
+		.name		= _name,			\
+		.parent_names	= (const char *[]) { _pname },	\
+		.num_parents	= 1,				\
+		.flags		= _f,				\
+		.div_off	= _reg,				\
+		.div_shift	= _shift,			\
+		.div_width	= _width,			\
+		.div_flags	= _cf,				\
+		.div_val	= _v,				\
+		.div_table	= _dtable,			\
+	}
+
+#define INTEL_GATE(_id, _name, _pname, _f, _reg,		\
+	     _shift, _cf, _v)					\
+	{							\
+		.id		= _id,				\
+		.type		= intel_clk_gate,		\
+		.name		= _name,			\
+		.parent_names	= (const char *[]) { _pname },	\
+		.num_parents	= !_pname ? 0 : 1,		\
+		.flags		= _f,				\
+		.gate_off	= _reg,				\
+		.gate_shift	= _shift,			\
+		.gate_flags	= _cf,				\
+		.gate_val	= _v,				\
+	}
+
+#define INTEL_FIXED(_id, _name, _pname, _f, _reg,		\
+	      _shift, _width, _cf, _freq, _v)			\
+	{							\
+		.id		= _id,				\
+		.type		= intel_clk_fixed,		\
+		.name		= _name,			\
+		.parent_names	= (const char *[]) { _pname },	\
+		.num_parents	= !_pname ? 0 : 1,		\
+		.flags		= _f,				\
+		.div_off	= _reg,				\
+		.div_shift	= _shift,			\
+		.div_width	= _width,			\
+		.div_flags	= _cf,				\
+		.div_val	= _v,				\
+		.mux_flags	= _freq,			\
+	}
+
+#define INTEL_FIXED_FACTOR(_id, _name, _pname, _f, _reg,	\
+	       _shift, _width, _cf, _v, _m, _d)			\
+	{							\
+		.id		= _id,				\
+		.type		= intel_clk_fixed_factor,	\
+		.name		= _name,			\
+		.parent_names	= (const char *[]) { _pname },	\
+		.num_parents	= 1,				\
+		.flags		= _f,				\
+		.div_off	= _reg,				\
+		.div_shift	= _shift,			\
+		.div_width	= _width,			\
+		.div_flags	= _cf,				\
+		.div_val	= _v,				\
+		.mult		= _m,				\
+		.div		= _d,				\
+	}
+
+void intel_set_clk_val(struct regmap *map, u32 reg, u8 shift,
+		       u8 width, u32 set_val);
+u32 intel_get_clk_val(struct regmap *map, u32 reg, u8 shift, u8 width);
+void intel_clk_add_lookup(struct intel_clk_provider *ctx,
+			  struct clk *clk, unsigned int id);
+void __init intel_clk_of_add_provider(struct device_node *np,
+				      struct intel_clk_provider *ctx);
+struct intel_clk_provider * __init
+intel_clk_init(struct device_node *np, struct regmap *map,
+	       unsigned int nr_clks);
+void __init intel_clk_register_osc(struct intel_clk_provider *ctx,
+				   struct intel_osc_clk *osc,
+				   unsigned int nr_clks);
+void intel_clk_register_branches(struct intel_clk_provider *ctx,
+				 struct intel_clk_branch *list,
+				 unsigned int nr_clk);
+void intel_clk_register_plls(struct intel_clk_provider *ctx,
+			     struct intel_pll_clk *list, unsigned int nr_clk);
+#endif /* __INTEL_CLK_H */
diff --git a/drivers/clk/intel/clk-grx500.c b/drivers/clk/intel/clk-grx500.c
new file mode 100644
index 000000000000..5c2546f82579
--- /dev/null
+++ b/drivers/clk/intel/clk-grx500.c
@@ -0,0 +1,168 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Copyright (C) 2018 Intel Corporation.
+ *  Zhu YiXin <Yixin.zhu@intel.com>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+#include <dt-bindings/clock/intel,grx500-clk.h>
+
+#include "clk-cgu-pll.h"
+#include "clk-cgu.h"
+
+#define PLL_DIV_WIDTH		4
+
+/* Gate1 clock shift */
+#define G_VCODEC_SHIFT		2
+#define G_DMA0_SHIFT		5
+#define G_USB0_SHIFT		6
+#define G_SPI1_SHIFT		7
+#define G_SPI0_SHIFT		8
+#define G_CBM_SHIFT		9
+#define G_EBU_SHIFT		10
+#define G_SSO_SHIFT		11
+#define G_GPTC0_SHIFT		12
+#define G_GPTC1_SHIFT		13
+#define G_GPTC2_SHIFT		14
+#define G_UART_SHIFT		17
+#define G_CPYTO_SHIFT		20
+#define G_SECPT_SHIFT		21
+#define G_TOE_SHIFT		22
+#define G_MPE_SHIFT		23
+#define G_TDM_SHIFT		25
+#define G_PAE_SHIFT		26
+#define G_USB1_SHIFT		27
+#define G_SWITCH_SHIFT		28
+
+/* Gate2 clock shift */
+#define G_PCIE0_SHIFT		1
+#define G_PCIE1_SHIFT		17
+#define G_PCIE2_SHIFT		25
+
+/* Register definition */
+#define GRX500_PLL0A_CFG0	0x0004
+#define GRX500_PLL0A_CFG1	0x0008
+#define GRX500_PLL0B_CFG0	0x0034
+#define GRX500_PLL0B_CFG1	0x0038
+#define GRX500_LCPLL_CFG0	0x0094
+#define GRX500_LCPLL_CFG1	0x0098
+#define GRX500_IF_CLK		0x00c4
+#define GRX500_CLK_GSR1		0x0120
+#define GRX500_CLK_GSR2		0x0130
+
+static const struct clk_div_table pll_div[] = {
+	{1,	2},
+	{2,	3},
+	{3,	4},
+	{4,	5},
+	{5,	6},
+	{6,	8},
+	{7,	10},
+	{8,	12},
+	{9,	16},
+	{10,	20},
+	{11,	24},
+	{12,	32},
+	{13,	40},
+	{14,	48},
+	{15,	64}
+};
+
+enum grx500_plls {
+	pll0a, pll0b, pll3,
+};
+
+PNAME(pll_p)	= { "osc" };
+PNAME(cpu_p)	= { "cpu0", "cpu1" };
+
+static struct intel_osc_clk grx500_osc_clks[] __initdata = {
+	INTEL_OSC(CLK_OSC, "osc", "intel,osc-frequency", 40000000),
+};
+
+static struct intel_pll_clk grx500_pll_clks[] __initdata = {
+	[pll0a] = INTEL_PLL(CLK_PLL0A, pll_grx500, "pll0a",
+		      pll_p, 0, GRX500_PLL0A_CFG0, NULL, 0, 0, 0),
+	[pll0b] = INTEL_PLL(CLK_PLL0B, pll_grx500, "pll0b",
+		      pll_p, 0, GRX500_PLL0B_CFG0, NULL, 0, 0, 0),
+	[pll3] = INTEL_PLL(CLK_PLL3, pll_grx500, "pll3",
+		     pll_p, 0, GRX500_LCPLL_CFG0, NULL, 0, 0, 0),
+};
+
+static struct intel_clk_branch grx500_branch_clks[] __initdata = {
+	INTEL_DIV(CLK_CBM, "cbm", "pll0a", 0, GRX500_PLL0A_CFG1,
+		  0, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_NGI, "ngi", "pll0a", 0, GRX500_PLL0A_CFG1,
+		  4, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_SSX4, "ssx4", "pll0a", 0, GRX500_PLL0A_CFG1,
+		  8, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_CPU0, "cpu0", "pll0a", 0, GRX500_PLL0A_CFG1,
+		  12, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_PAE, "pae", "pll0b", 0, GRX500_PLL0B_CFG1,
+		  0, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_GSWIP, "gswip", "pll0b", 0, GRX500_PLL0B_CFG1,
+		  4, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_DDR, "ddr", "pll0b", 0, GRX500_PLL0B_CFG1,
+		  8, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_DIV(CLK_CPU1, "cpu1", "pll0b", 0, GRX500_PLL0B_CFG1,
+		  12, PLL_DIV_WIDTH, 0, 0, pll_div),
+	INTEL_MUX(CLK_CPU, "cpu", cpu_p, CLK_SET_RATE_PARENT,
+		  GRX500_PLL0A_CFG1, 29, 1, 0, 0),
+	INTEL_GATE(GCLK_DMA0, "g_dma0", NULL, 0, GRX500_CLK_GSR1,
+		   G_DMA0_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_USB0, "g_usb0", NULL, 0, GRX500_CLK_GSR1,
+		   G_USB0_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_GPTC0, "g_gptc0", NULL, 0, GRX500_CLK_GSR1,
+		   G_GPTC0_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_GPTC1, "g_gptc1", NULL, 0, GRX500_CLK_GSR1,
+		   G_GPTC1_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_GPTC2, "g_gptc2", NULL, 0, GRX500_CLK_GSR1,
+		   G_GPTC2_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_UART, "g_uart", NULL, 0, GRX500_CLK_GSR1,
+		   G_UART_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_PCIE0, "g_pcie0", NULL, 0, GRX500_CLK_GSR2,
+		   G_PCIE0_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_PCIE1, "g_pcie1", NULL, 0, GRX500_CLK_GSR2,
+		   G_PCIE1_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_PCIE2, "g_pcie2", NULL, 0, GRX500_CLK_GSR2,
+		   G_PCIE2_SHIFT, GATE_CLK_HW, 0),
+	INTEL_GATE(GCLK_I2C, "g_i2c", NULL, 0, 0, 0, GATE_CLK_VT, 0),
+	INTEL_FIXED(CLK_VOICE, "voice", NULL, 0, GRX500_IF_CLK, 14, 2,
+		    CLOCK_FLAG_VAL_INIT, 8192000, 2),
+	INTEL_FIXED_FACTOR(CLK_DDRPHY, "ddrphy", "ddr", 0, 0, 0,
+			   0, 0, 0, 2, 1),
+	INTEL_FIXED_FACTOR(CLK_PCIE, "pcie", "pll3", 0, 0, 0,
+			   0, 0, 0, 1, 40),
+};
+
+static void __init grx500_clk_init(struct device_node *np)
+{
+	struct intel_clk_provider *ctx;
+	struct regmap *map;
+
+	map = syscon_node_to_regmap(np);
+	if (IS_ERR(map))
+		return;
+
+	ctx = intel_clk_init(np, map, CLK_NR_CLKS);
+	if (IS_ERR(ctx)) {
+		regmap_exit(map);
+		return;
+	}
+
+	intel_clk_register_osc(ctx, grx500_osc_clks,
+			       ARRAY_SIZE(grx500_osc_clks));
+	intel_clk_register_plls(ctx, grx500_pll_clks,
+				ARRAY_SIZE(grx500_pll_clks));
+	intel_clk_register_branches(ctx, grx500_branch_clks,
+				    ARRAY_SIZE(grx500_branch_clks));
+	of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
+
+	pr_debug("%s clk init done!\n", __func__);
+}
+
+CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
diff --git a/include/dt-bindings/clock/intel,grx500-clk.h b/include/dt-bindings/clock/intel,grx500-clk.h
new file mode 100644
index 000000000000..eb7daecea7dc
--- /dev/null
+++ b/include/dt-bindings/clock/intel,grx500-clk.h
@@ -0,0 +1,69 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Copyright (C) 2018 Intel Corporation.
+ *  Zhu YiXin <Yixin.zhu@intel.com>
+ *
+ */
+
+#ifndef __INTEL_GRX500_CLK_H
+#define __INTEL_GRX500_CLK_H
+
+/* OSC clock */
+#define CLK_OSC			1
+
+/* PLL clocks */
+#define CLK_PLL0A		2
+#define CLK_PLL0B		3
+#define CLK_PLL3		4
+
+/* clocks under pll0a */
+#define CLK_CBM			11
+#define CLK_NGI			12
+#define CLK_SSX4		13
+#define CLK_CPU0		14
+
+/* clocks under pll0b */
+#define CLK_PAE			21
+#define CLK_GSWIP		22
+#define CLK_DDR			23
+#define CLK_CPU1		24
+
+/* clocks under pll3 */
+#define CLK_PCIE		33
+
+/* clocks under gate1 */
+#define GCLK_V_CODEC		106
+#define GCLK_DMA0		107
+#define GCLK_USB0		108
+#define GCLK_SPI1		109
+#define GCLK_SPI0		110
+#define GCLK_CBM		111
+#define GCLK_EBU		112
+#define GCLK_SSO		113
+#define GCLK_GPTC0		114
+#define GCLK_GPTC1		115
+#define GCLK_GPTC2		116
+#define GCLK_UART		117
+#define GCLK_EIP97		118
+#define GCLK_EIP123		119
+#define GCLK_TOE		120
+#define GCLK_MPE		121
+#define GCLK_TDM		122
+#define GCLK_PAE		123
+#define GCLK_USB1		124
+#define GCLK_GSWIP		125
+
+/* clocks under gate2 */
+#define GCLK_PCIE0		126
+#define GCLK_PCIE1		127
+#define GCLK_PCIE2		128
+
+/* other clocks */
+#define CLK_CPU			150
+#define CLK_DDRPHY		151
+#define GCLK_I2C		152
+#define CLK_VOICE		153
+
+#define CLK_NR_CLKS		200
+
+#endif /* __INTEL_GRX500_CLK_H */