All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kever Yang <kever.yang@rock-chips.com>
To: "Heiko Stübner" <heiko@sntech.de>,
	"Doug Anderson" <dianders@chromium.org>
Cc: "Sonny Rao" <sonnyrao@chromium.org>,
	"Addy Ke" <addy.ke@rock-chips.com>,
	"Eddie Cai" <cf@rock-chips.com>,
	"Jianqun Xu" <xjq@rock-chips.com>,
	"han jiang" <hj@rock-chips.com>,
	"\"戴克霖 (Jack)\"" <dkl@rock-chips.com>,
	"Tao Huang" <huangtao@rock-chips.com>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	"Mike Turquette" <mturquette@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] clk: rockchip: disable unused clocks
Date: Thu, 30 Oct 2014 09:10:49 +0800	[thread overview]
Message-ID: <54519019.6010903@rock-chips.com> (raw)
In-Reply-To: <13348555.zj6gsMcCpK@diego>

Hi

On 10/30/2014 05:53 AM, Heiko Stübner wrote:
> Am Mittwoch, 29. Oktober 2014, 13:50:20 schrieb Doug Anderson:
>> Kever,
>>
>> On Wed, Oct 29, 2014 at 3:06 AM, Kever Yang <kever.yang@rock-chips.com>
> wrote:
>>> The rockchip clock driver use CLK_IGNORE_UNUSED flag to make sure
>>> all the clocks are available like default power on state.
>>> We have implement the clock manage in most of rockchip drivers,
>>> it is time to remove it for power save.
>>> Instead we add CLK_IGNORE_UNUSED for some clock nodes which should
>>> be on during boot or no module driver in kernel will initialize it.
>>>
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> ---
>>>
>>>   drivers/clk/rockchip/clk-rk3188.c |  32 ++++-----
>>>   drivers/clk/rockchip/clk-rk3288.c | 146
>>>   +++++++++++++++++++------------------- drivers/clk/rockchip/clk.c
>>>   |   9 ---
>>>   3 files changed, 90 insertions(+), 97 deletions(-)
>> Your patch didn't seem to apply to the top of Heiko's
>> "v3.19-clk/next".  Where should it apply to?
I apply the patch on the top of linux-next, I will apply it on Heiko's
"v3.19-clk/next" next version.
>>
>>> -       GATE(ACLK_CPU, "aclk_cpu", "aclk_cpu_pre", 0,
>>> +       GATE(0, "aclk_cpu", "aclk_cpu_pre", CLK_IGNORE_UNUSED,
>>>
>>>                          RK3288_CLKGATE_CON(0), 3, GFLAGS),
>> It seems strange to me that you're removing the ACLK_CPU ID here.  Was
>> that on purpose?
Sorry for that, I didn't mean to removing any ID here, will fix in next 
version.
>>
>>> -       COMPOSITE_NOMUX(PCLK_CPU, "pclk_cpu", "aclk_cpu_pre", 0,
>>> +       COMPOSITE_NOMUX(0, "pclk_cpu", "aclk_cpu_pre", CLK_IGNORE_UNUSED,
>>>
>>>                          RK3288_CLKSEL_CON(1), 12, 3, DFLAGS,
>>>                          RK3288_CLKGATE_CON(0), 5, GFLAGS),
>> Here too for PCLK_CPU.  There are a few others as well.
> yeah, they should keep their clock ids
>
>
>> I'll also say that when I applied this atop my local tree that USB
>> stopped working.  I just see:
>>
>> [ 1647.626747] hub 2-1:1.0: hub_port_status failed (err = -110)
>> [ 1657.626746] hub 2-1:1.0: hub_port_status failed (err = -110)
>>
>> ...over and over and over again.
> maybe Kever's dwc2 clock-handling patch [0] might help there, as the dwc2-host
> currently does not seem to do any clock handling at all.
I think Heiko is right, I test in my evb with following change and dwc2
works well:
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -654,8 +654,8 @@ static struct rockchip_clk_branch 
rk3288_clk_branches[] __initdata = {

         /* hclk_peri gates */
         GATE(0, "hclk_peri_matrix", "hclk_peri", CLK_IGNORE_UNUSED, 
RK3288_CLKGATE_CON(6), 0, GFLAGS),
-       GATE(HCLK_OTG0, "hclk_otg0", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 4, GFLAGS),
-       GATE(HCLK_USBHOST0, "hclk_host0", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 6, GFLAGS),
+       GATE(HCLK_OTG0, "hclk_otg0", "hclk_peri", CLK_IGNORE_UNUSED, 
RK3288_CLKGATE_CON(7), 4, GFLAGS),
+       GATE(HCLK_USBHOST0, "hclk_host0", "hclk_peri", 
CLK_IGNORE_UNUSED, RK3288_CLKGATE_CON(7), 6, GFLAGS),
         GATE(HCLK_USBHOST1, "hclk_host1", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 7, GFLAGS),
         GATE(HCLK_HSIC, "hclk_hsic", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 8, GFLAGS),
         GATE(0, "hclk_usb_peri", "hclk_peri", CLK_IGNORE_UNUSED, 
RK3288_CLKGATE_CON(7), 9, GFLAGS),

- Kever


WARNING: multiple messages have this Message-ID (diff)
From: kever.yang@rock-chips.com (Kever Yang)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: rockchip: disable unused clocks
Date: Thu, 30 Oct 2014 09:10:49 +0800	[thread overview]
Message-ID: <54519019.6010903@rock-chips.com> (raw)
In-Reply-To: <13348555.zj6gsMcCpK@diego>

Hi

On 10/30/2014 05:53 AM, Heiko St?bner wrote:
> Am Mittwoch, 29. Oktober 2014, 13:50:20 schrieb Doug Anderson:
>> Kever,
>>
>> On Wed, Oct 29, 2014 at 3:06 AM, Kever Yang <kever.yang@rock-chips.com>
> wrote:
>>> The rockchip clock driver use CLK_IGNORE_UNUSED flag to make sure
>>> all the clocks are available like default power on state.
>>> We have implement the clock manage in most of rockchip drivers,
>>> it is time to remove it for power save.
>>> Instead we add CLK_IGNORE_UNUSED for some clock nodes which should
>>> be on during boot or no module driver in kernel will initialize it.
>>>
>>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>>> ---
>>>
>>>   drivers/clk/rockchip/clk-rk3188.c |  32 ++++-----
>>>   drivers/clk/rockchip/clk-rk3288.c | 146
>>>   +++++++++++++++++++------------------- drivers/clk/rockchip/clk.c
>>>   |   9 ---
>>>   3 files changed, 90 insertions(+), 97 deletions(-)
>> Your patch didn't seem to apply to the top of Heiko's
>> "v3.19-clk/next".  Where should it apply to?
I apply the patch on the top of linux-next, I will apply it on Heiko's
"v3.19-clk/next" next version.
>>
>>> -       GATE(ACLK_CPU, "aclk_cpu", "aclk_cpu_pre", 0,
>>> +       GATE(0, "aclk_cpu", "aclk_cpu_pre", CLK_IGNORE_UNUSED,
>>>
>>>                          RK3288_CLKGATE_CON(0), 3, GFLAGS),
>> It seems strange to me that you're removing the ACLK_CPU ID here.  Was
>> that on purpose?
Sorry for that, I didn't mean to removing any ID here, will fix in next 
version.
>>
>>> -       COMPOSITE_NOMUX(PCLK_CPU, "pclk_cpu", "aclk_cpu_pre", 0,
>>> +       COMPOSITE_NOMUX(0, "pclk_cpu", "aclk_cpu_pre", CLK_IGNORE_UNUSED,
>>>
>>>                          RK3288_CLKSEL_CON(1), 12, 3, DFLAGS,
>>>                          RK3288_CLKGATE_CON(0), 5, GFLAGS),
>> Here too for PCLK_CPU.  There are a few others as well.
> yeah, they should keep their clock ids
>
>
>> I'll also say that when I applied this atop my local tree that USB
>> stopped working.  I just see:
>>
>> [ 1647.626747] hub 2-1:1.0: hub_port_status failed (err = -110)
>> [ 1657.626746] hub 2-1:1.0: hub_port_status failed (err = -110)
>>
>> ...over and over and over again.
> maybe Kever's dwc2 clock-handling patch [0] might help there, as the dwc2-host
> currently does not seem to do any clock handling at all.
I think Heiko is right, I test in my evb with following change and dwc2
works well:
--- a/drivers/clk/rockchip/clk-rk3288.c
+++ b/drivers/clk/rockchip/clk-rk3288.c
@@ -654,8 +654,8 @@ static struct rockchip_clk_branch 
rk3288_clk_branches[] __initdata = {

         /* hclk_peri gates */
         GATE(0, "hclk_peri_matrix", "hclk_peri", CLK_IGNORE_UNUSED, 
RK3288_CLKGATE_CON(6), 0, GFLAGS),
-       GATE(HCLK_OTG0, "hclk_otg0", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 4, GFLAGS),
-       GATE(HCLK_USBHOST0, "hclk_host0", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 6, GFLAGS),
+       GATE(HCLK_OTG0, "hclk_otg0", "hclk_peri", CLK_IGNORE_UNUSED, 
RK3288_CLKGATE_CON(7), 4, GFLAGS),
+       GATE(HCLK_USBHOST0, "hclk_host0", "hclk_peri", 
CLK_IGNORE_UNUSED, RK3288_CLKGATE_CON(7), 6, GFLAGS),
         GATE(HCLK_USBHOST1, "hclk_host1", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 7, GFLAGS),
         GATE(HCLK_HSIC, "hclk_hsic", "hclk_peri", 0, 
RK3288_CLKGATE_CON(7), 8, GFLAGS),
         GATE(0, "hclk_usb_peri", "hclk_peri", CLK_IGNORE_UNUSED, 
RK3288_CLKGATE_CON(7), 9, GFLAGS),

- Kever

  reply	other threads:[~2014-10-30  1:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-29 10:06 [PATCH] clk: rockchip: disable unused clocks Kever Yang
2014-10-29 10:06 ` Kever Yang
2014-10-29 19:05 ` Heiko Stübner
2014-10-29 19:05   ` Heiko Stübner
2014-10-29 20:50 ` Doug Anderson
2014-10-29 20:50   ` Doug Anderson
2014-10-29 21:53   ` Heiko Stübner
2014-10-29 21:53     ` Heiko Stübner
2014-10-30  1:10     ` Kever Yang [this message]
2014-10-30  1:10       ` Kever Yang
2014-10-30  4:21       ` Doug Anderson
2014-10-30  4:21         ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54519019.6010903@rock-chips.com \
    --to=kever.yang@rock-chips.com \
    --cc=addy.ke@rock-chips.com \
    --cc=cf@rock-chips.com \
    --cc=dianders@chromium.org \
    --cc=dkl@rock-chips.com \
    --cc=heiko@sntech.de \
    --cc=hj@rock-chips.com \
    --cc=huangtao@rock-chips.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mturquette@linaro.org \
    --cc=sonnyrao@chromium.org \
    --cc=xjq@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.