linux-kernel - Re: [PATCH v9 3/5] clk: imx: add SCCG PLL type

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]

Message-ID: <20181107202624.oya2runvbbwcilzu@fsr-ub1664-175>
Date:   Wed, 7 Nov 2018 20:26:25 +0000
From:   Abel Vesa <abel.vesa@....com>
To:     Stephen Boyd <sboyd@...nel.org>
CC:     Andrey Smirnov <andrew.smirnov@...il.com>,
        Anson Huang <anson.huang@....com>,
        "A.s. Dong" <aisheng.dong@....com>,
        Fabio Estevam <fabio.estevam@....com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Rob Herring <robh@...nel.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        dl-linux-imx <linux-imx@....com>, Abel Vesa <abelvesa@...ux.com>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Michael Turquette <mturquette@...libre.com>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        "open list:COMMON CLK FRAMEWORK" <linux-clk@...r.kernel.org>
Subject: Re: [PATCH v9 3/5] clk: imx: add SCCG PLL type

On Wed, Nov 07, 2018 at 11:01:02AM -0800, Stephen Boyd wrote:
> Quoting Abel Vesa (2018-11-07 03:54:45)
> > On Wed, Oct 17, 2018 at 12:55:52PM -0700, Stephen Boyd wrote:
> > > Quoting Abel Vesa (2018-09-24 03:39:55)
> > > > +static unsigned long clk_pll2_recalc_rate(struct clk_hw *hw,
> > > > +                                        unsigned long parent_rate)
> > > > +{
> > > > +       struct clk_sccg_pll *pll = to_clk_sccg_pll(hw);
> > > > +       u32 val, ref, divr1, divf1, divr2, divf2;
> > > > +       u64 temp64;
> > > > +
> > > > +       val = readl_relaxed(pll->base + PLL_CFG0);
> > > > +       switch (FIELD_GET(PLL_REF_MASK, val)) {
> > > > +       case 0:
> > > > +               ref = OSC_25M;
> > > > +               break;
> > > > +       case 1:
> > > > +               ref = OSC_27M;
> > > > +               break;
> > > > +       default:
> > > > +               ref = OSC_25M;
> > > 
> > > Does this information not come through 'parent_rate'?
> > > 
> > 
> > No. So basically both pll1 and pll2 and the divider after it form together this SCCG:
> > 
> > https://www.nxp.com/docs/en/reference-manual/IMX8MDQLQRM.pdf#page=834
> > 
> > See: Figure 5-8. SSCG PLL Block Diagram
> 
> Thanks for the link!
> 
> > 
> > We're basically reading the input of the pll 1 in order to compute the output of the entire SCCG.
> > 
> > I know it's a mess. I'm working on cleaning it up, but for now we need this in in order to boot up.
> 
> What's the plan to clean it up?

So I'm doing this in our internal tree first to make sure I don't break the
other (newer) socs.

I already have a prototype in testing but it's a long way to upstream it.

Basically, I'm replacing all of this with a single, more like a composite,
more complex, clock type that does all the magic inside.

One of the problems is the fact that the bypasses can have the same sources
and in my case, I'm implementing that as a list of parents name, but the
parent names list doesn't work with duplicates, so I have to find some other
way to do it.

Once I have something clean and tested enough I'll send it upstream.

> 
> > 
> > > > +               break;
> > > > +       }
> > > > +
> > > > +       val = readl_relaxed(pll->base + PLL_CFG2);
> > > > +       divr1 = FIELD_GET(PLL_DIVR1_MASK, val);
> > > > +       divr2 = FIELD_GET(PLL_DIVR2_MASK, val);
> > > > +       divf1 = FIELD_GET(PLL_DIVF1_MASK, val);
> > > > +       divf2 = FIELD_GET(PLL_DIVF2_MASK, val);
> > > > +
> > > > +       temp64 = ref * 2;
> > > > +       temp64 *= (divf1 + 1) * (divf2 + 1);
> > > > +
> > > > +       do_div(temp64, (divr1 + 1) * (divr2 + 1));
> > > 
> > > Nitpicks: A comment with the equation may be helpful to newcomers.
> > 
> > Since the SCCG is contructed by multiple different types of clocks here, the equation doesn't help
> > since it is spread in all constructing blocks.
> 
> Ok.
> 

--