[PATCH 1/4] clk: fixed-rate: Don't open code kstrdup()

[PATCH 1/4] clk: fixed-rate: Don't open code kstrdup()

[Mark Brown]

Save a little code by using the library function.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk-fixed-rate.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index 90c79fb..cf84e41 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -45,7 +45,6 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 {
 	struct clk_fixed_rate *fixed;
 	char **parent_names = NULL;
-	u8 len;
 
 	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
 
@@ -63,14 +62,10 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 		if (! parent_names)
 			goto out;
 
-		len = sizeof(char) * strlen(parent_name);
-
-		parent_names[0] = kmalloc(len, GFP_KERNEL);
+		parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
 
 		if (!parent_names[0])
 			goto out;
-
-		strncpy(parent_names[0], parent_name, len);
 	}
 
 out:
-- 
1.7.9.1

[PATCH 2/4] clk: Constify parent name arrays

[Mark Brown]

Drivers should be able to declare their arrays of parent names as const
so the APIs need to accept const arguments.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/clk/clk-fixed-rate.c |    2 +-
 drivers/clk/clk-mux.c        |    2 +-
 drivers/clk/clk.c            |    2 +-
 include/linux/clk-private.h  |    2 +-
 include/linux/clk-provider.h |    6 +++---
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
index cf84e41..16bf679 100644
--- a/drivers/clk/clk-fixed-rate.c
+++ b/drivers/clk/clk-fixed-rate.c
@@ -44,7 +44,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
 		unsigned long fixed_rate)
 {
 	struct clk_fixed_rate *fixed;
-	char **parent_names = NULL;
+	const char **parent_names = NULL;
 
 	fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
 
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index c71ad1f..bf9f42e 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -91,7 +91,7 @@ struct clk_ops clk_mux_ops = {
 EXPORT_SYMBOL_GPL(clk_mux_ops);
 
 struct clk *clk_register_mux(struct device *dev, const char *name,
-		char **parent_names, u8 num_parents, unsigned long flags,
+		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock)
 {
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 9cf6f59..33ef0df 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1322,7 +1322,7 @@ out:
  */
 struct clk *clk_register(struct device *dev, const char *name,
 		const struct clk_ops *ops, struct clk_hw *hw,
-		char **parent_names, u8 num_parents, unsigned long flags)
+		const char **parent_names, u8 num_parents, unsigned long flags)
 {
 	struct clk *clk;
 
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index 5e4312b..164e115 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -30,7 +30,7 @@ struct clk {
 	const struct clk_ops	*ops;
 	struct clk_hw		*hw;
 	struct clk		*parent;
-	char			**parent_names;
+	const char		**parent_names;
 	struct clk		**parents;
 	u8			num_parents;
 	unsigned long		rate;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5508897..340acbc 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -212,7 +212,7 @@ struct clk_divider {
 	u8		width;
 	u8		flags;
 	spinlock_t	*lock;
-	char		*parent[1];
+	const char	*parent[1];
 };
 
 #define CLK_DIVIDER_ONE_BASED		BIT(0)
@@ -253,7 +253,7 @@ struct clk_mux {
 #define CLK_MUX_INDEX_BIT		BIT(1)
 
 struct clk *clk_register_mux(struct device *dev, const char *name,
-		char **parent_names, u8 num_parents, unsigned long flags,
+		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock);
 
@@ -274,7 +274,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
  */
 struct clk *clk_register(struct device *dev, const char *name,
 		const struct clk_ops *ops, struct clk_hw *hw,
-		char **parent_names, u8 num_parents, unsigned long flags);
+		const char **parent_names, u8 num_parents, unsigned long flags);
 
 /* helper functions */
 const char *__clk_get_name(struct clk *clk);
-- 
1.7.9.1

[PATCH 3/4] clk: Provide dummy clk_unregister()

[Mark Brown]

While there's no actual implementation behind it having the call to use
in drivers makes them feel neater from a driver author point of view. An
actual implementation can wait for someone who needs to use the function
in a real system.

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

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 33ef0df..31419ca 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1344,6 +1344,18 @@ struct clk *clk_register(struct device *dev, const char *name,
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
+/**
+ * clk_unregister - unregister a currently registered clock
+ * @clk: clock to unregister
+ *
+ * Currently unimplemented.
+ */
+int clk_unregister(struct clk *clk)
+{
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(clk_unregister);
+
 /***        clk rate change notifiers        ***/
 
 /**
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 340acbc..57c0bd1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -275,6 +275,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 struct clk *clk_register(struct device *dev, const char *name,
 		const struct clk_ops *ops, struct clk_hw *hw,
 		const char **parent_names, u8 num_parents, unsigned long flags);
+int clk_unregister(struct clk *clk);
 
 /* helper functions */
 const char *__clk_get_name(struct clk *clk);
-- 
1.7.9.1

[PATCH 3/4] clk: Provide dummy clk_unregister()

[Turquette, Mike]

On Wed, Mar 21, 2012 at 1:01 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> While there's no actual implementation behind it having the call to use
> in drivers makes them feel neater from a driver author point of view. An
> actual implementation can wait for someone who needs to use the function
> in a real system.

Thanks for the patch Mark.  I'll include this in my fixes branch.

Regards,
Mike

> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
> ?drivers/clk/clk.c ? ? ? ? ? ?| ? 12 ++++++++++++
> ?include/linux/clk-provider.h | ? ?1 +
> ?2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 33ef0df..31419ca 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1344,6 +1344,18 @@ struct clk *clk_register(struct device *dev, const char *name,
> ?}
> ?EXPORT_SYMBOL_GPL(clk_register);
>
> +/**
> + * clk_unregister - unregister a currently registered clock
> + * @clk: clock to unregister
> + *
> + * Currently unimplemented.
> + */
> +int clk_unregister(struct clk *clk)
> +{
> + ? ? ? return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(clk_unregister);
> +
> ?/*** ? ? ? ?clk rate change notifiers ? ? ? ?***/
>
> ?/**
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 340acbc..57c0bd1 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -275,6 +275,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
> ?struct clk *clk_register(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?const struct clk_ops *ops, struct clk_hw *hw,
> ? ? ? ? ? ? ? ?const char **parent_names, u8 num_parents, unsigned long flags);
> +int clk_unregister(struct clk *clk);
>
> ?/* helper functions */
> ?const char *__clk_get_name(struct clk *clk);
> --
> 1.7.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/4] clk: wm831x: 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.

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      |    7 +
 drivers/clk/Makefile     |    1 +
 drivers/clk/clk-wm831x.c |  412 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 421 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clk/clk-wm831x.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 19b25e7..bf8ddc3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7475,6 +7475,7 @@ W:	http://opensource.wolfsonmicro.com/content/linux-drivers-wolfson-devices
 S:	Supported
 F:	Documentation/hwmon/wm83??
 F:	arch/arm/mach-s3c64xx/mach-crag6410*
+F:	drivers/clk/clk-wm83*.c
 F:	drivers/leds/leds-wm83*.c
 F:	drivers/hwmon/wm83??-hwmon.c
 F:	drivers/input/misc/wm831x-on.c
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 165e1fe..367ba74 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -44,4 +44,11 @@ config COMMON_CLK_DEBUG
 	  clk_flags, clk_prepare_count, clk_enable_count &
 	  clk_notifier_count.
 
+config COMMON_CLK_WM831X
+	tristate "Clock driver for WM831x/2x PMICs"
+	depends on MFD_WM831X
+	---help---
+          Supports the clocking subsystem of the WM831x/2x series of
+	  PMICs from Wolfson Microlectronics.
+
 endmenu
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 1f736bc..a155102 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
 				   clk-mux.o clk-divider.o
+obj-$(CONFIG_COMMON_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..24efd31
--- /dev/null
+++ b/drivers/clk/clk-wm831x.c
@@ -0,0 +1,412 @@
+/*
+ * WM831x clock control
+ *
+ * Copyright 2011-2 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/clk-provider.h>
+#include <linux/delay.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_is_enabled(struct clk_hw *hw)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  xtal_hw);
+
+	return clkdata->xtal_ena;
+}
+
+static unsigned long wm831x_xtal_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  xtal_hw);
+
+	if (clkdata->xtal_ena)
+		return 32768;
+	else
+		return 0;
+}
+
+static const struct clk_ops wm831x_xtal_ops = {
+	.is_enabled = wm831x_xtal_is_enabled,
+	.recalc_rate = wm831x_xtal_recalc_rate,
+};
+
+static const unsigned long wm831x_fll_auto_rates[] = {
+	 2048000,
+	11289600,
+	12000000,
+	12288000,
+	19200000,
+	22579600,
+	24000000,
+	24576000,
+};
+
+static int wm831x_fll_is_enabled(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_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) != 0;
+}
+
+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);
+
+	udelay_range(2000, 2000);
+
+	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,
+					    unsigned long parent_rate)
+{
+	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 long wm831x_fll_round_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *unused)
+{
+	int best = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(wm831x_fll_auto_rates); i++)
+		if (abs(wm831x_fll_auto_rates[i] - rate) <
+		    abs(wm831x_fll_auto_rates[best] - rate))
+			best = i;
+
+	return wm831x_fll_auto_rates[best];
+}
+
+static int wm831x_fll_set_rate(struct clk_hw *hw, unsigned long 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_is_enabled(hw))
+		return -EPERM;
+
+	return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_2,
+			       WM831X_FLL_AUTO_FREQ_MASK, i);
+}
+
+static const char *wm831x_fll_parents[] = {
+	"xtal",
+	"clkin",
+};
+
+static u8 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 0;
+	}
+
+	if (ret & WM831X_FLL_AUTO)
+		return 0;
+
+	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 0;
+	}
+
+	switch (ret & WM831X_FLL_CLK_SRC_MASK) {
+	case 0:
+		return 0;
+	case 1:
+		return 1;
+	default:
+		dev_err(wm831x->dev, "Unsupported FLL clock source %d\n",
+			ret & WM831X_FLL_CLK_SRC_MASK);
+		return 0;
+	}
+}
+
+static const struct clk_ops wm831x_fll_ops = {
+	.is_enabled = wm831x_fll_is_enabled,
+	.prepare = wm831x_fll_prepare,
+	.unprepare = wm831x_fll_unprepare,
+	.round_rate = wm831x_fll_round_rate,
+	.recalc_rate = wm831x_fll_recalc_rate,
+	.set_rate = wm831x_fll_set_rate,
+	.get_parent = wm831x_fll_get_parent,
+};
+
+static int wm831x_clkout_is_enabled(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 true;
+	}
+
+	return (ret & WM831X_CLKOUT_ENA) != 0;
+}
+
+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,
+					       unsigned long parent_rate)
+{
+	return parent_rate;
+}
+
+static const char *wm831x_clkout_parents[] = {
+	"xtal",
+	"fll",
+};
+
+static u8 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 0;
+	}
+
+	if (ret & WM831X_CLKOUT_SRC)
+		return 0;
+	else
+		return 1;
+}
+
+static int wm831x_clkout_set_parent(struct clk_hw *hw, u8 parent)
+{
+	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
+						  clkout_hw);
+	struct wm831x *wm831x = clkdata->wm831x;
+
+	if (parent > ARRAY_SIZE(wm831x_clkout_parents))
+		return -EINVAL;
+
+	return wm831x_set_bits(wm831x, WM831X_CLOCK_CONTROL_1,
+			       WM831X_CLKOUT_SRC,
+			       parent << WM831X_CLKOUT_SRC_SHIFT);
+}
+
+static const struct clk_ops wm831x_clkout_ops = {
+	.is_enabled = wm831x_clkout_is_enabled,
+	.prepare = wm831x_clkout_prepare,
+	.unprepare = wm831x_clkout_unprepare,
+	.recalc_rate = wm831x_clkout_recalc_rate,
+	.get_parent = wm831x_clkout_get_parent,
+	.set_parent = wm831x_clkout_set_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 = devm_kzalloc(&pdev->dev, 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);
+		return ret;
+	}
+	clkdata->xtal_ena = ret & WM831X_XTAL_ENA;
+
+	if (!clk_register(&pdev->dev, "xtal", &wm831x_xtal_ops,
+			  &clkdata->xtal_hw, NULL, 0, CLK_IS_ROOT))
+		return -EINVAL;
+
+	if (!clk_register(&pdev->dev, "fll", &wm831x_fll_ops, &clkdata->fll_hw,
+			  wm831x_fll_parents, ARRAY_SIZE(wm831x_fll_parents),
+			  0)) {
+		ret = -EINVAL;
+		goto err_xtal;
+	}
+
+	if (!clk_register(&pdev->dev, "clkout", &wm831x_clkout_ops,
+			  &clkdata->clkout_hw, wm831x_clkout_parents,
+			  ARRAY_SIZE(wm831x_clkout_parents),
+			  CLK_SET_RATE_PARENT)) {
+		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);
+	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);
+
+	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,
+	},
+};
+
+module_platform_driver(wm831x_clk_driver);
+
+/* 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.9.1

[PATCH 4/4] clk: wm831x: Add initial WM831x clock driver

[Sascha Hauer]

On Wed, Mar 21, 2012 at 08:01:22PM +0000, Mark Brown wrote:
> 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.
> 
> 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>
> ---

[...]

> +
> +static unsigned long wm831x_clkout_recalc_rate(struct clk_hw *hw,
> +					       unsigned long parent_rate)
> +{
> +	return parent_rate;
> +}

You should be able to drop this. The clock framework will use the parent
rate automatically if recalc_rate is not present.

> +
> +static int wm831x_clkout_set_parent(struct clk_hw *hw, u8 parent)
> +{
> +	struct wm831x_clk *clkdata = container_of(hw, struct wm831x_clk,
> +						  clkout_hw);
> +	struct wm831x *wm831x = clkdata->wm831x;
> +
> +	if (parent > ARRAY_SIZE(wm831x_clkout_parents))
> +		return -EINVAL;

Unless you mistrust the clock framework isn't necessary. The framework
checks this already.

> +
> +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 = devm_kzalloc(&pdev->dev, 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);
> +		return ret;
> +	}
> +	clkdata->xtal_ena = ret & WM831X_XTAL_ENA;
> +
> +	if (!clk_register(&pdev->dev, "xtal", &wm831x_xtal_ops,
> +			  &clkdata->xtal_hw, NULL, 0, CLK_IS_ROOT))
> +		return -EINVAL;

The clock names are unique identifiers for the clock, so clocks in
drivers should probably have dev_name encoded into them.

You could also use the fixed rate generic clock here.

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 4/4] clk: wm831x: Add initial WM831x clock driver

[Mark Brown]

On Wed, Mar 21, 2012 at 11:26:22PM +0100, Sascha Hauer wrote:
> On Wed, Mar 21, 2012 at 08:01:22PM +0000, Mark Brown wrote:

> > +	if (!clk_register(&pdev->dev, "xtal", &wm831x_xtal_ops,
> > +			  &clkdata->xtal_hw, NULL, 0, CLK_IS_ROOT))
> > +		return -EINVAL;

> The clock names are unique identifiers for the clock, so clocks in
> drivers should probably have dev_name encoded into them.

No, that's not sensible.  We shouldn't be open coding this into each
individual driver that provides clocks, and we shouldn't have clock
users having to guess at what scheme the driver author used to dedupe
the clocks.  As a driver author you would assume that the reason we're
providing the struct device to the registration function in the first
place is so that the core has the information it needs to do that.

I did provide patches to do what you suggest in the core for one of the
earlier versions of the API, I have to say I didn't check to see if they
got dropped during the general lulls.

> You could also use the fixed rate generic clock here.

It doesn't really do what I want with reporting the enables - for
diagnostic purposes I wanted to always have the clock present but
non-enablable so we could actually look at the clock tree and see 
what's going on directly when there's no clock coming out.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120322/c06289f3/attachment.sig>

[PATCH 4/4] clk: wm831x: Add initial WM831x clock driver

[Sascha Hauer]

On Thu, Mar 22, 2012 at 11:15:31AM +0000, Mark Brown wrote:
> On Wed, Mar 21, 2012 at 11:26:22PM +0100, Sascha Hauer wrote:
> > On Wed, Mar 21, 2012 at 08:01:22PM +0000, Mark Brown wrote:
> 
> > > +	if (!clk_register(&pdev->dev, "xtal", &wm831x_xtal_ops,
> > > +			  &clkdata->xtal_hw, NULL, 0, CLK_IS_ROOT))
> > > +		return -EINVAL;
> 
> > The clock names are unique identifiers for the clock, so clocks in
> > drivers should probably have dev_name encoded into them.
> 
> No, that's not sensible.  We shouldn't be open coding this into each
> individual driver that provides clocks, and we shouldn't have clock
> users having to guess at what scheme the driver author used to dedupe
> the clocks.  As a driver author you would assume that the reason we're
> providing the struct device to the registration function in the first
> place is so that the core has the information it needs to do that.
> 
> I did provide patches to do what you suggest in the core for one of the
> earlier versions of the API, I have to say I didn't check to see if they
> got dropped during the general lulls.

It seems they got dropped. Currently the clock framework does nothing
with the device argument. But right, it could use this argument to
generate a suitable name.

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 4/4] clk: wm831x: Add initial WM831x clock driver

[Mark Brown]

On Thu, Mar 22, 2012 at 12:26:13PM +0100, Sascha Hauer wrote:
> On Thu, Mar 22, 2012 at 11:15:31AM +0000, Mark Brown wrote:

> > No, that's not sensible.  We shouldn't be open coding this into each
> > individual driver that provides clocks, and we shouldn't have clock
> > users having to guess at what scheme the driver author used to dedupe
> > the clocks.  As a driver author you would assume that the reason we're

> It seems they got dropped. Currently the clock framework does nothing
> with the device argument. But right, it could use this argument to
> generate a suitable name.

Indeed, and as with adding the stub clk_unregister() it seems like it's
better at this point to just code the drivers as we'd expect them to
work and fill out the framework to handle this rather than introducing
workarounds in the drivers.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120322/5d75411a/attachment.sig>

[PATCH 4/4] clk: wm831x: Add initial WM831x clock driver

[Turquette, Mike]

On Thu, Mar 22, 2012 at 4:33 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Mar 22, 2012 at 12:26:13PM +0100, Sascha Hauer wrote:
>> On Thu, Mar 22, 2012 at 11:15:31AM +0000, Mark Brown wrote:
>
>> > No, that's not sensible. ?We shouldn't be open coding this into each
>> > individual driver that provides clocks, and we shouldn't have clock
>> > users having to guess at what scheme the driver author used to dedupe
>> > the clocks. ?As a driver author you would assume that the reason we're
>
>> It seems they got dropped. Currently the clock framework does nothing
>> with the device argument. But right, it could use this argument to
>> generate a suitable name.
>
> Indeed, and as with adding the stub clk_unregister() it seems like it's
> better at this point to just code the drivers as we'd expect them to
> work and fill out the framework to handle this rather than introducing
> workarounds in the drivers.

Agreed.  I put this off some time back, but I figured the WM831X
driver would bring the issue back up.

I'm happy for the core to concatenate the strings when a struct device
*dev is passed in.  However the reason I dropped this in the first
place is that I have some ideas on providing something like clk_get
directly from the common clk core and I hadn't figured out all of the
details yet.  For now we can handle the string concatenation in the
core and figure out those tricky details if/when the time comes to
provide a clk_get which is more closely linked to the clock framework
implementation.

I'll roll in a patch for my fixes series.

Regards,
Mike

[PATCH 4/4] clk: wm831x: Add initial WM831x clock driver

[Mark Brown]

On Thu, Mar 22, 2012 at 11:35:29AM -0700, Turquette, Mike wrote:

> I'm happy for the core to concatenate the strings when a struct device
> *dev is passed in.  However the reason I dropped this in the first
> place is that I have some ideas on providing something like clk_get
> directly from the common clk core and I hadn't figured out all of the
> details yet.  For now we can handle the string concatenation in the
> core and figure out those tricky details if/when the time comes to
> provide a clk_get which is more closely linked to the clock framework
> implementation.

Yeah, it's that sort of feature that most pushes against doing it in the
drivers - for example, the regulator API never ends up doing anything
like the concatenation.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120322/cdebbec9/attachment.sig>

[PATCH 1/4] clk: fixed-rate: Don't open code kstrdup()

[Turquette, Mike]

On Wed, Mar 21, 2012 at 1:01 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> Save a little code by using the library function.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Hi Mark,

Thanks for the patch.  I've already solved this in my clk-fixes branch
(working on it now, not yet on the list).  I'm going to re-use the
same approach for the fixed-rate clock as is used for the divider and
gate; the registration functions for those types store the parent_name
in the struct clk_foo and use kstrdup as your patch does below.

Thanks again,
Mike

> ---
> ?drivers/clk/clk-fixed-rate.c | ? ?7 +------
> ?1 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c
> index 90c79fb..cf84e41 100644
> --- a/drivers/clk/clk-fixed-rate.c
> +++ b/drivers/clk/clk-fixed-rate.c
> @@ -45,7 +45,6 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ?{
> ? ? ? ?struct clk_fixed_rate *fixed;
> ? ? ? ?char **parent_names = NULL;
> - ? ? ? u8 len;
>
> ? ? ? ?fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL);
>
> @@ -63,14 +62,10 @@ struct clk *clk_register_fixed_rate(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?if (! parent_names)
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
>
> - ? ? ? ? ? ? ? len = sizeof(char) * strlen(parent_name);
> -
> - ? ? ? ? ? ? ? parent_names[0] = kmalloc(len, GFP_KERNEL);
> + ? ? ? ? ? ? ? parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
>
> ? ? ? ? ? ? ? ?if (!parent_names[0])
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
> -
> - ? ? ? ? ? ? ? strncpy(parent_names[0], parent_name, len);
> ? ? ? ?}
>
> ?out:
> --
> 1.7.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel