Hi all,
These five patches are hopefully the final set of core framework changes for 3.5. There is the obligatory MAINTAINERS file change, three new fixes and the fixed-factor clock patch. That last patch is being reposted since three bugs were found in it (one on the list, two in my testing).
If no one complains about these then I'll commit them to clk-next and (finally) send my pull request to Arnd.
Note that the Orion patches aren't here, but I figure that Andrew L. probably wants to check those against the final clk-next before I pull them.
Regards, Mike
Mike Turquette (4): MAINTAINERS: add entry for common clk framework clk: prevent spurious parent rate propagation clk: remove COMMON_CLK_DISABLE_UNUSED clk: mux: assign init data
Sascha Hauer (1): clk: add a fixed factor clock
MAINTAINERS | 10 ++++ drivers/clk/Kconfig | 11 ----- drivers/clk/Makefile | 2 +- drivers/clk/clk-fixed-factor.c | 95 ++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk-mux.c | 1 + drivers/clk/clk.c | 9 +++- include/linux/clk-private.h | 20 ++++++++ include/linux/clk-provider.h | 23 ++++++++++ 8 files changed, 156 insertions(+), 15 deletions(-) create mode 100644 drivers/clk/clk-fixed-factor.c
Signed-off-by: Mike Turquette mturquette@linaro.org --- MAINTAINERS | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS index 1a2f8f5..164e9a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1882,6 +1882,16 @@ F: Documentation/filesystems/coda.txt F: fs/coda/ F: include/linux/coda*.h
+COMMON CLK FRAMEWORK +M: Mike Turquette mturquette@ti.com +M: Mike Turquette mturquette@linaro.org +L: linux-arm-kernel@lists.infradead.org (same as CLK API & CLKDEV) +T: git git://git.linaro.org/people/mturquette/linux.git +S: Maintained +F: drivers/clk/clk.c +F: drivers/clk/clk-* +F: include/linux/clk-pr* + COMMON INTERNET FILE SYSTEM (CIFS) M: Steve French sfrench@samba.org L: linux-cifs@vger.kernel.org
Patch 'clk: always pass parent_rate into .round_rate' made a subtle change to the semantics of .round_rate. It is now expected for the parent's rate to always be passed in, simplifying the implemenation of various .round_rate callback definitions.
However the patch also introduced a bug in clk_calc_new_rates whereby a clock without the CLK_SET_RATE_PARENT flag set could still propagate a rate change up to a parent clock if the the .round_rate callback modified the &best_parent_rate value in any way.
This patch fixes the issue at the framework level (in clk_calc_new_rates) by specifically handling the case where the CLK_SET_RATE_PARENT flag is not set.
Signed-off-by: Mike Turquette mturquette@linaro.org Reported-by: Sascha Hauers.hauer@pengutronix.de --- drivers/clk/clk.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 8149764..7ceca0e 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -774,12 +774,18 @@ static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) if (IS_ERR_OR_NULL(clk)) return NULL;
+ /* save parent rate, if it exists */ + if (clk->parent) + best_parent_rate = clk->parent->rate; + /* never propagate up to the parent */ if (!(clk->flags & CLK_SET_RATE_PARENT)) { if (!clk->ops->round_rate) { clk->new_rate = clk->rate; return NULL; } + new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate); + goto out; }
/* need clk->parent from here on out */ @@ -795,7 +801,6 @@ static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) goto out; }
- best_parent_rate = clk->parent->rate; new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate);
if (best_parent_rate != clk->parent->rate) {
On Sun, May 06, 2012 at 10:08:27PM -0700, Mike Turquette wrote:
Patch 'clk: always pass parent_rate into .round_rate' made a subtle change to the semantics of .round_rate. It is now expected for the parent's rate to always be passed in, simplifying the implemenation of various .round_rate callback definitions.
However the patch also introduced a bug in clk_calc_new_rates whereby a clock without the CLK_SET_RATE_PARENT flag set could still propagate a rate change up to a parent clock if the the .round_rate callback modified the &best_parent_rate value in any way.
This patch fixes the issue at the framework level (in clk_calc_new_rates) by specifically handling the case where the CLK_SET_RATE_PARENT flag is not set.
Signed-off-by: Mike Turquette mturquette@linaro.org Reported-by: Sascha Hauers.hauer@pengutronix.de
I think a warning might be useful when a clk driver tries to change the parent rate even if it is not allowed to. This can be added in a later patch, so
Acked-by: Sascha Hauer s.hauer@pengutronix.de
Sascha
drivers/clk/clk.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 8149764..7ceca0e 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -774,12 +774,18 @@ static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) if (IS_ERR_OR_NULL(clk)) return NULL;
- /* save parent rate, if it exists */
- if (clk->parent)
best_parent_rate = clk->parent->rate;- /* never propagate up to the parent */ if (!(clk->flags & CLK_SET_RATE_PARENT)) { if (!clk->ops->round_rate) { clk->new_rate = clk->rate; return NULL; }
new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate);goto out;/* need clk->parent from here on out */ @@ -795,7 +801,6 @@ static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) goto out; }
- best_parent_rate = clk->parent->rate; new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate);
if (best_parent_rate != clk->parent->rate) { -- 1.7.5.4
On Mon, May 7, 2012 at 12:58 AM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Sun, May 06, 2012 at 10:08:27PM -0700, Mike Turquette wrote:
Patch 'clk: always pass parent_rate into .round_rate' made a subtle change to the semantics of .round_rate. It is now expected for the parent's rate to always be passed in, simplifying the implemenation of various .round_rate callback definitions.
However the patch also introduced a bug in clk_calc_new_rates whereby a clock without the CLK_SET_RATE_PARENT flag set could still propagate a rate change up to a parent clock if the the .round_rate callback modified the &best_parent_rate value in any way.
This patch fixes the issue at the framework level (in clk_calc_new_rates) by specifically handling the case where the CLK_SET_RATE_PARENT flag is not set.
Signed-off-by: Mike Turquette mturquette@linaro.org Reported-by: Sascha Hauers.hauer@pengutronix.de
I think a warning might be useful when a clk driver tries to change the parent rate even if it is not allowed to. This can be added in a later patch, so
A fine idea.
Acked-by: Sascha Hauer s.hauer@pengutronix.de
Thanks, Mike
Exposing this option generates confusion and incorrect behavior for single-image builds across platforms. Enable this behavior permanently.
Signed-off-by: Mike Turquette mturquette@linaro.org --- drivers/clk/Kconfig | 11 ----------- drivers/clk/clk.c | 2 -- 2 files changed, 0 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index f05a60d..4864407 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -23,17 +23,6 @@ config COMMON_CLK menu "Common Clock Framework" depends on COMMON_CLK
-config COMMON_CLK_DISABLE_UNUSED - bool "Disabled unused clocks at boot" - depends on COMMON_CLK - ---help--- - Traverses the entire clock tree and disables any clocks that are - enabled in hardware but have not been enabled by any device drivers. - This saves power and keeps the software model of the clock in line - with reality. - - If in doubt, say "N". - config COMMON_CLK_DEBUG bool "DebugFS representation of clock tree" depends on COMMON_CLK diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 7ceca0e..e5d5dc1 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -196,7 +196,6 @@ late_initcall(clk_debug_init); static inline int clk_debug_register(struct clk *clk) { return 0; } #endif
-#ifdef CONFIG_COMMON_CLK_DISABLE_UNUSED /* caller must hold prepare_lock */ static void clk_disable_unused_subtree(struct clk *clk) { @@ -246,7 +245,6 @@ static int clk_disable_unused(void) return 0; } late_initcall(clk_disable_unused); -#endif
/*** helper functions ***/
On 05/06/2012 10:08 PM, Mike Turquette wrote:
Exposing this option generates confusion and incorrect behavior for single-image builds across platforms. Enable this behavior permanently.
Signed-off-by: Mike Turquettemturquette@linaro.org
drivers/clk/Kconfig | 11 ----------- drivers/clk/clk.c | 2 -- 2 files changed, 0 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index f05a60d..4864407 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -23,17 +23,6 @@ config COMMON_CLK menu "Common Clock Framework" depends on COMMON_CLK
-config COMMON_CLK_DISABLE_UNUSED
- bool "Disabled unused clocks at boot"
- depends on COMMON_CLK
- ---help---
Traverses the entire clock tree and disables any clocks that areenabled in hardware but have not been enabled by any device drivers.This saves power and keeps the software model of the clock in linewith reality.If in doubt, say "N".- config COMMON_CLK_DEBUG bool "DebugFS representation of clock tree" depends on COMMON_CLK
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 7ceca0e..e5d5dc1 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -196,7 +196,6 @@ late_initcall(clk_debug_init); static inline int clk_debug_register(struct clk *clk) { return 0; } #endif
-#ifdef CONFIG_COMMON_CLK_DISABLE_UNUSED /* caller must hold prepare_lock */ static void clk_disable_unused_subtree(struct clk *clk) { @@ -246,7 +245,6 @@ static int clk_disable_unused(void) return 0; } late_initcall(clk_disable_unused); -#endif
/*** helper functions ***/
Good decision! Acked.
-Saravana
The original conversion to struct clk_hw_init failed to add the pointer assignment in clk_register_mux.
Signed-off-by: Mike Turquette mturquette@linaro.org Reported-by: Sascha Hauer s.hauer@pengutronix.de --- drivers/clk/clk-mux.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 8e97491..fd36a8e 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -116,6 +116,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, mux->width = width; mux->flags = clk_mux_flags; mux->lock = lock; + mux->hw.init = &init;
clk = clk_register(dev, &mux->hw);
From: Sascha Hauer s.hauer@pengutronix.de
Having fixed factors/dividers in hardware is a common pattern, so add a basic clock type doing this. It basically describes a fixed factor clock using a nominator and a denominator.
Signed-off-by: Sascha Hauer s.hauer@pengutronix.de Reviewed-by: Viresh Kumar viresh.kumar@st.com [mturquette@linaro.org: constify parent_names in static init macro] [mturquette@linaro.org: copy/paste bug from mux in static init macro] [mturquette@linaro.org: fix error handling in clk_register_fixed_factor] Signed-off-by: Mike Turquette mturquette@linaro.org --- drivers/clk/Makefile | 2 +- drivers/clk/clk-fixed-factor.c | 95 ++++++++++++++++++++++++++++++++++++++++ include/linux/clk-private.h | 20 ++++++++ include/linux/clk-provider.h | 23 ++++++++++ 4 files changed, 139 insertions(+), 1 deletions(-) create mode 100644 drivers/clk/clk-fixed-factor.c
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 1f736bc..24aa714 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -1,4 +1,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 + clk-mux.o clk-divider.o clk-fixed-factor.o diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c new file mode 100644 index 0000000..1f2da01 --- /dev/null +++ b/drivers/clk/clk-fixed-factor.c @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2011 Sascha Hauer, Pengutronix s.hauer@pengutronix.de + * + * 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/module.h> +#include <linux/clk-provider.h> +#include <linux/slab.h> +#include <linux/err.h> + +/* + * DOC: basic fixed multiplier and divider clock that cannot gate + * + * Traits of this clock: + * prepare - clk_prepare only ensures that parents are prepared + * enable - clk_enable only ensures that parents are enabled + * rate - rate is fixed. clk->rate = parent->rate / div * mult + * parent - fixed parent. No clk_set_parent support + */ + +#define to_clk_fixed_factor(_hw) container_of(_hw, struct clk_fixed_factor, hw) + +static unsigned long clk_factor_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_fixed_factor *fix = to_clk_fixed_factor(hw); + + return (parent_rate / fix->div) * fix->mult; +} + +static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + struct clk_fixed_factor *fix = to_clk_fixed_factor(hw); + + if (__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT) { + unsigned long best_parent; + + best_parent = (rate / fix->mult) * fix->div; + *prate = __clk_round_rate(__clk_get_parent(hw->clk), + best_parent); + } + + return (*prate / fix->div) * fix->mult; +} + +static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + return 0; +} + +struct clk_ops clk_fixed_factor_ops = { + .round_rate = clk_factor_round_rate, + .set_rate = clk_factor_set_rate, + .recalc_rate = clk_factor_recalc_rate, +}; +EXPORT_SYMBOL_GPL(clk_fixed_factor_ops); + +struct clk *clk_register_fixed_factor(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + unsigned int mult, unsigned int div) +{ + struct clk_fixed_factor *fix; + struct clk_init_data init; + struct clk *clk; + + fix = kmalloc(sizeof(*fix), GFP_KERNEL); + if (!fix) { + pr_err("%s: could not allocate fixed factor clk\n", __func__); + return ERR_PTR(-ENOMEM); + } + + /* struct clk_fixed_factor assignments */ + fix->mult = mult; + fix->div = div; + fix->hw.init = &init; + + init.name = name; + init.ops = &clk_fixed_factor_ops; + init.flags = flags; + init.parent_names = &parent_name; + init.num_parents = 1; + + clk = clk_register(dev, &fix->hw); + + if (IS_ERR(clk)) + kfree(fix); + + return clk; +} diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index b258532..eb3f84b 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -143,6 +143,26 @@ struct clk { DEFINE_CLK(_name, clk_mux_ops, _flags, _parent_names, \ _parents);
+#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, \ + _parent_ptr, _flags, \ + _mult, _div) \ + static struct clk _name; \ + static const char *_name##_parent_names[] = { \ + _parent_name, \ + }; \ + static struct clk *_name##_parents[] = { \ + _parent_ptr, \ + }; \ + static struct clk_fixed_factor _name##_hw = { \ + .hw = { \ + .clk = &_name, \ + }, \ + .mult = _mult, \ + .div = _div, \ + }; \ + DEFINE_CLK(_name, clk_fixed_factor_ops, _flags, \ + _name##_parent_names, _name##_parents); + /** * __clk_init - initialize the data structures in a struct clk * @dev: device initializing this clk, placeholder for now diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 5db3412..c1c23b9 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -277,6 +277,29 @@ struct clk *clk_register_mux(struct device *dev, const char *name, u8 clk_mux_flags, spinlock_t *lock);
/** + * struct clk_fixed_factor - fixed multiplier and divider clock + * + * @hw: handle between common and hardware-specific interfaces + * @mult: multiplier + * @div: divider + * + * Clock with a fixed multiplier and divider. The output frequency is the + * parent clock rate divided by div and multiplied by mult. + * Implements .recalc_rate, .set_rate and .round_rate + */ + +struct clk_fixed_factor { + struct clk_hw hw; + unsigned int mult; + unsigned int div; +}; + +extern struct clk_ops clk_fixed_factor_ops; +struct clk *clk_register_fixed_factor(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + unsigned int mult, unsigned int div); + +/** * clk_register - allocate a new clock, register it and return an opaque cookie * @dev: device that is registering this clock * @hw: link to hardware-specific clock data
On 05/06/2012 10:08 PM, Mike Turquette wrote:
From: Sascha Hauers.hauer@pengutronix.de
Having fixed factors/dividers in hardware is a common pattern, so add a basic clock type doing this. It basically describes a fixed factor clock using a nominator and a denominator.
Signed-off-by: Sascha Hauers.hauer@pengutronix.de Reviewed-by: Viresh Kumarviresh.kumar@st.com [mturquette@linaro.org: constify parent_names in static init macro] [mturquette@linaro.org: copy/paste bug from mux in static init macro] [mturquette@linaro.org: fix error handling in clk_register_fixed_factor] Signed-off-by: Mike Turquettemturquette@linaro.org
drivers/clk/Makefile | 2 +- drivers/clk/clk-fixed-factor.c | 95 ++++++++++++++++++++++++++++++++++++++++ include/linux/clk-private.h | 20 ++++++++ include/linux/clk-provider.h | 23 ++++++++++ 4 files changed, 139 insertions(+), 1 deletions(-) create mode 100644 drivers/clk/clk-fixed-factor.c
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 1f736bc..24aa714 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -1,4 +1,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
clk-mux.o clk-divider.o clk-fixed-factor.odiff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c new file mode 100644 index 0000000..1f2da01 --- /dev/null +++ b/drivers/clk/clk-fixed-factor.c @@ -0,0 +1,95 @@ +/*
- Copyright (C) 2011 Sascha Hauer, Pengutronixs.hauer@pengutronix.de
- 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/module.h> +#include<linux/clk-provider.h> +#include<linux/slab.h> +#include<linux/err.h>
+/*
- DOC: basic fixed multiplier and divider clock that cannot gate
- Traits of this clock:
- prepare - clk_prepare only ensures that parents are prepared
- enable - clk_enable only ensures that parents are enabled
- rate - rate is fixed. clk->rate = parent->rate / div * mult
- parent - fixed parent. No clk_set_parent support
- */
+#define to_clk_fixed_factor(_hw) container_of(_hw, struct clk_fixed_factor, hw)
+static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)+{
- struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
- return (parent_rate / fix->div) * fix->mult;
Wouldn't multiplying and then dividing result in better accuracy? Is it worth doing that (since we will have to account for overflow)?
+}
+static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)+{
- struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
- if (__clk_get_flags(hw->clk)& CLK_SET_RATE_PARENT) {
unsigned long best_parent;best_parent = (rate / fix->mult) * fix->div;*prate = __clk_round_rate(__clk_get_parent(hw->clk),best_parent);- }
- return (*prate / fix->div) * fix->mult;
+}
+static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)+{
- return 0;
+}
+struct clk_ops clk_fixed_factor_ops = {
- .round_rate = clk_factor_round_rate,
- .set_rate = clk_factor_set_rate,
- .recalc_rate = clk_factor_recalc_rate,
+}; +EXPORT_SYMBOL_GPL(clk_fixed_factor_ops);
+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,unsigned int mult, unsigned int div)+{
- struct clk_fixed_factor *fix;
- struct clk_init_data init;
- struct clk *clk;
- fix = kmalloc(sizeof(*fix), GFP_KERNEL);
- if (!fix) {
pr_err("%s: could not allocate fixed factor clk\n", __func__);return ERR_PTR(-ENOMEM);- }
Can we add a check for mult <= div? It doesn't look like this clock is meant to capture clock multipliers (if there is even anything like that other than PLLs).
- /* struct clk_fixed_factor assignments */
- fix->mult = mult;
- fix->div = div;
- fix->hw.init =&init;
- init.name = name;
- init.ops =&clk_fixed_factor_ops;
- init.flags = flags;
- init.parent_names =&parent_name;
- init.num_parents = 1;
- clk = clk_register(dev,&fix->hw);
- if (IS_ERR(clk))
kfree(fix);- return clk;
+} diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index b258532..eb3f84b 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -143,6 +143,26 @@ struct clk { DEFINE_CLK(_name, clk_mux_ops, _flags, _parent_names, \ _parents);
+#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, \
_parent_ptr, _flags, \_mult, _div) \- static struct clk _name; \
- static const char *_name##_parent_names[] = { \
_parent_name, \- }; \
- static struct clk *_name##_parents[] = { \
_parent_ptr, \- }; \
- static struct clk_fixed_factor _name##_hw = { \
.hw = { \.clk =&_name, \}, \.mult = _mult, \.div = _div, \- }; \
- DEFINE_CLK(_name, clk_fixed_factor_ops, _flags, \
_name##_parent_names, _name##_parents);
I would prefer not defining a macro for this. But if we are going to do it, can we please at least stop doing nested macros here? It would be much easier for a new comer to read if we don't nest these clock macros.
Also, should we stop adding support for fully static allocation for new clock types? Since it's supposed to be going away soon. Since Mike didn't add this clock type, I'm guessing he doesn't need the clock type now and hence doesn't need static allocation support for it.
/**
- __clk_init - initialize the data structures in a struct clk
- @dev: device initializing this clk, placeholder for now
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 5db3412..c1c23b9 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -277,6 +277,29 @@ struct clk *clk_register_mux(struct device *dev, const char *name, u8 clk_mux_flags, spinlock_t *lock);
/**
- struct clk_fixed_factor - fixed multiplier and divider clock
- @hw: handle between common and hardware-specific interfaces
- @mult: multiplier
- @div: divider
- Clock with a fixed multiplier and divider. The output frequency is the
- parent clock rate divided by div and multiplied by mult.
- Implements .recalc_rate, .set_rate and .round_rate
- */
+struct clk_fixed_factor {
- struct clk_hw hw;
- unsigned int mult;
- unsigned int div;
+};
+extern struct clk_ops clk_fixed_factor_ops; +struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,unsigned int mult, unsigned int div);
Should we have one header file for each common clock type? We seem to be adding a lot of those (which is good), but it almost feels like clock-provider get out of hand soon.
Thanks, Saravana
On Mon, May 7, 2012 at 12:54 PM, Saravana Kannan skannan@codeaurora.org wrote:
On 05/06/2012 10:08 PM, Mike Turquette wrote:
From: Sascha Hauers.hauer@pengutronix.de +struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
- const char *parent_name, unsigned long flags,
- unsigned int mult, unsigned int div)
+{
- struct clk_fixed_factor *fix;
- struct clk_init_data init;
- struct clk *clk;
- fix = kmalloc(sizeof(*fix), GFP_KERNEL);
- if (!fix) {
- pr_err("%s: could not allocate fixed factor clk\n",
__func__);
- return ERR_PTR(-ENOMEM);
- }
Can we add a check for mult <= div? It doesn't look like this clock is meant to capture clock multipliers (if there is even anything like that other than PLLs).
I don't think we should enforce a rule like that. Some folks with static PLLs that never change their rates (and maybe are set up by the bootloader) might find this clock type to be perfect for them as a multiplier.
<snip>
+#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, \
- _parent_ptr, _flags, \
- _mult, _div) \
- static struct clk _name; \
- static const char *_name##_parent_names[] = { \
- _parent_name, \
- }; \
- static struct clk *_name##_parents[] = { \
- _parent_ptr, \
- }; \
- static struct clk_fixed_factor _name##_hw = { \
- .hw = { \
- .clk =&_name, \
- }, \
- .mult = _mult, \
- .div = _div, \
- }; \
- DEFINE_CLK(_name, clk_fixed_factor_ops, _flags, \
- _name##_parent_names, _name##_parents);
I would prefer not defining a macro for this. But if we are going to do it, can we please at least stop doing nested macros here? It would be much easier for a new comer to read if we don't nest these clock macros.
This macro follows the others in every way. Why should we make it look less uniform?
Also, should we stop adding support for fully static allocation for new clock types? Since it's supposed to be going away soon. Since Mike didn't add this clock type, I'm guessing he doesn't need the clock type now and hence doesn't need static allocation support for it.
I'm afraid I don't follow. I do need this clock in fact (see https://github.com/mturquette/linux/commits/clk-next-may06-omap), and my platform's data is still static.
/** * __clk_init - initialize the data structures in a struct clk * @dev: device initializing this clk, placeholder for now diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 5db3412..c1c23b9 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -277,6 +277,29 @@ struct clk *clk_register_mux(struct device *dev, const char *name, u8 clk_mux_flags, spinlock_t *lock);
/**
- struct clk_fixed_factor - fixed multiplier and divider clock
- @hw: handle between common and hardware-specific
interfaces
- @mult: multiplier
- @div: divider
- Clock with a fixed multiplier and divider. The output frequency is the
- parent clock rate divided by div and multiplied by mult.
- Implements .recalc_rate, .set_rate and .round_rate
- */
+struct clk_fixed_factor {
- struct clk_hw hw;
- unsigned int mult;
- unsigned int div;
+};
+extern struct clk_ops clk_fixed_factor_ops; +struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
- const char *parent_name, unsigned long flags,
- unsigned int mult, unsigned int div);
Should we have one header file for each common clock type? We seem to be adding a lot of those (which is good), but it almost feels like clock-provider get out of hand soon.
I think clk-provider.h is fine for now. It's a good one-stop-shop for people that are just now figuring out what basic clock types exist and applying them to their platform. The file itself is only 336 lines which is hardly out of control...
Regards, Mike
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.
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 05/07/2012 01:14 PM, Turquette, Mike wrote:
On Mon, May 7, 2012 at 12:54 PM, Saravana Kannanskannan@codeaurora.org wrote:
On 05/06/2012 10:08 PM, Mike Turquette wrote:
From: Sascha Hauers.hauer@pengutronix.de +struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,unsigned int mult, unsigned int div)+{
struct clk_fixed_factor *fix;struct clk_init_data init;struct clk *clk;fix = kmalloc(sizeof(*fix), GFP_KERNEL);if (!fix) {pr_err("%s: could not allocate fixed factor clk\n",__func__);
return ERR_PTR(-ENOMEM);}Can we add a check for mult<= div? It doesn't look like this clock is meant to capture clock multipliers (if there is even anything like that other than PLLs).
I don't think we should enforce a rule like that. Some folks with static PLLs that never change their rates (and maybe are set up by the bootloader) might find this clock type to be perfect for them as a multiplier.
I would think those clocks would have some type of register control. At least for enable/disable. This clock just seems to implement a simple integer divider/fractional multiplier. I think we should add this check.
<snip> >> +#define DEFINE_CLK_FIXED_FACTOR(_name, _parent_name, \ >> + _parent_ptr, _flags, \ >> + _mult, _div) \ >> + static struct clk _name; \ >> + static const char *_name##_parent_names[] = { \ >> + _parent_name, \ >> + }; \ >> + static struct clk *_name##_parents[] = { \ >> + _parent_ptr, \ >> + }; \ >> + static struct clk_fixed_factor _name##_hw = { \ >> + .hw = { \ >> + .clk =&_name, \ >> + }, \ >> + .mult = _mult, \ >> + .div = _div, \ >> + }; \ >> + DEFINE_CLK(_name, clk_fixed_factor_ops, _flags, \ >> + _name##_parent_names, _name##_parents); >> + > > > I would prefer not defining a macro for this. But if we are going to do it, > can we please at least stop doing nested macros here? It would be much > easier for a new comer to read if we don't nest these clock macros.
This macro follows the others in every way. Why should we make it look less uniform?
May be the other macros should be refactored to not be nested too?
Also, should we stop adding support for fully static allocation for new clock types? Since it's supposed to be going away soon. Since Mike didn't add this clock type, I'm guessing he doesn't need the clock type now and hence doesn't need static allocation support for it.
I'm afraid I don't follow. I do need this clock in fact (see https://github.com/mturquette/linux/commits/clk-next-may06-omap), and my platform's data is still static.
Never mind. If you are using this clock type, then it's okay to support static init.
Should we have one header file for each common clock type? We seem to be adding a lot of those (which is good), but it almost feels like clock-provider get out of hand soon.
I think clk-provider.h is fine for now. It's a good one-stop-shop for people that are just now figuring out what basic clock types exist and applying them to their platform. The file itself is only 336 lines which is hardly out of control...
I still prefer them to be split out since one doesn't need to include (and be recompiled when it changes) stuff they don't care about. But it's not yet a significant point to argue about. So, let's wait and see how it goes.
-Saravana
Note that the Orion patches aren't here, but I figure that Andrew L. probably wants to check those against the final clk-next before I pull them.
Hi Mike
Here is a pull request. There is only one small change since the previous version. The last patch no longer has
select COMMON_CLK_DISABLE_UNUSED.
Thanks Andrew
The following changes since commit d49d10779b9b9d1c07dc61bf58a7995835b7d32f:
clk: clk_set_rate() must fail if CLK_SET_RATE_GATE is set and clk is enabled (2012-05-07 09:57:58 +0200)
are available in the git repository at: git://github.com/lunn/orion-clk.git v3.4-rc5-clk-next-orion-v2
Andrew Lunn (14): [ARM: Orion] Add clocks using the generic clk infrastructure. [ARM: Orion: SPI] Add clk/clkdev support. [ARM: Orion: Eth] Add clk/clkdev support. [ARM: Orion: WDT] Add clk/clkdev support [ARM: Orion: UART] Get the clock rate via clk_get_rate(). [ARM: Orion: SATA] Add per channel clk/clkdev support. [ARM: Orion: EHCI] Add support for enabling clocks [ARM: Orion: NAND] Add support for clk, if there is one. [ARM: Orion: SDIO] Add support for clk. [ARM: Orion: CESA] Add support for clk [ARM: Orion: XOR] Add support for clk [ARM: Orion: PCIE] Add support for clk [ARM: Orion: Audio] Add clk/clkdev support [ARM: Kirkwood] Replace clock gating
arch/arm/Kconfig | 2 + arch/arm/mach-dove/common.c | 40 ++- arch/arm/mach-dove/dove-db-setup.c | 1 - arch/arm/mach-kirkwood/board-dreamplug.c | 1 - arch/arm/mach-kirkwood/board-dt.c | 3 + arch/arm/mach-kirkwood/common.c | 274 ++++++++++++++------- arch/arm/mach-kirkwood/common.h | 1 + arch/arm/mach-kirkwood/include/mach/bridge-regs.h | 16 ++ arch/arm/mach-kirkwood/mv88f6281gtw_ge-setup.c | 1 - arch/arm/mach-kirkwood/pcie.c | 25 ++- arch/arm/mach-kirkwood/rd88f6192-nas-setup.c | 1 - arch/arm/mach-kirkwood/t5325-setup.c | 1 - arch/arm/mach-kirkwood/tsx1x-common.c | 1 - arch/arm/mach-mv78xx0/common.c | 45 +++-- arch/arm/mach-orion5x/common.c | 27 ++- arch/arm/mach-orion5x/rd88f6183ap-ge-setup.c | 1 - arch/arm/plat-orion/common.c | 104 +++++---- arch/arm/plat-orion/include/plat/common.h | 34 ++-- arch/arm/plat-orion/include/plat/orion_wdt.h | 18 -- arch/arm/plat-orion/pcie.c | 4 +- drivers/ata/sata_mv.c | 40 +++- drivers/crypto/mv_cesa.c | 14 + drivers/dma/mv_xor.c | 15 ++ drivers/dma/mv_xor.h | 1 + drivers/mmc/host/mvsdio.c | 14 + drivers/mtd/nand/orion_nand.c | 18 ++ drivers/net/ethernet/marvell/mv643xx_eth.c | 42 +++- drivers/spi/spi-orion.c | 30 ++- drivers/usb/host/ehci-orion.c | 16 ++ drivers/watchdog/orion_wdt.c | 16 +- include/linux/mv643xx_eth.h | 1 - include/linux/spi/orion_spi.h | 17 -- sound/soc/kirkwood/kirkwood-i2s.c | 13 + sound/soc/kirkwood/kirkwood.h | 1 + 34 files changed, 575 insertions(+), 263 deletions(-) delete mode 100644 arch/arm/plat-orion/include/plat/orion_wdt.h delete mode 100644 include/linux/spi/orion_spi.h
On Sun, May 06, 2012 at 10:08:25PM -0700, Mike Turquette wrote:
If no one complains about these then I'll commit them to clk-next and (finally) send my pull request to Arnd.
On mach-mxs:
Tested-by: Shawn Guo shawn.guo@linaro.org
Mike,
I haven't seen too many outstanding comments on these patches, except the following one that Saravana put on clk-fixed-factor.c, which should be easy to fix.
"
return (parent_rate / fix->div) * fix->mult;
Wouldn't multiplying and then dividing result in better accuracy? "
Can you please quickly fix it and publish the patches on your clk-next, so that I can ask Arnd to pull my platform porting, on which mach-mxs device tree series depends.
On Tue, May 8, 2012 at 9:12 AM, Shawn Guo shawn.guo@linaro.org wrote:
On Sun, May 06, 2012 at 10:08:25PM -0700, Mike Turquette wrote:
If no one complains about these then I'll commit them to clk-next and (finally) send my pull request to Arnd.
On mach-mxs:
Tested-by: Shawn Guo shawn.guo@linaro.org
Mike,
I haven't seen too many outstanding comments on these patches, except the following one that Saravana put on clk-fixed-factor.c, which should be easy to fix.
"
- return (parent_rate / fix->div) * fix->mult;
Wouldn't multiplying and then dividing result in better accuracy? "
Can you please quickly fix it and publish the patches on your clk-next, so that I can ask Arnd to pull my platform porting, on which mach-mxs device tree series depends.
I'll fix it up on my end and send the pull request to Arnd.
Regards, Mike
-- Regards, Shawn
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 20120506-22:08, Mike Turquette wrote:
Hi all,
These five patches are hopefully the final set of core framework changes for 3.5. There is the obligatory MAINTAINERS file change, three new fixes and the fixed-factor clock patch. That last patch is being reposted since three bugs were found in it (one on the list, two in my testing).
If no one complains about these then I'll commit them to clk-next and (finally) send my pull request to Arnd.
Arnd,
Please pull the following changes since commit 66f75a5d028beaf67c931435fdc3e7823125730c:
Linux 3.4-rc4 (2012-04-21 14:47:52 -0700)
are available in the git repository at: git://git.linaro.org/people/mturquette/linux.git clk-next
Mark Brown (2): clk: Remove comment for end of CONFIG_COMMON_CLK section clk: Constify parent name arrays
Mike Turquette (10): clk: core: correct clk_set_rate kerneldoc clk: core: remove dead code paths clk: core: clk_calc_new_rates handles NULL parents clk: core: enforce clk_ops consistency clk: core: copy parent_names & return error codes clk: basic: improve parent_names & return errors MAINTAINERS: add entry for common clk framework clk: prevent spurious parent rate propagation clk: remove COMMON_CLK_DISABLE_UNUSED clk: mux: assign init data
Rajendra Nayak (2): clk: Make clk_get_rate() return 0 on error clk: constify parent name arrays in macros
Rob Herring (2): clk: select CLKDEV_LOOKUP for COMMON_CLK clk: remove trailing whitespace from clk.h
Saravana Kannan (1): clk: Use a separate struct for holding init data.
Sascha Hauer (1): clk: add a fixed factor clock
Shawn Guo (7): clk: use kzalloc in clk_register_mux clk: remove unnecessary EXPORT_SYMBOL_GPL clk: add "const" for clk_ops of basic clks clk: declare clk_ops of basic clks in clk-provider.h clk: always pass parent_rate into .round_rate clk: pass parent_rate into .set_rate clk: propagate round_rate for CLK_SET_RATE_PARENT case
Viresh Kumar (5): clk: Fix typo in comment clk: clk-gate: Create clk_gate_endisable() clk: clk-private: Add DEFINE_CLK macro clk: Don't set clk->new_rate twice clk: clk_set_rate() must fail if CLK_SET_RATE_GATE is set and clk is enabled
MAINTAINERS | 10 ++ drivers/clk/Kconfig | 12 +-- drivers/clk/Makefile | 2 +- drivers/clk/clk-divider.c | 68 +++++----- drivers/clk/clk-fixed-factor.c | 95 ++++++++++++++ drivers/clk/clk-fixed-rate.c | 49 ++++---- drivers/clk/clk-gate.c | 104 ++++++++-------- drivers/clk/clk-mux.c | 27 +++-- drivers/clk/clk.c | 270 +++++++++++++++++++++++++-------------- include/linux/clk-private.h | 99 +++++++-------- include/linux/clk-provider.h | 118 ++++++++++++------ include/linux/clk.h | 6 +- 12 files changed, 537 insertions(+), 323 deletions(-) create mode 100644 drivers/clk/clk-fixed-factor.c
Note that the Orion patches aren't here, but I figure that Andrew L. probably wants to check those against the final clk-next before I pull them.
Regards, Mike
Mike Turquette (4): MAINTAINERS: add entry for common clk framework clk: prevent spurious parent rate propagation clk: remove COMMON_CLK_DISABLE_UNUSED clk: mux: assign init data
Sascha Hauer (1): clk: add a fixed factor clock
MAINTAINERS | 10 ++++ drivers/clk/Kconfig | 11 ----- drivers/clk/Makefile | 2 +- drivers/clk/clk-fixed-factor.c | 95 ++++++++++++++++++++++++++++++++++++++++ drivers/clk/clk-mux.c | 1 + drivers/clk/clk.c | 9 +++- include/linux/clk-private.h | 20 ++++++++ include/linux/clk-provider.h | 23 ++++++++++ 8 files changed, 156 insertions(+), 15 deletions(-) create mode 100644 drivers/clk/clk-fixed-factor.c
-- 1.7.5.4
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Tuesday 08 May 2012, Turquette, Mike wrote:
Arnd,
Please pull the following changes since commit 66f75a5d028beaf67c931435fdc3e7823125730c:
Linux 3.4-rc4 (2012-04-21 14:47:52 -0700)
are available in the git repository at: git://git.linaro.org/people/mturquette/linux.git clk-next
Hi Mike, sorry for the delay. Olof was handling the other pull requests this week, and he probably didn't see this one or the discussion leading up to it, because he was not on Cc.
I've pulled it into a next/clock branch now and it should show up in the for-next branch when it gets rebuilt without the spear patches that are currently in it and have older confliction versions of some of the same patches.
Arnd
-
Andrew Lunn -
Arnd Bergmann -
Mike Turquette -
Saravana Kannan -
Sascha Hauer -
Shawn Guo -
Turquette, Mike