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
