linux-kernel - Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

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:   Fri, 27 Apr 2018 15:00:18 +0200
From:   Marcel Ziswiler <marcel@...wiler.com>
To:     Dmitry Osipenko <digetx@...il.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Peter De Schrijver <pdeschrijver@...dia.com>,
        Prashant Gaikwad <pgaikwad@...dia.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Marc Dietrich <marvin24@....de>
Cc:     linux-clk@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/4] clk: tegra20: Add DEV1/DEV2 OSC dividers

On Fri, 2018-04-27 at 15:54 +0300, Dmitry Osipenko wrote:
> Hi Marcel,
> 
> On 27.04.2018 15:33,  Ziswiler wrote:
> > Hi Dmitry
> > 
> > Isn't the CLK_RST_CONTROLLER_MISC_CLK_ENB_0 the other way around
> > e.g.
> > DEV1_OSC_DIV_SEL at bit 23:22 and DEV2_OSC_DIV_SEL at 21:20?
> > 
> > On Fri, 2018-04-27 at 02:58 +0300, Dmitry Osipenko wrote:
> > > CDEV1/CDEV2 clocks could have corresponding oscillator clock
> > > divider
> > > as
> > > a parent. Add these dividers in order to be able to provide that
> > > parent
> > > option.
> > > 
> > > Signed-off-by: Dmitry Osipenko <digetx@...il.com>
> > > ---
> > >  drivers/clk/tegra/clk-tegra20.c | 12 ++++++++++++
> > >  1 file changed, 12 insertions(+)
> > > 
> > > diff --git a/drivers/clk/tegra/clk-tegra20.c
> > > b/drivers/clk/tegra/clk-
> > > tegra20.c
> > > index 0ee56dd04cec..16cf4108f2ff 100644
> > > --- a/drivers/clk/tegra/clk-tegra20.c
> > > +++ b/drivers/clk/tegra/clk-tegra20.c
> > > @@ -26,6 +26,8 @@
> > >  #include "clk.h"
> > >  #include "clk-id.h"
> > >  
> > > +#define MISC_CLK_ENB 0x48
> > > +
> > >  #define OSC_CTRL 0x50
> > >  #define OSC_CTRL_OSC_FREQ_MASK (3<<30)
> > >  #define OSC_CTRL_OSC_FREQ_13MHZ (0<<30)
> > > @@ -831,6 +833,16 @@ static void __init
> > > tegra20_periph_clk_init(void)
> > >  				    periph_clk_enb_refcnt);
> > >  	clks[TEGRA20_CLK_PEX] = clk;
> > >  
> > > +	/* cdev1 OSC divider */
> > > +	clk_register_divider(NULL, "cdev1_osc_div", "clk_m",
> > > +			     0, clk_base + MISC_CLK_ENB, 20, 2,
> > 
> > So it would be:
> > 
> > +			     0, clk_base + MISC_CLK_ENB, 22, 2,
> > 
> > > +			     CLK_DIVIDER_POWER_OF_TWO, NULL);
> > > +
> > > +	/* cdev2 OSC divider */
> > > +	clk_register_divider(NULL, "cdev2_osc_div", "clk_m",
> > > +			     0, clk_base + MISC_CLK_ENB, 22, 2,
> > 
> > And:
> > 
> > +			     0, clk_base + MISC_CLK_ENB, 20, 2,
> > 
> > > +			     CLK_DIVIDER_POWER_OF_TWO, NULL);
> > > +
> > >  	/* cdev1 */
> > >  	clk = clk_register_fixed_rate(NULL, "cdev1_fixed", NULL,
> > > 0,
> > > 26000000);
> > >  	clk = tegra_clk_register_periph_gate("cdev1",
> > > "cdev1_fixed",
> > > 0,
> 
> Indeed, good catch! I'll wait for more comments and then re-spin
> patchset with
> the fix. Thank you.

You are very welcome. Thank you!

Other than that it all looks proper and works fine at least in the
configuration we use on Colibri T20. So you may add my reviewed and
tested bys to the whole series:

Reviewed-by: Marcel Ziswiler <marcel@...wiler.com>
Tested-by: Marcel Ziswiler <marcel@...wiler.com>