All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag
@ 2015-04-07  7:46 Joonyoung Shim
  2015-04-07  7:46 ` [PATCH 2/2] clk: divider: fix to set parent rate from " Joonyoung Shim
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Joonyoung Shim @ 2015-04-07  7:46 UTC (permalink / raw)
  To: linux-clk
  Cc: linux-kernel, mturquette, sboyd, haojian.zhuang, james.hogan,
	jy0922.shim

Even if use CLK_DIVIDER_READ_ONLY flag, divider setting can be changed
by set_rate callback. Don't change divider setting from set_rate
callback of divider with CLK_DIVIDER_READ_ONLY flag.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/clk/clk-divider.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 25006a8..ce34d29a 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -384,6 +384,9 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned long flags = 0;
 	u32 val;
 
+	if (divider->flags & CLK_DIVIDER_READ_ONLY)
+		return 0;
+
 	value = divider_get_val(rate, parent_rate, divider->table,
 				divider->width, divider->flags);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag
  2015-04-07  7:46 [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag Joonyoung Shim
@ 2015-04-07  7:46 ` Joonyoung Shim
  2015-05-12 23:25     ` Michael Turquette
  2015-06-04 22:42   ` Stephen Boyd
  2015-05-12 23:59 ` [PATCH 1/2] clk: divider: don't set_rate with " Stephen Boyd
  2015-06-04 22:44 ` Stephen Boyd
  2 siblings, 2 replies; 10+ messages in thread
From: Joonyoung Shim @ 2015-04-07  7:46 UTC (permalink / raw)
  To: linux-clk
  Cc: linux-kernel, mturquette, sboyd, haojian.zhuang, james.hogan,
	jy0922.shim

The round_rate callback function will returns alway same parent clk rate
of divider with CLK_DIVIDER_READ_ONLY flag. If be used
CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
change parent clk rate anymore.

>From this case, this patch allows to change parent clk rate.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/clk/clk-divider.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index ce34d29a..37e285e 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
 		bestdiv = readl(divider->reg) >> divider->shift;
 		bestdiv &= div_mask(divider->width);
 		bestdiv = _get_div(divider->table, bestdiv, divider->flags);
+
+		if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
+			*prate = __clk_round_rate(__clk_get_parent(hw->clk),
+						  rate);
+
 		return DIV_ROUND_UP(*prate, bestdiv);
 	}
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag
  2015-04-07  7:46 ` [PATCH 2/2] clk: divider: fix to set parent rate from " Joonyoung Shim
@ 2015-05-12 23:25     ` Michael Turquette
  2015-06-04 22:42   ` Stephen Boyd
  1 sibling, 0 replies; 10+ messages in thread
From: Michael Turquette @ 2015-05-12 23:25 UTC (permalink / raw)
  To: Joonyoung Shim, linux-clk
  Cc: linux-kernel, sboyd, haojian.zhuang, james.hogan, jy0922.shim

Quoting Joonyoung Shim (2015-04-07 00:46:46)
> The round_rate callback function will returns alway same parent clk rate
> of divider with CLK_DIVIDER_READ_ONLY flag. If be used
> CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
> change parent clk rate anymore.
> 
> From this case, this patch allows to change parent clk rate.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/clk/clk-divider.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index ce34d29a..37e285e 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
>                 bestdiv = readl(divider->reg) >> divider->shift;
>                 bestdiv &= div_mask(divider->width);
>                 bestdiv = _get_div(divider->table, bestdiv, divider->flags);
> +
> +               if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> +                       *prate = __clk_round_rate(__clk_get_parent(hw->clk),
> +                                                 rate);
> +
>                 return DIV_ROUND_UP(*prate, bestdiv);
>         }
>  
> -- 
> 1.9.1
> 

Hello Joonyoung Shim,

Thanks for reporting the bug and providing a fix!

I've come up with an alternative solution to this. This patch should
replace both of your patches. Can you test this and see if it fixes the
problem for you?

Thanks,
Mike



>From 655dddad2700a30aaa397cd804422e0d9195efad Mon Sep 17 00:00:00 2001
From: Michael Turquette <mturquette@linaro.org>
Date: Tue, 12 May 2015 16:13:46 -0700
Subject: [PATCH] clk: divider: support read-only dividers

An arbitrary clock rate divider may be set out of reset, or perhaps by
the bootloader or something other than Linux. In these cases we may want
to know the frequency of the clock signal, but we do not want to allow
Linux to change it.

The CLK_DIVIDER_READ_ONLY flag was intended to express this, but the
functionality was missing in the code. Add read-only clk_ops for divider
clocks to handle this case.

For hardware with fixed dividers it is still best to use the
fixed-factor clock type.

Reported-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Michael Turquette <mturquette@linaro.org>
---
 drivers/clk/clk-divider.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 25006a8..5d2de26 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -412,6 +412,11 @@ const struct clk_ops clk_divider_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_divider_ops);
 
+const struct clk_ops clk_divider_ro_ops = {
+	.recalc_rate = clk_divider_recalc_rate,
+};
+EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
+
 static struct clk *_register_divider(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
@@ -437,7 +442,10 @@ static struct clk *_register_divider(struct device *dev, const char *name,
 	}
 
 	init.name = name;
-	init.ops = &clk_divider_ops;
+	if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
+		init.ops = &clk_divider_ro_ops;
+	else
+		init.ops = &clk_divider_ops;
 	init.flags = flags | CLK_IS_BASIC;
 	init.parent_names = (parent_name ? &parent_name: NULL);
 	init.num_parents = (parent_name ? 1 : 0);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag
@ 2015-05-12 23:25     ` Michael Turquette
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Turquette @ 2015-05-12 23:25 UTC (permalink / raw)
  To: Joonyoung Shim, linux-clk
  Cc: linux-kernel, sboyd, haojian.zhuang, james.hogan, jy0922.shim

Quoting Joonyoung Shim (2015-04-07 00:46:46)
> The round_rate callback function will returns alway same parent clk rate
> of divider with CLK_DIVIDER_READ_ONLY flag. If be used
> CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
> change parent clk rate anymore.
> =

> From this case, this patch allows to change parent clk rate.
> =

> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/clk/clk-divider.c | 5 +++++
>  1 file changed, 5 insertions(+)
> =

> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index ce34d29a..37e285e 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw=
, unsigned long rate,
>                 bestdiv =3D readl(divider->reg) >> divider->shift;
>                 bestdiv &=3D div_mask(divider->width);
>                 bestdiv =3D _get_div(divider->table, bestdiv, divider->fl=
ags);
> +
> +               if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> +                       *prate =3D __clk_round_rate(__clk_get_parent(hw->=
clk),
> +                                                 rate);
> +
>                 return DIV_ROUND_UP(*prate, bestdiv);
>         }
>  =

> -- =

> 1.9.1
> =


Hello Joonyoung Shim,

Thanks for reporting the bug and providing a fix!

I've come up with an alternative solution to this. This patch should
replace both of your patches. Can you test this and see if it fixes the
problem for you?

Thanks,
Mike



From=20655dddad2700a30aaa397cd804422e0d9195efad Mon Sep 17 00:00:00 2001
From: Michael Turquette <mturquette@linaro.org>
Date: Tue, 12 May 2015 16:13:46 -0700
Subject: [PATCH] clk: divider: support read-only dividers

An arbitrary clock rate divider may be set out of reset, or perhaps by
the bootloader or something other than Linux. In these cases we may want
to know the frequency of the clock signal, but we do not want to allow
Linux to change it.

The CLK_DIVIDER_READ_ONLY flag was intended to express this, but the
functionality was missing in the code. Add read-only clk_ops for divider
clocks to handle this case.

For hardware with fixed dividers it is still best to use the
fixed-factor clock type.

Reported-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Michael Turquette <mturquette@linaro.org>
---
 drivers/clk/clk-divider.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 25006a8..5d2de26 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -412,6 +412,11 @@ const struct clk_ops clk_divider_ops =3D {
 };
 EXPORT_SYMBOL_GPL(clk_divider_ops);
 =

+const struct clk_ops clk_divider_ro_ops =3D {
+	.recalc_rate =3D clk_divider_recalc_rate,
+};
+EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
+
 static struct clk *_register_divider(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
@@ -437,7 +442,10 @@ static struct clk *_register_divider(struct device *de=
v, const char *name,
 	}
 =

 	init.name =3D name;
-	init.ops =3D &clk_divider_ops;
+	if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
+		init.ops =3D &clk_divider_ro_ops;
+	else
+		init.ops =3D &clk_divider_ops;
 	init.flags =3D flags | CLK_IS_BASIC;
 	init.parent_names =3D (parent_name ? &parent_name: NULL);
 	init.num_parents =3D (parent_name ? 1 : 0);
-- =

1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag
  2015-05-12 23:25     ` Michael Turquette
  (?)
@ 2015-05-12 23:57     ` Stephen Boyd
  2015-05-15  2:12       ` Joonyoung Shim
  -1 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2015-05-12 23:57 UTC (permalink / raw)
  To: Michael Turquette
  Cc: Joonyoung Shim, linux-clk, linux-kernel, haojian.zhuang, james.hogan

On 05/12, Michael Turquette wrote:
> Quoting Joonyoung Shim (2015-04-07 00:46:46)
> > The round_rate callback function will returns alway same parent clk rate
> > of divider with CLK_DIVIDER_READ_ONLY flag. If be used
> > CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
> > change parent clk rate anymore.
> > 
> > From this case, this patch allows to change parent clk rate.
> > 
> > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> > ---
> >  drivers/clk/clk-divider.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> > index ce34d29a..37e285e 100644
> > --- a/drivers/clk/clk-divider.c
> > +++ b/drivers/clk/clk-divider.c
> > @@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
> >                 bestdiv = readl(divider->reg) >> divider->shift;
> >                 bestdiv &= div_mask(divider->width);
> >                 bestdiv = _get_div(divider->table, bestdiv, divider->flags);
> > +
> > +               if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> > +                       *prate = __clk_round_rate(__clk_get_parent(hw->clk),
> > +                                                 rate);
> > +
> >                 return DIV_ROUND_UP(*prate, bestdiv);
> >         }
> >  
> > -- 
> > 1.9.1
> > 
> 
> Hello Joonyoung Shim,
> 
> Thanks for reporting the bug and providing a fix!
> 
> I've come up with an alternative solution to this. This patch should
> replace both of your patches. Can you test this and see if it fixes the
> problem for you?
> 
> Thanks,
> Mike
> 
> 
> 
> From 655dddad2700a30aaa397cd804422e0d9195efad Mon Sep 17 00:00:00 2001
> From: Michael Turquette <mturquette@linaro.org>
> Date: Tue, 12 May 2015 16:13:46 -0700
> Subject: [PATCH] clk: divider: support read-only dividers
> 
> An arbitrary clock rate divider may be set out of reset, or perhaps by
> the bootloader or something other than Linux. In these cases we may want
> to know the frequency of the clock signal, but we do not want to allow
> Linux to change it.
> 
> The CLK_DIVIDER_READ_ONLY flag was intended to express this, but the
> functionality was missing in the code. Add read-only clk_ops for divider
> clocks to handle this case.
> 
> For hardware with fixed dividers it is still best to use the
> fixed-factor clock type.
> 
> Reported-by: Joonyoung Shim <jy0922.shim@samsung.com>
> Signed-off-by: Michael Turquette <mturquette@linaro.org>
> ---
>  drivers/clk/clk-divider.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 25006a8..5d2de26 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -412,6 +412,11 @@ const struct clk_ops clk_divider_ops = {
>  };
>  EXPORT_SYMBOL_GPL(clk_divider_ops);
>  
> +const struct clk_ops clk_divider_ro_ops = {
> +	.recalc_rate = clk_divider_recalc_rate,
> +};
> +EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
> +
>  static struct clk *_register_divider(struct device *dev, const char *name,
>  		const char *parent_name, unsigned long flags,
>  		void __iomem *reg, u8 shift, u8 width,
> @@ -437,7 +442,10 @@ static struct clk *_register_divider(struct device *dev, const char *name,
>  	}
>  
>  	init.name = name;
> -	init.ops = &clk_divider_ops;
> +	if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
> +		init.ops = &clk_divider_ro_ops;
> +	else
> +		init.ops = &clk_divider_ops;
>  	init.flags = flags | CLK_IS_BASIC;
>  	init.parent_names = (parent_name ? &parent_name: NULL);
>  	init.num_parents = (parent_name ? 1 : 0);
> -- 

Isn't this sort of reverting commit e6d5e7d90be9 (clk-divider:
Fix READ_ONLY when divider > 1, 2014-11-14)? 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag
  2015-04-07  7:46 [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag Joonyoung Shim
  2015-04-07  7:46 ` [PATCH 2/2] clk: divider: fix to set parent rate from " Joonyoung Shim
@ 2015-05-12 23:59 ` Stephen Boyd
  2015-05-15  1:52   ` Joonyoung Shim
  2015-06-04 22:44 ` Stephen Boyd
  2 siblings, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2015-05-12 23:59 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: linux-clk, linux-kernel, mturquette, haojian.zhuang, james.hogan

On 04/07, Joonyoung Shim wrote:
> Even if use CLK_DIVIDER_READ_ONLY flag, divider setting can be changed
> by set_rate callback. Don't change divider setting from set_rate
> callback of divider with CLK_DIVIDER_READ_ONLY flag.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---

Is the rate actually changing? Or is it just a problem that we
may be writing the register to the same value it already is?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag
  2015-05-12 23:59 ` [PATCH 1/2] clk: divider: don't set_rate with " Stephen Boyd
@ 2015-05-15  1:52   ` Joonyoung Shim
  0 siblings, 0 replies; 10+ messages in thread
From: Joonyoung Shim @ 2015-05-15  1:52 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-clk, linux-kernel, mturquette, haojian.zhuang, james.hogan

Hi Stephen,

On 05/13/2015 08:59 AM, Stephen Boyd wrote:
> On 04/07, Joonyoung Shim wrote:
>> Even if use CLK_DIVIDER_READ_ONLY flag, divider setting can be changed
>> by set_rate callback. Don't change divider setting from set_rate
>> callback of divider with CLK_DIVIDER_READ_ONLY flag.
>>
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> ---
> 
> Is the rate actually changing? Or is it just a problem that we
> may be writing the register to the same value it already is?
> 

If rate and parant_rate are different, it can write the register to
different value. Even if the value is same but i think it's unnecessary
to re-write the register.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag
  2015-05-12 23:57     ` Stephen Boyd
@ 2015-05-15  2:12       ` Joonyoung Shim
  0 siblings, 0 replies; 10+ messages in thread
From: Joonyoung Shim @ 2015-05-15  2:12 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: linux-clk, linux-kernel, haojian.zhuang, james.hogan

Hi Michael,

On 05/13/2015 08:57 AM, Stephen Boyd wrote:
> On 05/12, Michael Turquette wrote:
>> Quoting Joonyoung Shim (2015-04-07 00:46:46)
>>> The round_rate callback function will returns alway same parent clk rate
>>> of divider with CLK_DIVIDER_READ_ONLY flag. If be used
>>> CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
>>> change parent clk rate anymore.
>>>
>>> From this case, this patch allows to change parent clk rate.
>>>
>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>> ---
>>>  drivers/clk/clk-divider.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>>> index ce34d29a..37e285e 100644
>>> --- a/drivers/clk/clk-divider.c
>>> +++ b/drivers/clk/clk-divider.c
>>> @@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
>>>                 bestdiv = readl(divider->reg) >> divider->shift;
>>>                 bestdiv &= div_mask(divider->width);
>>>                 bestdiv = _get_div(divider->table, bestdiv, divider->flags);
>>> +
>>> +               if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
>>> +                       *prate = __clk_round_rate(__clk_get_parent(hw->clk),
>>> +                                                 rate);
>>> +
>>>                 return DIV_ROUND_UP(*prate, bestdiv);
>>>         }
>>>  
>>> -- 
>>> 1.9.1
>>>
>>
>> Hello Joonyoung Shim,
>>
>> Thanks for reporting the bug and providing a fix!
>>
>> I've come up with an alternative solution to this. This patch should
>> replace both of your patches. Can you test this and see if it fixes the
>> problem for you?
>>

Yes, it works.

>> Thanks,
>> Mike
>>
>>
>>
>> From 655dddad2700a30aaa397cd804422e0d9195efad Mon Sep 17 00:00:00 2001
>> From: Michael Turquette <mturquette@linaro.org>
>> Date: Tue, 12 May 2015 16:13:46 -0700
>> Subject: [PATCH] clk: divider: support read-only dividers
>>
>> An arbitrary clock rate divider may be set out of reset, or perhaps by
>> the bootloader or something other than Linux. In these cases we may want
>> to know the frequency of the clock signal, but we do not want to allow
>> Linux to change it.
>>
>> The CLK_DIVIDER_READ_ONLY flag was intended to express this, but the
>> functionality was missing in the code. Add read-only clk_ops for divider
>> clocks to handle this case.
>>
>> For hardware with fixed dividers it is still best to use the
>> fixed-factor clock type.
>>
>> Reported-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> Signed-off-by: Michael Turquette <mturquette@linaro.org>
>> ---
>>  drivers/clk/clk-divider.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> index 25006a8..5d2de26 100644
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -412,6 +412,11 @@ const struct clk_ops clk_divider_ops = {
>>  };
>>  EXPORT_SYMBOL_GPL(clk_divider_ops);
>>  
>> +const struct clk_ops clk_divider_ro_ops = {
>> +	.recalc_rate = clk_divider_recalc_rate,
>> +};
>> +EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
>> +
>>  static struct clk *_register_divider(struct device *dev, const char *name,
>>  		const char *parent_name, unsigned long flags,
>>  		void __iomem *reg, u8 shift, u8 width,
>> @@ -437,7 +442,10 @@ static struct clk *_register_divider(struct device *dev, const char *name,
>>  	}
>>  
>>  	init.name = name;
>> -	init.ops = &clk_divider_ops;
>> +	if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
>> +		init.ops = &clk_divider_ro_ops;
>> +	else
>> +		init.ops = &clk_divider_ops;
>>  	init.flags = flags | CLK_IS_BASIC;
>>  	init.parent_names = (parent_name ? &parent_name: NULL);
>>  	init.num_parents = (parent_name ? 1 : 0);
>> -- 
> 
> Isn't this sort of reverting commit e6d5e7d90be9 (clk-divider:
> Fix READ_ONLY when divider > 1, 2014-11-14)? 
> 

So i had abandoned to retry commit e6d5e7d90be9.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] clk: divider: fix to set parent rate from CLK_DIVIDER_READ_ONLY flag
  2015-04-07  7:46 ` [PATCH 2/2] clk: divider: fix to set parent rate from " Joonyoung Shim
  2015-05-12 23:25     ` Michael Turquette
@ 2015-06-04 22:42   ` Stephen Boyd
  1 sibling, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2015-06-04 22:42 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: linux-clk, linux-kernel, mturquette, haojian.zhuang, james.hogan

On 04/07, Joonyoung Shim wrote:
> The round_rate callback function will returns alway same parent clk rate
> of divider with CLK_DIVIDER_READ_ONLY flag. If be used
> CLK_SET_RATE_PARENT flag with CLK_DIVIDER_READ_ONLY flag, then never
> change parent clk rate anymore.
> 
> From this case, this patch allows to change parent clk rate.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/clk/clk-divider.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index ce34d29a..37e285e 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -352,6 +352,11 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate,
>  		bestdiv = readl(divider->reg) >> divider->shift;
>  		bestdiv &= div_mask(divider->width);
>  		bestdiv = _get_div(divider->table, bestdiv, divider->flags);
> +
> +		if ((__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> +			*prate = __clk_round_rate(__clk_get_parent(hw->clk),
> +						  rate);
> +
>  		return DIV_ROUND_UP(*prate, bestdiv);

Doesn't this assume that the divider is 1? Otherwise we should be
multiplying the rate up by whatever the divider is that we have
in the hardware.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag
  2015-04-07  7:46 [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag Joonyoung Shim
  2015-04-07  7:46 ` [PATCH 2/2] clk: divider: fix to set parent rate from " Joonyoung Shim
  2015-05-12 23:59 ` [PATCH 1/2] clk: divider: don't set_rate with " Stephen Boyd
@ 2015-06-04 22:44 ` Stephen Boyd
  2 siblings, 0 replies; 10+ messages in thread
From: Stephen Boyd @ 2015-06-04 22:44 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: linux-clk, linux-kernel, mturquette, haojian.zhuang, james.hogan

On 04/07, Joonyoung Shim wrote:
> Even if use CLK_DIVIDER_READ_ONLY flag, divider setting can be changed
> by set_rate callback. Don't change divider setting from set_rate
> callback of divider with CLK_DIVIDER_READ_ONLY flag.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/clk/clk-divider.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 25006a8..ce34d29a 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -384,6 +384,9 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>  	unsigned long flags = 0;
>  	u32 val;
>  
> +	if (divider->flags & CLK_DIVIDER_READ_ONLY)
> +		return 0;
> +
>  	value = divider_get_val(rate, parent_rate, divider->table,
>  				divider->width, divider->flags);
>  

I wonder if it would make more sense to have different ops for
read only dividers. We would need to have an empty clk_set_rate
op in the case where the CCF tries to set the rate to what it
already is and then the proper recalc_rate and round_rate ops for
read only devices. At the least, this patch looks correct.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-06-04 22:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-07  7:46 [PATCH 1/2] clk: divider: don't set_rate with CLK_DIVIDER_READ_ONLY flag Joonyoung Shim
2015-04-07  7:46 ` [PATCH 2/2] clk: divider: fix to set parent rate from " Joonyoung Shim
2015-05-12 23:25   ` Michael Turquette
2015-05-12 23:25     ` Michael Turquette
2015-05-12 23:57     ` Stephen Boyd
2015-05-15  2:12       ` Joonyoung Shim
2015-06-04 22:42   ` Stephen Boyd
2015-05-12 23:59 ` [PATCH 1/2] clk: divider: don't set_rate with " Stephen Boyd
2015-05-15  1:52   ` Joonyoung Shim
2015-06-04 22:44 ` Stephen Boyd

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.