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


Groups > linux.kernel > #1514827

Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks

From Stephen Boyd <sboyd@codeaurora.org>
Newsgroups linux.kernel
Subject Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks
Date 2016-11-03 21:40 +0100
Message-ID <szwXM-lr-27@gated-at.bofh.it> (permalink)
References <svKwh-6el-13@gated-at.bofh.it> <svKwh-6el-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 10/24, Sricharan R wrote:
> With the venus subcore0/1 gdscs(powerdomains) in
> hw controlled mode, the clock controller does not handle
> the status bit for the clocks in that domain. So avoid
> checking for the status bit of those clocks by setting the
> BRANCH_HALT_DELAY flag. This avoids the WARN_ONs which
> otherwise occurs when enabling/disabling those clocks.
> 
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>

A better design would be to check if the associated GDSC is in hw
control mode and then skip the checks because the clocks are no
longer under the control of the registers. I presume we only
enable the clocks here to turn on parent clocks which don't turn
on/off automatically, i.e. the PLL.

Given that hw control is a static decision I guess that is an
over-engineered solution though? The problem is that this seems
brittle because we have to keep two things in sync, the branches
and the gdsc. So I guess this is ok, but it deserves a comment
like "GDSC is in HW control" so we know what's going on. Also the
commit text could be more explicit that clocks within the gdsc
power domain don't work when the gdsc is off, and with hw control
of a gdsc we can't tell when the gdsc may be off or on.

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

Back to linux.kernel | Previous | NextNext in thread | Find similar | Unroll thread


Thread

Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus  core0/1 clks Stephen Boyd <sboyd@codeaurora.org> - 2016-11-03 21:40 +0100
  RE: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks "Sricharan" <sricharan@codeaurora.org> - 2016-11-04 10:20 +0100
    Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus  core0/1 clks 'Stephen Boyd' <sboyd@codeaurora.org> - 2016-11-04 21:30 +0100
      Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus  core0/1 clks Rajendra Nayak <rnayak@codeaurora.org> - 2016-11-07 06:50 +0100
        Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus  core0/1 clks 'Stephen Boyd' <sboyd@codeaurora.org> - 2016-11-08 23:40 +0100
          RE: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks "Sricharan" <sricharan@codeaurora.org> - 2016-11-09 18:00 +0100
            Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus  core0/1 clks Rajendra Nayak <rnayak@codeaurora.org> - 2016-11-10 03:40 +0100
              RE: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus core0/1 clks "Sricharan" <sricharan@codeaurora.org> - 2016-11-10 04:30 +0100
            Re: [PATCH 3/3] clk: qcom: Set BRANCH_HALT_DELAY flags for venus  core0/1 clks 'Stephen Boyd' <sboyd@codeaurora.org> - 2016-11-11 00:40 +0100

csiph-web