diff mbox

[v2,6/7] clk: Add initial WM831x clock driver

Message ID 1316730422-20027-7-git-send-email-mturquette@ti.com
State New
Headers show

Commit Message

Mike Turquette Sept. 22, 2011, 10:27 p.m. UTC 
From: Mark Brown <broonie@opensource.wolfsonmicro.com>

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>
Signed-off-by: Mike Turquette <mturquette@ti.com>
---
Changes since v1:
Changed CLK_SET_RATE_PROPAGATE to CLK_PARENT_RATE_CHANGE
Converted to clk_register to the original style, without passing in
	struct device *dev.  This is due to Mark's clock name collision
	patch being dropped.

 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

Comments

Grant Likely Sept. 25, 2011, 4:08 a.m. UTC | #1 
On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:
> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> 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>
> Signed-off-by: Mike Turquette <mturquette@ti.com>

A few minor comments below.  Otherwise looks fine to me.

> +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 devm_kzalloc() is used, then all the kfree unwinding can be
dropped.

> +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;

No need for this song-and-dance.  The driver core is pretty well
debugged.  Just use "return platform_driver_register(...);"

g.
Mike Turquette Sept. 25, 2011, 5:29 a.m. UTC | #2 
On Sat, Sep 24, 2011 at 9:08 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:
>> From: Mark Brown <broonie@opensource.wolfsonmicro.com>
>>
>> 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>
>> Signed-off-by: Mike Turquette <mturquette@ti.com>
>
> A few minor comments below.  Otherwise looks fine to me.
>
>> +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 devm_kzalloc() is used, then all the kfree unwinding can be
> dropped.
>
>> +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;
>
> No need for this song-and-dance.  The driver core is pretty well
> debugged.  Just use "return platform_driver_register(...);"

Grant,

Thanks for the review.

Mark,

I know you're not carrying this whole set of patches but do you want
to rework this and resend or do you just want me to fix it up?
Changes are trivial if you don't want to touch it.

Regards,
Mike

> g.
>
Mark Brown Sept. 26, 2011, 9:38 a.m. UTC | #3 
On Sat, Sep 24, 2011 at 10:08:36PM -0600, Grant Likely wrote:
> On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:

> > +	ret = platform_driver_register(&wm831x_clk_driver);
> > +	if (ret != 0)
> > +		pr_err("Failed to register WM831x clock driver: %d\n", ret);
> > +
> > +	return ret;

> No need for this song-and-dance.  The driver core is pretty well
> debugged.  Just use "return platform_driver_register(...);"

No, that's not helpful.  The issue isn't the device probe code itself,
the issue is things like module unload not doing what they're supposed
to do and leaving the device lying around or something - there's rather
more going on than just the plain API call.
Grant Likely Oct. 4, 2011, 6:18 p.m. UTC | #4 
On Mon, Sep 26, 2011 at 10:38:58AM +0100, Mark Brown wrote:
> On Sat, Sep 24, 2011 at 10:08:36PM -0600, Grant Likely wrote:
> > On Thu, Sep 22, 2011 at 03:27:01PM -0700, Mike Turquette wrote:
> 
> > > +	ret = platform_driver_register(&wm831x_clk_driver);
> > > +	if (ret != 0)
> > > +		pr_err("Failed to register WM831x clock driver: %d\n", ret);
> > > +
> > > +	return ret;
> 
> > No need for this song-and-dance.  The driver core is pretty well
> > debugged.  Just use "return platform_driver_register(...);"
> 
> No, that's not helpful.  The issue isn't the device probe code itself,
> the issue is things like module unload not doing what they're supposed
> to do and leaving the device lying around or something - there's rather
> more going on than just the plain API call.

Then lets fix the core code.  I see this pattern show up again and
again of extra boilerplate going around
platform_driver_{register,unregister}().  That says to me that there
either needs to be a new helper, or the core code needs to be made
more verbose.

In fact, I've been considering adding a macro for
{platform,i2c,spi,...}_drivers that does all the module boilerplate
for the common case of only registering a driver at init time.
Something like:

#define module_platform_driver(__driver) \
int __driver##_init(void) \
{ \
	return platform_driver_register(&(__driver)); \
} \
module_init(__driver##_init); \
void ##__driver##_exit(void) \
{ \
	platform_driver_unregister(&(__driver)); \
} \
module_exit(##__driver##_exit);

It's not a lot of code, but I dislike how much boilerplate every
single driver has to use if it doesn't do anything special.

g.
Mark Brown Oct. 4, 2011, 8:50 p.m. UTC | #5 
On Tue, Oct 04, 2011 at 12:18:18PM -0600, Grant Likely wrote:
> On Mon, Sep 26, 2011 at 10:38:58AM +0100, Mark Brown wrote:

> > No, that's not helpful.  The issue isn't the device probe code itself,
> > the issue is things like module unload not doing what they're supposed
> > to do and leaving the device lying around or something - there's rather
> > more going on than just the plain API call.

> Then lets fix the core code.  I see this pattern show up again and
> again of extra boilerplate going around
> platform_driver_{register,unregister}().  That says to me that there
> either needs to be a new helper, or the core code needs to be made
> more verbose.

I'd go with the latter, it's pretty much the approach the subsystems I
help maintain have been taking.  In fast paths it's a bit different but
in slow paths it tends to be helpful to know why things just fell over.

> In fact, I've been considering adding a macro for
> {platform,i2c,spi,...}_drivers that does all the module boilerplate
> for the common case of only registering a driver at init time.
> Something like:

> #define module_platform_driver(__driver) \
> int __driver##_init(void) \
> { \
> 	return platform_driver_register(&(__driver)); \
> } \
> module_init(__driver##_init); \
> void ##__driver##_exit(void) \
> { \
> 	platform_driver_unregister(&(__driver)); \
> } \
> module_exit(##__driver##_exit);

> It's not a lot of code, but I dislike how much boilerplate every
> single driver has to use if it doesn't do anything special.

Yeah, this sort of stuff would be helpful - there's quite a bit of
boilerplate you end up having to write to get drivers going.
Grant Likely Oct. 4, 2011, 11:22 p.m. UTC | #6 
On Tue, Oct 04, 2011 at 09:50:02PM +0100, Mark Brown wrote:
> On Tue, Oct 04, 2011 at 12:18:18PM -0600, Grant Likely wrote:
> > #define module_platform_driver(__driver) \
> > int __driver##_init(void) \
> > { \
> > 	return platform_driver_register(&(__driver)); \
> > } \
> > module_init(__driver##_init); \
> > void ##__driver##_exit(void) \
> > { \
> > 	platform_driver_unregister(&(__driver)); \
> > } \
> > module_exit(##__driver##_exit);
> 
> > It's not a lot of code, but I dislike how much boilerplate every
> > single driver has to use if it doesn't do anything special.
> 
> Yeah, this sort of stuff would be helpful - there's quite a bit of
> boilerplate you end up having to write to get drivers going.

Draft patch written, and posting to the list soon...

g.
diff mbox

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index ae8820e..a100ea0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7208,6 +7208,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/input/misc/wm831x-on.c
 F:	drivers/input/touchscreen/wm831x-ts.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..bfcbcb5
--- /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_PARENT_RATE_CHANGE;
+}
+
+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_xtal_ops, &clkdata->xtal_hw,
+			  "xtal")) {
+		ret = -EINVAL;
+		goto err_alloc;
+	}
+
+	if (!clk_register(&wm831x_fll_ops, &clkdata->fll_hw,
+			  "fll")) {
+		ret = -EINVAL;
+		goto err_xtal;
+	}
+
+	if (!clk_register(&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");