linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] init some clock rate from dts for rk3288
@ 2014-10-07  9:33 Kever Yang
  2014-10-07  9:33 ` [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate Kever Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Kever Yang @ 2014-10-07  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch add init rate for PLLs and some bus clock  from dts for rk3288,
add two clock rate of 400M and 500M into rate table for we will use it.


Kever Yang (2):
  clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate
  ARM: dts: enable init rate for clock

 arch/arm/boot/dts/rk3288.dtsi     | 10 ++++++++++
 drivers/clk/rockchip/clk-rk3288.c |  2 ++
 2 files changed, 12 insertions(+)

-- 
1.9.1

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

* [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate
  2014-10-07  9:33 [PATCH 0/2] init some clock rate from dts for rk3288 Kever Yang
@ 2014-10-07  9:33 ` Kever Yang
  2014-10-07 16:56   ` Doug Anderson
  2014-10-07  9:33 ` [PATCH 2/2] ARM: dts: enable init rate for clock Kever Yang
  2014-10-07 17:11 ` [PATCH 0/2] init some clock rate from dts for rk3288 Doug Anderson
  2 siblings, 1 reply; 10+ messages in thread
From: Kever Yang @ 2014-10-07  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

This patch add 400MHz and 500MHz to clock rate table for rk3288.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

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

diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
index d053529..f6ea9c6 100644
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -86,8 +86,10 @@ struct rockchip_pll_rate_table rk3288_pll_rates[] = {
 	RK3066_PLL_RATE( 594000000, 2, 198, 4),
 	RK3066_PLL_RATE( 552000000, 1, 46, 2),
 	RK3066_PLL_RATE( 504000000, 1, 84, 4),
+	RK3066_PLL_RATE( 500000000, 3, 125, 2),
 	RK3066_PLL_RATE( 456000000, 1, 76, 4),
 	RK3066_PLL_RATE( 408000000, 1, 68, 4),
+	RK3066_PLL_RATE( 400000000, 3, 50, 1),
 	RK3066_PLL_RATE( 384000000, 2, 128, 4),
 	RK3066_PLL_RATE( 360000000, 1, 60, 4),
 	RK3066_PLL_RATE( 312000000, 1, 52, 4),
-- 
1.9.1

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

* [PATCH 2/2] ARM: dts: enable init rate for clock
  2014-10-07  9:33 [PATCH 0/2] init some clock rate from dts for rk3288 Kever Yang
  2014-10-07  9:33 ` [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate Kever Yang
@ 2014-10-07  9:33 ` Kever Yang
  2014-10-07 17:03   ` Doug Anderson
  2014-10-07 17:11 ` [PATCH 0/2] init some clock rate from dts for rk3288 Doug Anderson
  2 siblings, 1 reply; 10+ messages in thread
From: Kever Yang @ 2014-10-07  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

We need to initialize PLL rate and some of bus clock rate while
kernel init, for there is no other module will do that.

Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
---

 arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index 874e66d..2f4519b 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -455,6 +455,16 @@
 		rockchip,grf = <&grf>;
 		#clock-cells = <1>;
 		#reset-cells = <1>;
+		assigned-clocks = <&cru PLL_GPLL>, <&cru PLL_CPLL>,
+				  <&cru PLL_NPLL>, <&cru ACLK_CPU>,
+				  <&cru HCLK_CPU>, <&cru PCLK_CPU>,
+				  <&cru ACLK_PERI>, <&cru HCLK_PERI>,
+				  <&cru PCLK_PERI>;
+		assigned-clock-rates = <594000000>, <400000000>,
+				       <500000000>, <300000000>,
+				       <150000000>, <75000000>,
+				       <300000000>, <150000000>,
+				       <75000000>;
 	};
 
 	grf: syscon at ff770000 {
-- 
1.9.1

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

* [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate
  2014-10-07  9:33 ` [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate Kever Yang
@ 2014-10-07 16:56   ` Doug Anderson
  0 siblings, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2014-10-07 16:56 UTC (permalink / raw)
  To: linux-arm-kernel

Kever,

On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com> wrote:
> This patch add 400MHz and 500MHz to clock rate table for rk3288.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
>  drivers/clk/rockchip/clk-rk3288.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3288.c b/drivers/clk/rockchip/clk-rk3288.c
> index d053529..f6ea9c6 100644
> --- a/drivers/clk/rockchip/clk-rk3288.c
> +++ b/drivers/clk/rockchip/clk-rk3288.c
> @@ -86,8 +86,10 @@ struct rockchip_pll_rate_table rk3288_pll_rates[] = {
>         RK3066_PLL_RATE( 594000000, 2, 198, 4),
>         RK3066_PLL_RATE( 552000000, 1, 46, 2),
>         RK3066_PLL_RATE( 504000000, 1, 84, 4),
> +       RK3066_PLL_RATE( 500000000, 3, 125, 2),
>         RK3066_PLL_RATE( 456000000, 1, 76, 4),
>         RK3066_PLL_RATE( 408000000, 1, 68, 4),
> +       RK3066_PLL_RATE( 400000000, 3, 50, 1),

This violates the constraints in the TRM.  That says:

Fvco = (Fin/NR)*NF value range requirement: 440MHz ? 2200MHz

I believe you end up with 400Mhz, not 440.  Are you aware of this?  Is
there some reason it's OK to violate the TRM in this case?

-Doug

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

* [PATCH 2/2] ARM: dts: enable init rate for clock
  2014-10-07  9:33 ` [PATCH 2/2] ARM: dts: enable init rate for clock Kever Yang
@ 2014-10-07 17:03   ` Doug Anderson
  2014-10-07 18:27     ` Heiko Stübner
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Anderson @ 2014-10-07 17:03 UTC (permalink / raw)
  To: linux-arm-kernel

Kever,

On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com> wrote:
> We need to initialize PLL rate and some of bus clock rate while
> kernel init, for there is no other module will do that.
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
>  arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> index 874e66d..2f4519b 100644
> --- a/arch/arm/boot/dts/rk3288.dtsi
> +++ b/arch/arm/boot/dts/rk3288.dtsi
> @@ -455,6 +455,16 @@
>                 rockchip,grf = <&grf>;
>                 #clock-cells = <1>;
>                 #reset-cells = <1>;
> +               assigned-clocks = <&cru PLL_GPLL>, <&cru PLL_CPLL>,
> +                                 <&cru PLL_NPLL>, <&cru ACLK_CPU>,
> +                                 <&cru HCLK_CPU>, <&cru PCLK_CPU>,
> +                                 <&cru ACLK_PERI>, <&cru HCLK_PERI>,
> +                                 <&cru PCLK_PERI>;
> +               assigned-clock-rates = <594000000>, <400000000>,
> +                                      <500000000>, <300000000>,

When I boot up, I see that ACLK_CPU was 297000000.  You specified
300000000.  Did you expect to get 300?  If you expected 297, I think
you should put 297.  If you expected 300 then we have some debugging
to do.  Note: I'm not quite sure how you'd expect to get 300 given
that none of the PLLs divide evenly to 300...


> +                                      <150000000>, <75000000>,

Similarly, I see 148500000, 74250000

> +                                      <300000000>, <150000000>,

297000000, 148500000

> +                                      <75000000>;

74250000

>         };
>
>         grf: syscon at ff770000 {
> --
> 1.9.1
>

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

* [PATCH 0/2] init some clock rate from dts for rk3288
  2014-10-07  9:33 [PATCH 0/2] init some clock rate from dts for rk3288 Kever Yang
  2014-10-07  9:33 ` [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate Kever Yang
  2014-10-07  9:33 ` [PATCH 2/2] ARM: dts: enable init rate for clock Kever Yang
@ 2014-10-07 17:11 ` Doug Anderson
  2 siblings, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2014-10-07 17:11 UTC (permalink / raw)
  To: linux-arm-kernel

Kever,

On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com> wrote:
> This patch add init rate for PLLs and some bus clock  from dts for rk3288,
> add two clock rate of 400M and 500M into rate table for we will use it.

In the cover letter you should probably mention that your series won't
work properly unless <https://patchwork.kernel.org/patch/5038781/> is
applied.  When I don't have that patch then "aclk_cpu" doesn't get set
properly.

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

* [PATCH 2/2] ARM: dts: enable init rate for clock
  2014-10-07 17:03   ` Doug Anderson
@ 2014-10-07 18:27     ` Heiko Stübner
  2014-10-07 19:20       ` Doug Anderson
  0 siblings, 1 reply; 10+ messages in thread
From: Heiko Stübner @ 2014-10-07 18:27 UTC (permalink / raw)
  To: linux-arm-kernel

Am Dienstag, 7. Oktober 2014, 10:03:19 schrieb Doug Anderson:
> Kever,
> 
> On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com> 
wrote:
> > We need to initialize PLL rate and some of bus clock rate while
> > kernel init, for there is no other module will do that.
> > 
> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> > ---
> > 
> >  arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> > index 874e66d..2f4519b 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> > @@ -455,6 +455,16 @@
> > 
> >                 rockchip,grf = <&grf>;
> >                 #clock-cells = <1>;
> >                 #reset-cells = <1>;
> > 
> > +               assigned-clocks = <&cru PLL_GPLL>, <&cru PLL_CPLL>,
> > +                                 <&cru PLL_NPLL>, <&cru ACLK_CPU>,
> > +                                 <&cru HCLK_CPU>, <&cru PCLK_CPU>,
> > +                                 <&cru ACLK_PERI>, <&cru HCLK_PERI>,
> > +                                 <&cru PCLK_PERI>;
> > +               assigned-clock-rates = <594000000>, <400000000>,
> > +                                      <500000000>, <300000000>,
> 
> When I boot up, I see that ACLK_CPU was 297000000.  You specified
> 300000000.  Did you expect to get 300?  If you expected 297, I think
> you should put 297.  If you expected 300 then we have some debugging
> to do.  Note: I'm not quite sure how you'd expect to get 300 given
> that none of the PLLs divide evenly to 300...

I'd think 300 is simply the target value. I.e. take the closest rate <= 300 
MHz, same for 150 etc. I somehow like the approach of specifying what the rate 
_should_ ideally be :-) .

Also reduces the amount of thougths necessary (and possible head-scratching) 
when adapting the pll rates to some other constraints (child-clocks already 
have the target rates and cannot drop to some even stranger value).


Heiko


> 
> > +                                      <150000000>, <75000000>,
> 
> Similarly, I see 148500000, 74250000
> 
> > +                                      <300000000>, <150000000>,
> 
> 297000000, 148500000
> 
> > +                                      <75000000>;
> 
> 74250000
> 
> >         };
> >         
> >         grf: syscon at ff770000 {
> > 
> > --
> > 1.9.1

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

* [PATCH 2/2] ARM: dts: enable init rate for clock
  2014-10-07 18:27     ` Heiko Stübner
@ 2014-10-07 19:20       ` Doug Anderson
  2014-10-07 19:37         ` Heiko Stübner
  0 siblings, 1 reply; 10+ messages in thread
From: Doug Anderson @ 2014-10-07 19:20 UTC (permalink / raw)
  To: linux-arm-kernel

Heiko,

On Tue, Oct 7, 2014 at 11:27 AM, Heiko St?bner <heiko@sntech.de> wrote:
> Am Dienstag, 7. Oktober 2014, 10:03:19 schrieb Doug Anderson:
>> Kever,
>>
>> On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com>
> wrote:
>> > We need to initialize PLL rate and some of bus clock rate while
>> > kernel init, for there is no other module will do that.
>> >
>> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> > ---
>> >
>> >  arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
>> >  1 file changed, 10 insertions(+)
>> >
>> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
>> > index 874e66d..2f4519b 100644
>> > --- a/arch/arm/boot/dts/rk3288.dtsi
>> > +++ b/arch/arm/boot/dts/rk3288.dtsi
>> > @@ -455,6 +455,16 @@
>> >
>> >                 rockchip,grf = <&grf>;
>> >                 #clock-cells = <1>;
>> >                 #reset-cells = <1>;
>> >
>> > +               assigned-clocks = <&cru PLL_GPLL>, <&cru PLL_CPLL>,
>> > +                                 <&cru PLL_NPLL>, <&cru ACLK_CPU>,
>> > +                                 <&cru HCLK_CPU>, <&cru PCLK_CPU>,
>> > +                                 <&cru ACLK_PERI>, <&cru HCLK_PERI>,
>> > +                                 <&cru PCLK_PERI>;
>> > +               assigned-clock-rates = <594000000>, <400000000>,
>> > +                                      <500000000>, <300000000>,
>>
>> When I boot up, I see that ACLK_CPU was 297000000.  You specified
>> 300000000.  Did you expect to get 300?  If you expected 297, I think
>> you should put 297.  If you expected 300 then we have some debugging
>> to do.  Note: I'm not quite sure how you'd expect to get 300 given
>> that none of the PLLs divide evenly to 300...
>
> I'd think 300 is simply the target value. I.e. take the closest rate <= 300
> MHz, same for 150 etc. I somehow like the approach of specifying what the rate
> _should_ ideally be :-) .
>
> Also reduces the amount of thougths necessary (and possible head-scratching)
> when adapting the pll rates to some other constraints (child-clocks already
> have the target rates and cannot drop to some even stranger value).

OK, fair enough.  I guess maybe I've overly paranoid and like to make
sure that someone has thought through exactly what various core clocks
should be and made sure that the voltage matched and that clocks were
never out of spec.  ...but if others really like specifying more ideal
values then I'm OK with it.  I will say that I'd really like the PLLs
to be exact, though.  I think those are now...

-Doug

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

* [PATCH 2/2] ARM: dts: enable init rate for clock
  2014-10-07 19:20       ` Doug Anderson
@ 2014-10-07 19:37         ` Heiko Stübner
  2014-10-07 21:10           ` Doug Anderson
  0 siblings, 1 reply; 10+ messages in thread
From: Heiko Stübner @ 2014-10-07 19:37 UTC (permalink / raw)
  To: linux-arm-kernel

Am Dienstag, 7. Oktober 2014, 12:20:38 schrieb Doug Anderson:
> Heiko,
> 
> On Tue, Oct 7, 2014 at 11:27 AM, Heiko St?bner <heiko@sntech.de> wrote:
> > Am Dienstag, 7. Oktober 2014, 10:03:19 schrieb Doug Anderson:
> >> Kever,
> >> 
> >> On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com>
> > 
> > wrote:
> >> > We need to initialize PLL rate and some of bus clock rate while
> >> > kernel init, for there is no other module will do that.
> >> > 
> >> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> >> > ---
> >> > 
> >> >  arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
> >> >  1 file changed, 10 insertions(+)
> >> > 
> >> > diff --git a/arch/arm/boot/dts/rk3288.dtsi
> >> > b/arch/arm/boot/dts/rk3288.dtsi
> >> > index 874e66d..2f4519b 100644
> >> > --- a/arch/arm/boot/dts/rk3288.dtsi
> >> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> >> > @@ -455,6 +455,16 @@
> >> > 
> >> >                 rockchip,grf = <&grf>;
> >> >                 #clock-cells = <1>;
> >> >                 #reset-cells = <1>;
> >> > 
> >> > +               assigned-clocks = <&cru PLL_GPLL>, <&cru PLL_CPLL>,
> >> > +                                 <&cru PLL_NPLL>, <&cru ACLK_CPU>,
> >> > +                                 <&cru HCLK_CPU>, <&cru PCLK_CPU>,
> >> > +                                 <&cru ACLK_PERI>, <&cru HCLK_PERI>,
> >> > +                                 <&cru PCLK_PERI>;
> >> > +               assigned-clock-rates = <594000000>, <400000000>,
> >> > +                                      <500000000>, <300000000>,
> >> 
> >> When I boot up, I see that ACLK_CPU was 297000000.  You specified
> >> 300000000.  Did you expect to get 300?  If you expected 297, I think
> >> you should put 297.  If you expected 300 then we have some debugging
> >> to do.  Note: I'm not quite sure how you'd expect to get 300 given
> >> that none of the PLLs divide evenly to 300...
> > 
> > I'd think 300 is simply the target value. I.e. take the closest rate <=
> > 300
> > MHz, same for 150 etc. I somehow like the approach of specifying what the
> > rate _should_ ideally be :-) .
> > 
> > Also reduces the amount of thougths necessary (and possible
> > head-scratching) when adapting the pll rates to some other constraints
> > (child-clocks already have the target rates and cannot drop to some even
> > stranger value).
> OK, fair enough.  I guess maybe I've overly paranoid and like to make
> sure that someone has thought through exactly what various core clocks
> should be and made sure that the voltage matched and that clocks were
> never out of spec.  ...but if others really like specifying more ideal
> values then I'm OK with it.

Especially as clocks like ACLK_CPU can select from different plls (cpll and 
gpll on the rk3288) it would be even more difficult to predict which path the 
clock selection would take. So it makes sense to specifiy the value we wish for 
and let the common-clock-framework do its magic to select the best sources.


>I will say that I'd really like the PLLs
> to be exact, though.  I think those are now...

agreed. PLL rates should be exact.

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

* [PATCH 2/2] ARM: dts: enable init rate for clock
  2014-10-07 19:37         ` Heiko Stübner
@ 2014-10-07 21:10           ` Doug Anderson
  0 siblings, 0 replies; 10+ messages in thread
From: Doug Anderson @ 2014-10-07 21:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Oct 7, 2014 at 12:37 PM, Heiko St?bner <heiko@sntech.de> wrote:
> Am Dienstag, 7. Oktober 2014, 12:20:38 schrieb Doug Anderson:
>> Heiko,
>>
>> On Tue, Oct 7, 2014 at 11:27 AM, Heiko St?bner <heiko@sntech.de> wrote:
>> > Am Dienstag, 7. Oktober 2014, 10:03:19 schrieb Doug Anderson:
>> >> Kever,
>> >>
>> >> On Tue, Oct 7, 2014 at 2:33 AM, Kever Yang <kever.yang@rock-chips.com>
>> >
>> > wrote:
>> >> > We need to initialize PLL rate and some of bus clock rate while
>> >> > kernel init, for there is no other module will do that.
>> >> >
>> >> > Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> >> > ---
>> >> >
>> >> >  arch/arm/boot/dts/rk3288.dtsi | 10 ++++++++++
>> >> >  1 file changed, 10 insertions(+)
>> >> >
>> >> > diff --git a/arch/arm/boot/dts/rk3288.dtsi
>> >> > b/arch/arm/boot/dts/rk3288.dtsi
>> >> > index 874e66d..2f4519b 100644
>> >> > --- a/arch/arm/boot/dts/rk3288.dtsi
>> >> > +++ b/arch/arm/boot/dts/rk3288.dtsi
>> >> > @@ -455,6 +455,16 @@
>> >> >
>> >> >                 rockchip,grf = <&grf>;
>> >> >                 #clock-cells = <1>;
>> >> >                 #reset-cells = <1>;
>> >> >
>> >> > +               assigned-clocks = <&cru PLL_GPLL>, <&cru PLL_CPLL>,
>> >> > +                                 <&cru PLL_NPLL>, <&cru ACLK_CPU>,
>> >> > +                                 <&cru HCLK_CPU>, <&cru PCLK_CPU>,
>> >> > +                                 <&cru ACLK_PERI>, <&cru HCLK_PERI>,
>> >> > +                                 <&cru PCLK_PERI>;
>> >> > +               assigned-clock-rates = <594000000>, <400000000>,
>> >> > +                                      <500000000>, <300000000>,
>> >>
>> >> When I boot up, I see that ACLK_CPU was 297000000.  You specified
>> >> 300000000.  Did you expect to get 300?  If you expected 297, I think
>> >> you should put 297.  If you expected 300 then we have some debugging
>> >> to do.  Note: I'm not quite sure how you'd expect to get 300 given
>> >> that none of the PLLs divide evenly to 300...
>> >
>> > I'd think 300 is simply the target value. I.e. take the closest rate <=
>> > 300
>> > MHz, same for 150 etc. I somehow like the approach of specifying what the
>> > rate _should_ ideally be :-) .
>> >
>> > Also reduces the amount of thougths necessary (and possible
>> > head-scratching) when adapting the pll rates to some other constraints
>> > (child-clocks already have the target rates and cannot drop to some even
>> > stranger value).
>> OK, fair enough.  I guess maybe I've overly paranoid and like to make
>> sure that someone has thought through exactly what various core clocks
>> should be and made sure that the voltage matched and that clocks were
>> never out of spec.  ...but if others really like specifying more ideal
>> values then I'm OK with it.
>
> Especially as clocks like ACLK_CPU can select from different plls (cpll and
> gpll on the rk3288) it would be even more difficult to predict which path the
> clock selection would take. So it makes sense to specifiy the value we wish for
> and let the common-clock-framework do its magic to select the best sources.
>
>
>>I will say that I'd really like the PLLs
>> to be exact, though.  I think those are now...
>
> agreed. PLL rates should be exact.

OK, fair enough.  Then you can give my:

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

...of course I'd still like comments addressed for part 1 of this series.  ;)

-Doug

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

end of thread, other threads:[~2014-10-07 21:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-07  9:33 [PATCH 0/2] init some clock rate from dts for rk3288 Kever Yang
2014-10-07  9:33 ` [PATCH 1/2] clk: rockchip: add 400MHz and 500MHz for rk3288 clock rate Kever Yang
2014-10-07 16:56   ` Doug Anderson
2014-10-07  9:33 ` [PATCH 2/2] ARM: dts: enable init rate for clock Kever Yang
2014-10-07 17:03   ` Doug Anderson
2014-10-07 18:27     ` Heiko Stübner
2014-10-07 19:20       ` Doug Anderson
2014-10-07 19:37         ` Heiko Stübner
2014-10-07 21:10           ` Doug Anderson
2014-10-07 17:11 ` [PATCH 0/2] init some clock rate from dts for rk3288 Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).