more clk-next fixes
cover more clk-next fixes
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
Signature · 1 line
1.7.5.4
PATCH 1/5 MAINTAINERS: add entry for common clk framework
--- MAINTAINERS | 10 ++++++++++ 1 files changed, 10 insertions(+), 0 deletions(-)
MAINTAINERS +10 -0
@@ -1882,6 +1882,16 @@ F: Documentation/filesystems/coda.txt F: fs/coda/ F: include/linux/coda*.h +COMMON CLK FRAMEWORK +M: Mike Turquette <[email protected]> +M: Mike Turquette <[email protected]> +L: linux-arm-kernel at 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 <[email protected]> L: linux-cifs at vger.kernel.org
Signature · 1 line
1.7.5.4
PATCH 2/5 clk: prevent spurious parent rate propagation
In reply to Mike TurquettePatch '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.
--- drivers/clk/clk.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
drivers/clk/clk.c +6 -1
@@ -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) {
Signature · 1 line
1.7.5.4
PATCH 2/5 clk: prevent spurious parent rate propagation
Quoted 10 lines · Mike Turquette
Patch 'clk: always pass
parent_rateinto .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_rateswhereby a clock without theCLK_SET_RATE_PARENTflag set could still propagate a rate change up to a parent clock if the the .round_rate callback modified the&best_parent_ratevalue in any way.
Quoted 6 lines
This patch fixes the issue at the framework level (in
clk_calc_new_rates) by specifically handling the case where theCLK_SET_RATE_PARENTflag is not set.Signed-off-by Mike Turquette <[email protected]>Reported-by Sascha Hauer<[email protected]>
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
Sascha
Quoted 39 lines
--- drivers/clk/clk.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)drivers/clk/clk.c +6
-1@@ -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) {Signature · 1 line
1.7.5.4
Signature · 4 lines
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 2/5 clk: prevent spurious parent rate propagation
Quoted 15 lines · Sascha Hauer
Quoted 14 lines · Mike Turquette
Patch 'clk: always pass
parent_rateinto .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_rateswhereby a clock without theCLK_SET_RATE_PARENTflag set could still propagate a rate change up to a parent clock if the the .round_rate callback modified the&best_parent_ratevalue in any way.This patch fixes the issue at the framework level (in
clk_calc_new_rates) by specifically handling the case where theCLK_SET_RATE_PARENTflag is not set.
Quoted 7 lines
Quoted 2 lines
Signed-off-by Mike Turquette <[email protected]>Reported-by Sascha Hauer<[email protected]>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.
Quoted 2 lines
Acked-by Sascha Hauer <[email protected]>
Thanks,
Mike
PATCH 3/5 clk: remove COMMON_CLK_DISABLE_UNUSED
In reply to Mike TurquetteExposing this option generates confusion and incorrect behavior for single-image builds across platforms. Enable this behavior permanently.
--- drivers/clk/Kconfig | 11 ----------- drivers/clk/clk.c | 2 -- 2 files changed, 0 insertions(+), 13 deletions(-)
drivers/clk/Kconfig +0 -11
@@ -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
drivers/clk/clk.c +0 -2
@@ -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 ***/
Signature · 1 line
1.7.5.4
PATCH 3/5 clk: remove COMMON_CLK_DISABLE_UNUSED
Quoted 44 lines · Mike Turquette
Exposing this option generates confusion and incorrect behavior for single-image builds across platforms. Enable this behavior permanently.
Signed-off-by Mike Turquette<[email protected]>--- drivers/clk/Kconfig | 11 ----------- drivers/clk/clk.c | 2 -- 2 files changed, 0 insertions(+), 13 deletions(-)drivers/clk/Kconfig +0
-11@@ -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_CLKdrivers/clk/clk.c +0
-1@@ -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) {
drivers/clk/clk.c static int clk_disable_unused(void) · line 250
Quoted 7 lines
drivers/clk/clk.c +0
-1@@ -246,7 +245,6 @@ static int clk_disable_unused(void)return 0 ; }late_initcall (clk_disable_unused); -#endif /*** helper functions ***/
Good decision! Acked.
-Saravana
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 4/5 clk: mux: assign init data
In reply to Mike TurquetteThe original conversion to struct clk_hw_init failed to add the pointer assignment in clk_register_mux.
--- drivers/clk/clk-mux.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/clk/clk-mux.c +1 -0
@@ -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);
Signature · 1 line
1.7.5.4
PATCH 5/5 clk: add a fixed factor clock
In reply to Mike TurquetteFrom: Sascha Hauer <[email protected]>
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.
--- 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
drivers/clk/Makefile +1 -1
@@ -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
drivers/clk/clk-fixed-factor.c new file +95 -0
95 lines not on this page · raw message
include/linux/clk-private.h +20 -0
@@ -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
include/linux/clk-provider.h +23 -0
@@ -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
Signature · 1 line
1.7.5.4
PATCH 5/5 clk: add a fixed factor clock
Quoted 58 lines · Mike Turquette
From: Sascha Hauer<[email protected]>
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<[email protected]>Reviewed-by Viresh Kumar<[email protected]>[mturquette at linaro.org: constify parent_names in static init macro][mturquette at linaro.org: copy/paste bug from mux in static init macro][mturquette at linaro.org: fix error handling in clk_register_fixed_factor]Signed-off-by Mike Turquette<[email protected]>--- 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.cdrivers/clk/Makefile +1
-1@@ -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.odrivers/clk/clk-fixed-factor.c new file +22
-0@@ -0,0 +1,95 @@ +/* +* Copyright (C) 2011 Sascha Hauer, Pengutronix<[email protected]> +* +* 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
drivers/clk/clk-fixed-factor.c line 32
Quoted 10 lines
drivers/clk/clk-fixed-factor.c new file +10
-0+*/ + +#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)?
Quoted 34 lines
drivers/clk/clk-fixed-factor.c new file +34
-0+} + +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)
drivers/clk/clk-fixed-factor.c line 76
Quoted 10 lines
drivers/clk/clk-fixed-factor.c new file +10
-0+{ +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).
Quoted 37 lines
drivers/clk/clk-fixed-factor.c new file +19
-0+ +/* 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; +}include/linux/clk-private.h +10
-0@@ -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, \ + }; \
include/linux/clk-private.h struct clk { · line 165
Quoted 10 lines
include/linux/clk-private.h +10
-0+ 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.
Quoted 22 lines
include/linux/clk-private.h +0
-0/** * __clk_init - initialize the data structures in a struct clk * @dev: device initializing this clk, placeholder for now include/linux/clk-provider.h +11
-0@@ -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 +*/ +
include/linux/clk-provider.h struct clk *clk_register_mux(struct device *dev, const char *name, · line 300
Quoted 10 lines
include/linux/clk-provider.h +10
-0+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
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 5/5 clk: add a fixed factor clock
Quoted 14 lines · Saravana Kannan
Quoted 13 lines · Mike Turquette
From: Sascha Hauer<[email protected]>
+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)(quoted diff) +8
-0+{ + ? ? ? 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",
Quoted 9 lines
Quoted 3 lines
__func__);
+ ? ? ? ? ? ? ? returnERR_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>
Quoted 17 lines
Quoted 17 lines
+#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 structclk_fixed_factor_name##_hw = { ? ? ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk =&_name, ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? ? .mult =_mult, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? ? ? ? ? .div =_div, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
Quoted 8 lines
Quoted 3 lines
+ ? ? ? 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?
Quoted 5 lines
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.
Quoted 33 lines
Quoted 33 lines
?/**
? *__clk_init- initialize the data structures in a struct clk ? * @dev: ? ? ?device initializing this clk, placeholder for nowinclude/linux/clk-provider.h +0
-0@@ -277,6 +277,29 @@ struct clk *clk_register_mux(struct device *dev,const char *name, ? ? ? ? ? ? ? ?u8 clk_mux_flags, spinlock_t *lock);?/**
+ * structclk_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
+ */include/linux/clk-provider.h +9
-0+ +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
Quoted 9 lines
Quoted 3 lines
*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
Quoted 12 lines
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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 5/5 clk: add a fixed factor clock
Quoted 25 lines · Mike
Quoted 14 lines · Saravana Kannan
Quoted 13 lines · Mike Turquette
From: Sascha Hauer<[email protected]>
+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,drivers/clk/clk-fixed-factor.c new file +10
-0+ 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",Quoted 9 lines
Review
Quoted 3 lines
__func__);(quoted diff) +2
-0+ 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).
Quoted 4 lines
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.
Quoted 27 lines
<snip>
Quoted 17 lines
Quoted 17 lines
include/linux/clk-private.h +17
-0+#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, \ + }; \Quoted 8 lines
include/linux/clk-private.h struct clk { · line 165
Quoted 3 lines
include/linux/clk-private.h +3
-0+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.
Quoted 2 lines
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?
Quoted 9 lines
Quoted 5 lines
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.
Quoted 9 lines
Quoted 4 lines
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.his 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
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
cover more clk-next fixes
In reply to Mike TurquetteQuoted 3 lines
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
cover more clk-next fixes
In reply to Mike TurquetteQuoted 3 lines · Mike Turquette
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:
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.
"
Quoted 1 line
+ 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.
Signature · 2 lines
Regards, Shawn
cover more clk-next fixes
Quoted 15 lines · Shawn Guo
Quoted 3 lines · Mike Turquette
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 <[email protected]>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."
Quoted 8 lines
Quoted 1 line
+ ? ? 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
Quoted 9 lines
--
Regards,
Shawn_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
cover more clk-next fixes
In reply to Mike TurquetteQuoted 11 lines · Mike Turquette
Hi all,
These five patches are hopefully the final set of core framework changes for 3.5. There is the obligatory
MAINTAINERSfile 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 commit66f75a5d028beaf67c931435fdc3e7823125730c:
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
Quoted 35 lines
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,
MikeMike Turquette (4):
MAINTAINERS: add entry for common clk framework
clk: prevent spurious parent rate propagation
clk: removeCOMMON_CLK_DISABLE_UNUSED
clk: mux: assign init dataSascha Hauer (1):
clk: add a fixed factor clockMAINTAINERS | 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 at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
cover more clk-next fixes
Quoted 10 lines · Mike
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.gitclk-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