Re: [RFC PATCH 1/2] clk: qcom: implement RCG2 'parked' clock support

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 04/10/2023 01:31, Dmitry Baryshkov wrote:
> clk_rcg2_shared_ops implements support for the case of the RCG which
> must not be completely turned off. However its design has one major
> drawback: it doesn't allow us to properly implement the is_enabled
> callback, which causes different kinds of misbehaviour from the CCF.
> 
> Follow the idea behind clk_regmap_phy_mux_ops and implement the new
> clk_rcg2_parked_ops. It also targets the clocks which must not be fully
> switched off (and shared most of the implementation with
> clk_rcg2_shared_ops). The major difference is that it requires that the
> parent map doesn't conain the safe (parked) clock source. Instead if the
> CFG_REG register points to the safe source, the clock is considered to
> be disabled.

Why not have a new bit in .flags ?

Instead of lying about the clock being off, mark the clock as "parked", 
or "safe parked" or whatever term we choose for it ?

I feel 'disabled' should mean disabled and 'enabled' should me enabled 
when I read a value from debugfs and if we are parking a clock it should 
have a clear means of being flagged as a clock that should be parked.

An example. I recently inherited some autogenerated code for camcc on 
sc8280xp.

One of the clocks is marked as CLK_IS_CRITICAL by the autogen code 
meaning "never switch off" but CLK_IS_CRITICAL stops the camcc driver 
from doing runtime pm_ops to power collapse.

The solution I have is to remove CLK_IS_CRITICAL and to hard-code in 
probe() the clock to always on.

But if we had a CLK_DISABLE_SAFE_PARK flag - then not just for rcg but 
for branch clocks we could differentiate away from hard-coded always on 
in probe...

?

---
bod