Hi,
This is again a set of patches to unify the management of clocks and

allow easy registration and unregistration of them. This is neccessary

to cleanly support such devices as toshiba mobile companion chips,

sa1111 companion, ASIC3 companion, etc. Also it brings code unification,

especially for a lot of arm sub-arches which share nearly the same code.
Changes from the previous patchset:

* Change the name as requested by Ben: clocklib -> generic clk (genclk)

* Move from lib/clocklib.c to kernel/genclk/*

* Fix locking in generic code
* Rebase PXA patch to the current Russell's tree.

* Interlock access to CKEN registers on PXA.
--

With best wishes

Dmitry
--

Any particular reason why you've not changed the enable and disable
--

Ben (ben@fluff.org, http://www.fluff.org/)
  'a smiley only costs 4 bytes'

--

Please read my previous mail. This was specifically requested by Andrew
--

With best wishes

Dmitry
--

Make SA-1100 use clocklib for clocks support.
Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>

---

 arch/arm/Kconfig             |    1 +

 arch/arm/mach-sa1100/clock.c |  114 ++++++------------------------------------

 2 files changed, 17 insertions(+), 98 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

index bf0dd41..d86ad6a 100644

--- a/arch/arm/Kconfig

+++ b/arch/arm/Kconfig

@@ -460,6 +460,7 @@ config ARCH_SA1100

 	select GENERIC_CLOCKEVENTS

 	select TICK_ONESHOT

 	select HAVE_GPIO_LIB

+	select HAVE_GENERIC_CLOCKS

 	help

 	  Support for StrongARM 11x0 based boards.
diff --git a/arch/arm/mach-sa1100/clock.c b/arch/arm/mach-sa1100/clock.c

index fc97fe5..472eec1 100644

--- a/arch/arm/mach-sa1100/clock.c

+++ b/arch/arm/mach-sa1100/clock.c

@@ -3,88 +3,12 @@

  */

 #include <linux/module.h>

 #include <linux/kernel.h>

-#include <linux/list.h>

-#include <linux/errno.h>

-#include <linux/err.h>

-#include <linux/string.h>

 #include <linux/clk.h>

-#include <linux/spinlock.h>

-#include <linux/mutex.h>

+#include <linux/generic_clk.h>
 #include <asm/hardware.h>
-/*

- * Very simple clock implementation - we only have one clock to

- * deal with at the moment, so we only match using the "name".

- */

-struct clk {

-	struct list_head	node;

-	unsigned long		rate;

-	const char		*name;

-	unsigned int		enabled;

-	void			(*enable)(void);

-	void			(*disable)(void);

-};

-

-static LIST_HEAD(clocks);

-static DEFINE_MUTEX(clocks_mutex);

-static DEFINE_SPINLOCK(clocks_lock);

-

-struct clk *clk_get(struct device *dev, const char *id)

-{

-	struct clk *p, *clk = ERR_PTR(-ENOENT);

-

-	mutex_lock(&clocks_mutex);

-	list_for_each_entry(p, &clocks, node) {

-		if (strcmp(id, p->name) == 0) {

-			clk = p;

-			break;

-		}

-	}

-	mutex_unlock(&clocks_mutex);

-

-	return clk;

-}

-EXPORT_SYMBOL(clk_get);

-

-void clk_put(struct clk *clk)

-{

-}

-EXPORT_SYMBOL(clk_put);

-...
[ message continues ]

---

 arch/arm/Kconfig           |    1 +

 arch/arm/mach-pxa/clock.c  |  163 ++++++++++++++++++--------------------------

 arch/arm/mach-pxa/clock.h  |  120 ++++++++++++++++----------------

 arch/arm/mach-pxa/pxa25x.c |   80 ++++++++++++---------

 arch/arm/mach-pxa/pxa27x.c |   80 +++++++++++++--------

 arch/arm/mach-pxa/pxa300.c |   10 ++-

 arch/arm/mach-pxa/pxa320.c |    4 +-

 arch/arm/mach-pxa/pxa3xx.c |  113 ++++++++++++++++++++----------

 8 files changed, 305 insertions(+), 266 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

index ba5d8df..bf0dd41 100644

--- a/arch/arm/Kconfig

+++ b/arch/arm/Kconfig

@@ -431,6 +431,7 @@ config ARCH_PXA

 	select GENERIC_TIME

 	select GENERIC_CLOCKEVENTS

 	select TICK_ONESHOT

+	select HAVE_GENERIC_CLOCKS

 	help

 	  Support for Intel/Marvell's PXA2xx/PXA3xx processor line.
diff --git a/arch/arm/mach-pxa/clock.c b/arch/arm/mach-pxa/clock.c

index b2e847a..9831c58 100644

--- a/arch/arm/mach-pxa/clock.c

+++ b/arch/arm/mach-pxa/clock.c

@@ -3,155 +3,124 @@

  */

 #include <linux/module.h>

 #include <linux/kernel.h>

-#include <linux/list.h>

-#include <linux/errno.h>

-#include <linux/err.h>

-#include <linux/string.h>

 #include <linux/clk.h>

-#include <linux/spinlock.h>

-#include <linux/platform_device.h>

+#include <linux/generic_clk.h>

 #include <linux/delay.h>
 #include <asm/arch/pxa2xx-regs.h>

 #include <asm/arch/pxa2xx-gpio.h>

 #include <asm/hardware.h>
-#include "devices.h"

-#include "generic.h"

+//#include "devices.h"

+//#include "generic.h"

 #include "clock.h"
-static LIST_HEAD(clocks);

-static DEFINE_MUTEX(clocks_mutex);

-static DEFINE_SPINLOCK(clocks_lock);

-

-static struct clk *clk_lookup(struct device *dev, const char *id)

+static int clk_gpio11_enable(struct clk *clk)

 {

-	struct clk *p;

-

-	list_for_each_entry(p, &clocks, node)

-		if (strcmp(id, p->name) == 0 && p->dev == dev)

-			return p;

-...
[ message continues ]

Provide a generic framework that platform may choose

to support clocks api. In particular this provides

platform-independant struct clk definition, a full

implementation of clocks api and a set of functions

for registering and unregistering clocks in a safe way.
Signed-off-by: Dmitry Baryshkov <dbaryshkov@gmail.com>

---

 include/linux/generic_clk.h    |   72 +++++++++++

 init/Kconfig                   |    3 +

 kernel/Makefile                |    2 +

 kernel/genclk/Makefile         |    2 +

 kernel/genclk/genclk.c         |  264 ++++++++++++++++++++++++++++++++++++++++

 kernel/genclk/genclk.h         |   21 +++

 kernel/genclk/genclk_debugfs.c |   97 +++++++++++++++

 7 files changed, 461 insertions(+), 0 deletions(-)

 create mode 100644 include/linux/generic_clk.h

 create mode 100644 kernel/genclk/Makefile

 create mode 100644 kernel/genclk/genclk.c

 create mode 100644 kernel/genclk/genclk.h

 create mode 100644 kernel/genclk/genclk_debugfs.c
diff --git a/include/linux/generic_clk.h b/include/linux/generic_clk.h

new file mode 100644

index 0000000..c211966

--- /dev/null

+++ b/include/linux/generic_clk.h

@@ -0,0 +1,72 @@

+/*

+ * Copyright (C) 2008 Dmitry Baryshkov

+ *

+ * This file is released under the GPL v2.

+ */

+

+#ifndef GENERIC_CLK_H

+#define GENERIC_CLK_H

+

+#include <linux/spinlock.h>

+#include <linux/kref.h>

+

+struct clk;

+struct device;

+

+/**

+ * struct clk_ops - generic clock management operations

+ * @can_get: checks whether passed device can get this clock

+ * @set_parent: reconfigures the clock to use specified parent

+ * @set_mode: enable or disable specified clock

+ * @get_rate: obtain the current clock rate of a specified clock

+ * @set_rate: set the clock rate for a specified clock

+ * @round_rate: adjust a reate to the exact rate a clock can provide

+ *

+ * This structure specifies clock operations that are used to configure

+ * specific clock.

+ */

+struct clk_ops {

+	int (*can_get)(struct clk *clk, struct device *dev);

...
[ message continues ]

So I'll merge this into -mm and, unless there be suitably-serious

objections, into 2.6.27.
The other two patches got rejects all over the place - so many that I

didn't even attempt to fix them.  Perhaps you were developing againt

Linus's tree, which really is not a successful strategy late in the
hm, what's this unusual-but-uncommented code doing in here?  Please

always comment unusual code.
There is already a zero-byte version of `struct lock_class_key' in

include/linux/lockdep.h, dependent upon CONFIG_LOCKDEP.
whoa.  We have a kernel-wide symbol called "locks"?
afaict that won't cause any linkage errors, but those compilation units

which already have static variables called "clocks" will explode if

they deliberately or accidentally include your header file.
I suggest that this be renamed to something more subsystem-specific.

genclk_clocks?
Or, better, refactor the code a bit and make it static.  Do other

compilation units _really_ need to go poking into genclk's internals? 
As this is a new kernel-wide utility library, it is appropriate that

all of its public interfaces (at least) be documented.  An appropriate

way of doing that is via kerneldoc annotation.  Please don't forget to

document return values and call environment prerequisites (eg: requires
Please always test code with all kernel debug options enabled.  See
Could do
	if (!WARN_ON(IS_ERR(dir)))

		clk->dir = dir;
Or not.  What you have there is clearer.
Quite a bit of rework is needed here and it needs to be fully retested.

I won't apply it after all.
--

Please consider this version of patch.
I've:

* cleaned up the points you have pointed at:

* * moved locks and list to genclk_ namespace

* * made them static

* * provided documentation for new API and a link to linux/clk.h

    as a doc for implemented API

* reworked lockdep handling to be more clean and error-prone

* some small fixes
--

With best wishes

Dmitry
--

That is, the stuff that's not already documented in <linux/clk.h>;

that's where clk_get_parent() is documented, for example.
I'd suggest a separate (for review!) doc patch to add this to the

stuff that's at the end of Documentation/DocBook/kernel-api.tmpl

in 2.6.26 ... possibly the existing clk.h interface stuff should

become a <sect1> and the new implementation glue should become

a sibling <sect1>, that will cleanly separate the two.
- Dave

--

argh.  That's why I missed it - please don't document stuff in header

files.  The usual approach is to document interfaces at the

implementation site.
Except for structs where of course there is no choice.  But then,

the .h file _is_ the definition site.
--

That doesn't work.  In this case, linux/clk.h _is_ the only source of

the API.  The API is just a defined set of functions.
How an architecture or machine class implements it is at the discressiom

of whoever's writing that code.  The point being, that we have a fixed

core API which drivers can use which is portable across different SoCs.
However, there certainly are implementation specific issues which

architecture code needs to know about - hence why the implementation

has (until genclk) been left completely up to that code to decide and

implement the interface.
So, the only place for that documentation is the header file.  Documenting

an implementation will just confuse the issue and lead people into thinking

that it somehow represents the way things should be done.
Even genclk doesn't represent "The Way Things Should Be Done" - it's just

yet another implementation of the interface.
--

Russell King

 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/

 maintainer of:

--

When it's a pure interface, I don't see a better option than having the

kerneldoc live in the header file ...
What Dmitry has provided is one implementation framework.  It's not

clear it's general enough to become "the" implementation framework, or

that it should be.  ISTR that it's not ready to handle OMAP clocks

yet ... those would be the acid test for any proposal that there be
That matches <linux/clk.h> to a tee:  the header *is* the definition

of the interface.
- dave
--

Strange. They were developed against Russell's arm:devel. Please check
Fine, I'll drop that conditional compilation and always use struct

lock_class_key.

The whole thing is required to make lockdep work with clocklib.

Otherwise it thinks that all locks inside struct clk belong to the

same class and "detects" deadlock when code tries to lock a clock,
The header file defining the "clocks" and clocks_lock is for private

use only, e.g. the debugfs code. I will change that to
The updated patch will be posted few hours later.
--

With best wishes

Dmitry

--

Because the release handler might not like being called under

that lock.  I didn't check...

--