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

[Stephen Boyd <sboyd@codeaurora.org>]

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