[PATCH 0/7] clk: atlas7: a bind of fixes

[PATCH 0/7] clk: atlas7: a bind of fixes

[Barry Song]

Guo Zeng (5):
  clk: atlas7: add lost pwm unit clks
  clk: atlas7: export mux clks so that driver can use it
  clk: atlas7: fix the clock tree for bluetooth stuff
  clk: sirf: fix bit field and its root clk for coresight_tpiu
  clk: atlas7: fix pll missed divide NR in fraction mode

Yibo Cai (2):
  clk: sirf: fix integer overflow in dto rate calculation
  clk: atlas7: replace dto resolution magic number by macro

 drivers/clk/sirf/clk-atlas7.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

-- 
1.9.1

[PATCH 1/7] clk: atlas7: add lost pwm unit clks

[Barry Song]

From: Guo Zeng <Guo.Zeng@csr.com>

Signed-off-by: Guo Zeng <Guo.Zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index db8ab69..a92e7ea 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -1174,6 +1174,10 @@ static struct atlas7_unit_init_data unit_list[] __initdata = {
 	{ 135, "thbtm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 7, &leaf8_gate_lock },
 	{ 136, "btslow", "xinw_fixdiv_btslow", 0, SIRFSOC_CLKC_ROOT_CLK_EN1_SET, 25, &root1_gate_lock },
 	{ 137, "a7ca_btslow", "btslow", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 0, &leaf8_gate_lock },
+	{ 138 , "pwm_io", "io_mux", 0, SIRFSOC_CLKC_LEAF_CLK_EN0_SET, 0, &leaf0_gate_lock },
+	{ 139 , "pwm_xin", "xin", 0, SIRFSOC_CLKC_LEAF_CLK_EN0_SET, 1, &leaf0_gate_lock },
+	{ 140 , "pwm_xinw", "xinw", 0, SIRFSOC_CLKC_LEAF_CLK_EN0_SET, 2, &leaf0_gate_lock },
+	{ 141 , "thcgum_sys", "sys_mux", 0, SIRFSOC_CLKC_LEAF_CLK_EN0_SET, 3, &leaf0_gate_lock },
 };
 
 static struct clk *atlas7_clks[ARRAY_SIZE(unit_list)];
-- 
1.9.1

[PATCH 2/7] clk: atlas7: export mux clks so that driver can use it

[Barry Song]

From: Guo Zeng <guo.zeng@csr.com>

this patch makes mux clks can be referenced by device drivers.

Signed-off-by: Guo Zeng <guo.zeng@csr.com>
Signed-off-by: Barry Song <Barry.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index a92e7ea..d01dce3 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -1180,7 +1180,7 @@ static struct atlas7_unit_init_data unit_list[] __initdata = {
 	{ 141 , "thcgum_sys", "sys_mux", 0, SIRFSOC_CLKC_LEAF_CLK_EN0_SET, 3, &leaf0_gate_lock },
 };
 
-static struct clk *atlas7_clks[ARRAY_SIZE(unit_list)];
+static struct clk *atlas7_clks[ARRAY_SIZE(unit_list) + ARRAY_SIZE(mux_list)];
 
 static int unit_clk_is_enabled(struct clk_hw *hw)
 {
@@ -1613,6 +1613,7 @@ static void __init atlas7_clk_init(struct device_node *np)
 			       sirfsoc_clk_vbase + mux->mux_offset,
 			       mux->shift, mux->width,
 			       mux->mux_flags, NULL);
+		atlas7_clks[ARRAY_SIZE(unit_list) + i] = clk;
 		BUG_ON(!clk);
 	}
 
@@ -1624,7 +1625,7 @@ static void __init atlas7_clk_init(struct device_node *np)
 	}
 
 	clk_data.clks = atlas7_clks;
-	clk_data.clk_num = ARRAY_SIZE(unit_list);
+	clk_data.clk_num = ARRAY_SIZE(unit_list) + ARRAY_SIZE(mux_list);
 
 	ret = of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 	BUG_ON(ret);
-- 
1.9.1

[PATCH 2/7] clk: atlas7: export mux clks so that driver can use it

[Stephen Boyd]

On 07/27/2015 11:27 PM, Barry Song wrote:
> From: Guo Zeng <guo.zeng@csr.com>
>
> this patch makes mux clks can be referenced by device drivers.
>
> Signed-off-by: Guo Zeng <guo.zeng@csr.com>
> Signed-off-by: Barry Song <Barry.Song@csr.com>
> ---

The subject makes this sounds like we're exporting clocks to drivers via
extern, but that's not happening. Perhaps the subject could be clarified
to say something like "export mux clks so that consumers can get them"?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 2/7] clk: atlas7: export mux clks so that driver can use it

[Barry Song]

2015-07-29 5:56 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 07/27/2015 11:27 PM, Barry Song wrote:
>> From: Guo Zeng <guo.zeng@csr.com>
>>
>> this patch makes mux clks can be referenced by device drivers.
>>
>> Signed-off-by: Guo Zeng <guo.zeng@csr.com>
>> Signed-off-by: Barry Song <Barry.Song@csr.com>
>> ---
>
> The subject makes this sounds like we're exporting clocks to drivers via
> extern, but that's not happening. Perhaps the subject could be clarified
> to say something like "export mux clks so that consumers can get them"?
>

ok. sounds good.

-barry

[PATCH 3/7] clk: sirf: fix integer overflow in dto rate calculation

[Barry Song]

From: Yibo Cai <yibo.cai@csr.com>

I cannot believe that I spend quite a lot time in finding this bug.
It seems a pitfall people tend to fall in.

In "int64 = int32 * int32", conversion from 32-bits to 64-bits comes
after the multiplication. So this statement may not work as expected.

Signed-off-by: Yibo Cai <yibo.cai@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index d01dce3..cf489a5 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -519,7 +519,7 @@ static unsigned long dto_clk_recalc_rate(struct clk_hw *hw,
 static long dto_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long *parent_rate)
 {
-	u64 dividend = rate * (1 << 29);
+	u64 dividend = (u64)rate * (1 << 29);
 
 	do_div(dividend, *parent_rate);
 	dividend *= *parent_rate;
@@ -531,7 +531,7 @@ static long dto_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 static int dto_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long parent_rate)
 {
-	u64 dividend = rate * (1 << 29);
+	u64 dividend = (u64)rate * (1 << 29);
 	struct clk_dto *clk = to_dtoclk(hw);
 
 	do_div(dividend, parent_rate);
-- 
1.9.1

[PATCH 4/7] clk: atlas7: replace dto resolution magic number by macro

[Barry Song]

From: Yibo Cai <yibo.cai@csr.com>

Signed-off-by: Yibo Cai <yibo.cai@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index cf489a5..1000421 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -465,6 +465,9 @@ static struct clk_pll clk_sys3pll = {
  *  double resolution mode:fout = fin * finc / 2^29
  *  normal mode:fout = fin * finc / 2^28
  */
+#define DTO_RESL_DOUBLE	(1ULL << 29)
+#define DTO_RESL_NORMAL	(1ULL << 28)
+
 static int dto_clk_is_enabled(struct clk_hw *hw)
 {
 	struct clk_dto *clk = to_dtoclk(hw);
@@ -509,9 +512,9 @@ static unsigned long dto_clk_recalc_rate(struct clk_hw *hw,
 	rate *= finc;
 	if (droff & BIT(0))
 		/* Double resolution off */
-		do_div(rate, 1 << 28);
+		do_div(rate, DTO_RESL_NORMAL);
 	else
-		do_div(rate, 1 << 29);
+		do_div(rate, DTO_RESL_DOUBLE);
 
 	return rate;
 }
@@ -519,11 +522,11 @@ static unsigned long dto_clk_recalc_rate(struct clk_hw *hw,
 static long dto_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long *parent_rate)
 {
-	u64 dividend = (u64)rate * (1 << 29);
+	u64 dividend = rate * DTO_RESL_DOUBLE;
 
 	do_div(dividend, *parent_rate);
 	dividend *= *parent_rate;
-	do_div(dividend, 1 << 29);
+	do_div(dividend, DTO_RESL_DOUBLE);
 
 	return dividend;
 }
@@ -531,7 +534,7 @@ static long dto_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 static int dto_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long parent_rate)
 {
-	u64 dividend = (u64)rate * (1 << 29);
+	u64 dividend = rate * DTO_RESL_DOUBLE;
 	struct clk_dto *clk = to_dtoclk(hw);
 
 	do_div(dividend, parent_rate);
-- 
1.9.1

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Barry Song]

From: Guo Zeng <guo.zeng@csr.com>

normally dmac should depends on only one clock, to operate
dmac internal register, but dmac4 is very special case, it
normally dmac should depends on only one clock, to operate
dmac internal register, but dmac4 is very special case, it
depends on two additional clock, the reason is that dmac4
is wrapped in hw into bt a7ca module, and accessing dmac4
internal register would also require that the a7ca_io and
related bt macro io clk also enabled.
here workaround this by setting depend clk into parent of
dmac4, and also related clks, to reflect dependency.
noc_io
    -btm_noc_clk
            -a7ca_io
                    -dmac4_io
                                    -uart6_io
                                    -usp3_io

Signed-off-by: Guo Zeng <guo.zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index 1000421..c1788df 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -1169,10 +1169,10 @@ static struct atlas7_unit_init_data unit_list[] __initdata = {
 	{ 127, "vss_sdr", "gpum_sdr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 1, &leaf7_gate_lock },
 	{ 128, "thgpum_nocr", "gpum_nocr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 2, &leaf7_gate_lock },
 	{ 129, "a7ca_btss", "btm_btss", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 1, &leaf8_gate_lock },
-	{ 130, "dmac4_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 2, &leaf8_gate_lock },
-	{ 131, "uart6_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 3, &leaf8_gate_lock },
-	{ 132, "usp3_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 4, &leaf8_gate_lock },
-	{ 133, "a7ca_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 5, &leaf8_gate_lock },
+	{ 130 , "dmac4_io", "a7ca_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 2, &leaf8_gate_lock },
+	{ 131 , "uart6_io", "dmac4_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 3, &leaf8_gate_lock },
+	{ 132 , "usp3_io", "dmac4_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 4, &leaf8_gate_lock },
+	{ 133 , "a7ca_io", "noc_btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 5, &leaf8_gate_lock },
 	{ 134, "noc_btm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 6, &leaf8_gate_lock },
 	{ 135, "thbtm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 7, &leaf8_gate_lock },
 	{ 136, "btslow", "xinw_fixdiv_btslow", 0, SIRFSOC_CLKC_ROOT_CLK_EN1_SET, 25, &root1_gate_lock },
-- 
1.9.1

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Stephen Boyd]

On 07/27/2015 11:27 PM, Barry Song wrote:
> From: Guo Zeng <guo.zeng@csr.com>
>
> normally dmac should depends on only one clock, to operate
> dmac internal register, but dmac4 is very special case, it
> normally dmac should depends on only one clock, to operate
> dmac internal register, but dmac4 is very special case, it

The sentence is duplicated twice here

> depends on two additional clock, the reason is that dmac4
> is wrapped in hw into bt a7ca module, and accessing dmac4
> internal register would also require that the a7ca_io and
> related bt macro io clk also enabled.
> here workaround this by setting depend clk into parent of
> dmac4, and also related clks, to reflect dependency.

We don't put clocks as parents of other clocks just because the child
depends on the parent to be accessed. It isn't very clear from the
description, but it sounds like a7ca_io is just another clock that the
bluetooth driver needs to control? We want to express the true clk
hierarchy, not some psuedo dependency tree.

> noc_io
>     -btm_noc_clk
>             -a7ca_io
>                     -dmac4_io
>                                     -uart6_io
>                                     -usp3_io
>
> Signed-off-by: Guo Zeng <guo.zeng@csr.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  drivers/clk/sirf/clk-atlas7.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
> index 1000421..c1788df 100644
> --- a/drivers/clk/sirf/clk-atlas7.c
> +++ b/drivers/clk/sirf/clk-atlas7.c
> @@ -1169,10 +1169,10 @@ static struct atlas7_unit_init_data unit_list[] __initdata = {
>  	{ 127, "vss_sdr", "gpum_sdr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 1, &leaf7_gate_lock },
>  	{ 128, "thgpum_nocr", "gpum_nocr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 2, &leaf7_gate_lock },
>  	{ 129, "a7ca_btss", "btm_btss", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 1, &leaf8_gate_lock },
> -	{ 130, "dmac4_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 2, &leaf8_gate_lock },
> -	{ 131, "uart6_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 3, &leaf8_gate_lock },
> -	{ 132, "usp3_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 4, &leaf8_gate_lock },
> -	{ 133, "a7ca_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 5, &leaf8_gate_lock },
> +	{ 130 , "dmac4_io", "a7ca_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 2, &leaf8_gate_lock },
> +	{ 131 , "uart6_io", "dmac4_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 3, &leaf8_gate_lock },
> +	{ 132 , "usp3_io", "dmac4_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 4, &leaf8_gate_lock },
> +	{ 133 , "a7ca_io", "noc_btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 5, &leaf8_gate_lock },
>  	{ 134, "noc_btm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 6, &leaf8_gate_lock },
>  	{ 135, "thbtm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 7, &leaf8_gate_lock },
>  	{ 136, "btslow", "xinw_fixdiv_btslow", 0, SIRFSOC_CLKC_ROOT_CLK_EN1_SET, 25, &root1_gate_lock },


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Barry Song]

2015-07-29 6:07 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 07/27/2015 11:27 PM, Barry Song wrote:
>> From: Guo Zeng <guo.zeng@csr.com>
>>
>> normally dmac should depends on only one clock, to operate
>> dmac internal register, but dmac4 is very special case, it
>> normally dmac should depends on only one clock, to operate
>> dmac internal register, but dmac4 is very special case, it
>
> The sentence is duplicated twice here
>
>> depends on two additional clock, the reason is that dmac4
>> is wrapped in hw into bt a7ca module, and accessing dmac4
>> internal register would also require that the a7ca_io and
>> related bt macro io clk also enabled.
>> here workaround this by setting depend clk into parent of
>> dmac4, and also related clks, to reflect dependency.
>
> We don't put clocks as parents of other clocks just because the child
> depends on the parent to be accessed. It isn't very clear from the
> description, but it sounds like a7ca_io is just another clock that the
> bluetooth driver needs to control? We want to express the true clk
> hierarchy, not some psuedo dependency tree.
>

Stephen, generically i agree the codes should be a map of real
hardware connection. here the problem is real hardware connection is
very much difficult for its drivers implementation and these clocks
are buggy designed in HW. they are not a tree, they are a "network".

see attached diagram(hope LKML will not reject the attachment), we are
not a tree,  that means btm_noc_clk depends on btm_io_clk, a7ca_io_clk
depends on btm_io_clk, dmac4_io_clk depends on btm_noc_clk,
a7ca_io_clk and btm_noc_clk....
usp3_io_clk depends on uart6_io_clk, dmac4_io_clk, btm_noc_clk,
a7ca_io_clk and btm_noc_clk.

so in the driver, we have a choice as below:
in uart6 driver, we take all of the clocks.
in dmac4 driver, we take all of btm_noc_clk, a7ca_io_clk and btm_noc_clk.
...
what a ugly code! that is what we did before.

so here, we moved to do a not-real clock tree,  that means
we makes
1.btm_io_clk the parent of btm_noc_clk
2. btm_noc_clk the parent of a7ca_io_clk
3. a7ca_io_clk the parent of dmac4_io_clk
...

but it doesn't break any dependency of these clocks, and make all
clients driver as clean as a simple tree.
i strongly think we should keep this "workaround", otherwise, we have
many pain in all of the related drivers.

>> noc_io
>>     -btm_noc_clk
>>             -a7ca_io
>>                     -dmac4_io
>>                                     -uart6_io
>>                                     -usp3_io
>>
>> Signed-off-by: Guo Zeng <guo.zeng@csr.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  drivers/clk/sirf/clk-atlas7.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
>> index 1000421..c1788df 100644
>> --- a/drivers/clk/sirf/clk-atlas7.c
>> +++ b/drivers/clk/sirf/clk-atlas7.c
>> @@ -1169,10 +1169,10 @@ static struct atlas7_unit_init_data unit_list[] __initdata = {
>>       { 127, "vss_sdr", "gpum_sdr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 1, &leaf7_gate_lock },
>>       { 128, "thgpum_nocr", "gpum_nocr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 2, &leaf7_gate_lock },
>>       { 129, "a7ca_btss", "btm_btss", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 1, &leaf8_gate_lock },
>> -     { 130, "dmac4_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 2, &leaf8_gate_lock },
>> -     { 131, "uart6_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 3, &leaf8_gate_lock },
>> -     { 132, "usp3_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 4, &leaf8_gate_lock },
>> -     { 133, "a7ca_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 5, &leaf8_gate_lock },
>> +     { 130 , "dmac4_io", "a7ca_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 2, &leaf8_gate_lock },
>> +     { 131 , "uart6_io", "dmac4_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 3, &leaf8_gate_lock },
>> +     { 132 , "usp3_io", "dmac4_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 4, &leaf8_gate_lock },
>> +     { 133 , "a7ca_io", "noc_btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 5, &leaf8_gate_lock },
>>       { 134, "noc_btm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 6, &leaf8_gate_lock },
>>       { 135, "thbtm_io", "btm_io", 0, SIRFSOC_CLKC_LEAF_CLK_EN8_SET, 7, &leaf8_gate_lock },
>>       { 136, "btslow", "xinw_fixdiv_btslow", 0, SIRFSOC_CLKC_ROOT_CLK_EN1_SET, 25, &root1_gate_lock },
>

-barry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image001.png
Type: image/png
Size: 27774 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150729/73e9f8b5/attachment-0001.png>

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Stephen Boyd]

On 07/29/2015 07:52 AM, Barry Song wrote:
> 2015-07-29 6:07 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
>> On 07/27/2015 11:27 PM, Barry Song wrote:
>>> From: Guo Zeng <guo.zeng@csr.com>
>>>
>>> normally dmac should depends on only one clock, to operate
>>> dmac internal register, but dmac4 is very special case, it
>>> normally dmac should depends on only one clock, to operate
>>> dmac internal register, but dmac4 is very special case, it
>> The sentence is duplicated twice here
>>
>>> depends on two additional clock, the reason is that dmac4
>>> is wrapped in hw into bt a7ca module, and accessing dmac4
>>> internal register would also require that the a7ca_io and
>>> related bt macro io clk also enabled.
>>> here workaround this by setting depend clk into parent of
>>> dmac4, and also related clks, to reflect dependency.
>> We don't put clocks as parents of other clocks just because the child
>> depends on the parent to be accessed. It isn't very clear from the
>> description, but it sounds like a7ca_io is just another clock that the
>> bluetooth driver needs to control? We want to express the true clk
>> hierarchy, not some psuedo dependency tree.
>>
> Stephen, generically i agree the codes should be a map of real
> hardware connection. here the problem is real hardware connection is
> very much difficult for its drivers implementation and these clocks
> are buggy designed in HW. they are not a tree, they are a "network".
>
> see attached diagram(hope LKML will not reject the attachment), we are
> not a tree,  that means btm_noc_clk depends on btm_io_clk, a7ca_io_clk
> depends on btm_io_clk, dmac4_io_clk depends on btm_noc_clk,
> a7ca_io_clk and btm_noc_clk....
> usp3_io_clk depends on uart6_io_clk, dmac4_io_clk, btm_noc_clk,
> a7ca_io_clk and btm_noc_clk.
>
> so in the driver, we have a choice as below:
> in uart6 driver, we take all of the clocks.
> in dmac4 driver, we take all of btm_noc_clk, a7ca_io_clk and btm_noc_clk.
> ...
> what a ugly code! that is what we did before.
>
> so here, we moved to do a not-real clock tree,  that means
> we makes
> 1.btm_io_clk the parent of btm_noc_clk
> 2. btm_noc_clk the parent of a7ca_io_clk
> 3. a7ca_io_clk the parent of dmac4_io_clk
> ...
>
> but it doesn't break any dependency of these clocks, and make all
> clients driver as clean as a simple tree.
> i strongly think we should keep this "workaround", otherwise, we have
> many pain in all of the related drivers.
>
>

Thanks for the nice PNG. It seems like you have a typical bus topology 
where UART and USP need to go through DMAC and the Data NOC to get out 
on the system bus. btm_io_clk is the true parent of all the clocks.

I can only guess that A7CA is some sort of device that you have to go 
through from the CPU side to access the UART, USP, and DMAC hardware 
blocks. Is that right? Can you please describe what each of these blocks 
do and if there are linux drivers for them?

One way to model this would be to make UART and USP children of the DMAC 
hardware block. And then make a driver/device up for the NOC and have 
the DMAC be a child of the NOC device. Then the NOC would probe and be 
runtime PM enabled to turn on the btm_noc_clk whenever it or its 
children are enabled. Then the DMAC device would do the same thing, but 
with the dmac4_io_clk, and the UART and USP devices would get the 
uart6_io_clk and usp3_io_clk respectively and enable/disable them when 
the device is active.

That leaves us with the a7ca_io_clk, which is really just the clock for 
a bus that register read/writes go through. In designs I've seen, I 
usually left that clock under the control of each individual device that 
sits on the bus. So in this case, the DMAC, USP and UART device drivers 
would all need to get the a7ca_io_clk as their "interface" clock and 
make sure to enable or disable it whenever the driver wants to 
read/write the registers. I'm not sure if something else is done on the 
APB for you, but that may be all that's necessary.

This actually points out a big problem we have right now in Linux, which 
is the lack of proper power management for the type of bus topology you 
see in embedded systems. Some of the genpd work going on may also be 
another solution to this problem, where we could group multiple devices 
into generic power domains that know to turn on 2 or 3 of the clocks. 
For example, the DMAC, USP, and UART device could all be in the APB 
power domain so that the a7ca_io_clk is enabled whenever one of those 
devices is active. And there could be another power domain for the NOC 
that encompasses all devices that are sitting on that NOC.

Long story short, solving these sorts of problems in the clk framework 
is typically the easy route that people take, although it's the wrong 
route. We're not expressing these sorts of complicated bus topologies in 
the clk framework. We're only expressing the clk tree (which is already 
quite complicated enough). Your drivers can still be simple, but don't 
make it more complicated than it has to be in the clk framework for that.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Barry Song]

2015-08-06 9:44 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 07/29/2015 07:52 AM, Barry Song wrote:
>>
>> 2015-07-29 6:07 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
>>>
>>> On 07/27/2015 11:27 PM, Barry Song wrote:
>>>>
>>>> From: Guo Zeng <guo.zeng@csr.com>
>>>>
>>>> normally dmac should depends on only one clock, to operate
>>>> dmac internal register, but dmac4 is very special case, it
>>>> normally dmac should depends on only one clock, to operate
>>>> dmac internal register, but dmac4 is very special case, it
>>>
>>> The sentence is duplicated twice here
>>>
>>>> depends on two additional clock, the reason is that dmac4
>>>> is wrapped in hw into bt a7ca module, and accessing dmac4
>>>> internal register would also require that the a7ca_io and
>>>> related bt macro io clk also enabled.
>>>> here workaround this by setting depend clk into parent of
>>>> dmac4, and also related clks, to reflect dependency.
>>>
>>> We don't put clocks as parents of other clocks just because the child
>>> depends on the parent to be accessed. It isn't very clear from the
>>> description, but it sounds like a7ca_io is just another clock that the
>>> bluetooth driver needs to control? We want to express the true clk
>>> hierarchy, not some psuedo dependency tree.
>>>
>> Stephen, generically i agree the codes should be a map of real
>> hardware connection. here the problem is real hardware connection is
>> very much difficult for its drivers implementation and these clocks
>> are buggy designed in HW. they are not a tree, they are a "network".
>>
>> see attached diagram(hope LKML will not reject the attachment), we are
>> not a tree,  that means btm_noc_clk depends on btm_io_clk, a7ca_io_clk
>> depends on btm_io_clk, dmac4_io_clk depends on btm_noc_clk,
>> a7ca_io_clk and btm_noc_clk....
>> usp3_io_clk depends on uart6_io_clk, dmac4_io_clk, btm_noc_clk,
>> a7ca_io_clk and btm_noc_clk.
>>
>> so in the driver, we have a choice as below:
>> in uart6 driver, we take all of the clocks.
>> in dmac4 driver, we take all of btm_noc_clk, a7ca_io_clk and btm_noc_clk.
>> ...
>> what a ugly code! that is what we did before.
>>
>> so here, we moved to do a not-real clock tree,  that means
>> we makes
>> 1.btm_io_clk the parent of btm_noc_clk
>> 2. btm_noc_clk the parent of a7ca_io_clk
>> 3. a7ca_io_clk the parent of dmac4_io_clk
>> ...
>>
>> but it doesn't break any dependency of these clocks, and make all
>> clients driver as clean as a simple tree.
>> i strongly think we should keep this "workaround", otherwise, we have
>> many pain in all of the related drivers.
>>
>>
>
> Thanks for the nice PNG. It seems like you have a typical bus topology where

i feel very happy you can read this diagram carefully, then we can do
some deeper discussion.

> UART and USP need to go through DMAC and the Data NOC to get out on the
> system bus.

actually only uart6 and usp3 need to go through DMAC and the Data NOC
to get out on the
system bus. so you know how painful it is to change uart and usp
drivers to only put some
strange clocks for one node, but keep others nodes have normal clock tree?

the code might be as below:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=52bec4ed4ef83f1a14dbcfd1a97e35f77c6e261e

but after moving to this clock workaround we are talking about, we
reverted the above patch:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4b8038dca0c0ccf5e4689cc4fbbbf4f3728304be

we got a beautiful uart driver.

> btm_io_clk is the true parent of all the clocks

right, btm_io_clk is the true parent of all the clocks

>
> I can only guess that A7CA is some sort of device that you have to go
> through from the CPU side to access the UART, USP, and DMAC hardware blocks.
> Is that right? Can you please describe what each of these blocks do and if
> there are linux drivers for them?

a7ca is bluetooth module embedded in the chip.
uart, usp(can be a general serial port), dmac have linux drivers in:
1. drivers/tty/serial/sirfsoc_uart.c
2. drivers/dma/sirf-dma.c

>
> One way to model this would be to make UART and USP children of the DMAC
> hardware block. And then make a driver/device up for the NOC and have the
> DMAC be a child of the NOC device. Then the NOC would probe and be runtime
> PM enabled to turn on the btm_noc_clk whenever it or its children are
> enabled. Then the DMAC device would do the same thing, but with the
> dmac4_io_clk, and the UART and USP devices would get the uart6_io_clk and
> usp3_io_clk respectively and enable/disable them when the device is active.
>
> That leaves us with the a7ca_io_clk, which is really just the clock for a
> bus that register read/writes go through. In designs I've seen, I usually
> left that clock under the control of each individual device that sits on the
> bus. So in this case, the DMAC, USP and UART device drivers would all need
> to get the a7ca_io_clk as their "interface" clock and make sure to enable or
> disable it whenever the driver wants to read/write the registers. I'm not
> sure if something else is done on the APB for you, but that may be all
> that's necessary.
>
> This actually points out a big problem we have right now in Linux, which is
> the lack of proper power management for the type of bus topology you see in
> embedded systems. Some of the genpd work going on may also be another
> solution to this problem, where we could group multiple devices into generic
> power domains that know to turn on 2 or 3 of the clocks. For example, the
> DMAC, USP, and UART device could all be in the APB power domain so that the
> a7ca_io_clk is enabled whenever one of those devices is active. And there
> could be another power domain for the NOC that encompasses all devices that
> are sitting on that NOC.
>
> Long story short, solving these sorts of problems in the clk framework is
> typically the easy route that people take, although it's the wrong route.
> We're not expressing these sorts of complicated bus topologies in the clk
> framework. We're only expressing the clk tree (which is already quite
> complicated enough). Your drivers can still be simple, but don't make it
> more complicated than it has to be in the clk framework for that.
>

it does explain the current situation well. but how painful it is to
express this kind of bus in any driver subsystem.
for exmaple, to access usp3, clocks in the whole topo should be open.
but it seems the whole topo should not
be visible to the client driver like uart/dmac4.

if we stand in the position of dmac4, it only needs to know its clock
should be enabled, why it needs to know
the complex topo in the complex bus?

so the below two are both wrong

1. take many clocks in the uart, dmac driver
2. ignore the topo and "simulate" one simple clk tree

if we have to choice one, i prefer it is 2.

-barry

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Barry Song]

2015-08-06 9:44 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 07/29/2015 07:52 AM, Barry Song wrote:
>>
>> 2015-07-29 6:07 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
>>>
>>> On 07/27/2015 11:27 PM, Barry Song wrote:
>>>>
>>>> From: Guo Zeng <guo.zeng@csr.com>
>>>>
>>>> normally dmac should depends on only one clock, to operate
>>>> dmac internal register, but dmac4 is very special case, it
>>>> normally dmac should depends on only one clock, to operate
>>>> dmac internal register, but dmac4 is very special case, it
>>>
>>> The sentence is duplicated twice here
>>>
>>>> depends on two additional clock, the reason is that dmac4
>>>> is wrapped in hw into bt a7ca module, and accessing dmac4
>>>> internal register would also require that the a7ca_io and
>>>> related bt macro io clk also enabled.
>>>> here workaround this by setting depend clk into parent of
>>>> dmac4, and also related clks, to reflect dependency.
>>>
>>> We don't put clocks as parents of other clocks just because the child
>>> depends on the parent to be accessed. It isn't very clear from the
>>> description, but it sounds like a7ca_io is just another clock that the
>>> bluetooth driver needs to control? We want to express the true clk
>>> hierarchy, not some psuedo dependency tree.
>>>
>> Stephen, generically i agree the codes should be a map of real
>> hardware connection. here the problem is real hardware connection is
>> very much difficult for its drivers implementation and these clocks
>> are buggy designed in HW. they are not a tree, they are a "network".
>>
>> see attached diagram(hope LKML will not reject the attachment), we are
>> not a tree,  that means btm_noc_clk depends on btm_io_clk, a7ca_io_clk
>> depends on btm_io_clk, dmac4_io_clk depends on btm_noc_clk,
>> a7ca_io_clk and btm_noc_clk....
>> usp3_io_clk depends on uart6_io_clk, dmac4_io_clk, btm_noc_clk,
>> a7ca_io_clk and btm_noc_clk.
>>
>> so in the driver, we have a choice as below:
>> in uart6 driver, we take all of the clocks.
>> in dmac4 driver, we take all of btm_noc_clk, a7ca_io_clk and btm_noc_clk.
>> ...
>> what a ugly code! that is what we did before.
>>
>> so here, we moved to do a not-real clock tree,  that means
>> we makes
>> 1.btm_io_clk the parent of btm_noc_clk
>> 2. btm_noc_clk the parent of a7ca_io_clk
>> 3. a7ca_io_clk the parent of dmac4_io_clk
>> ...
>>
>> but it doesn't break any dependency of these clocks, and make all
>> clients driver as clean as a simple tree.
>> i strongly think we should keep this "workaround", otherwise, we have
>> many pain in all of the related drivers.
>>
>>
>
> Thanks for the nice PNG. It seems like you have a typical bus topology where
> UART and USP need to go through DMAC and the Data NOC to get out on the
> system bus. btm_io_clk is the true parent of all the clocks.
>
> I can only guess that A7CA is some sort of device that you have to go
> through from the CPU side to access the UART, USP, and DMAC hardware blocks.
> Is that right? Can you please describe what each of these blocks do and if
> there are linux drivers for them?
>
> One way to model this would be to make UART and USP children of the DMAC
> hardware block. And then make a driver/device up for the NOC and have the
> DMAC be a child of the NOC device. Then the NOC would probe and be runtime
> PM enabled to turn on the btm_noc_clk whenever it or its children are
> enabled. Then the DMAC device would do the same thing, but with the
> dmac4_io_clk, and the UART and USP devices would get the uart6_io_clk and
> usp3_io_clk respectively and enable/disable them when the device is active.
>
> That leaves us with the a7ca_io_clk, which is really just the clock for a
> bus that register read/writes go through. In designs I've seen, I usually
> left that clock under the control of each individual device that sits on the
> bus. So in this case, the DMAC, USP and UART device drivers would all need
> to get the a7ca_io_clk as their "interface" clock and make sure to enable or
> disable it whenever the driver wants to read/write the registers. I'm not
> sure if something else is done on the APB for you, but that may be all
> that's necessary.
>
> This actually points out a big problem we have right now in Linux, which is
> the lack of proper power management for the type of bus topology you see in
> embedded systems. Some of the genpd work going on may also be another
> solution to this problem, where we could group multiple devices into generic
> power domains that know to turn on 2 or 3 of the clocks. For example, the
> DMAC, USP, and UART device could all be in the APB power domain so that the
> a7ca_io_clk is enabled whenever one of those devices is active. And there
> could be another power domain for the NOC that encompasses all devices that
> are sitting on that NOC.
>

Stephen, this should be a possible option, i will talk with HW guys to
figure out the relationship of power domain, and check whether we
handle this issue by the power domain solution.

btw, would you apply other patches in this series at first? we can
hold on this one to find a final solution.

-barry

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Stephen Boyd]

On 08/10, Barry Song wrote:
> 2015-08-06 9:44 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> >
> > This actually points out a big problem we have right now in Linux, which is
> > the lack of proper power management for the type of bus topology you see in
> > embedded systems. Some of the genpd work going on may also be another
> > solution to this problem, where we could group multiple devices into generic
> > power domains that know to turn on 2 or 3 of the clocks. For example, the
> > DMAC, USP, and UART device could all be in the APB power domain so that the
> > a7ca_io_clk is enabled whenever one of those devices is active. And there
> > could be another power domain for the NOC that encompasses all devices that
> > are sitting on that NOC.
> >
> 
> Stephen, this should be a possible option, i will talk with HW guys to
> figure out the relationship of power domain, and check whether we
> handle this issue by the power domain solution.
> 
> btw, would you apply other patches in this series at first? we can
> hold on this one to find a final solution.
> 

Yes. I'll apply the rest to clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 5/7] clk: atlas7: fix the clock tree for bluetooth stuff

[Barry Song]

2015-08-12 8:02 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 08/10, Barry Song wrote:
>> 2015-08-06 9:44 GMT+08:00 Stephen Boyd <sboyd@codeaurora.org>:
>> >
>> > This actually points out a big problem we have right now in Linux, which is
>> > the lack of proper power management for the type of bus topology you see in
>> > embedded systems. Some of the genpd work going on may also be another
>> > solution to this problem, where we could group multiple devices into generic
>> > power domains that know to turn on 2 or 3 of the clocks. For example, the
>> > DMAC, USP, and UART device could all be in the APB power domain so that the
>> > a7ca_io_clk is enabled whenever one of those devices is active. And there
>> > could be another power domain for the NOC that encompasses all devices that
>> > are sitting on that NOC.
>> >
>>
>> Stephen, this should be a possible option, i will talk with HW guys to
>> figure out the relationship of power domain, and check whether we
>> handle this issue by the power domain solution.
>>
>> btw, would you apply other patches in this series at first? we can
>> hold on this one to find a final solution.
>>
>
> Yes. I'll apply the rest to clk-next.

Stephen, thanks a lot. i think more about the power domain solution.
we can re-look at the attached diagram(btm.png) again.

usp3 requires btm_io_clk and uart6_io_clk;
uart6_io requires btm_io_clk and dmac4_io_clk;
dmac4_io requires btm_io_clk and a7ca_io_clk;
a7ca_io requires btm_io_clk and btm_noc_clk;

if we put these things in a power domain, it seems we can enable these
clocks together or disable them together. but it seems we can't
describe the dependency how.

for example, if someone wants to use usp3, it means all clocks should
be enabled.
but if someone only wants to use a7ca_io, it means only btm_io_clk and
btm_noc_clk should be enabled, others should be disabled.
and if someone wants to use dmac4_io, then btm_io_clk , btm_noc_clk
and a7ca_io_clk should be enabled, others should be disabled.

it is difficult to find any power domain things can really resolve the
right dependency of the clock except that we can do such a power
domain as diagram pd.png. but the problem is this is the powerdomain
design in the real hardware.

it turns out the current patch really resolves all problems except
that it is not a right description to clock tree. but it does build a
good dependency map for the relationship of the clocks.

-barry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: btm.PNG
Type: image/png
Size: 5993 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150812/0795135e/attachment.png>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pd.png
Type: image/png
Size: 4527 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150812/0795135e/attachment-0001.png>

[PATCH 6/7] clk: sirf: fix bit field and its root clk for coresight_tpiu

[Barry Song]

From: Guo Zeng <guo.zeng@csr.com>

bit4 thcpum_cpudiv4_clken 	r/w
thcpum_cpudiv4_clk clock enable (default: 1)
Root clock CPU_CLK must be enabled for this clock to be enabled

bit3 coresight_tpiu_clken 	r/w
coresight_tpiu_clk clock enable (default: 0)
Root clock TPIU_CLK must be enabled for this clock to be enabled

Signed-off-by: Guo Zeng <guo.zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index c1788df..4fc33da 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -1164,7 +1164,7 @@ static struct atlas7_unit_init_data unit_list[] __initdata = {
 	{ 122, "spram1_cpudiv2", "cpum_cpu", 0, SIRFSOC_CLKC_LEAF_CLK_EN6_SET, 0, &leaf6_gate_lock },
 	{ 123, "spram2_cpudiv2", "cpum_cpu", 0, SIRFSOC_CLKC_LEAF_CLK_EN6_SET, 1, &leaf6_gate_lock },
 	{ 124, "coresight_cpudiv2", "cpum_cpu", 0, SIRFSOC_CLKC_LEAF_CLK_EN6_SET, 2, &leaf6_gate_lock },
-	{ 125, "thcpum_cpudiv4", "cpum_cpu", 0, SIRFSOC_CLKC_LEAF_CLK_EN6_SET, 3, &leaf6_gate_lock },
+	{ 125 , "coresight_tpiu", "cpum_tpiu", 0, SIRFSOC_CLKC_LEAF_CLK_EN6_SET, 3, &leaf6_gate_lock },
 	{ 126, "graphic_gpu", "gpum_gpu", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 0, &leaf7_gate_lock },
 	{ 127, "vss_sdr", "gpum_sdr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 1, &leaf7_gate_lock },
 	{ 128, "thgpum_nocr", "gpum_nocr", 0, SIRFSOC_CLKC_LEAF_CLK_EN7_SET, 2, &leaf7_gate_lock },
-- 
1.9.1

[PATCH 7/7] clk: atlas7: fix pll missed divide NR in fraction mode

[Barry Song]

From: Guo Zeng <guo.zeng@csr.com>

PLL VCO frequency is given by Fvco = Fref * 2 * NF / NR
in integer-N mode, or by Fvco = Fref * SSN / NR in Spread
Spectrum (fractional-N) mode. Thus fix the missing part
of NR

Signed-off-by: Guo Zeng <guo.zeng@csr.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/clk/sirf/clk-atlas7.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/sirf/clk-atlas7.c b/drivers/clk/sirf/clk-atlas7.c
index 4fc33da..19f417c 100644
--- a/drivers/clk/sirf/clk-atlas7.c
+++ b/drivers/clk/sirf/clk-atlas7.c
@@ -358,6 +358,7 @@ static unsigned long pll_clk_recalc_rate(struct clk_hw *hw,
 	if (regctrl0 & SIRFSOC_ABPLL_CTRL0_SSEN) {
 		rate = fin;
 		rate *= 1 << 24;
+		do_div(rate, nr);
 		do_div(rate, (256 * ((ssdiv >> ssdepth) << ssdepth)
 			+ (ssmod << ssdepth)));
 	} else {
-- 
1.9.1