On Wed, Oct 04, 2023 at 03:31:24AM +0300, 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.
I have some behaviors in mind when reading this, others might not.
Please give some specific behavior(s) here.
Thanks,
Bjorn
>
> 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.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
> drivers/clk/qcom/clk-rcg.h | 1 +
> drivers/clk/qcom/clk-rcg2.c | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index e6d84c8c7989..9fbbf1251564 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -176,6 +176,7 @@ extern const struct clk_ops clk_byte2_ops;
> extern const struct clk_ops clk_pixel_ops;
> extern const struct clk_ops clk_gfx3d_ops;
> extern const struct clk_ops clk_rcg2_shared_ops;
> +extern const struct clk_ops clk_rcg2_parked_ops;
> extern const struct clk_ops clk_dp_ops;
>
> struct clk_rcg_dfs_data {
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 5183c74b074f..3f52abf0025e 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -5,6 +5,7 @@
>
> #include <linux/kernel.h>
> #include <linux/bitops.h>
> +#include <linux/bitfield.h>
> #include <linux/err.h>
> #include <linux/bug.h>
> #include <linux/export.h>
> @@ -1150,6 +1151,39 @@ const struct clk_ops clk_rcg2_shared_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
>
> +static int clk_rcg2_park_is_enabled(struct clk_hw *hw)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + u32 cfg;
> + int ret;
> +
> + if (!clk_rcg2_is_enabled(hw))
> + return false;
> +
> + ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CFG_REG, &cfg);
> + if (ret)
> + return ret;
> +
> + return FIELD_GET(CFG_SRC_SEL_MASK, cfg) != rcg->safe_src_index;
> +}
> +
> +/*
> + * Unlike clk_rcg2_shared_ops, the safe_src_index aka XO must NOT be present in
> + * parent_map. This allows us to implement proper is_enabled callback.
> + */
> +const struct clk_ops clk_rcg2_parked_ops = {
> + .is_enabled = clk_rcg2_park_is_enabled,
> + .enable = clk_rcg2_shared_enable,
> + .disable = clk_rcg2_shared_disable,
> + .get_parent = clk_rcg2_shared_get_parent,
> + .set_parent = clk_rcg2_shared_set_parent,
> + .recalc_rate = clk_rcg2_shared_recalc_rate,
> + .determine_rate = clk_rcg2_determine_rate,
> + .set_rate = clk_rcg2_shared_set_rate,
> + .set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
> +};
> +EXPORT_SYMBOL_GPL(clk_rcg2_parked_ops);
> +
> /* Common APIs to be used for DFS based RCGR */
> static void clk_rcg2_dfs_populate_freq(struct clk_hw *hw, unsigned int l,
> struct freq_tbl *f)
> --
> 2.39.2
>
