Re: [RFC PATCH v1 1/6] rockchip: rockchip: add new clock-type for the ddrclk
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
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
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
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
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
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
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
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
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
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; +