Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1411032 > unrolled thread

[RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support

Started byLin Huang <hl@rock-chips.com>
First post2016-06-01 11:40 +0200
Last post2016-06-02 03:40 +0200
Articles 4 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support Lin Huang <hl@rock-chips.com> - 2016-06-01 11:40 +0200
    Re: [RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support Doug Anderson <dianders@chromium.org> - 2016-06-01 17:30 +0200
      Re: [RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support Heiko Stübner <heiko@sntech.de> - 2016-06-01 17:50 +0200
        Re: [RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support hl <hl@rock-chips.com> - 2016-06-02 03:40 +0200

#1411032 — [RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support

FromLin Huang <hl@rock-chips.com>
Date2016-06-01 11:40 +0200
Subject[RFC PATCH 2/4] clk: rockchip: rk3399: add ddrc clock support
Message-ID<rFaN4-6FG-35@gated-at.bofh.it>
add ddrc clock setting, so we can do ddr frequency
scaling on rk3399 platform in future.

Signed-off-by: Lin Huang <hl@rock-chips.com>
---
 drivers/clk/rockchip/clk-rk3399.c      | 16 ++++++++++++++++
 include/dt-bindings/clock/rk3399-cru.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index f1d8e44..749ea59 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -118,6 +118,10 @@ PNAME(mux_armclkb_p)				= { "clk_core_b_lpll_src",
 						    "clk_core_b_bpll_src",
 						    "clk_core_b_dpll_src",
 						    "clk_core_b_gpll_src" };
+PNAME(mux_ddrclk_p)				= { "clk_ddrc_lpll_src",
+						    "clk_ddrc_bpll_src",
+						    "clk_ddrc_dpll_src",
+						    "clk_ddrc_gpll_src" };
 PNAME(mux_aclk_cci_p)				= { "cpll_aclk_cci_src",
 						    "gpll_aclk_cci_src",
 						    "npll_aclk_cci_src",
@@ -1377,6 +1381,18 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
 	COMPOSITE_NOMUX(0, "clk_test", "clk_test_pre", CLK_IGNORE_UNUSED,
 			RK3368_CLKSEL_CON(58), 0, 5, DFLAGS,
 			RK3368_CLKGATE_CON(13), 11, GFLAGS),
+
+	/* ddrc */
+	GATE(0, "clk_ddrc_lpll_src", "lpll", CLK_IGNORE_UNUSED,
+	     RK3399_CLKGATE_CON(3), 0, GFLAGS),
+	GATE(0, "clk_ddrc_bpll_src", "bpll", CLK_IGNORE_UNUSED,
+	     RK3399_CLKGATE_CON(3), 1, GFLAGS),
+	GATE(0, "clk_ddrc_dpll_src", "dpll", CLK_IGNORE_UNUSED,
+	     RK3399_CLKGATE_CON(3), 2, GFLAGS),
+	GATE(0, "clk_ddrc_gpll_src", "gpll", CLK_IGNORE_UNUSED,
+	     RK3399_CLKGATE_CON(3), 3, GFLAGS),
+	COMPOSITE_DDRC(SCLK_DDRCLK, "clk_ddrc", mux_ddrclk_p, CLK_IGNORE_UNUSED,
+		       RK3399_CLKSEL_CON(6), 4, 2, MFLAGS, 0, 3, DFLAGS),
 };
 
 static struct rockchip_clk_branch rk3399_clk_pmu_branches[] __initdata = {
diff --git a/include/dt-bindings/clock/rk3399-cru.h b/include/dt-bindings/clock/rk3399-cru.h
index 50a44cf..8a0f0442 100644
--- a/include/dt-bindings/clock/rk3399-cru.h
+++ b/include/dt-bindings/clock/rk3399-cru.h
@@ -131,6 +131,7 @@
 #define SCLK_DPHY_RX0_CFG		165
 #define SCLK_RMII_SRC			166
 #define SCLK_PCIEPHY_REF100M		167
+#define SCLK_DDRCLK			168
 
 #define DCLK_VOP0			180
 #define DCLK_VOP1			181
-- 
1.9.1

[toc] | [next] | [standalone]


#1411322

FromDoug Anderson <dianders@chromium.org>
Date2016-06-01 17:30 +0200
Message-ID<rFgfM-1DF-13@gated-at.bofh.it>
In reply to#1411032
Lin Huang,

On Wed, Jun 1, 2016 at 2:35 AM, Lin Huang <hl@rock-chips.com> wrote:
> add ddrc clock setting, so we can do ddr frequency
> scaling on rk3399 platform in future.
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
>  drivers/clk/rockchip/clk-rk3399.c      | 16 ++++++++++++++++
>  include/dt-bindings/clock/rk3399-cru.h |  1 +
>  2 files changed, 17 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index f1d8e44..749ea59 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -118,6 +118,10 @@ PNAME(mux_armclkb_p)                               = { "clk_core_b_lpll_src",
>                                                     "clk_core_b_bpll_src",
>                                                     "clk_core_b_dpll_src",
>                                                     "clk_core_b_gpll_src" };
> +PNAME(mux_ddrclk_p)                            = { "clk_ddrc_lpll_src",
> +                                                   "clk_ddrc_bpll_src",
> +                                                   "clk_ddrc_dpll_src",
> +                                                   "clk_ddrc_gpll_src" };
>  PNAME(mux_aclk_cci_p)                          = { "cpll_aclk_cci_src",
>                                                     "gpll_aclk_cci_src",
>                                                     "npll_aclk_cci_src",
> @@ -1377,6 +1381,18 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
>         COMPOSITE_NOMUX(0, "clk_test", "clk_test_pre", CLK_IGNORE_UNUSED,
>                         RK3368_CLKSEL_CON(58), 0, 5, DFLAGS,
>                         RK3368_CLKGATE_CON(13), 11, GFLAGS),
> +
> +       /* ddrc */
> +       GATE(0, "clk_ddrc_lpll_src", "lpll", CLK_IGNORE_UNUSED,
> +            RK3399_CLKGATE_CON(3), 0, GFLAGS),
> +       GATE(0, "clk_ddrc_bpll_src", "bpll", CLK_IGNORE_UNUSED,
> +            RK3399_CLKGATE_CON(3), 1, GFLAGS),
> +       GATE(0, "clk_ddrc_dpll_src", "dpll", CLK_IGNORE_UNUSED,
> +            RK3399_CLKGATE_CON(3), 2, GFLAGS),
> +       GATE(0, "clk_ddrc_gpll_src", "gpll", CLK_IGNORE_UNUSED,
> +            RK3399_CLKGATE_CON(3), 3, GFLAGS),
> +       COMPOSITE_DDRC(SCLK_DDRCLK, "clk_ddrc", mux_ddrclk_p, CLK_IGNORE_UNUSED,
> +                      RK3399_CLKSEL_CON(6), 4, 2, MFLAGS, 0, 3, DFLAGS),

It seems slightly unfortunate that we need CLK_IGNORE_UNUSED on these.
Only one of these will ever be used at once and it would be awfully
nice if the others could get gated, right?

...presumably this is needed because we might not have an actual
driver for DDR Freq and we definitely want to make sure that clk_ddrc
is enabled in that case.  I guess what we really want is something
like CLK_ENABLE_HAND_OFF eventually, but until then I think you might
get slightly better behavior by getting rid of all of these
CLK_IGNORE_UNUSED and setting "clk_ddrc" as a critical clock, either
using the table in this file or the new flag.

>  };
>
>  static struct rockchip_clk_branch rk3399_clk_pmu_branches[] __initdata = {
> diff --git a/include/dt-bindings/clock/rk3399-cru.h b/include/dt-bindings/clock/rk3399-cru.h
> index 50a44cf..8a0f0442 100644
> --- a/include/dt-bindings/clock/rk3399-cru.h
> +++ b/include/dt-bindings/clock/rk3399-cru.h
> @@ -131,6 +131,7 @@
>  #define SCLK_DPHY_RX0_CFG              165
>  #define SCLK_RMII_SRC                  166
>  #define SCLK_PCIEPHY_REF100M           167
> +#define SCLK_DDRCLK                    168

Almost certainly you'll want to create a separate patch for the
dt-bindings change since it will need to land in a different tree so
it can be pulled into both Heiko's clock topic branch and dts64 topic
branch.



-Doug

[toc] | [prev] | [next] | [standalone]


#1411357

FromHeiko Stübner <heiko@sntech.de>
Date2016-06-01 17:50 +0200
Message-ID<rFgz8-1Kx-9@gated-at.bofh.it>
In reply to#1411322
Am Mittwoch, 1. Juni 2016, 08:24:48 schrieb Doug Anderson:
> Lin Huang,
> 
> On Wed, Jun 1, 2016 at 2:35 AM, Lin Huang <hl@rock-chips.com> wrote:
> > add ddrc clock setting, so we can do ddr frequency
> > scaling on rk3399 platform in future.
> > 
> > Signed-off-by: Lin Huang <hl@rock-chips.com>
> > ---
> > 
> >  drivers/clk/rockchip/clk-rk3399.c      | 16 ++++++++++++++++
> >  include/dt-bindings/clock/rk3399-cru.h |  1 +
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/drivers/clk/rockchip/clk-rk3399.c
> > b/drivers/clk/rockchip/clk-rk3399.c index f1d8e44..749ea59 100644
> > --- a/drivers/clk/rockchip/clk-rk3399.c
> > +++ b/drivers/clk/rockchip/clk-rk3399.c
> > @@ -118,6 +118,10 @@ PNAME(mux_armclkb_p)                               =
> > { "clk_core_b_lpll_src",> 
> >                                                     "clk_core_b_bpll_src",
> >                                                     "clk_core_b_dpll_src",
> >                                                     "clk_core_b_gpll_src"
> >                                                     };
> > 
> > +PNAME(mux_ddrclk_p)                            = { "clk_ddrc_lpll_src",
> > +                                                   "clk_ddrc_bpll_src",
> > +                                                   "clk_ddrc_dpll_src",
> > +                                                   "clk_ddrc_gpll_src" };
> > 
> >  PNAME(mux_aclk_cci_p)                          = { "cpll_aclk_cci_src",
> >  
> >                                                     "gpll_aclk_cci_src",
> >                                                     "npll_aclk_cci_src",
> > 
> > @@ -1377,6 +1381,18 @@ static struct rockchip_clk_branch
> > rk3399_clk_branches[] __initdata = {> 
> >         COMPOSITE_NOMUX(0, "clk_test", "clk_test_pre", CLK_IGNORE_UNUSED,
> >         
> >                         RK3368_CLKSEL_CON(58), 0, 5, DFLAGS,
> >                         RK3368_CLKGATE_CON(13), 11, GFLAGS),
> > 
> > +
> > +       /* ddrc */
> > +       GATE(0, "clk_ddrc_lpll_src", "lpll", CLK_IGNORE_UNUSED,
> > +            RK3399_CLKGATE_CON(3), 0, GFLAGS),
> > +       GATE(0, "clk_ddrc_bpll_src", "bpll", CLK_IGNORE_UNUSED,
> > +            RK3399_CLKGATE_CON(3), 1, GFLAGS),
> > +       GATE(0, "clk_ddrc_dpll_src", "dpll", CLK_IGNORE_UNUSED,
> > +            RK3399_CLKGATE_CON(3), 2, GFLAGS),
> > +       GATE(0, "clk_ddrc_gpll_src", "gpll", CLK_IGNORE_UNUSED,
> > +            RK3399_CLKGATE_CON(3), 3, GFLAGS),
> > +       COMPOSITE_DDRC(SCLK_DDRCLK, "clk_ddrc", mux_ddrclk_p,
> > CLK_IGNORE_UNUSED, +                      RK3399_CLKSEL_CON(6), 4, 2,
> > MFLAGS, 0, 3, DFLAGS),
> It seems slightly unfortunate that we need CLK_IGNORE_UNUSED on these.
> Only one of these will ever be used at once and it would be awfully
> nice if the others could get gated, right?
> 
> ...presumably this is needed because we might not have an actual
> driver for DDR Freq and we definitely want to make sure that clk_ddrc
> is enabled in that case.  I guess what we really want is something
> like CLK_ENABLE_HAND_OFF eventually, but until then I think you might
> get slightly better behavior by getting rid of all of these
> CLK_IGNORE_UNUSED and setting "clk_ddrc" as a critical clock, either
> using the table in this file or the new flag.

My current feeling is that staying with the homegrown solution for critical 
clocks might be preferable until the clk-handoff mechanism lands as well, as 
our critical clocks fall into both categories and I'd like to not touch 
everything twice.

The clock above should be controlled by the dcf, so falls into the handoff 
category (critical until a driver picks up the clock).

Also mixing both new and old approach might get confusing.

But I'm definitly open to counter-arguments :-)


> >  };
> >  
> >  static struct rockchip_clk_branch rk3399_clk_pmu_branches[] __initdata =
> >  {
> > 
> > diff --git a/include/dt-bindings/clock/rk3399-cru.h
> > b/include/dt-bindings/clock/rk3399-cru.h index 50a44cf..8a0f0442 100644
> > --- a/include/dt-bindings/clock/rk3399-cru.h
> > +++ b/include/dt-bindings/clock/rk3399-cru.h
> > @@ -131,6 +131,7 @@
> > 
> >  #define SCLK_DPHY_RX0_CFG              165
> >  #define SCLK_RMII_SRC                  166
> >  #define SCLK_PCIEPHY_REF100M           167
> > 
> > +#define SCLK_DDRCLK                    168
> 
> Almost certainly you'll want to create a separate patch for the
> dt-bindings change since it will need to land in a different tree so
> it can be pulled into both Heiko's clock topic branch and dts64 topic
> branch.

correct.

[toc] | [prev] | [next] | [standalone]


#1411745

Fromhl <hl@rock-chips.com>
Date2016-06-02 03:40 +0200
Message-ID<rFpM5-7QO-15@gated-at.bofh.it>
In reply to#1411357
Hi Doug&Heiko,

On 2016年06月01日 23:46, Heiko Stübner wrote:
> Am Mittwoch, 1. Juni 2016, 08:24:48 schrieb Doug Anderson:
>> Lin Huang,
>>
>> On Wed, Jun 1, 2016 at 2:35 AM, Lin Huang <hl@rock-chips.com> wrote:
>>> add ddrc clock setting, so we can do ddr frequency
>>> scaling on rk3399 platform in future.
>>>
>>> Signed-off-by: Lin Huang <hl@rock-chips.com>
>>> ---
>>>
>>>   drivers/clk/rockchip/clk-rk3399.c      | 16 ++++++++++++++++
>>>   include/dt-bindings/clock/rk3399-cru.h |  1 +
>>>   2 files changed, 17 insertions(+)
>>>
>>> diff --git a/drivers/clk/rockchip/clk-rk3399.c
>>> b/drivers/clk/rockchip/clk-rk3399.c index f1d8e44..749ea59 100644
>>> --- a/drivers/clk/rockchip/clk-rk3399.c
>>> +++ b/drivers/clk/rockchip/clk-rk3399.c
>>> @@ -118,6 +118,10 @@ PNAME(mux_armclkb_p)                               =
>>> { "clk_core_b_lpll_src",>
>>>                                                      "clk_core_b_bpll_src",
>>>                                                      "clk_core_b_dpll_src",
>>>                                                      "clk_core_b_gpll_src"
>>>                                                      };
>>>
>>> +PNAME(mux_ddrclk_p)                            = { "clk_ddrc_lpll_src",
>>> +                                                   "clk_ddrc_bpll_src",
>>> +                                                   "clk_ddrc_dpll_src",
>>> +                                                   "clk_ddrc_gpll_src" };
>>>
>>>   PNAME(mux_aclk_cci_p)                          = { "cpll_aclk_cci_src",
>>>   
>>>                                                      "gpll_aclk_cci_src",
>>>                                                      "npll_aclk_cci_src",
>>>
>>> @@ -1377,6 +1381,18 @@ static struct rockchip_clk_branch
>>> rk3399_clk_branches[] __initdata = {>
>>>          COMPOSITE_NOMUX(0, "clk_test", "clk_test_pre", CLK_IGNORE_UNUSED,
>>>          
>>>                          RK3368_CLKSEL_CON(58), 0, 5, DFLAGS,
>>>                          RK3368_CLKGATE_CON(13), 11, GFLAGS),
>>>
>>> +
>>> +       /* ddrc */
>>> +       GATE(0, "clk_ddrc_lpll_src", "lpll", CLK_IGNORE_UNUSED,
>>> +            RK3399_CLKGATE_CON(3), 0, GFLAGS),
>>> +       GATE(0, "clk_ddrc_bpll_src", "bpll", CLK_IGNORE_UNUSED,
>>> +            RK3399_CLKGATE_CON(3), 1, GFLAGS),
>>> +       GATE(0, "clk_ddrc_dpll_src", "dpll", CLK_IGNORE_UNUSED,
>>> +            RK3399_CLKGATE_CON(3), 2, GFLAGS),
>>> +       GATE(0, "clk_ddrc_gpll_src", "gpll", CLK_IGNORE_UNUSED,
>>> +            RK3399_CLKGATE_CON(3), 3, GFLAGS),
>>> +       COMPOSITE_DDRC(SCLK_DDRCLK, "clk_ddrc", mux_ddrclk_p,
>>> CLK_IGNORE_UNUSED, +                      RK3399_CLKSEL_CON(6), 4, 2,
>>> MFLAGS, 0, 3, DFLAGS),
>> It seems slightly unfortunate that we need CLK_IGNORE_UNUSED on these.
>> Only one of these will ever be used at once and it would be awfully
>> nice if the others could get gated, right?
>>
>> ...presumably this is needed because we might not have an actual
>> driver for DDR Freq and we definitely want to make sure that clk_ddrc
>> is enabled in that case.  I guess what we really want is something
>> like CLK_ENABLE_HAND_OFF eventually, but until then I think you might
>> get slightly better behavior by getting rid of all of these
>> CLK_IGNORE_UNUSED and setting "clk_ddrc" as a critical clock, either
>> using the table in this file or the new flag.
> My current feeling is that staying with the homegrown solution for critical
> clocks might be preferable until the clk-handoff mechanism lands as well, as
> our critical clocks fall into both categories and I'd like to not touch
> everything twice.
>
> The clock above should be controlled by the dcf, so falls into the handoff
> category (critical until a driver picks up the clock).
>
> Also mixing both new and old approach might get confusing.
>
> But I'm definitly open to counter-arguments :-)
     I will remove CLK_IGNORE_UNUSED flag  and set
     "clk_ddrc"  as critical clock in next version. Besides, i think we 
should also
     set "clk_ddrc_dpll_src" as critical clock, since we always use dpll 
as ddr clock source,
     and it should always on.
>
>>>   };
>>>   
>>>   static struct rockchip_clk_branch rk3399_clk_pmu_branches[] __initdata =
>>>   {
>>>
>>> diff --git a/include/dt-bindings/clock/rk3399-cru.h
>>> b/include/dt-bindings/clock/rk3399-cru.h index 50a44cf..8a0f0442 100644
>>> --- a/include/dt-bindings/clock/rk3399-cru.h
>>> +++ b/include/dt-bindings/clock/rk3399-cru.h
>>> @@ -131,6 +131,7 @@
>>>
>>>   #define SCLK_DPHY_RX0_CFG              165
>>>   #define SCLK_RMII_SRC                  166
>>>   #define SCLK_PCIEPHY_REF100M           167
>>>
>>> +#define SCLK_DDRCLK                    168
>> Almost certainly you'll want to create a separate patch for the
>> dt-bindings change since it will need to land in a different tree so
>> it can be pulled into both Heiko's clock topic branch and dts64 topic
>> branch.
> correct.
     will fix next version, thanks.
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>

-- 
Lin Huang

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web