Hi Lin,

It looks good with only a few minor comments.

On 2016/6/3 17:55, Lin Huang wrote:

On new rockchip platform(rk3399 etc), there have dcf controller to do ddr frequency scaling, and this controller will implement in arm-trust-firmware. We add a special clock-type to handle that.

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• None

drivers/clk/rockchip/Makefile | 1 + drivers/clk/rockchip/clk-ddr.c | 147 +++++++++++++++++++++++++++++++++++++++++ drivers/clk/rockchip/clk.c | 9 +++ drivers/clk/rockchip/clk.h | 27 ++++++++ 4 files changed, 184 insertions(+) create mode 100644 drivers/clk/rockchip/clk-ddr.c

diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index f47a2fa..b5f2c8e 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -8,6 +8,7 @@ obj-y += clk-pll.o obj-y += clk-cpu.o obj-y += clk-inverter.o obj-y += clk-mmc-phase.o +obj-y += clk-ddr.o obj-$(CONFIG_RESET_CONTROLLER) += softrst.o

obj-y += clk-rk3036.o diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c new file mode 100644 index 0000000..5b6630d --- /dev/null +++ b/drivers/clk/rockchip/clk-ddr.c @@ -0,0 +1,147 @@ +/*

• • Copyright (c) 2016 Rockchip Electronics Co. Ltd.
• • Author: Lin Huang hl@rock-chips.com
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• */

+#include <linux/of.h> +#include <linux/slab.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include "clk.h"

alphabetical order would be better.

+struct rockchip_ddrclk {

• struct clk_hw hw;
• void __iomem *reg_base;
• int mux_offset;
• int mux_shift;
• int mux_width;
• int mux_flag;
• int div_shift;
• int div_width;
• int div_flag;
• spinlock_t *lock;

+};

+#define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw) +#define val_mask(width) ((1 << (width)) - 1)

+static int rockchip_ddrclk_set_rate(struct clk_hw *hw, unsigned long drate,

• 		    unsigned long prate)
  

+{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• unsigned long flags;
• spin_lock_irqsave(ddrclk->lock, flags);
• /* TODO: set ddr rate in bl31 */
• spin_unlock_irqrestore(ddrclk->lock, flags);

What do you wanna protect here?

• return 0;

+}

+static unsigned long +rockchip_ddrclk_recalc_rate(struct clk_hw *hw,

• 	    unsigned long parent_rate)
  

+{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• int val;
• val = clk_readl(ddrclk->reg_base +

• 	ddrclk->mux_offset) >> ddrclk->div_shift;
  

• val &= val_mask(ddrclk->div_width);
• return DIV_ROUND_UP_ULL((u64)parent_rate, val + 1);

+}

+static long clk_ddrclk_round_rate(struct clk_hw *hw, unsigned long rate,

• 		  unsigned long *prate)
  

+{

• return rate;

+}

+static u8 rockchip_ddrclk_get_parent(struct clk_hw *hw) +{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• int num_parents = clk_hw_get_num_parents(hw);
• u32 val;
• val = clk_readl(ddrclk->reg_base +

• 	ddrclk->mux_offset) >> ddrclk->mux_shift;
  

• val &= val_mask(ddrclk->mux_width);
• if (val >= num_parents)

• return -EINVAL;
  

• return val;

+}

+static const struct clk_ops rockchip_ddrclk_ops = {

• .recalc_rate = rockchip_ddrclk_recalc_rate,
• .set_rate = rockchip_ddrclk_set_rate,
• .round_rate = clk_ddrclk_round_rate,
• .get_parent = rockchip_ddrclk_get_parent,

+};

+struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,

• 			 const char *const *parent_names,
  

• 			 u8 num_parents, int mux_offset,
  

• 			 int mux_shift, int mux_width,
  

• 			 int mux_flag, int div_shift,
  

• 			 int div_width, int div_flag,
  

• 			 void __iomem *reg_base,
  

• 			 spinlock_t *lock)
  

+{

• struct rockchip_ddrclk *ddrclk;
• struct clk_init_data init;
• struct clk *clk;
• int ret;
• ddrclk = kzalloc(sizeof(*ddrclk), GFP_KERNEL);
• if (!ddrclk)

• return ERR_PTR(-ENOMEM);
  

• init.name = name;
• init.parent_names = parent_names;
• init.num_parents = num_parents;
• init.ops = &rockchip_ddrclk_ops;
• init.flags = flags;
• init.flags |= CLK_SET_RATE_NO_REPARENT;
• init.flags |= CLK_GET_RATE_NOCACHE;

can we combine these two?

• ddrclk->reg_base = reg_base;
• ddrclk->lock = lock;
• ddrclk->hw.init = &init;
• ddrclk->mux_offset = mux_offset;
• ddrclk->mux_shift = mux_shift;
• ddrclk->mux_width = mux_width;
• ddrclk->mux_flag = mux_flag;
• ddrclk->div_shift = div_shift;
• ddrclk->div_width = div_width;
• ddrclk->div_flag = div_flag;
• clk = clk_register(NULL, &ddrclk->hw);
• if (IS_ERR(clk)) {

• pr_err("%s: could not register ddrclk %s\n", __func__,	name);
  

• ret = PTR_ERR(clk);
  

Why do you need PTR_ERR and then convert back again.

• goto free_ddrclk;
  

• }
• return clk;

+free_ddrclk:

• kfree(ddrclk);
• return ERR_PTR(ret);

+} diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c index 7ffd134..6ac1aa5 100644 --- a/drivers/clk/rockchip/clk.c +++ b/drivers/clk/rockchip/clk.c @@ -484,6 +484,15 @@ void __init rockchip_clk_register_branches( list->gate_offset, list->gate_shift, list->gate_flags, flags, &ctx->lock); break;

• case branch_ddrc:
  

• 	clk = rockchip_clk_register_ddrclk(
  

• 		list->name, list->flags,
  

• 		list->parent_names, list->num_parents,
  

• 		list->muxdiv_offset, list->mux_shift,
  

• 		list->mux_width, list->mux_flags,
  

• 		list->div_shift, list->div_width,
  

• 		list->div_flags, ctx->reg_base, &ctx->lock);
  

• 	break;
  

  }

  /* none of the cases above matched */

diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h index 2194ffa..4033fe4 100644 --- a/drivers/clk/rockchip/clk.h +++ b/drivers/clk/rockchip/clk.h @@ -281,6 +281,13 @@ struct clk *rockchip_clk_register_mmc(const char *name, const char *const *parent_names, u8 num_parents, void __iomem *reg, int shift);

+struct clk *rockchip_clk_register_ddrclk(const char *name, int flags,

• 	const char *const *parent_names, u8 num_parents,
  

• 	int mux_offset, int mux_shift, int mux_width,
  

• 	int mux_flag, int div_shift, int div_width,
  

• 	int div_flag, void __iomem *reg_base,
  

• 	spinlock_t *lock);
  

#define ROCKCHIP_INVERTER_HIWORD_MASK BIT(0)

struct clk *rockchip_clk_register_inverter(const char *name, @@ -299,6 +306,7 @@ enum rockchip_clk_branch_type { branch_mmc, branch_inverter, branch_factor,

• branch_ddrc,

};

struct rockchip_clk_branch { @@ -488,6 +496,25 @@ struct rockchip_clk_branch { .child = ch, \ }

+#define COMPOSITE_DDRC(_id, cname, pnames, f, mo, ms, mw, mf, \

• 	 ds, dw, df)				\
  

it seems a duplication exept for branch_type.

• { \

• .id		= _id,				\
  

• .branch_type	= branch_ddrc,			\
  

• .name		= cname,			\
  

• .parent_names	= pnames,			\
  

• .num_parents	= ARRAY_SIZE(pnames),		\
  

• .flags		= f,				\
  

• .muxdiv_offset	= mo,				\
  

• .mux_shift	= ms,				\
  

• .mux_width	= mw,				\
  

• .mux_flags	= mf,				\
  

• .div_shift	= ds,				\
  

• .div_width	= dw,				\
  

• .div_flags	= df,				\
  

• .gate_offset	= -1,				\
  

• }

#define MUX(_id, cname, pnames, f, o, s, w, mf) \ { \ .id = _id, \

Am Freitag, 3. Juni 2016, 17:55:14 schrieb Lin Huang:

On new rockchip platform(rk3399 etc), there have dcf controller to do ddr frequency scaling, and this controller will implement in arm-trust-firmware. We add a special clock-type to handle that.

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• None

drivers/clk/rockchip/Makefile | 1 + drivers/clk/rockchip/clk-ddr.c | 147 +++++++++++++++++++++++++++++++++++++++++ drivers/clk/rockchip/clk.c | 9 +++ drivers/clk/rockchip/clk.h | 27 ++++++++ 4 files changed, 184 insertions(+) create mode 100644 drivers/clk/rockchip/clk-ddr.c

diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index f47a2fa..b5f2c8e 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -8,6 +8,7 @@ obj-y += clk-pll.o obj-y += clk-cpu.o obj-y += clk-inverter.o obj-y += clk-mmc-phase.o +obj-y += clk-ddr.o obj-$(CONFIG_RESET_CONTROLLER) += softrst.o

obj-y += clk-rk3036.o diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c new file mode 100644 index 0000000..5b6630d --- /dev/null +++ b/drivers/clk/rockchip/clk-ddr.c @@ -0,0 +1,147 @@ +/*

• • Copyright (c) 2016 Rockchip Electronics Co. Ltd.
• • Author: Lin Huang hl@rock-chips.com
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• */

+#include <linux/of.h> +#include <linux/slab.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include "clk.h"

+struct rockchip_ddrclk {

• struct clk_hw hw;
• void __iomem *reg_base;
• int mux_offset;
• int mux_shift;
• int mux_width;
• int mux_flag;
• int div_shift;
• int div_width;
• int div_flag;
• spinlock_t *lock;

+};

+#define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw) +#define val_mask(width) ((1 << (width)) - 1)

maybe use GENMASK?

+static int rockchip_ddrclk_set_rate(struct clk_hw *hw, unsigned long drate,

• 		    unsigned long prate)
  

+{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• unsigned long flags;
• spin_lock_irqsave(ddrclk->lock, flags);
• /* TODO: set ddr rate in bl31 */

I expect this interface to be in existence and merged into the main ATF first.

Right now the whole clock-type does nothing more than a simple COMPOSITE with added CLK_DIVIDER_READ_ONLY | CLK_MUX_READ_ONLY.

Also Mike is propably still working in the so called coordinated rate change for clocks needing special handling on rate changes, which might provide a second approach to this.

So please, first of all get the ATF-interface merged and meanwhile if you need to read the clock-rate, just use a regular composite, with the read-only flags.

• spin_unlock_irqrestore(ddrclk->lock, flags);
• return 0;

+}

+static unsigned long +rockchip_ddrclk_recalc_rate(struct clk_hw *hw,

• 	    unsigned long parent_rate)
  

+{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• int val;
• val = clk_readl(ddrclk->reg_base +

• 	ddrclk->mux_offset) >> ddrclk->div_shift;
  

• val &= val_mask(ddrclk->div_width);
• return DIV_ROUND_UP_ULL((u64)parent_rate, val + 1);

return divider_recalc_rate(...)

Heiko

Hi Heiko,

On 2016年06月03日 20:51, Heiko Stübner wrote:

Am Freitag, 3. Juni 2016, 17:55:14 schrieb Lin Huang:

...

On new rockchip platform(rk3399 etc), there have dcf controller to do ddr frequency scaling, and this controller will implement in arm-trust-firmware. We add a special clock-type to handle that.

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• None

  drivers/clk/rockchip/Makefile | 1 + drivers/clk/rockchip/clk-ddr.c | 147

+++++++++++++++++++++++++++++++++++++++++ drivers/clk/rockchip/clk.c | 9 +++ drivers/clk/rockchip/clk.h | 27 ++++++++ 4 files changed, 184 insertions(+) create mode 100644 drivers/clk/rockchip/clk-ddr.c

diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index f47a2fa..b5f2c8e 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -8,6 +8,7 @@ obj-y += clk-pll.o obj-y += clk-cpu.o obj-y += clk-inverter.o obj-y += clk-mmc-phase.o +obj-y += clk-ddr.o obj-$(CONFIG_RESET_CONTROLLER) += softrst.o

obj-y += clk-rk3036.o diff --git a/drivers/clk/rockchip/clk-ddr.c b/drivers/clk/rockchip/clk-ddr.c new file mode 100644 index 0000000..5b6630d --- /dev/null +++ b/drivers/clk/rockchip/clk-ddr.c @@ -0,0 +1,147 @@ +/*

• • Copyright (c) 2016 Rockchip Electronics Co. Ltd.
• • Author: Lin Huang hl@rock-chips.com
• • This program is free software; you can redistribute it and/or modify
• • it under the terms of the GNU General Public License as published by
• • the Free Software Foundation; either version 2 of the License, or
• • (at your option) any later version.
• • This program is distributed in the hope that it will be useful,
• • but WITHOUT ANY WARRANTY; without even the implied warranty of
• • MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
• • GNU General Public License for more details.
• */

+#include <linux/of.h> +#include <linux/slab.h> +#include <linux/io.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include "clk.h"

+struct rockchip_ddrclk {

• struct clk_hw hw;
• void __iomem *reg_base;
• int mux_offset;
• int mux_shift;
• int mux_width;
• int mux_flag;
• int div_shift;
• int div_width;
• int div_flag;
• spinlock_t *lock;

+};

+#define to_rockchip_ddrclk_hw(hw) container_of(hw, struct rockchip_ddrclk, hw) +#define val_mask(width) ((1 << (width)) - 1)

maybe use GENMASK?

Oh, yes, we can use it. Will fix next version.

...

+static int rockchip_ddrclk_set_rate(struct clk_hw *hw, unsigned long drate,

• 		    unsigned long prate)
  

+{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• unsigned long flags;
• spin_lock_irqsave(ddrclk->lock, flags);
• /* TODO: set ddr rate in bl31 */

I expect this interface to be in existence and merged into the main ATF first.

Right now the whole clock-type does nothing more than a simple COMPOSITE with added CLK_DIVIDER_READ_ONLY | CLK_MUX_READ_ONLY.

Also Mike is propably still working in the so called coordinated rate change for clocks needing special handling on rate changes, which might provide a second approach to this.

You mean there is a patch set can handle it now? Can you tell me the ID, I want to check it.

So please, first of all get the ATF-interface merged and meanwhile if you need to read the clock-rate, just use a regular composite, with the read-only flags.

My colleague are wroking on ATF code now, I agree with you, We can not merge this patch set before ATF-interface merged. And we do not need to add a new code if we only want to read ddr clock rate(we can get the ddr rate to read dpll), so i prefer to keep this function for now, and do review first, once the ATF code ready, we can merge soon it(i hope :-P ) .

...

• spin_unlock_irqrestore(ddrclk->lock, flags);
• return 0;

+}

+static unsigned long +rockchip_ddrclk_recalc_rate(struct clk_hw *hw,

• 	    unsigned long parent_rate)
  

+{

• struct rockchip_ddrclk *ddrclk = to_rockchip_ddrclk_hw(hw);
• int val;
• val = clk_readl(ddrclk->reg_base +

• 	ddrclk->mux_offset) >> ddrclk->div_shift;
  

• val &= val_mask(ddrclk->div_width);
• return DIV_ROUND_UP_ULL((u64)parent_rate, val + 1);

return divider_recalc_rate(...)

got it , thank you.

Heiko

Hi Lin

How about merge it into your patch#3.

On 2016/6/3 17:55, Lin Huang wrote:

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• None

include/dt-bindings/clock/rk3399-cru.h | 1 + 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/rk3399-cru.h b/include/dt-bindings/clock/rk3399-cru.h index 50a44cf..8a0f0442 100644 --- a/include/dt-bindings/clock/rk3399-cru.h +++ b/include/dt-bindings/clock/rk3399-cru.h @@ -131,6 +131,7 @@ #define SCLK_DPHY_RX0_CFG 165 #define SCLK_RMII_SRC 166 #define SCLK_PCIEPHY_REF100M 167 +#define SCLK_DDRCLK 168

#define DCLK_VOP0 180 #define DCLK_VOP1 181

在 2016/6/3 20:36, Heiko Stübner 写道:

Hi Shawn,

Am Freitag, 3. Juni 2016, 20:34:52 schrieb Shawn Lin:

...

How about merge it into your patch#3.

see comments from Doug and me on previous version.

clock-ids should always be separate patches, as we will need them in both clock and devicetree branches, so they must be in a separate branch shared between clock and dts branches.

Ah, I missed the previous version as I think it should be from non-version to v2 rather than from non-version to v1, which IIRC is from Doug. :)

Anyway, thanks for explaining this.

Heiko

...

On 2016/6/3 17:55, Lin Huang wrote:

...

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• None

include/dt-bindings/clock/rk3399-cru.h | 1 + 1 file changed, 1 insertion(+)

diff --git a/include/dt-bindings/clock/rk3399-cru.h b/include/dt-bindings/clock/rk3399-cru.h index 50a44cf..8a0f0442 100644 --- a/include/dt-bindings/clock/rk3399-cru.h +++ b/include/dt-bindings/clock/rk3399-cru.h @@ -131,6 +131,7 @@

#define SCLK_DPHY_RX0_CFG 165 #define SCLK_RMII_SRC 166 #define SCLK_PCIEPHY_REF100M 167

+#define SCLK_DDRCLK 168

#define DCLK_VOP0 180 #define DCLK_VOP1 181

---

Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip

Hi Heiko,

On 2016年06月03日 20:56, Heiko Stübner wrote:

Am Freitag, 3. Juni 2016, 17:55:16 schrieb Lin Huang:

...

add ddrc clock setting, so we can do ddr frequency scaling on rk3399 platform in future.

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• remove ddrc source CLK_IGNORE_UNUSED flag, Suggestion by Doug

• move clk_ddrc and clk_ddrc_dpll_src to critical, Suggestion by Doug

  drivers/clk/rockchip/clk-rk3399.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c index f1d8e44..29afb88 100644 --- a/drivers/clk/rockchip/clk-rk3399.c +++ b/drivers/clk/rockchip/clk-rk3399.c

[...]

...

@@ -1377,6 +1381,18 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = { COMPOSITE_NOMUX(0, "clk_test", "clk_test_pre", CLK_IGNORE_UNUSED, RK3368_CLKSEL_CON(58), 0, 5, DFLAGS, RK3368_CLKGATE_CON(13), 11, GFLAGS),

• /* ddrc */
• GATE(0, "clk_ddrc_lpll_src", "lpll", 0, RK3399_CLKGATE_CON(3),

•     0, GFLAGS),
  

• GATE(0, "clk_ddrc_bpll_src", "bpll", 0, RK3399_CLKGATE_CON(3),

•     1, GFLAGS),
  

• GATE(0, "clk_ddrc_dpll_src", "dpll", 0, RK3399_CLKGATE_CON(3),

•     2, GFLAGS),
  

• GATE(0, "clk_ddrc_gpll_src", "gpll", 0, RK3399_CLKGATE_CON(3),

•     3, GFLAGS),
  

• COMPOSITE_DDRC(SCLK_DDRCLK, "clk_ddrc", mux_ddrclk_p, 0,

•        RK3399_CLKSEL_CON(6), 4, 2, MFLAGS, 0, 3, DFLAGS),
  

  };

as said in the other patch, just make this a regular COMPOSITE_NOGATE with CLK_DIVIDER_READ_ONLY | CLK_MUX_READ_ONLY until that interface to the ATF exists and is approved.

That way you can still read back the clock rate without anything changing the clock-rate, but we don't need to add duplicate code for it.

...

static struct rockchip_clk_branch rk3399_clk_pmu_branches[] __initdata = { @@ -1487,6 +1503,10 @@ static const char *const rk3399_cru_critical_clocks[] __initconst = { "gpll_hclk_perilp1_src", "gpll_aclk_perilp0_src", "gpll_aclk_perihp_src",

• /* ddrc */
• "clk_ddrc_dpll_src",

Why does your clk_ddrc_dpll_src need a separate critical entry. Any code changing the clk_ddrc parent should make sure the new parent is enabled. (The clock-framework of course does this already).

Okay, thank you.

...

• "clk_ddrc", };

  static const char *const rk3399_pmucru_critical_clocks[] __initconst = {

Heiko

Hi Lin,

I add the some comment on below. If you modify it, You can add my acked-by tag. Looks good to me.

Acked-by: Chanwoo Choi cw00.choi@samsung.com

Also, I'd like you to add me to mail thread on next version because I'm supporter of devfreq-event.

On 2016년 06월 03일 18:55, Lin Huang wrote:

on rk3399 platform, there is dfi conroller can monitor ddr load, base on this result, we can do ddr freqency scaling.

Signed-off-by: Lin Huang hl@rock-chips.com

Changes in v1:

• NOne

drivers/devfreq/event/Kconfig | 7 + drivers/devfreq/event/Makefile | 1 + drivers/devfreq/event/rockchip-dfi.c | 265 +++++++++++++++++++++++++++++++++++ 3 files changed, 273 insertions(+) create mode 100644 drivers/devfreq/event/rockchip-dfi.c

diff --git a/drivers/devfreq/event/Kconfig b/drivers/devfreq/event/Kconfig index 1e8b4f4..6ebdc13 100644 --- a/drivers/devfreq/event/Kconfig +++ b/drivers/devfreq/event/Kconfig @@ -30,4 +30,11 @@ config DEVFREQ_EVENT_EXYNOS_PPMU (Platform Performance Monitoring Unit) counters to estimate the utilization of each module.

+config DEVFREQ_EVENT_ROCKCHIP_DFI

• bool "ROCKCHIP DFI DEVFREQ event Driver"
• depends on ARCH_ROCKCHIP
• help

•  This add the devfreq-event driver for Rockchip SoC. It provides DFI
  

You better to full name of 'DFI' abbreviation as following: - DFI (Dxxx Fxxx Ixxx)

•  driver to count ddr load.
  

endif # PM_DEVFREQ_EVENT diff --git a/drivers/devfreq/event/Makefile b/drivers/devfreq/event/Makefile index 3d6afd3..dda7090 100644 --- a/drivers/devfreq/event/Makefile +++ b/drivers/devfreq/event/Makefile @@ -2,3 +2,4 @@

obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_NOCP) += exynos-nocp.o obj-$(CONFIG_DEVFREQ_EVENT_EXYNOS_PPMU) += exynos-ppmu.o +obj-$(CONFIG_DEVFREQ_EVENT_ROCKCHIP_DFI) += rockchip-dfi.o diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c new file mode 100644 index 0000000..e3b020f9 --- /dev/null +++ b/drivers/devfreq/event/rockchip-dfi.c @@ -0,0 +1,265 @@ +/*

• • Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
• • Author: Lin Huang hl@rock-chips.com
• • This program is free software; you can redistribute it and/or modify it
• • under the terms and conditions of the GNU General Public License,
• • version 2, as published by the Free Software Foundation.
• • This program is distributed in the hope it will be useful, but WITHOUT
• • ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
• • FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
• • more details.
• */

+#include <linux/clk.h> +#include <linux/devfreq-event.h> +#include <linux/kernel.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/list.h> +#include <linux/of.h>

+#define RK3399_DMC_NUM_CH 2

+/* DDRMON_CTRL */ +#define DDRMON_CTRL 0x04 +#define CLR_DDRMON_CTRL (0x1f0000 << 0) +#define LPDDR4_EN (0x10001 << 4) +#define HARDWARE_EN (0x10001 << 3) +#define LPDDR3_EN (0x10001 << 2) +#define SOFTWARE_EN (0x10001 << 1) +#define TIME_CNT_EN (0x10001 << 0)

+#define DDRMON_CH0_COUNT_NUM 0x28 +#define DDRMON_CH0_DFI_ACCESS_NUM 0x2c +#define DDRMON_CH1_COUNT_NUM 0x3c +#define DDRMON_CH1_DFI_ACCESS_NUM 0x40

+/* pmu grf */ +#define PMUGRF_OS_REG2 0x308 +#define DDRTYPE_SHIFT 13 +#define DDRTYPE_MASK 7

+enum {

• DDR3 = 3,
• LPDDR3 = 6,
• LPDDR4 = 7,
• UNUSED = 0xFF

+};

+struct dmc_usage {

• u32 access;
• u32 total;

+};

+struct rockchip_dfi {

• struct devfreq_event_dev *edev;
• struct devfreq_event_desc *desc;
• struct dmc_usage ch_usage[RK3399_DMC_NUM_CH];
• struct device *dev;
• void __iomem *regs;
• struct regmap *regmap_pmu;
• struct clk *clk;

+};

+static void rockchip_dfi_start_hardware_counter(struct devfreq_event_dev *edev) +{

• struct rockchip_dfi *info = devfreq_event_get_drvdata(edev);
• void __iomem *dfi_regs = info->regs;
• u32 val;
• u32 ddr_type;
• /* get ddr type */
• regmap_read(info->regmap_pmu, PMUGRF_OS_REG2, &val);
• ddr_type = (val >> DDRTYPE_SHIFT) & DDRTYPE_MASK;
• /* clear DDRMON_CTRL setting */
• writel_relaxed(CLR_DDRMON_CTRL, dfi_regs + DDRMON_CTRL);
• /* set ddr type to dfi */
• if (ddr_type == LPDDR3)

• writel_relaxed(LPDDR3_EN, dfi_regs + DDRMON_CTRL);
  

• else if (ddr_type == LPDDR4)

• writel_relaxed(LPDDR4_EN, dfi_regs + DDRMON_CTRL);
  

• /* enable count, use software mode */
• writel_relaxed(SOFTWARE_EN, dfi_regs + DDRMON_CTRL);

+}

+static void rockchip_dfi_stop_hardware_counter(struct devfreq_event_dev *edev) +{

• struct rockchip_dfi *info = devfreq_event_get_drvdata(edev);
• void __iomem *dfi_regs = info->regs;
• u32 val;
• val = readl_relaxed(dfi_regs + DDRMON_CTRL);
• val &= ~SOFTWARE_EN;
• writel_relaxed(val, dfi_regs + DDRMON_CTRL);

+}

+static int rockchip_dfi_get_busier_ch(struct devfreq_event_dev *edev) +{

• struct rockchip_dfi *info = devfreq_event_get_drvdata(edev);
• u32 tmp, max = 0;
• u32 i, busier_ch = 0;
• void __iomem *dfi_regs = info->regs;
• rockchip_dfi_stop_hardware_counter(edev);
• /* Find out which channel is busier */
• for (i = 0; i < RK3399_DMC_NUM_CH; i++) {

• info->ch_usage[i].access = readl_relaxed(dfi_regs +
  

• 		DDRMON_CH0_DFI_ACCESS_NUM + i * 20);
  

• info->ch_usage[i].total = readl_relaxed(dfi_regs +
  

• 		DDRMON_CH0_COUNT_NUM + i * 20);
  

• tmp = info->ch_usage[i].access;
  

• if (tmp > max) {
  

• 	busier_ch = i;
  

• 	max = tmp;
  

• }
  

• }
• rockchip_dfi_start_hardware_counter(edev);
• return busier_ch;

+}

+static int rockchip_dfi_disable(struct devfreq_event_dev *edev) +{

• struct rockchip_dfi *info = devfreq_event_get_drvdata(edev);
• rockchip_dfi_stop_hardware_counter(edev);
• clk_disable(info->clk);

I prefer to change the function as following. I add the comment on below in probe() - clk_disable -> clk_disable_unprepare()

• return 0;

+}

+static int rockchip_dfi_enable(struct devfreq_event_dev *edev) +{

• struct rockchip_dfi *info = devfreq_event_get_drvdata(edev);
• int ret;
• ret = clk_enable(info->clk);

I prefer to change the function as following. I add the comment on below in probe() - clk_enable -> clk_prepare_enable()

• if (ret)

• return ret;
  

• rockchip_dfi_start_hardware_counter(edev);
• return 0;

+}

+static int rockchip_dfi_set_event(struct devfreq_event_dev *edev) +{

• return 0;

+}

+static int rockchip_dfi_get_event(struct devfreq_event_dev *edev,

• 		  struct devfreq_event_data *edata)
  

+{

• struct rockchip_dfi *info = devfreq_event_get_drvdata(edev);
• int busier_ch;
• busier_ch = rockchip_dfi_get_busier_ch(edev);
• edata->load_count = info->ch_usage[busier_ch].access;
• edata->total_count = info->ch_usage[busier_ch].total;
• return 0;

+}

+static const struct devfreq_event_ops rockchip_dfi_ops = {

• .disable = rockchip_dfi_disable,
• .enable = rockchip_dfi_enable,
• .get_event = rockchip_dfi_get_event,
• .set_event = rockchip_dfi_set_event,

+};

+static const struct of_device_id rockchip_dfi_id_match[] = {

• { .compatible = "rockchip,rk3399-dfi" },
• { },

+};

+static int rockchip_dfi_probe(struct platform_device *pdev) +{

• struct device *dev = &pdev->dev;
• struct rockchip_dfi *data;
• struct resource *res;
• struct devfreq_event_desc *desc;
• int ret;
• struct device_node *np = pdev->dev.of_node, *node;
• data = devm_kzalloc(dev, sizeof(struct rockchip_dfi), GFP_KERNEL);
• if (!data)

• return -ENOMEM;
  

• res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
• data->regs = devm_ioremap_resource(&pdev->dev, res);
• if (IS_ERR(data->regs))

• return PTR_ERR(data->regs);
  

• data->clk = devm_clk_get(dev, "pclk_ddr_mon");
• if (IS_ERR(data->clk)) {

• dev_err(dev, "Cannot get the clk dmc_clk\n");
  

• return PTR_ERR(data->clk);
  

• };
• /* try to find the optional reference to the pmu syscon */
• node = of_parse_phandle(np, "rockchip,pmu", 0);
• if (node) {

• data->regmap_pmu = syscon_node_to_regmap(node);
  

• if (IS_ERR(data->regmap_pmu))
  

• 	return PTR_ERR(data->regmap_pmu);
  

• }
• data->dev = dev;
• desc = devm_kzalloc(dev, sizeof(*desc), GFP_KERNEL);
• if (!desc)

• return -ENOMEM;
  

• desc->ops = &rockchip_dfi_ops;
• desc->driver_data = data;
• desc->name = np->name;
• data->desc = desc;
• data->edev = devm_devfreq_event_add_edev(&pdev->dev, desc);
• if (IS_ERR(data->edev)) {

• ret = PTR_ERR(data->edev);
  

• dev_err(&pdev->dev,
  

• 	"failed to add devfreq-event device\n");
  

• return ret;
  

You can simply return the PTR_ERR(data->edev) without 'ret' variable as following:

return PTR_ERR(data->edev);

• }
• ret = clk_prepare_enable(data->clk);
• if (ret) {

• dev_err(&pdev->dev, "failed to enable clk: %d\n", ret);
  

• clk_disable_unprepare(data->clk);
  

• return ret;
  

• }

The following two functions handle the clock. So, rockchip_dfi_probe() don't need to enable the clock. Just pass the role of clock control to the following functions. Because of calling the twice of enable function of clock, the usage count of clock is mismatch when disabling the clock. - rockchip_dfi_enable(struct devfreq_event_dev *edev) enable the clock. - rockchip_dfi_disable(struct devfreq_event_dev *edev) disable the clock.

• platform_set_drvdata(pdev, data);
• return 0;

+}

+static int rockchip_dfi_remove(struct platform_device *pdev) +{

• return 0;

+}

If the rockchip_dfi_remove() don't do anything, you can remove it

+static struct platform_driver rockchip_dfi_driver = {

• .probe = rockchip_dfi_probe,
• .remove = rockchip_dfi_remove,

ditto.

• .driver = {

• .name	= "rockchip-dfi",
  

• .of_match_table = rockchip_dfi_id_match,
  

• },

+}; +module_platform_driver(rockchip_dfi_driver);

+MODULE_LICENSE("GPL v2");

You need to add MODULE_AUTHOR("").

+MODULE_DESCRIPTION("Rockchip dfi driver");

Remove unneeded blank line

Regards, Chanwoo Choi