linux-kernel - Re: [PATCH E 11/14] OMAP clock: track child clocks

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: <alpine.DEB.2.00.0902122345160.14667@utopia.booyaka.com>
Date:	Fri, 13 Feb 2009 00:01:37 -0700 (MST)
From:	Paul Walmsley <paul@...an.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
cc:	linux-arm-kernel@...ts.arm.linux.org.uk,
	linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org,
	r-woodruff2@...com, Tony Lindgren <tony@...mide.com>
Subject: Re: [PATCH E 11/14] OMAP clock: track child clocks

(cc'ing Richard Woodruff)

Hello Russell,

On Mon, 9 Feb 2009, Russell King - ARM Linux wrote:

> On Thu, Jan 29, 2009 at 10:06:08PM +0000, Russell King - ARM Linux wrote:
> > @@ -780,7 +780,7 @@ int omap2_clk_set_parent(struct clk *clk, struct clk *new_parent)
> >  	if (clk->usecount > 0)
> >  		_omap2_clk_enable(clk);
> >  
> > -	clk->parent = new_parent;
> > +	clk_reparent(clk, new_parent);
> 
> While looking at the DPLL patches, I've realised that omap2_clk_set_parent()
> is buggy, as are any other places which reparent the clock (thankfully
> the only other place is in the initialisation code where it doesn't
> matter.)
> 
> Consider what happens when a clock is enabled - we walk up the tree
> enabling all parents.  If we then change the clock's parent, and
> then disable the child, we will again walk up the tree, but since
> we've reparented it, it will be a different clock tree.  The result
> is that the ancestors clock usage counts, and therefore their enable
> status, will end up getting screwed up.

Agreed.

> This brings up a question: what we currently do here is:
> 
> - disable the child
> - program clksel
> - enable the child
> - change child->parent
> 
> If we add in the parent handling, there are two possibilities:
> 
> - disable the child
> - enable the new parent tree
> - program clksel
> - change child->parent
> - disable the old parent tree
> - enable the child
> 
> OR
> 
> - disable the child and the old parent tree
> - program clksel
> - change child->parent
> - enable the new parent tree and the child
> 
> (note those 'and's have implied ordering).
> 
> Is there anything which dictates one approach over the other?
> Obviously the latter approach results in something smaller and
> cleaner, but might not be technically correct.

I don't know of any hardware reason to prefer one approach over the other, 
but Richard might know better.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/