Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-05 Thread hl

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#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(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





--
Lin Huang




Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-05 Thread hl

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#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(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





--
Lin Huang




Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-05 Thread hl

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#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





--
Lin Huang




Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-05 Thread hl

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#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





--
Lin Huang




Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-05 Thread hl

Hi Shawn,

On 2016年06月03日 20:29, Shawn Lin wrote:

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include "clk.h"
+


alphabetical order would be better.

Okay, will fix next version, thank you.



+struct rockchip_ddrclk {
+struct clk_hwhw;
+void __iomem*reg_base;
+intmux_offset;
+intmux_shift;
+intmux_width;
+intmux_flag;
+intdiv_shift;
+intdiv_width;
+intdiv_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?

I am not sure for now, when i finish this function,
if there nothing need protect i will remove it. Thank you.



+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 = 

Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-05 Thread hl

Hi Shawn,

On 2016年06月03日 20:29, Shawn Lin wrote:

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include "clk.h"
+


alphabetical order would be better.

Okay, will fix next version, thank you.



+struct rockchip_ddrclk {
+struct clk_hwhw;
+void __iomem*reg_base;
+intmux_offset;
+intmux_shift;
+intmux_width;
+intmux_flag;
+intdiv_shift;
+intdiv_width;
+intdiv_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?

I am not sure for now, when i finish this function,
if there nothing need protect i will remove it. Thank you.



+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 = _ddrclk_ops;
+
+init.flags = flags;
+

Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-03 Thread Heiko Stübner
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 
> ---
> 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 000..5b6630d
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-ddr.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
> + * Author: Lin Huang 
> + *
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#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


Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-03 Thread Heiko Stübner
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 
> ---
> 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 000..5b6630d
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-ddr.c
> @@ -0,0 +1,147 @@
> +/*
> + * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
> + * Author: Lin Huang 
> + *
> + * 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 
> +#include 
> +#include 
> +#include 
> +#include 
> +#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


Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-03 Thread Shawn Lin

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#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 

Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk

2016-06-03 Thread Shawn Lin

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 
---
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 000..5b6630d
--- /dev/null
+++ b/drivers/clk/rockchip/clk-ddr.c
@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2016 Rockchip Electronics Co. Ltd.
+ * Author: Lin Huang 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#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;
+