Re: Locking in the clk API, part 2: clk_prepare/clk_unprepare

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Fri, Feb 04, 2011 at 11:21:20AM +0100, Uwe Kleine-König wrote:
> I happily point out that the prepare_count needs to be protected by a
> spinlock and you need a flag that signals a prepare or unprepare is
> currently running.

It's really simple.  You don't use a struct clk pointer in any way until
you've called a clk_get() to get a pointer.  So what's the problem with
ensuring that you do clk_prepare() on it before you register whatever
services may end up calling clk_enable().

That is good practice.  It's precisely the same practice which says that
you shall not register device drivers with subsystems, thereby making
them visible, until you're absolutely ready in the driver to start taking
requests to use your driver.  Precisely the same thing applies here.

In other words, to go back to the UART console driver case, in the
UART console setup function, you do this:

	clk = clk_get(...);
	if (IS_ERR(clk))
		return PTR_ERR(clk);

	err = clk_prepare(clk);
	if (err) {
		clk_put(clk);
		return err;
	}

	rate = clk_get_rate(clk);
	... setup UART, setup baud rate according to rate ...

	return 0;

So, this means that clk_enable() in the console write function will not
be called until after the initialization function has finished - by which
time clk_prepare() will have completed.

There is no need for any kind of spinlocking, atomic types or other such
crap for the prepare count.  We do not care about concurrent clk_enables().

The only time you'd need such games as you're suggesting is if you're still
promoting your idea about calling clk_prepare() from clk_enable() "in case
driver writers forget it", which is soo broken it's untrue.