[PATCH 1/2] Add a common struct clk
Richard Cochran
richardcochran at gmail.com
Thu Jan 6 11:07:52 EST 2011
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
More information about the linux-arm-kernel
mailing list