lkml.org 
[lkml]   [2011]   [Jan]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] Add a common struct clk
On Wed, Jan 05, 2011 at 11:51:02AM +0800, Jeremy Kerr wrote:

> + * The @lock member provides either a spinlock or a mutex to protect (at least)
> + * @enable_count. The type of lock used will depend on @flags; if CLK_ATOMIC is
> + * set, then the core clock code will use a spinlock, otherwise a mutex. This
> + * lock will be acquired during clk_enable and clk_disable, so for atomic
> + * clocks, these ops callbacks must not sleep.
> + *
> + * The choice of atomic or non-atomic clock depends on how the clock is enabled.
> + * Typically, you'll want to use a non-atomic clock. For clocks that need to be
> + * enabled/disabled in interrupt context, use CLK_ATOMIC. Note that atomic
> + * clocks with parents will typically cascade enable/disable operations to
> + * their parent, so the parent of an atomic clock *must* be atomic too.

...

> +struct clk {
> + const struct clk_ops *ops;
> + unsigned int enable_count;
> + int flags;
> + union {
> + struct mutex mutex;
> + spinlock_t spinlock;
> + } lock;
> +};

Here you have a "polymorphic" lock, where the clock instance knows
which type it is supposed to be. I got flak from David Miller and
others trying to do the same thing with the mdio_bus:

http://kerneltrap.org/mailarchive/linux-netdev/2010/7/6/6280618

The criticism, applied to your case, is that the clk_enable() caller
cannot know whether it is safe to make the call or not. I was told,
"there has got to be a better way."

Richard



\
 
 \ /
  Last update: 2011-01-06 17:11    [W:0.148 / U:0.328 seconds]
©2003-2014 Jasper Spaans. Advertise on this site