linux-kernel - Re: [PATCH 2/6] clk: qcom: apss-ipq6018: fix apcs_alias0_clk

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 11 Jul 2022 15:23:44 +0200
From:   Robert Marko <robimarko@...il.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, sivaprak@...eaurora.org,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        linux-clk@...r.kernel.org,
        Devicetree List <devicetree@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/6] clk: qcom: apss-ipq6018: fix apcs_alias0_clk_src

On Mon, 11 Jul 2022 at 14:48, Dmitry Baryshkov
<dmitry.baryshkov@...aro.org> wrote:
>
> On Mon, 11 Jul 2022 at 14:22, Robert Marko <robimarko@...il.com> wrote:
> >
> > While working on IPQ8074 APSS driver it was discovered that IPQ6018 and
> > IPQ8074 use almost the same PLL and APSS clocks, however APSS driver is
> > currently broken.
> >
> > More precisely apcs_alias0_clk_src is broken, it was added as regmap_mux
> > clock.
> > However after debugging why it was always stuck at 800Mhz, it was figured
> > out that its not regmap_mux compatible at all.
> > It is a simple mux but it uses RCG2 register layout and control bits, so
>
> To utilize control bits, you probably should also use

Hi,
I am not really sure what you mean here?

>
> > utilize the new clk_rcg2_mux_closest_ops to correctly drive it while not
> > having to provide a dummy frequency table.
>
> Could you please clarify this. Your new rcg2 ops seems to be literally
> equivalent to the clk_regmap_mux_closest_ops provided the shift and
> width are set correctly..

Well, I have tried playing with the clk_regmap_mux_closest_ops but I
just cannot get it
to work.

The width like you pointed out should be 8, register offset is
currently pointing at the RCG control
register and not the CFG one, so it obviously does not work.

Setting the register to 0x54 and shift to 8 will just fail silently,
leaving the shift at 7 and correcting
the register won't work as RCG control bits are not utilized at all
with regmap_mux and DIRTY_CFG
is active when I manually look at the register.

So, I am really not sure how clk_regmap_mux_closest_ops are supposed
to work here at all.

Regards,
Robert
>
> > While we are here, use ARRAY_SIZE for number of parents.
> >
> > Tested on IPQ6018-CP01-C1 reference board and multiple IPQ8074 boards.
> >
> > Fixes: 5e77b4ef1b19 ("clk: qcom: Add ipq6018 apss clock controller")
> > Signed-off-by: Robert Marko <robimarko@...il.com>
> > ---
> >  drivers/clk/qcom/apss-ipq6018.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/apss-ipq6018.c b/drivers/clk/qcom/apss-ipq6018.c
> > index d78ff2f310bf..be952d417ded 100644
> > --- a/drivers/clk/qcom/apss-ipq6018.c
> > +++ b/drivers/clk/qcom/apss-ipq6018.c
> > @@ -16,7 +16,7 @@
> >  #include "clk-regmap.h"
> >  #include "clk-branch.h"
> >  #include "clk-alpha-pll.h"
> > -#include "clk-regmap-mux.h"
> > +#include "clk-rcg.h"
> >
> >  enum {
> >         P_XO,
> > @@ -33,16 +33,15 @@ static const struct parent_map parents_apcs_alias0_clk_src_map[] = {
> >         { P_APSS_PLL_EARLY, 5 },
> >  };
> >
> > -static struct clk_regmap_mux apcs_alias0_clk_src = {
> > -       .reg = 0x0050,
> > -       .width = 3,
> > -       .shift = 7,
>
> Judging from rcg2 ops, .shift should be set to 8.
>
> > +static struct clk_rcg2 apcs_alias0_clk_src = {
> > +       .cmd_rcgr = 0x0050,
> > +       .hid_width = 5,
> >         .parent_map = parents_apcs_alias0_clk_src_map,
> >         .clkr.hw.init = &(struct clk_init_data){
> >                 .name = "apcs_alias0_clk_src",
> >                 .parent_data = parents_apcs_alias0_clk_src,
> > -               .num_parents = 2,
> > -               .ops = &clk_regmap_mux_closest_ops,
> > +               .num_parents = ARRAY_SIZE(parents_apcs_alias0_clk_src),
> > +               .ops = &clk_rcg2_mux_closest_ops,
> >                 .flags = CLK_SET_RATE_PARENT,
> >         },
> >  };
> > --
> > 2.36.1
> >
>
>
> --
> With best wishes
> Dmitry