Thread (27 messages) 27 messages, 10 authors, 2011-01-18

[PATCH 1/2] Add a common struct clk

From: richardcochran@gmail.com (Richard Cochran)
Date: 2011-01-06 16:08:07
Also in: lkml

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help