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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Jul 2023 17:15:47 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Satya Priya Kakitapalli <quic_skakitap@...cinc.com>
Cc:     Konrad Dybcio <konrad.dybcio@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Jagadeesh Kona <quic_jkona@...cinc.com>,
        Taniya Das <quic_tdas@...cinc.com>
Subject: Re: [RESEND] clk: qcom: rcg: Update rcg configuration before enabling it

On Wed, 12 Jul 2023 at 04:49, Satya Priya Kakitapalli
<quic_skakitap@...cinc.com> wrote:
>
> From: Taniya Das <quic_tdas@...cinc.com>
>
> 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@...cinc.com>

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