[PATCH 0/11] generic struct clk patches

[PATCH 0/11] generic struct clk patches

[Mark Brown]

I got a request from Mike to post the updates I'd done to the generic
struct clk so this is a repost of the patches Jeremy did plus what I've
got sitting on top of them.  Everything is exactly the same as the
last posting, just resending.

[PATCH 01/11] clk: Add a generic clock infrastructure

[Mark Brown]

From: Jeremy Kerr <jeremy.kerr@canonical.com>

We currently have ~21 definitions of struct clk in the ARM architecture,
each defined on a per-platform basis. This makes it difficult to define
platform- (or architecture-) independent clock sources without making
assumptions about struct clk, and impossible to compile two
platforms with different struct clks into a single image.

This change is an effort to unify struct clk where possible, by defining
a common struct clk, and a set of clock operations. Different clock
implementations can set their own operations, and have a standard
interface for generic code. The callback interface is exposed to the
kernel proper, while the clock implementations only need to be seen by
the platform internals.

The interface is split into two halves:

 * struct clk, which is the generic-device-driver interface. This
   provides a set of functions which drivers may use to request
   enable/disable, query or manipulate in a hardware-independent manner.

 * struct clk_hw and struct clk_hw_ops, which is the hardware-specific
   interface. Clock drivers implement the ops, which allow the core
   clock code to implement the generic 'struct clk' API.

This allows us to share clock code among platforms, and makes it
possible to dynamically create clock devices in platform-independent
code.

Platforms can enable the generic struct clock through
CONFIG_GENERIC_CLK. In this case, the clock infrastructure consists of a
common, opaque struct clk, and a set of clock operations (defined per
type of clock):

  struct clk_hw_ops {
  	int		(*prepare)(struct clk_hw *);
  	void		(*unprepare)(struct clk_hw *);
  	int		(*enable)(struct clk_hw *);
  	void		(*disable)(struct clk_hw *);
  	unsigned long	(*recalc_rate)(struct clk_hw *);
  	int		(*set_rate)(struct clk_hw *,
  					unsigned long, unsigned long *);
  	long		(*round_rate)(struct clk_hw *, unsigned long);
  	int		(*set_parent)(struct clk_hw *, struct clk *);
  	struct clk *	(*get_parent)(struct clk_hw *);
  };

Platform clock code can register a clock through clk_register, passing a
set of operations, and a pointer to hardware-specific data:

  struct clk_hw_foo {
  	struct clk_hw clk;
  	void __iomem *enable_reg;
  };

  #define to_clk_foo(c) offsetof(c, clk_hw_foo, clk)

  static int clk_foo_enable(struct clk_hw *clk)
  {
  	struct clk_foo *foo = to_clk_foo(clk);
  	raw_writeb(foo->enable_reg, 1);
  	return 0;
  }

  struct clk_hw_ops clk_foo_ops = {
  	.enable = clk_foo_enable,
  };

And in the platform initialisation code:

  struct clk_foo my_clk_foo;

  void init_clocks(void)
  {
  	my_clk_foo.enable_reg = ioremap(...);

  	clk_register(&clk_foo_ops, &my_clk_foo, NULL);
  }

Changes from Thomas Gleixner <tglx@linutronix.de>.

The common clock definitions are based on a development patch from Ben
Herrenschmidt <benh@kernel.crashing.org>.

TODO:

 * We don't keep any internal reference to the clock topology at present.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/Kconfig  |    3 +
 drivers/clk/Makefile |    1 +
 drivers/clk/clk.c    |  229 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/clkdev.c |    7 ++
 include/linux/clk.h  |  102 ++++++++++++++++++++--
 5 files changed, 332 insertions(+), 10 deletions(-)
 create mode 100644 drivers/clk/clk.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 3530927..c53ed59 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -5,3 +5,6 @@ config CLKDEV_LOOKUP
 
 config HAVE_MACH_CLKDEV
 	bool
+
+config GENERIC_CLK
+	bool
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 07613fa..570d5b9 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -1,2 +1,3 @@
 
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
+obj-$(CONFIG_GENERIC_CLK)	+= clk.o
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
new file mode 100644
index 0000000..ad90a90
--- /dev/null
+++ b/drivers/clk/clk.c
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2010-2011 Canonical Ltd <jeremy.kerr@canonical.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Standard functionality for the common clock API.
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+struct clk {
+	const char		*name;
+	struct clk_hw_ops	*ops;
+	struct clk_hw		*hw;
+	unsigned int		enable_count;
+	unsigned int		prepare_count;
+	struct clk		*parent;
+	unsigned long		rate;
+};
+
+static DEFINE_SPINLOCK(enable_lock);
+static DEFINE_MUTEX(prepare_lock);
+
+static void __clk_unprepare(struct clk *clk)
+{
+	if (!clk)
+		return;
+
+	if (WARN_ON(clk->prepare_count == 0))
+		return;
+
+	if (--clk->prepare_count > 0)
+		return;
+
+	WARN_ON(clk->enable_count > 0);
+
+	if (clk->ops->unprepare)
+		clk->ops->unprepare(clk->hw);
+
+	__clk_unprepare(clk->parent);
+}
+
+void clk_unprepare(struct clk *clk)
+{
+	mutex_lock(&prepare_lock);
+	__clk_unprepare(clk);
+	mutex_unlock(&prepare_lock);
+}
+EXPORT_SYMBOL_GPL(clk_unprepare);
+
+static int __clk_prepare(struct clk *clk)
+{
+	int ret = 0;
+
+	if (!clk)
+		return 0;
+
+	if (clk->prepare_count == 0) {
+		ret = __clk_prepare(clk->parent);
+		if (ret)
+			return ret;
+
+		if (clk->ops->prepare) {
+			ret = clk->ops->prepare(clk->hw);
+			if (ret) {
+				__clk_unprepare(clk->parent);
+				return ret;
+			}
+		}
+	}
+
+	clk->prepare_count++;
+
+	return 0;
+}
+
+int clk_prepare(struct clk *clk)
+{
+	int ret;
+
+	mutex_lock(&prepare_lock);
+	ret = __clk_prepare(clk);
+	mutex_unlock(&prepare_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(clk_prepare);
+
+static void __clk_disable(struct clk *clk)
+{
+	if (!clk)
+		return;
+
+	if (WARN_ON(clk->enable_count == 0))
+		return;
+
+	if (--clk->enable_count > 0)
+		return;
+
+	if (clk->ops->disable)
+		clk->ops->disable(clk->hw);
+	__clk_disable(clk->parent);
+}
+
+void clk_disable(struct clk *clk)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&enable_lock, flags);
+	__clk_disable(clk);
+	spin_unlock_irqrestore(&enable_lock, flags);
+}
+EXPORT_SYMBOL_GPL(clk_disable);
+
+static int __clk_enable(struct clk *clk)
+{
+	int ret;
+
+	if (!clk)
+		return 0;
+
+	if (WARN_ON(clk->prepare_count == 0))
+		return -ESHUTDOWN;
+
+
+	if (clk->enable_count == 0) {
+		ret = __clk_enable(clk->parent);
+		if (ret)
+			return ret;
+
+		if (clk->ops->enable) {
+			ret = clk->ops->enable(clk->hw);
+			if (ret) {
+				__clk_disable(clk->parent);
+				return ret;
+			}
+		}
+	}
+
+	clk->enable_count++;
+	return 0;
+}
+
+int clk_enable(struct clk *clk)
+{
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&enable_lock, flags);
+	ret = __clk_enable(clk);
+	spin_unlock_irqrestore(&enable_lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(clk_enable);
+
+unsigned long clk_get_rate(struct clk *clk)
+{
+	if (!clk)
+		return 0;
+	return clk->rate;
+}
+EXPORT_SYMBOL_GPL(clk_get_rate);
+
+long clk_round_rate(struct clk *clk, unsigned long rate)
+{
+	if (clk && clk->ops->round_rate)
+		return clk->ops->round_rate(clk->hw, rate);
+	return rate;
+}
+EXPORT_SYMBOL_GPL(clk_round_rate);
+
+int clk_set_rate(struct clk *clk, unsigned long rate)
+{
+	/* not yet implemented */
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(clk_set_rate);
+
+struct clk *clk_get_parent(struct clk *clk)
+{
+	if (!clk)
+		return NULL;
+
+	return clk->parent;
+}
+EXPORT_SYMBOL_GPL(clk_get_parent);
+
+int clk_set_parent(struct clk *clk, struct clk *parent)
+{
+	/* not yet implemented */
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(clk_set_parent);
+
+struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
+		const char *name)
+{
+	struct clk *clk;
+
+	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
+	if (!clk)
+		return NULL;
+
+	clk->name = name;
+	clk->ops = ops;
+	clk->hw = hw;
+	hw->clk = clk;
+
+	/* Query the hardware for parent and initial rate */
+
+	if (clk->ops->get_parent)
+		/* We don't to lock against prepare/enable here, as
+		 * the clock is not yet accessible from anywhere */
+		clk->parent = clk->ops->get_parent(clk->hw);
+
+	if (clk->ops->recalc_rate)
+		clk->rate = clk->ops->recalc_rate(clk->hw);
+
+
+	return clk;
+}
+EXPORT_SYMBOL_GPL(clk_register);
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6db161f..e2a9719 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -23,6 +23,13 @@
 static LIST_HEAD(clocks);
 static DEFINE_MUTEX(clocks_mutex);
 
+/* For USE_COMMON_STRUCT_CLK, these are provided in clk.c, but not exported
+ * through other headers; we don't want them used anywhere but here. */
+#ifdef CONFIG_USE_COMMON_STRUCT_CLK
+extern int __clk_get(struct clk *clk);
+extern void __clk_put(struct clk *clk);
+#endif
+
 /*
  * Find the correct struct clk for the device and connection ID.
  * We do slightly fuzzy matching here:
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 1d37f42..93ff870 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -3,6 +3,7 @@
  *
  *  Copyright (C) 2004 ARM Limited.
  *  Written by Deep Blue Solutions Limited.
+ *  Copyright (c) 2010-2011 Jeremy Kerr <jeremy.kerr@canonical.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -11,17 +12,103 @@
 #ifndef __LINUX_CLK_H
 #define __LINUX_CLK_H
 
+#include <linux/err.h>
+#include <linux/spinlock.h>
+
 struct device;
 
-/*
- * The base API.
+struct clk;
+
+#ifdef CONFIG_GENERIC_CLK
+
+struct clk_hw {
+	struct clk *clk;
+};
+
+/**
+ * struct clk_hw_ops -  Callback operations for hardware clocks; these are to
+ * be provided by the clock implementation, and will be called by drivers
+ * through the clk_* API.
+ *
+ * @prepare:	Prepare the clock for enabling. This must not return until
+ *		the clock is fully prepared, and it's safe to call clk_enable.
+ *		This callback is intended to allow clock implementations to
+ *		do any initialisation that may sleep. Called with
+ *		prepare_lock held.
+ *
+ * @unprepare:	Release the clock from its prepared state. This will typically
+ *		undo any work done in the @prepare callback. Called with
+ *		prepare_lock held.
+ *
+ * @enable:	Enable the clock atomically. This must not return until the
+ *		clock is generating a valid clock signal, usable by consumer
+ *		devices. Called with enable_lock held. This function must not
+ *		sleep.
+ *
+ * @disable:	Disable the clock atomically. Called with enable_lock held.
+ *		This function must not sleep.
+ *
+ * @recalc_rate	Recalculate the rate of this clock, by quering hardware
+ *		and/or the clock's parent. Called with the global clock mutex
+ *		held. Optional, but recommended - if this op is not set,
+ *		clk_get_rate will return 0.
+ *
+ * @get_parent	Query the parent of this clock; for clocks with multiple
+ *		possible parents, query the hardware for the current
+ *		parent. Currently only called when the clock is first
+ *		registered.
+ *
+ * The clk_enable/clk_disable and clk_prepare/clk_unprepare pairs allow
+ * implementations to split any work between atomic (enable) and sleepable
+ * (prepare) contexts.  If a clock requires sleeping code to be turned on, this
+ * should be done in clk_prepare. Switching that will not sleep should be done
+ * in clk_enable.
+ *
+ * Typically, drivers will call clk_prepare when a clock may be needed later
+ * (eg. when a device is opened), and clk_enable when the clock is actually
+ * required (eg. from an interrupt). Note that clk_prepare *must* have been
+ * called before clk_enable.
+ */
+struct clk_hw_ops {
+	int		(*prepare)(struct clk_hw *);
+	void		(*unprepare)(struct clk_hw *);
+	int		(*enable)(struct clk_hw *);
+	void		(*disable)(struct clk_hw *);
+	unsigned long	(*recalc_rate)(struct clk_hw *);
+	long		(*round_rate)(struct clk_hw *, unsigned long);
+	struct clk *	(*get_parent)(struct clk_hw *);
+};
+
+/**
+ * clk_prepare - prepare clock for atomic enabling.
+ *
+ * @clk: The clock to prepare
+ *
+ * Do any possibly sleeping initialisation on @clk, allowing the clock to be
+ * later enabled atomically (via clk_enable). This function may sleep.
  */
+int clk_prepare(struct clk *clk);
 
+/**
+ * clk_unprepare - release clock from prepared state
+ *
+ * @clk: The clock to release
+ *
+ * Do any (possibly sleeping) cleanup on clk. This function may sleep.
+ */
+void clk_unprepare(struct clk *clk);
+
+#else /* !CONFIG_GENERIC_CLK */
 
 /*
- * struct clk - an machine class defined object / cookie.
+ * For !CONFIG_GENERIC_CLK, we don't enforce any atomicity
+ * requirements for clk_enable/clk_disable, so the prepare and unprepare
+ * functions are no-ops
  */
-struct clk;
+static inline int clk_prepare(struct clk *clk) { return 0; }
+static inline void clk_unprepare(struct clk *clk) { }
+
+#endif /* !CONFIG_GENERIC_CLK */
 
 /**
  * clk_get - lookup and obtain a reference to a clock producer.
@@ -67,6 +154,7 @@ void clk_disable(struct clk *clk);
 /**
  * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
  *		  This is only valid once the clock source has been enabled.
+ *		  Returns zero if the clock rate is unknown.
  * @clk: clock source
  */
 unsigned long clk_get_rate(struct clk *clk);
@@ -83,12 +171,6 @@ unsigned long clk_get_rate(struct clk *clk);
  */
 void clk_put(struct clk *clk);
 
-
-/*
- * The remaining APIs are optional for machine class support.
- */
-
-
 /**
  * clk_round_rate - adjust a rate to the exact rate a clock can provide
  * @clk: clock source
-- 
1.7.5.4

[PATCH 02/11] clk: Implement clk_set_rate

[Mark Brown]

From: Jeremy Kerr <jeremy.kerr@canonical.com>

Implemenent clk_set_rate by adding a set_rate callback to clk_hw_ops,
and core code to handle propagation of rate changes up and down the
clock tree.

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk.c   |   74 ++++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/clk.h |   12 ++++++++
 2 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ad90a90..3a4d70e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -21,6 +21,8 @@ struct clk {
 	unsigned int		enable_count;
 	unsigned int		prepare_count;
 	struct clk		*parent;
+	struct hlist_head	children;
+	struct hlist_node	child_node;
 	unsigned long		rate;
 };
 
@@ -176,10 +178,61 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
 }
 EXPORT_SYMBOL_GPL(clk_round_rate);
 
+/*
+ * clk_recalc_rates - Given a clock (with a recently updated clk->rate),
+ *	notify its children that the rate may need to be recalculated, using
+ *	ops->recalc_rate().
+ */
+static void clk_recalc_rates(struct clk *clk)
+{
+	struct hlist_node *tmp;
+	struct clk *child;
+
+	if (clk->ops->recalc_rate)
+		clk->rate = clk->ops->recalc_rate(clk->hw);
+
+	hlist_for_each_entry(child, tmp, &clk->children, child_node)
+		clk_recalc_rates(child);
+}
+
 int clk_set_rate(struct clk *clk, unsigned long rate)
 {
-	/* not yet implemented */
-	return -ENOSYS;
+	unsigned long parent_rate, new_rate;
+	int ret;
+
+	if (!clk->ops->set_rate)
+		return -ENOSYS;
+
+	new_rate = rate;
+
+	/* prevent racing with updates to the clock topology */
+	mutex_lock(&prepare_lock);
+
+propagate:
+	ret = clk->ops->set_rate(clk->hw, new_rate, &parent_rate);
+
+	if (ret < 0)
+		return ret;
+
+	/* ops->set_rate may require the parent's rate to change (to
+	 * parent_rate), we need to propagate the set_rate call to the
+	 * parent.
+	 */
+	if (ret == CLK_SET_RATE_PROPAGATE) {
+		new_rate = parent_rate;
+		clk = clk->parent;
+		goto propagate;
+	}
+
+	/* If successful (including propagation to the parent clock(s)),
+	 * recalculate the rates of the clock, including children.  We'll be
+	 * calling this on the 'parent-most' clock that we propagated to.
+	 */
+	clk_recalc_rates(clk);
+
+	mutex_unlock(&prepare_lock);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(clk_set_rate);
 
@@ -213,16 +266,25 @@ struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
 	clk->hw = hw;
 	hw->clk = clk;
 
-	/* Query the hardware for parent and initial rate */
+	/* Query the hardware for parent and initial rate. We may alter
+	 * the clock topology, making this clock available from the parent's
+	 * children list. So, we need to protect against concurrent
+	 * accesses through set_rate
+	 */
+	mutex_lock(&prepare_lock);
 
-	if (clk->ops->get_parent)
-		/* We don't to lock against prepare/enable here, as
-		 * the clock is not yet accessible from anywhere */
+	if (clk->ops->get_parent) {
 		clk->parent = clk->ops->get_parent(clk->hw);
+		if (clk->parent)
+			hlist_add_head(&clk->child_node,
+					&clk->parent->children);
+	}
 
 	if (clk->ops->recalc_rate)
 		clk->rate = clk->ops->recalc_rate(clk->hw);
 
+	mutex_unlock(&prepare_lock);
+
 
 	return clk;
 }
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 93ff870..e0969d2 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -58,6 +58,12 @@ struct clk_hw {
  *		parent. Currently only called when the clock is first
  *		registered.
  *
+ * @set_rate	Change the rate of this clock. If this callback returns
+ *		CLK_SET_RATE_PROPAGATE, the rate change will be propagated to
+ *		the parent clock (which may propagate again). The requested
+ *		rate of the parent is passed back from the callback in the
+ *		second 'unsigned long *' argument.
+ *
  * The clk_enable/clk_disable and clk_prepare/clk_unprepare pairs allow
  * implementations to split any work between atomic (enable) and sleepable
  * (prepare) contexts.  If a clock requires sleeping code to be turned on, this
@@ -76,9 +82,15 @@ struct clk_hw_ops {
 	void		(*disable)(struct clk_hw *);
 	unsigned long	(*recalc_rate)(struct clk_hw *);
 	long		(*round_rate)(struct clk_hw *, unsigned long);
+	int		(*set_rate)(struct clk_hw *,
+					unsigned long, unsigned long *);
 	struct clk *	(*get_parent)(struct clk_hw *);
 };
 
+enum {
+	CLK_SET_RATE_PROPAGATE = 1,
+};
+
 /**
  * clk_prepare - prepare clock for atomic enabling.
  *
-- 
1.7.5.4

[PATCH 02/11] clk: Implement clk_set_rate

[Sascha Hauer]

On Wed, Aug 24, 2011 at 02:15:50PM +0100, Mark Brown wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> Implemenent clk_set_rate by adding a set_rate callback to clk_hw_ops,
> and core code to handle propagation of rate changes up and down the
> clock tree.
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/clk/clk.c   |   74 ++++++++++++++++++++++++++++++++++++++++++++++----
>  include/linux/clk.h |   12 ++++++++
>  2 files changed, 80 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ad90a90..3a4d70e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -21,6 +21,8 @@ struct clk {
>  	unsigned int		enable_count;
>  	unsigned int		prepare_count;
>  	struct clk		*parent;
> +	struct hlist_head	children;
> +	struct hlist_node	child_node;
>  	unsigned long		rate;
>  };
>  
> @@ -176,10 +178,61 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
>  }
>  EXPORT_SYMBOL_GPL(clk_round_rate);
>  
> +/*
> + * clk_recalc_rates - Given a clock (with a recently updated clk->rate),
> + *	notify its children that the rate may need to be recalculated, using
> + *	ops->recalc_rate().
> + */
> +static void clk_recalc_rates(struct clk *clk)
> +{
> +	struct hlist_node *tmp;
> +	struct clk *child;
> +
> +	if (clk->ops->recalc_rate)
> +		clk->rate = clk->ops->recalc_rate(clk->hw);
> +
> +	hlist_for_each_entry(child, tmp, &clk->children, child_node)
> +		clk_recalc_rates(child);
> +}
> +
>  int clk_set_rate(struct clk *clk, unsigned long rate)
>  {
> -	/* not yet implemented */
> -	return -ENOSYS;
> +	unsigned long parent_rate, new_rate;
> +	int ret;
> +
> +	if (!clk->ops->set_rate)
> +		return -ENOSYS;
> +
> +	new_rate = rate;
> +
> +	/* prevent racing with updates to the clock topology */
> +	mutex_lock(&prepare_lock);
> +
> +propagate:
> +	ret = clk->ops->set_rate(clk->hw, new_rate, &parent_rate);

You have to check for clk->ops->set_rate != NULL here for the
propagation case.

> +
> +	if (ret < 0)
> +		return ret;

This returns with a mutex held. Also, some rates may already have
changed here. I suggest a goto recalc_rates here.

> +
> +	/* ops->set_rate may require the parent's rate to change (to
> +	 * parent_rate), we need to propagate the set_rate call to the
> +	 * parent.
> +	 */
> +	if (ret == CLK_SET_RATE_PROPAGATE) {
> +		new_rate = parent_rate;
> +		clk = clk->parent;
> +		goto propagate;

before propagating you should check if the parent already has the
desired rate. The parent may be a fixed clock which does not have
a set_rate function, still this function shall succeed in this
case.

> +	}
> +
> +	/* If successful (including propagation to the parent clock(s)),
> +	 * recalculate the rates of the clock, including children.  We'll be
> +	 * calling this on the 'parent-most' clock that we propagated to.
> +	 */

recalc_rates:

> +	clk_recalc_rates(clk);
> +
> +	mutex_unlock(&prepare_lock);
> +
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(clk_set_rate);
>  
> @@ -213,16 +266,25 @@ struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
>  	clk->hw = hw;
>  	hw->clk = clk;
>  
> -	/* Query the hardware for parent and initial rate */
> +	/* Query the hardware for parent and initial rate. We may alter
> +	 * the clock topology, making this clock available from the parent's
> +	 * children list. So, we need to protect against concurrent
> +	 * accesses through set_rate
> +	 */
> +	mutex_lock(&prepare_lock);
>  
> -	if (clk->ops->get_parent)
> -		/* We don't to lock against prepare/enable here, as
> -		 * the clock is not yet accessible from anywhere */
> +	if (clk->ops->get_parent) {
>  		clk->parent = clk->ops->get_parent(clk->hw);
> +		if (clk->parent)
> +			hlist_add_head(&clk->child_node,
> +					&clk->parent->children);
> +	}
>  
>  	if (clk->ops->recalc_rate)
>  		clk->rate = clk->ops->recalc_rate(clk->hw);
>  
> +	mutex_unlock(&prepare_lock);
> +
>  
>  	return clk;
>  }
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 93ff870..e0969d2 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -58,6 +58,12 @@ struct clk_hw {
>   *		parent. Currently only called when the clock is first
>   *		registered.
>   *
> + * @set_rate	Change the rate of this clock. If this callback returns
> + *		CLK_SET_RATE_PROPAGATE, the rate change will be propagated to
> + *		the parent clock (which may propagate again). The requested
> + *		rate of the parent is passed back from the callback in the
> + *		second 'unsigned long *' argument.
> + *
>   * The clk_enable/clk_disable and clk_prepare/clk_unprepare pairs allow
>   * implementations to split any work between atomic (enable) and sleepable
>   * (prepare) contexts.  If a clock requires sleeping code to be turned on, this
> @@ -76,9 +82,15 @@ struct clk_hw_ops {
>  	void		(*disable)(struct clk_hw *);
>  	unsigned long	(*recalc_rate)(struct clk_hw *);
>  	long		(*round_rate)(struct clk_hw *, unsigned long);
> +	int		(*set_rate)(struct clk_hw *,
> +					unsigned long, unsigned long *);
>  	struct clk *	(*get_parent)(struct clk_hw *);
>  };
>  
> +enum {
> +	CLK_SET_RATE_PROPAGATE = 1,
> +};
> +
>  /**
>   * clk_prepare - prepare clock for atomic enabling.
>   *
> -- 
> 1.7.5.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 02/11] clk: Implement clk_set_rate

[Mark Brown]

On Fri, Aug 26, 2011 at 10:53:27AM +0200, Sascha Hauer wrote:
> On Wed, Aug 24, 2011 at 02:15:50PM +0100, Mark Brown wrote:
> > From: Jeremy Kerr <jeremy.kerr@canonical.com>

> > +propagate:
> > +	ret = clk->ops->set_rate(clk->hw, new_rate, &parent_rate);

> You have to check for clk->ops->set_rate != NULL here for the
> propagation case.

Note that I'm just forwarding on Jeremy's patches here, I'm not likely
to be able to spend too much time developing the core.

[PATCH 02/11] clk: Implement clk_set_rate

[Sascha Hauer]

On Fri, Aug 26, 2011 at 09:55:10AM +0100, Mark Brown wrote:
> On Fri, Aug 26, 2011 at 10:53:27AM +0200, Sascha Hauer wrote:
> > On Wed, Aug 24, 2011 at 02:15:50PM +0100, Mark Brown wrote:
> > > From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> > > +propagate:
> > > +	ret = clk->ops->set_rate(clk->hw, new_rate, &parent_rate);
> 
> > You have to check for clk->ops->set_rate != NULL here for the
> > propagation case.
> 
> Note that I'm just forwarding on Jeremy's patches here, I'm not likely
> to be able to spend too much time developing the core.

Ok, then we have at least some pointer in the archives for the one
who continues on these patches.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 02/11] clk: Implement clk_set_rate

[Mark Brown]

On Fri, Aug 26, 2011 at 10:58:37AM +0200, Sascha Hauer wrote:
> On Fri, Aug 26, 2011 at 09:55:10AM +0100, Mark Brown wrote:

> > Note that I'm just forwarding on Jeremy's patches here, I'm not likely
> > to be able to spend too much time developing the core.

> Ok, then we have at least some pointer in the archives for the one
> who continues on these patches.

Yes, the review is definitely valuable - I just wanted to avoid any
disappointment if you were expecting me to do anything with it (and
perhaps even encourage you to do so yourself!).

[PATCH 02/11] clk: Implement clk_set_rate

[Turquette, Mike]

On Wed, Aug 24, 2011 at 6:15 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
>
> Implemenent clk_set_rate by adding a set_rate callback to clk_hw_ops,
> and core code to handle propagation of rate changes up and down the
> clock tree.

Propagating rates up the tree is wrong.  If a parent rate must be
changed then clk_set_rate should be called on the parent explicitly,
then clk_set_rate can be called on the target child clk.

I don't see any other way to do it that is safe, save for some batch
operation mechanism that takes predefined clock sub-trees into account
(which is likely only feasible for SoCs since their data manuals
provide all this info).

> +/*
> + * clk_recalc_rates - Given a clock (with a recently updated clk->rate),
> + * ? ? notify its children that the rate may need to be recalculated, using
> + * ? ? ops->recalc_rate().
> + */
> +static void clk_recalc_rates(struct clk *clk)
> +{
> + ? ? ? struct hlist_node *tmp;
> + ? ? ? struct clk *child;
> +
> + ? ? ? if (clk->ops->recalc_rate)
> + ? ? ? ? ? ? ? clk->rate = clk->ops->recalc_rate(clk->hw);
> +
> + ? ? ? hlist_for_each_entry(child, tmp, &clk->children, child_node)
> + ? ? ? ? ? ? ? clk_recalc_rates(child);

We need a rate post-change notifier here that drivers can subscribe
to.  Since many devices do not support having their clocks changed on
the fly while the device is enabled, a pre-change and post-change
notifier are necessary to notify devices downstream of the clock being
changed.  Imagine doing a rate change high up in the tree (PLL) and
you can see that many devices will need these notifiers propagated to
them.

> +}
> +
> ?int clk_set_rate(struct clk *clk, unsigned long rate)
> ?{
> - ? ? ? /* not yet implemented */
> - ? ? ? return -ENOSYS;
> + ? ? ? unsigned long parent_rate, new_rate;
> + ? ? ? int ret;
> +
> + ? ? ? if (!clk->ops->set_rate)
> + ? ? ? ? ? ? ? return -ENOSYS;
> +
> + ? ? ? new_rate = rate;
> +
> + ? ? ? /* prevent racing with updates to the clock topology */
> + ? ? ? mutex_lock(&prepare_lock);
> +
> +propagate:

Need a rate pre-change notifier here that walks the tree.  Drivers
subscribing to this notifier can save context and call their
pm_runtime_put or clk_disable bits so that they can survive the rate
change without any glitches before the rate actually changes.

> + ? ? ? ret = clk->ops->set_rate(clk->hw, new_rate, &parent_rate);
> +
> + ? ? ? if (ret < 0)
> + ? ? ? ? ? ? ? return ret;
> +
> + ? ? ? /* ops->set_rate may require the parent's rate to change (to
> + ? ? ? ?* parent_rate), we need to propagate the set_rate call to the
> + ? ? ? ?* parent.
> + ? ? ? ?*/
> + ? ? ? if (ret == CLK_SET_RATE_PROPAGATE) {
> + ? ? ? ? ? ? ? new_rate = parent_rate;
> + ? ? ? ? ? ? ? clk = clk->parent;
> + ? ? ? ? ? ? ? goto propagate;
> + ? ? ? }

Again, I feel this is wrong.  Rate change propagation should only flow
downstream.  Any rate change arbitration that requires changes
upstream should be considered and handled by the driver.

> +
> + ? ? ? /* If successful (including propagation to the parent clock(s)),
> + ? ? ? ?* recalculate the rates of the clock, including children. ?We'll be
> + ? ? ? ?* calling this on the 'parent-most' clock that we propagated to.
> + ? ? ? ?*/
> + ? ? ? clk_recalc_rates(clk);

As mentioned above, recalc should propagate rate post-change notification.

Regards,
Mike

[PATCH 02/11] clk: Implement clk_set_rate

[Stephen Boyd]

On 08/24/11 06:15, Mark Brown wrote:
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ad90a90..3a4d70e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -21,6 +21,8 @@ struct clk {
>  	unsigned int		enable_count;
>  	unsigned int		prepare_count;
>  	struct clk		*parent;
> +	struct hlist_head	children;
> +	struct hlist_node	child_node;

Shouldn't these hlist nodes be initialized in clk_register() via
INIT_HLIST_*()? I guess it's not explicitly needed because kzalloc is
used, but perhaps it is better to be explicit should the kzalloc change
to a kmalloc.

> @@ -213,16 +266,25 @@ struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
>  	clk->hw = hw;
>  	hw->clk = clk;
>  
> -	/* Query the hardware for parent and initial rate */
> +	/* Query the hardware for parent and initial rate. We may alter
> +	 * the clock topology, making this clock available from the parent's
> +	 * children list. So, we need to protect against concurrent
> +	 * accesses through set_rate
> +	 */
> +	mutex_lock(&prepare_lock);
>  
> -	if (clk->ops->get_parent)
> -		/* We don't to lock against prepare/enable here, as
> -		 * the clock is not yet accessible from anywhere */
> +	if (clk->ops->get_parent) {
>  		clk->parent = clk->ops->get_parent(clk->hw);
> +		if (clk->parent)
> +			hlist_add_head(&clk->child_node,
> +					&clk->parent->children);
> +	}
>  
>  	if (clk->ops->recalc_rate)
>  		clk->rate = clk->ops->recalc_rate(clk->hw);
>  
> +	mutex_unlock(&prepare_lock);
> +
>  
>  	return clk;
>  }
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 03/11] clk: Add fixed-rate clock

[Mark Brown]

From: Jeremy Kerr <jeremy.kerr@canonical.com>

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/Kconfig     |    4 ++++
 drivers/clk/Makefile    |    1 +
 drivers/clk/clk-fixed.c |   17 +++++++++++++++++
 include/linux/clk.h     |   14 ++++++++++++++
 4 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clk/clk-fixed.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index c53ed59..d8313d7 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -8,3 +8,7 @@ config HAVE_MACH_CLKDEV
 
 config GENERIC_CLK
 	bool
+
+config GENERIC_CLK_FIXED
+	bool
+	depends on GENERIC_CLK
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 570d5b9..9a3325a 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -1,3 +1,4 @@
 
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_GENERIC_CLK)	+= clk.o
+obj-$(CONFIG_GENERIC_CLK_FIXED)	+= clk-fixed.o
diff --git a/drivers/clk/clk-fixed.c b/drivers/clk/clk-fixed.c
new file mode 100644
index 0000000..47a27f9
--- /dev/null
+++ b/drivers/clk/clk-fixed.c
@@ -0,0 +1,17 @@
+
+#include <linux/clk.h>
+#include <linux/module.h>
+
+#define to_clk_fixed(c) container_of(c, struct clk_hw_fixed, hw)
+
+static unsigned long clk_fixed_recalc_rate(struct clk_hw *hw)
+{
+	return to_clk_fixed(hw)->rate;
+}
+
+struct clk_hw_ops clk_fixed_ops = {
+	.recalc_rate = clk_fixed_recalc_rate,
+};
+EXPORT_SYMBOL_GPL(clk_fixed_ops);
+
+
diff --git a/include/linux/clk.h b/include/linux/clk.h
index e0969d2..fd62e86 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -110,6 +110,20 @@ int clk_prepare(struct clk *clk);
  */
 void clk_unprepare(struct clk *clk);
 
+/* Base clock implementations. Platform clock implementations can use these
+ * directly, or 'subclass' as approprate */
+
+#ifdef CONFIG_GENERIC_CLK_FIXED
+
+struct clk_hw_fixed {
+	struct clk_hw	hw;
+	unsigned long	rate;
+};
+
+extern struct clk_hw_ops clk_fixed_ops;
+
+#endif /* CONFIG_GENERIC_CLK_FIXED */
+
 #else /* !CONFIG_GENERIC_CLK */
 
 /*
-- 
1.7.5.4

[PATCH 04/11] clk: Add simple gated clock

[Mark Brown]

From: Jeremy Kerr <jeremy.kerr@canonical.com>

Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/Kconfig    |    4 ++++
 drivers/clk/Makefile   |    1 +
 drivers/clk/clk-gate.c |   41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/clk.h    |   13 +++++++++++++
 4 files changed, 59 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clk/clk-gate.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index d8313d7..a78967c 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -12,3 +12,7 @@ config GENERIC_CLK
 config GENERIC_CLK_FIXED
 	bool
 	depends on GENERIC_CLK
+
+config GENERIC_CLK_GATE
+	bool
+	depends on GENERIC_CLK
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 9a3325a..d186446 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_GENERIC_CLK)	+= clk.o
 obj-$(CONFIG_GENERIC_CLK_FIXED)	+= clk-fixed.o
+obj-$(CONFIG_GENERIC_CLK_GATE)	+= clk-gate.o
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
new file mode 100644
index 0000000..833e0da
--- /dev/null
+++ b/drivers/clk/clk-gate.c
@@ -0,0 +1,41 @@
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <asm/io.h>
+
+#define to_clk_gate(clk) container_of(clk, struct clk_gate, hw)
+
+static unsigned long clk_gate_get_rate(struct clk_hw *clk)
+{
+	return clk_get_rate(clk_get_parent(clk->clk));
+}
+
+static int clk_gate_enable(struct clk_hw *clk)
+{
+	struct clk_gate *gate = to_clk_gate(clk);
+	u32 reg;
+
+	reg = __raw_readl(gate->reg);
+	reg |= 1 << gate->bit_idx;
+	__raw_writel(reg, gate->reg);
+
+	return 0;
+}
+
+static void clk_gate_disable(struct clk_hw *clk)
+{
+	struct clk_gate *gate = to_clk_gate(clk);
+	u32 reg;
+
+	reg = __raw_readl(gate->reg);
+	reg &= ~(1 << gate->bit_idx);
+	__raw_writel(reg, gate->reg);
+}
+
+struct clk_hw_ops clk_gate_ops = {
+	.recalc_rate = clk_gate_get_rate,
+	.enable = clk_gate_enable,
+	.disable = clk_gate_disable,
+};
+EXPORT_SYMBOL_GPL(clk_gate_ops);
+
diff --git a/include/linux/clk.h b/include/linux/clk.h
index fd62e86..7c26135 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -124,6 +124,19 @@ extern struct clk_hw_ops clk_fixed_ops;
 
 #endif /* CONFIG_GENERIC_CLK_FIXED */
 
+#ifdef CONFIG_GENERIC_CLK_GATE
+
+struct clk_gate {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u8		bit_idx;
+};
+
+extern struct clk_hw_ops clk_gate_ops;
+
+#endif /* CONFIG_GENERIC_CLK_GATE */
+
+
 #else /* !CONFIG_GENERIC_CLK */
 
 /*
-- 
1.7.5.4

[PATCH 04/11] clk: Add simple gated clock

[Jamie Iles]

Hi Mark, Jeremy,

On Wed, Aug 24, 2011 at 02:15:52PM +0100, Mark Brown wrote:
> From: Jeremy Kerr <jeremy.kerr@canonical.com>
> 
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

I've just ported my (currently out-of-tree) platform to use the common 
struct clk and it all works nicely (if I add a naive implementation of 
clk_set_parent()).

Our platform has gateable clocks where bits in the control register are 
set to disable the clocks rather than enable them.  I've used the patch 
below to add support for that.

Jamie

8<----

Subject: [PATCH] clk: allow gateable clocks to work with both polarities

Some devices (picoxcell in particular) have gateable clocks where the
control register is a set-to-disable register.  Create a new set of
clock ops that can use struct clk_gate with this register configuration.

Signed-off-by: Jamie Iles <jamie@jamieiles.com>
---
 drivers/clk/clk-gate.c |   45 +++++++++++++++++++++++++++++++++++++--------
 include/linux/clk.h    |    3 ++-
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 833e0da..30926e9 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -10,7 +10,7 @@ static unsigned long clk_gate_get_rate(struct clk_hw *clk)
 	return clk_get_rate(clk_get_parent(clk->clk));
 }
 
-static int clk_gate_enable(struct clk_hw *clk)
+static void clk_gate_set_bit(struct clk_hw *clk)
 {
 	struct clk_gate *gate = to_clk_gate(clk);
 	u32 reg;
@@ -18,11 +18,9 @@ static int clk_gate_enable(struct clk_hw *clk)
 	reg = __raw_readl(gate->reg);
 	reg |= 1 << gate->bit_idx;
 	__raw_writel(reg, gate->reg);
-
-	return 0;
 }
 
-static void clk_gate_disable(struct clk_hw *clk)
+static void clk_gate_clear_bit(struct clk_hw *clk)
 {
 	struct clk_gate *gate = to_clk_gate(clk);
 	u32 reg;
@@ -32,10 +30,41 @@ static void clk_gate_disable(struct clk_hw *clk)
 	__raw_writel(reg, gate->reg);
 }
 
-struct clk_hw_ops clk_gate_ops = {
+static int clk_gate_enable_set(struct clk_hw *clk)
+{
+	clk_gate_set_bit(clk);
+
+	return 0;
+}
+
+static void clk_gate_disable_clear(struct clk_hw *clk)
+{
+	clk_gate_clear_bit(clk);
+}
+
+struct clk_hw_ops clk_gate_set_enable_ops = {
+	.recalc_rate = clk_gate_get_rate,
+	.enable = clk_gate_enable_set,
+	.disable = clk_gate_disable_clear,
+};
+EXPORT_SYMBOL_GPL(clk_gate_set_enable_ops);
+
+static int clk_gate_enable_clear(struct clk_hw *clk)
+{
+	clk_gate_clear_bit(clk);
+
+	return 0;
+}
+
+static void clk_gate_disable_set(struct clk_hw *clk)
+{
+	clk_gate_set_bit(clk);
+}
+
+struct clk_hw_ops clk_gate_set_disable_ops = {
 	.recalc_rate = clk_gate_get_rate,
-	.enable = clk_gate_enable,
-	.disable = clk_gate_disable,
+	.enable = clk_gate_enable_clear,
+	.disable = clk_gate_disable_set,
 };
-EXPORT_SYMBOL_GPL(clk_gate_ops);
+EXPORT_SYMBOL_GPL(clk_gate_set_disable_ops);
 
diff --git a/include/linux/clk.h b/include/linux/clk.h
index cb1879b..d30314a 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -132,7 +132,8 @@ struct clk_gate {
 	u8		bit_idx;
 };
 
-extern struct clk_hw_ops clk_gate_ops;
+extern struct clk_hw_ops clk_gate_set_enable_ops;
+extern struct clk_hw_ops clk_gate_set_disable_ops;
 
 #endif /* CONFIG_GENERIC_CLK_GATE */
 
-- 
1.7.4.1

[PATCH 04/11] clk: Add simple gated clock

[Stephen Boyd]

On 08/24/11 06:15, Mark Brown wrote:
> +static int clk_gate_enable(struct clk_hw *clk)
> +{
> +	struct clk_gate *gate = to_clk_gate(clk);
> +	u32 reg;
> +
> +	reg = __raw_readl(gate->reg);
> +	reg |= 1 << gate->bit_idx;

The BIT() macro might be clearer.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 05/11] clk: Prototype and document clk_register()

[Mark Brown]

This allows the compiler to ensure drivers are using the correct prototype.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
---
 include/linux/clk.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/include/linux/clk.h b/include/linux/clk.h
index 7c26135..2ca4f66 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops;
 
 #endif /* CONFIG_GENERIC_CLK_GATE */
 
+/**
+ * clk_register - register and initialize a new clock
+ *
+ * @ops: ops for the new clock
+ * @hw: struct clk_hw to be passed to the ops of the new clock
+ * @name: name to use for the new clock
+ *
+ * Register a new clock with the clk subsytem.  Returns either a
+ * struct clk for the new clock or a NULL pointer.
+ */
+struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
+			 const char *name);
 
 #else /* !CONFIG_GENERIC_CLK */
 
-- 
1.7.5.4

[PATCH 05/11] clk: Prototype and document clk_register()

[Stephen Boyd]

On 08/24/11 06:15, Mark Brown wrote:
> @@ -136,6 +136,18 @@ extern struct clk_hw_ops clk_gate_ops;
>  
>  #endif /* CONFIG_GENERIC_CLK_GATE */
>  
> +/**
> + * clk_register - register and initialize a new clock
> + *
> + * @ops: ops for the new clock
> + * @hw: struct clk_hw to be passed to the ops of the new clock
> + * @name: name to use for the new clock
> + *
> + * Register a new clock with the clk subsytem.  Returns either a

s/subsytem/subsystem/

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 06/11] clk: Provide a dummy clk_unregister()

[Mark Brown]

Even though unregistration is not actually supported by the clk API it is
still useful to provide a clk_unregister() so that drivers can implement
their unregistration code. This saves having to go back later and update
them once unregistration is possible.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 include/linux/clk.h |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/include/linux/clk.h b/include/linux/clk.h
index 2ca4f66..df5c64f 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -149,6 +149,21 @@ extern struct clk_hw_ops clk_gate_ops;
 struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
 			 const char *name);
 
+/**
+ * clk_unregister - remove a clock
+ *
+ * @clk: clock to unregister
+ *
+ * Remove a clock from the clk subsystem.  This is currently not
+ * implemented but is provided to allow unregistration code to be
+ * written in drivers ready for use when an implementation is
+ * provided.
+ */
+static inline int clk_unregister(struct clk *clk)
+{
+	return -ENOTSUPP;
+}
+
 #else /* !CONFIG_GENERIC_CLK */
 
 /*
-- 
1.7.5.4

[PATCH 06/11] clk: Provide a dummy clk_unregister()

[Jamie Iles]

Hi Mark,

On Wed, Aug 24, 2011 at 02:15:54PM +0100, Mark Brown wrote:
> Even though unregistration is not actually supported by the clk API it is
> still useful to provide a clk_unregister() so that drivers can implement
> their unregistration code. This saves having to go back later and update
> them once unregistration is possible.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  include/linux/clk.h |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 2ca4f66..df5c64f 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -149,6 +149,21 @@ extern struct clk_hw_ops clk_gate_ops;
>  struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
>  			 const char *name);
>  
> +/**
> + * clk_unregister - remove a clock
> + *
> + * @clk: clock to unregister
> + *
> + * Remove a clock from the clk subsystem.  This is currently not
> + * implemented but is provided to allow unregistration code to be
> + * written in drivers ready for use when an implementation is
> + * provided.
> + */
> +static inline int clk_unregister(struct clk *clk)
> +{
> +	return -ENOTSUPP;
> +}

ENOTSUPP is only defined in include/linux/errno.h which isn't included 
here.  I think either linux/errno.h needs to be included or use 
EOPNOTSUPP?

Jamie

[PATCH 07/11] clk: Constify struct clk_hw_ops

[Mark Brown]

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk.c   |    4 ++--
 include/linux/clk.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3a4d70e..1df6e23 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -16,7 +16,7 @@
 
 struct clk {
 	const char		*name;
-	struct clk_hw_ops	*ops;
+	const struct clk_hw_ops	*ops;
 	struct clk_hw		*hw;
 	unsigned int		enable_count;
 	unsigned int		prepare_count;
@@ -252,7 +252,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 }
 EXPORT_SYMBOL_GPL(clk_set_parent);
 
-struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
+struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw,
 		const char *name)
 {
 	struct clk *clk;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index df5c64f..fb5e435 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -146,7 +146,7 @@ extern struct clk_hw_ops clk_gate_ops;
  * Register a new clock with the clk subsytem.  Returns either a
  * struct clk for the new clock or a NULL pointer.
  */
-struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
+struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw,
 			 const char *name);
 
 /**
-- 
1.7.5.4

[PATCH 08/11] clk: Avoid clock name collisions

[Mark Brown]

Currently the generic clk API identifies clocks internally using the name
of the clock. This is OK for on-SoC clocks where we have enough control to
disambiguate but doesn't work well for clocks provided on external chips
where a system design may include more than one instance of the same chip
(the Wolfson Speyside system is an example of this) or may have namespace
collisions.

Address this by allowing the clock provider to supply a struct device for
the clock for use in disambiguation. As a first pass if it is provided we
prefix the clock name with the dev_name() of the device.

In order to avoid needless noise in names and memory usage it is strongly
recommended that on-SoC clocks do not provide a struct device until the
implementation is improved.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk.c   |   36 ++++++++++++++++++++++++++++++++----
 include/linux/clk.h |   14 ++++++++++----
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 1df6e23..f36f637 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -13,6 +13,7 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/device.h>
 
 struct clk {
 	const char		*name;
@@ -252,20 +253,44 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
 }
 EXPORT_SYMBOL_GPL(clk_set_parent);
 
-struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw,
-		const char *name)
+struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops,
+			 struct clk_hw *hw, const char *name)
 {
 	struct clk *clk;
+	char *new_name;
+	size_t name_len;
 
 	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
 	if (!clk)
 		return NULL;
 
-	clk->name = name;
 	clk->ops = ops;
 	clk->hw = hw;
 	hw->clk = clk;
 
+	/* Since we currently match clock providers on a purely string
+	 * based method add a prefix based on the device name if a
+	 * device is provided.  When we have support for device tree
+	 * based clock matching it should be possible to avoid this
+	 * mangling and instead use the struct device to hook into
+	 * the bindings.
+	 *
+	 * As we don't currently support unregistering clocks we don't
+	 * need to worry about cleanup as yet.
+	 */
+	if (dev) {
+		name_len = strlen(name) + strlen(dev_name(dev)) + 2;
+		new_name = kzalloc(name_len, GFP_KERNEL);
+		if (!new_name)
+			goto err;
+
+		snprintf(new_name, name_len, "%s-%s", dev_name(dev), name);
+
+		clk->name = new_name;
+	} else {
+		clk->name = name;
+	}
+
 	/* Query the hardware for parent and initial rate. We may alter
 	 * the clock topology, making this clock available from the parent's
 	 * children list. So, we need to protect against concurrent
@@ -285,7 +310,10 @@ struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw,
 
 	mutex_unlock(&prepare_lock);
 
-
 	return clk;
+
+err:
+	kfree(clk);
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(clk_register);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index fb5e435..cb1879b 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -139,15 +139,21 @@ extern struct clk_hw_ops clk_gate_ops;
 /**
  * clk_register - register and initialize a new clock
  *
+ * @dev: device providing the clock or NULL
  * @ops: ops for the new clock
  * @hw: struct clk_hw to be passed to the ops of the new clock
  * @name: name to use for the new clock
  *
- * Register a new clock with the clk subsytem.  Returns either a
- * struct clk for the new clock or a NULL pointer.
+ * Register a new clock with the clk subsytem.  If dev is provided
+ * then it will be used to disambiguate between multiple instances of
+ * the same device in the system, typically this should only be done
+ * for devices that are not part of the core SoC unless device tree is
+ * in use.
+ *
+ * Returns either a struct clk for the new clock or a NULL pointer.
  */
-struct clk *clk_register(const struct clk_hw_ops *ops, struct clk_hw *hw,
-			 const char *name);
+struct clk *clk_register(struct device *dev, const struct clk_hw_ops *ops,
+			 struct clk_hw *hw, const char *name);
 
 /**
  * clk_unregister - remove a clock
-- 
1.7.5.4

[PATCH 08/11] clk: Avoid clock name collisions

[Stephen Boyd]

On 08/24/11 06:15, Mark Brown wrote:
> +	if (dev) {
> +		name_len = strlen(name) + strlen(dev_name(dev)) + 2;
> +		new_name = kzalloc(name_len, GFP_KERNEL);
> +		if (!new_name)
> +			goto err;
> +
> +		snprintf(new_name, name_len, "%s-%s", dev_name(dev), name);
> +

Perhaps use kasprintf() instead?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 09/11] clk: Add Kconfig option to build all generic clk drivers

[Mark Brown]

Currently drivers for the generic clk subsystem must be selected by
platforms using them in order to enable build. When doing development on
the API or generic build time testing it is useful to be able to build
unused drivers in order to improve coverage so supply a Kconfig option
which allows this.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/Kconfig |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index a78967c..95b42a3 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -1,4 +1,3 @@
-
 config CLKDEV_LOOKUP
 	bool
 	select HAVE_CLK
@@ -9,6 +8,16 @@ config HAVE_MACH_CLKDEV
 config GENERIC_CLK
 	bool
 
+config GENERIC_CLK_BUILD_TEST
+	bool "Build all generic clock drivers"
+	depends on EXPERIMENTAL && GENERIC_CLK
+	select GENERIC_CLK_FIXED
+	select GENERIC_CLK_GATE
+	help
+	   Enable all possible generic clock drivers.  This is only
+	   useful for improving build coverage, it is not useful for
+	   production kernel builds.
+
 config GENERIC_CLK_FIXED
 	bool
 	depends on GENERIC_CLK
-- 
1.7.5.4

[PATCH 10/11] clk: Add initial WM831x clock driver

[Mark Brown]

The WM831x and WM832x series of PMICs contain a flexible clocking
subsystem intended to provide always on and system core clocks.  It
features:

- A 32.768kHz crystal oscillator which can optionally be used to pass
  through an externally generated clock.
- A FLL which can be clocked from either the 32.768kHz oscillator or
  the CLKIN pin.
- A CLKOUT pin which can bring out either the oscillator or the FLL
  output.
- The 32.768kHz clock can also optionally be brought out on the GPIO
  pins of the device.

This driver fully supports the 32.768kHz oscillator and CLKOUT.  The FLL
is supported only in AUTO mode, the full flexibility of the FLL cannot
currently be used.  The use of clock references other than the internal
oscillator is not currently supported, and since clk_set_parent() is not
implemented in the generic clock API the clock tree configuration cannot
be changed at runtime.

Due to a lack of access to systems where the core SoC has been converted
to use the generic clock API this driver has been compile tested only.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 MAINTAINERS              |    1 +
 drivers/clk/Kconfig      |    5 +
 drivers/clk/Makefile     |    1 +
 drivers/clk/clk-wm831x.c |  386 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 393 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clk/clk-wm831x.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2be1684..fb3886b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7213,6 +7213,7 @@ T:	git git://opensource.wolfsonmicro.com/linux-2.6-audioplus
 W:	http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices
 S:	Supported
 F:	Documentation/hwmon/wm83??
+F:	drivers/clk/clk-wm83*.c
 F:	drivers/leds/leds-wm83*.c
 F:	drivers/mfd/wm8*.c
 F:	drivers/power/wm83*.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 95b42a3..8aca5ab 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -13,6 +13,7 @@ config GENERIC_CLK_BUILD_TEST
 	depends on EXPERIMENTAL && GENERIC_CLK
 	select GENERIC_CLK_FIXED
 	select GENERIC_CLK_GATE
+	select GENERIC_CLK_WM831X if MFD_WM831X=y
 	help
 	   Enable all possible generic clock drivers.  This is only
 	   useful for improving build coverage, it is not useful for
@@ -25,3 +26,7 @@ config GENERIC_CLK_FIXED
 config GENERIC_CLK_GATE
 	bool
 	depends on GENERIC_CLK
+
+config GENERIC_CLK_WM831X
+	tristate
+	depends on GENERIC_CLK && MFD_WM831X
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d186446..6628ad5 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_GENERIC_CLK)	+= clk.o
 obj-$(CONFIG_GENERIC_CLK_FIXED)	+= clk-fixed.o
 obj-$(CONFIG_GENERIC_CLK_GATE)	+= clk-gate.o
+obj-$(CONFIG_GENERIC_CLK_WM831X) += clk-wm831x.o
diff --git a/drivers/clk/clk-wm831x.c b/drivers/clk/clk-wm831x.c
new file mode 100644
index 0000000..5682acd
--- /dev/null
+++ b/drivers/clk/clk-wm831x.c
@@ -0,0 +1,386 @@
+/*
+ * WM831x clock control
+ *
+ * Copyright 2011 Wolfson Microelectronics PLC.
+ *
+ * Author: Mark Brown <broonie@opensource.wolfsonmicro.com>
+ *
+ *  This program is free software; you can redistribute  it and/or modify it
+ *  under  the terms of  the GNU General  Public License as published by the
+ *  Free Software Foundation;  either version 2 of the  License, or (at your
+ *  option) any later version.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/wm831x/core.h>
+
+struct wm831x_clk {
+	struct wm831x *wm831x;
+	struct clk_hw xtal_hw;
+	struct clk_hw fll_hw;
+	struct clk_hw clkout_hw;
+	bool xtal_ena;
+};
+
+static int wm831x_xtal_enable(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  xtal_hw);
+
+	if (clkdata->xtal_ena)
+		return 0;
+	else
+		return -EPERM;
+}
+
+static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  xtal_hw);
+
+	if (clkdata->xtal_ena)
+		return 32768;
+	else
+		return 0;
+}
+
+static long wm831x_xtal_round_rate(struct clk_hw *hw, unsigned long rate)
+{
+	return wm831x_xtal_recalc_rate(hw);
+}
+
+static const struct clk_hw_ops wm831x_xtal_ops = {
+	.enable = wm831x_xtal_enable,
+	.recalc_rate = wm831x_xtal_recalc_rate,
+	.round_rate = wm831x_xtal_round_rate,
+};
+
+static const unsigned long wm831x_fll_auto_rates[] = {
+	 2048000,
+	11289600,
+	12000000,
+	12288000,
+	19200000,
+	22579600,
+	24000000,
+	24576000,
+};
+
+static bool wm831x_fll_enabled(struct wm831x *wm831x)
+{
+	int ret;
+
+	ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_1);
+	if (ret < 0) {
+		dev_err(wm831x->dev, "Unable to read FLL_CONTROL_1: %d\n",
+			ret);
+		return true;
+	}
+
+	return ret & WM831X_FLL_ENA;
+}
+
+static int wm831x_fll_prepare(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  fll_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2,
+			      WM831X_FLL_ENA, WM831X_FLL_ENA);
+	if (ret != 0)
+		dev_crit(wm831x->dev, "Failed to enable FLL: %d\n", ret);
+
+	return ret;
+}
+
+static void wm831x_fll_unprepare(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  fll_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	ret = wm831x_set_bits(wm831x, WM831X_FLL_CONTROL_2, WM831X_FLL_ENA, 0);
+	if (ret != 0)
+		dev_crit(wm831x->dev, "Failed to disaable FLL: %d\n", ret);
+}
+
+static unsigned long wm831x_fll_recalc_rate(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  fll_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2);
+	if (ret < 0) {
+		dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n",
+			ret);
+		return 0;
+	}
+
+	if (ret & WM831X_FLL_AUTO)
+		return wm831x_fll_auto_rates[ret & WM831X_FLL_AUTO_FREQ_MASK];
+
+	dev_err(wm831x->dev, "FLL only supported in AUTO mode\n");
+	return 0;
+}
+
+static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long *parent_rate)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  fll_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++)
+		if (wm831x_fll_auto_rates[i] == rate)
+			break;
+	if (i == ARRAY_SIZE(wm831x_fll_auto_rates))
+		return -EINVAL;
+
+	if (wm831x_fll_enabled(wm831x))
+		return -EPERM;
+
+	return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2,
+			       WM831X_FLL_AUTO_FREQ_MASK, i);
+}
+
+static struct clk *wm831x_fll_get_parent(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  fll_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	/* AUTO mode is always clocked from the crystal */
+	ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2);
+	if (ret < 0) {
+		dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n",
+			ret);
+		return NULL;
+	}
+
+	if (ret & WM831X_FLL_AUTO)
+		return clkdata->xtal_hw.clk;
+
+	ret = wm831x_reg_read(wm831x, WM831X_FLL_CONTROL_5);
+	if (ret < 0) {
+		dev_err(wm831x->dev, "Unable to read FLL_CONTROL_5: %d\n",
+			ret);
+		return NULL;
+	}
+
+	switch (ret & WM831X_FLL_CLK_SRC_MASK) {
+	case 0:
+		return clkdata->xtal_hw.clk;
+	case 1:
+		dev_warn(wm831x->dev,
+			 "FLL clocked from CLKIN not yet supported\n");
+		return NULL;
+	default:
+		dev_err(wm831x->dev, "Unsupported FLL clock source %d\n",
+			ret & WM831X_FLL_CLK_SRC_MASK);
+		return NULL;
+	}
+}
+
+static const struct clk_hw_ops wm831x_fll_ops = {
+	.prepare = wm831x_fll_prepare,
+	.unprepare = wm831x_fll_unprepare,
+	.recalc_rate = wm831x_fll_recalc_rate,
+	.set_rate = wm831x_fll_set_rate,
+	.get_parent = wm831x_fll_get_parent,
+};
+
+static int wm831x_clkout_prepare(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  clkout_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	ret = wm831x_reg_unlock(wm831x);
+	if (ret != 0) {
+		dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret);
+		return ret;
+	}
+
+	ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1,
+			      WM831X_CLKOUT_ENA, WM831X_CLKOUT_ENA);
+	if (ret != 0)
+		dev_crit(wm831x->dev, "Failed to enable CLKOUT: %d\n", ret);
+
+	wm831x_reg_lock(wm831x);
+
+	return ret;
+}
+
+static void wm831x_clkout_unprepare(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  clkout_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	ret = wm831x_reg_unlock(wm831x);
+	if (ret != 0) {
+		dev_crit(wm831x->dev, "Failed to lock registers: %d\n", ret);
+		return;
+	}
+
+	ret = wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1,
+			      WM831X_CLKOUT_ENA, 0);
+	if (ret != 0)
+		dev_crit(wm831x->dev, "Failed to disable CLKOUT: %d\n", ret);
+
+	wm831x_reg_lock(wm831x);
+}
+
+static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw)
+{
+	return clk_get_rate(clk_get_parent(hw->clk));
+}
+
+static long wm831x_clkout_round_rate(struct clk_hw *hw, unsigned long rate)
+{
+	return clk_round_rate(clk_get_parent(hw->clk), rate);
+}
+
+static int wm831x_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *parent_rate)
+{
+	*parent_rate = rate;
+	return CLK_SET_RATE_PROPAGATE;
+}
+
+static struct clk *wm831x_clkout_get_parent(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  clkout_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+	int ret;
+
+	ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_1);
+	if (ret < 0) {
+		dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_1: %d\n",
+			ret);
+		return NULL;
+	}
+
+	if (ret & WM831X_CLKOUT_SRC)
+		return clkdata->xtal_hw.clk;
+	else
+		return clkdata->fll_hw.clk;
+}
+
+static const struct clk_hw_ops wm831x_clkout_ops = {
+	.prepare = wm831x_clkout_prepare,
+	.unprepare = wm831x_clkout_unprepare,
+	.recalc_rate = wm831x_clkout_recalc_rate,
+	.round_rate = wm831x_clkout_round_rate,
+	.set_rate = wm831x_clkout_set_rate,
+	.get_parent = wm831x_clkout_get_parent,
+};
+
+static __devinit int wm831x_clk_probe(struct platform_device *pdev)
+{
+	struct wm831x *wm831x = dev_get_drvdata(pdev->dev.parent);
+	struct wm831x_clk *clkdata;
+	int ret;
+
+	clkdata = kzalloc(sizeof(*clkdata), GFP_KERNEL);
+	if (!clkdata)
+		return -ENOMEM;
+
+	/* XTAL_ENA can only be set via OTP/InstantConfig so just read once */
+	ret = wm831x_reg_read(wm831x, WM831X_CLOCK_CONTROL_2);
+	if (ret < 0) {
+		dev_err(wm831x->dev, "Unable to read CLOCK_CONTROL_2: %d\n",
+			ret);
+		goto err_alloc;
+	}
+	clkdata->xtal_ena = ret & WM831X_XTAL_ENA;
+
+	if (!clk_register(wm831x->dev, &wm831x_xtal_ops, &clkdata->xtal_hw,
+			  "xtal")) {
+		ret = -EINVAL;
+		goto err_alloc;
+	}
+
+	if (!clk_register(wm831x->dev, &wm831x_fll_ops, &clkdata->fll_hw,
+			  "fll")) {
+		ret = -EINVAL;
+		goto err_xtal;
+	}
+
+	if (!clk_register(wm831x->dev, &wm831x_clkout_ops, &clkdata->clkout_hw,
+			  "clkout")) {
+		ret = -EINVAL;
+		goto err_fll;
+	}
+
+	dev_set_drvdata(&pdev->dev, clkdata);
+
+	return 0;
+
+err_fll:
+	clk_unregister(clkdata->fll_hw.clk);
+err_xtal:
+	clk_unregister(clkdata->xtal_hw.clk);
+err_alloc:
+	kfree(clkdata);
+	return ret;
+}
+
+static __devexit int wm831x_clk_remove(struct platform_device *pdev)
+{
+	struct wm831x_clk *clkdata = dev_get_drvdata(&pdev->dev);
+
+	clk_unregister(clkdata->clkout_hw.clk);
+	clk_unregister(clkdata->fll_hw.clk);
+	clk_unregister(clkdata->xtal_hw.clk);
+	kfree(clkdata);
+
+	return 0;
+}
+
+static struct platform_driver wm831x_clk_driver = {
+	.probe = wm831x_clk_probe,
+	.remove = __devexit_p(wm831x_clk_remove),
+	.driver		= {
+		.name	= "wm831x-clk",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init wm831x_clk_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&wm831x_clk_driver);
+	if (ret != 0)
+		pr_err("Failed to register WM831x clock driver: %d\n", ret);
+
+	return ret;
+}
+module_init(wm831x_clk_init);
+
+static void __exit wm831x_clk_exit(void)
+{
+	platform_driver_unregister(&wm831x_clk_driver);
+}
+module_exit(wm831x_clk_exit);
+
+/* Module information */
+MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_DESCRIPTION("WM831x clock driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:wm831x-clk");
-- 
1.7.5.4

[PATCH 11/11] x86: Enable generic clk API on x86

[Mark Brown]

Enable the generic clk API on x86, enabling use of the API by drivers for
x86 modules and also improving build coverage for clock API using devices.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 arch/x86/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ff6d4e7..02ece2e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -74,6 +74,7 @@ config X86
 	select HAVE_BPF_JIT if (X86_64 && NET)
 	select CLKEVT_I8253
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
+	select GENERIC_CLK
 
 config INSTRUCTION_DECODER
 	def_bool (KPROBES || PERF_EVENTS)
-- 
1.7.5.4

[PATCH 01/11] clk: Add a generic clock infrastructure

[Stephen Boyd]

On 08/24/11 06:15, Mark Brown wrote:
> +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
> +		const char *name)
> +{
> +	struct clk *clk;
> +
> +	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
> +	if (!clk)
> +		return NULL;
> +
> +	clk->name = name;
> +	clk->ops = ops;
> +	clk->hw = hw;
> +	hw->clk = clk;
> +
> +	/* Query the hardware for parent and initial rate */
> +
> +	if (clk->ops->get_parent)
> +		/* We don't to lock against prepare/enable here, as
> +		 * the clock is not yet accessible from anywhere */
> +		clk->parent = clk->ops->get_parent(clk->hw);
> +
> +	if (clk->ops->recalc_rate)
> +		clk->rate = clk->ops->recalc_rate(clk->hw);
> +
> +
> +	return clk;
> +}
> +EXPORT_SYMBOL_GPL(clk_register);

How will this play with clkdev? I think we'll need to add another
function on top of this one that allows you to register a clk_hw and an
array of clk_lookups at the same time. Otherwise we're going to get
about 10 different implementations of the same code.

Something like:

struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw, 
		const char *name, struct clk_lookup *lookups, 
		size_t num_lookups)


?

> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 1d37f42..93ff870 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -11,17 +12,103 @@
>  #ifndef __LINUX_CLK_H
>  #define __LINUX_CLK_H
>  
> +#include <linux/err.h>
> +#include <linux/spinlock.h>
> +

I don't see these includes used in this header.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 01/11] clk: Add a generic clock infrastructure

[Saravana Kannan]

On Wed, August 24, 2011 6:15 am, Mark Brown wrote:
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 1d37f42..93ff870 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h

> -struct clk;
> +static inline int clk_prepare(struct clk *clk) { return 0; }
> +static inline void clk_unprepare(struct clk *clk) { }
> +

You should add might_sleep() inside these stubs. That way, if any device
driver developers wants to move over to the new APIs before their arch
does, they get to see the warnings when they call prepare/unprepare in
atomic context.

Thanks,
Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

[PATCH 01/11] clk: Add a generic clock infrastructure

[Mark Brown]

On Tue, Sep 13, 2011 at 07:22:17PM -0700, Saravana Kannan wrote:
> On Wed, August 24, 2011 6:15 am, Mark Brown wrote:

> > -struct clk;
> > +static inline int clk_prepare(struct clk *clk) { return 0; }
> > +static inline void clk_unprepare(struct clk *clk) { }

> You should add might_sleep() inside these stubs. That way, if any device
> driver developers wants to move over to the new APIs before their arch
> does, they get to see the warnings when they call prepare/unprepare in
> atomic context.

Just to repeat previous clarifications as I said when I reposted I've no
intention of working on this, I'm just republishing Jeremy's work as
Mike needed a copy.