linux-kernel - Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU clock

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]

Message-ID: <20170928142050.gkbdqlwpfjmpdlg3@flea>
Date:   Thu, 28 Sep 2017 16:20:50 +0200
From:   Maxime Ripard <maxime.ripard@...e-electrons.com>
To:     icenowy@...c.io
Cc:     devicetree@...r.kernel.org, linux-sunxi@...glegroups.com,
        linux-kernel@...r.kernel.org, Chen-Yu Tsai <wens@...e.org>,
        linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/3] clk: sunxi-ng: add mux and pll notifiers for A64 CPU
 clock

On Thu, Sep 28, 2017 at 10:42:39AM +0000, icenowy@...c.io wrote:
> > On Sat, Sep 23, 2017 at 12:15:29AM +0000, Icenowy Zheng wrote:
> > > The A64 PLL_CPU clock has the same instability if some factor changed
> > > without the PLL gated like other SoCs with sun6i-style CCU, e.g. A33,
> > > H3.
> > > 
> > > Add the mux and pll notifiers for A64 CPU clock to workaround the
> > > problem.
> > > 
> > > Fixes: c6a0637460c2 ("clk: sunxi-ng: Add A64 clocks")
> > > Signed-off-by: Icenowy Zheng <icenowy@...c.io>
> > > ---
> > >  drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 28
> > > +++++++++++++++++++++++++++-
> > >  1 file changed, 27 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > index 2bb4cabf802f..b55fa69dd0c1 100644
> > > --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
> > > @@ -879,11 +879,26 @@ static const struct sunxi_ccu_desc
> > > sun50i_a64_ccu_desc = {
> > >  	.num_resets	= ARRAY_SIZE(sun50i_a64_ccu_resets),
> > >  };
> > > 
> > > +static struct ccu_pll_nb sun50i_a64_pll_cpu_nb = {
> > > +	.common	= &pll_cpux_clk.common,
> > > +	/* copy from pll_cpux_clk */
> > > +	.enable	= BIT(31),
> > > +	.lock	= BIT(28),
> > > +};
> > > +
> > > +static struct ccu_mux_nb sun50i_a64_cpu_nb = {
> > > +	.common		= &cpux_clk.common,
> > > +	.cm		= &cpux_clk.mux,
> > > +	.delay_us	= 1, /* > 8 clock cycles at 24 MHz */
> > > +	.bypass_index	= 1, /* index of 24 MHz oscillator */
> > > +};
> > > +
> > > 
> > >  static int sun50i_a64_ccu_probe(struct platform_device *pdev)
> > >  {
> > >  	struct resource *res;
> > >  	void __iomem *reg;
> > >  	u32 val;
> > > +	int ret;
> > > 
> > >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > >  	reg = devm_ioremap_resource(&pdev->dev, res);
> > > @@ -897,7 +912,18 @@ static int sun50i_a64_ccu_probe(struct
> > > platform_device *pdev)
> > > 
> > >  	writel(0x515, reg + SUN50I_A64_PLL_MIPI_REG);
> > > 
> > > -	return sunxi_ccu_probe(pdev->dev.of_node, reg,
> > > &sun50i_a64_ccu_desc);
> > > +	ret = sunxi_ccu_probe(pdev->dev.of_node, reg, &sun50i_a64_ccu_desc);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	/* Gate then ungate PLL CPU after any rate changes */
> > > +	ccu_pll_notifier_register(&sun50i_a64_pll_cpu_nb);
> > > +
> > > +	/* Reparent CPU during PLL CPU rate changes */
> > > +	ccu_mux_notifier_register(pll_cpux_clk.common.hw.clk,
> > > +				  &sun50i_a64_cpu_nb);
> > > +
> > > +	return 0;
> > 
> > So this is the fourth user of the exact same code, can you turn that
> > into a shared function?
> 
> I think it's not so worthful to extract the code, as:

It does, because the order is important. If you do not register the
notifiers in the right order, you have a bug, and:

> - the notifier structs contains info of the clocks

this should be passed as a parameter anyway,

> - A31 seems not to need the PLL notifier.

And you don't care about the ordering in that case, since there's just
one. If was talking about the H3, A64, R40 and A33 that all have that
code.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)