linux-kernel - Re: Locking in the clk API, part 2: clk_prepare/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]

Message-ID: <20110201212409.GU31216@n2100.arm.linux.org.uk>
Date:	Tue, 1 Feb 2011 21:24:09 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Nicolas Pitre <nicolas.pitre@...aro.org>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	Saravana Kannan <skannan@...eaurora.org>,
	linux-sh@...r.kernel.org,
	Ben Herrenschmidt <benh@...nel.crashing.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Paul Mundt <lethal@...ux-sh.org>, linux-kernel@...r.kernel.org,
	Dima Zavin <dmitriyz@...gle.com>,
	Ben Dooks <ben-linux@...ff.org>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: Locking in the clk API, part 2: clk_prepare/clk_unprepare

On Tue, Feb 01, 2011 at 12:59:11PM -0800, Stephen Boyd wrote:
> On 02/01/2011 07:24 AM, Russell King - ARM Linux wrote:
> > I'd also be tempted at this stage to build-in a no-op dummy clock,
> > that being the NULL clk:
> >
> > int clk_prepare(struct clk *clk)
> > {
> > 	int ret = 0;
> >
> > 	if (clk) {
> > 		mutex_lock(&clk->mutex);
> > 		if (clk->prepared == 0)
> > 			ret = clk->ops->prepare(clk);
> > 		if (ret == 0)
> > 			clk->prepared++;
> > 		mutex_unlock(&clk->mutex);
> > 	}
> >
> > 	return ret;
> > }
> 
> I'm afraid this will hide enable/disable imbalances on some targets and
> then expose them on others. Maybe its not a big problem though since
> this also elegantly handles the root(s) of the tree.

You can't catch enable/disable imbalances in the prepare code, and you
can't really catch them in the unprepare code either.

Consider two drivers sharing the same struct clk.  When the second driver
prepares the clock, the enable count could well be non-zero, caused by
the first driver.  Ditto for when the second driver is removed, and it
calls unprepare - the enable count may well be non-zero.

The only thing you can check is that when the prepare count is zero,
the enable count is also zero.  You can also check in clk_enable() and
clk_disable() that the prepare count is non-zero.

If you want tigher checking than that, you need to somehow identify and
match up the clk_prepare/clk_enable/clk_disable/clk_unprepare calls from
a particular driver instance.  Addresses of the functions don't work as
you can't be certain that driver code will be co-located within a certain
range.  Adding an additional argument to these functions which is driver
instance specific seems to be horrible too.
--
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/