Re: [RESEND] clk: qcom: rcg: Update rcg configuration before enabling it

[Dmitry Baryshkov]

On Wed, 12 Jul 2023 at 04:49, Satya Priya Kakitapalli
<quic_skakitap@xxxxxxxxxxx> wrote:
>
> From: Taniya Das <quic_tdas@xxxxxxxxxxx>
>
> If rcg is in disabled state when clk_rcg2_shared_set_rate is called, the
> new configuration is written to the configuration register but it won't be
> effective in h/w yet because update bit won't be set if rcg is in disabled
> state. Since the new configuration is not yet updated in h/w, dirty bit of
> configuration register will be set in such case. Clear the dirty bit and
> update the rcg to proper new configuration by setting the update bit before
> enabling the rcg.
>
> Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx>

I think there should be your S-o-B too, as you are resending this patch.

Next, should there be any Fixes tags? Probably this is a fix for 703db1f5da1e ?

> ---
> Resending this patch as there is no review for 2 months.
>
>  drivers/clk/qcom/clk-rcg2.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index e22baf3a7112..b25635feb617 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -982,7 +982,13 @@ static int clk_rcg2_set_force_enable(struct clk_hw *hw)
>  {
>         struct clk_rcg2 *rcg = to_clk_rcg2(hw);
>         const char *name = clk_hw_get_name(hw);
> -       int ret, count;
> +       int ret, count, val;
> +
> +       if (!__clk_is_enabled(hw->clk)) {

There is clk_hw_is_enabled(). Can you consider using it instead?

Also, I'd like to point out the traditional source of controversy,
confusion and troubles. The clk_rcg2_shared_ops does not have the
is_enabled callback. Will this code work as expected in this case?

> +               regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + CMD_REG, &val);
> +               if (val & CMD_DIRTY_CFG)
> +                       update_config(rcg);
> +       }
>
>         ret = regmap_update_bits(rcg->clkr.regmap, rcg->cmd_rcgr + CMD_REG,
>                                  CMD_ROOT_EN, CMD_ROOT_EN);
> --
> 2.25.1
>


-- 
With best wishes
Dmitry