This series collects many of the fixes posted for the recently merged common clock framework as well as some general clean-up. Most of the code classifies as a clean-up moreso than a bug fix; hopefully this is not a problem since the common clk framework is new code pulled for 3.4.
Patches are based on v3.4-rc2 and can be pulled from: git://git.linaro.org/people/mturquette/linux.git v3.4-rc2-clk-fixes
Please let me know I missed any critical fixes that were posted to the list already.
Arnd & Olof, if there are no objections to these changes can this get pulled through the arm-soc tree?
Thanks, Mike
Mark Brown (2): clk: Remove comment for end of CONFIG_COMMON_CLK section clk: Constify parent name arrays
Mike Turquette (6): 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
Rajendra Nayak (1): clk: Make clk_get_rate() return 0 on error
Shawn Guo (4): 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
drivers/clk/clk-divider.c | 51 +++++++++---- drivers/clk/clk-fixed-rate.c | 57 +++++++++------ drivers/clk/clk-gate.c | 53 ++++++++++----- drivers/clk/clk-mux.c | 16 +++-- drivers/clk/clk.c | 159 ++++++++++++++++++++++++++--------------- include/linux/clk-private.h | 14 +--- include/linux/clk-provider.h | 13 ++-- include/linux/clk.h | 2 +- 8 files changed, 230 insertions(+), 135 deletions(-)
Remove old and misleading documentation from the previous clk_set_rate implementaion.
Reported-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk.c | 41 +++++++++++------------------------------ 1 files changed, 11 insertions(+), 30 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 9cf6f59..3ed36d3 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -859,38 +859,19 @@ static void clk_change_rate(struct clk *clk) * @clk: the clk whose rate is being changed * @rate: the new rate for clk * - * In the simplest case clk_set_rate will only change the rate of clk. + * In the simplest case clk_set_rate will only adjust the rate of clk. * - * If clk has the CLK_SET_RATE_GATE flag set and it is enabled this call - * will fail; only when the clk is disabled will it be able to change - * its rate. + * Setting the CLK_SET_RATE_PARENT flag allows the rate change operation to + * propagate up to clk's parent; whether or not this happens depends on the + * outcome of clk's .round_rate implementation. If *parent_rate is unchanged + * after calling .round_rate then upstream parent propagation is ignored. If + * *parent_rate comes back with a new rate for clk's parent then we propagate + * up to clk's parent and set it's rate. Upward propagation will continue + * until either a clk does not support the CLK_SET_RATE_PARENT flag or + * .round_rate stops requesting changes to clk's parent_rate. * - * Setting the CLK_SET_RATE_PARENT flag allows clk_set_rate to - * recursively propagate up to clk's parent; whether or not this happens - * depends on the outcome of clk's .round_rate implementation. If - * *parent_rate is 0 after calling .round_rate then upstream parent - * propagation is ignored. If *parent_rate comes back with a new rate - * for clk's parent then we propagate up to clk's parent and set it's - * rate. Upward propagation will continue until either a clk does not - * support the CLK_SET_RATE_PARENT flag or .round_rate stops requesting - * changes to clk's parent_rate. If there is a failure during upstream - * propagation then clk_set_rate will unwind and restore each clk's rate - * that had been successfully changed. Afterwards a rate change abort - * notification will be propagated downstream, starting from the clk - * that failed. - * - * At the end of all of the rate setting, clk_set_rate internally calls - * __clk_recalc_rates and propagates the rate changes downstream, - * starting from the highest clk whose rate was changed. This has the - * added benefit of propagating post-rate change notifiers. - * - * Note that while post-rate change and rate change abort notifications - * are guaranteed to be sent to a clk only once per call to - * clk_set_rate, pre-change notifications will be sent for every clk - * whose rate is changed. Stacking pre-change notifications is noisy - * for the drivers subscribed to them, but this allows drivers to react - * to intermediate clk rate changes up until the point where the final - * rate is achieved at the end of upstream propagation. + * Rate changes are accomplished via tree traversal that also recalculates the + * rates for the clocks and fires off POST_RATE_CHANGE notifiers. * * Returns 0 on success, -EERROR otherwise. */
On 4/12/2012 6:32 AM, Mike Turquette wrote:
- If clk has the CLK_SET_RATE_GATE flag set and it is enabled this call
- will fail; only when the clk is disabled will it be able to change
- its rate.
Why is CLK_SET_RATE_GATE removed? I already sent a patch to fix clk_set_rate() for this. And i think it should be required.
On Wed, Apr 11, 2012 at 9:28 PM, Viresh Kumar viresh.kumar@st.com wrote:
On 4/12/2012 6:32 AM, Mike Turquette wrote:
- If clk has the CLK_SET_RATE_GATE flag set and it is enabled this call
- will fail; only when the clk is disabled will it be able to change
- its rate.
Why is CLK_SET_RATE_GATE removed? I already sent a patch to fix clk_set_rate() for this. And i think it should be required.
I removed it from the documentation since it was not present in the code. I was originally targeting this for an -rc and I was trying to limit the changes purely to fixes and cleanups, not new features.
I haven't finished digging through all of the responses yet, so my goal for where these patches are headed might change.
Regards, Mike
Hi Mike
A general question to all these patches.
Do you want to get them into 3.4-rc, or linux-next?
Thanks Andrew
On Thu, Apr 12, 2012 at 10:24 AM, Andrew Lunn andrew@lunn.ch wrote:
Hi Mike
A general question to all these patches.
Do you want to get them into 3.4-rc, or linux-next?
Thanks Andrew
In 0/13, I think he did ask Arnd to take these in for 3.4-rc even if they are not strictly bug fixes, since this is a new framework. Up to the arm-soc maintainers now.
/Amit
Some static inline dummy functions were left over from before the clock core was consolidated from several C files down to one. Remove them.
Reported-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3ed36d3..4daacf5 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -194,7 +194,7 @@ static int __init clk_debug_init(void) late_initcall(clk_debug_init); #else static inline int clk_debug_register(struct clk *clk) { return 0; } -#endif /* CONFIG_COMMON_CLK_DEBUG */ +#endif
#ifdef CONFIG_COMMON_CLK_DISABLE_UNUSED /* caller must hold prepare_lock */ @@ -246,9 +246,7 @@ static int clk_disable_unused(void) return 0; } late_initcall(clk_disable_unused); -#else -static inline int clk_disable_unused(struct clk *clk) { return 0; } -#endif /* CONFIG_COMMON_CLK_DISABLE_UNUSED */ +#endif
/*** helper functions ***/
On 4/12/2012 6:32 AM, Mike Turquette wrote:
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3ed36d3..4daacf5 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -194,7 +194,7 @@ static int __init clk_debug_init(void) late_initcall(clk_debug_init); #else static inline int clk_debug_register(struct clk *clk) { return 0; } -#endif /* CONFIG_COMMON_CLK_DEBUG */ +#endif
Why is this updated? Isn't this considered good practice anymore?
On Wed, Apr 11, 2012 at 11:14 PM, Viresh Kumar viresh.kumar@st.com wrote:
On 4/12/2012 6:32 AM, Mike Turquette wrote:
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 3ed36d3..4daacf5 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -194,7 +194,7 @@ static int __init clk_debug_init(void) late_initcall(clk_debug_init); #else static inline int clk_debug_register(struct clk *clk) { return 0; } -#endif /* CONFIG_COMMON_CLK_DEBUG */ +#endif
Why is this updated? Isn't this considered good practice anymore?
See comments in this thread: http://article.gmane.org/gmane.linux.kernel/1276102
Regards, Mike
It is possible to call clk_set_rate on a clock with a NULL parent. One such example is an adjustable-rate root clock. Ensure that clk_calc_new_rates does not dereference parent without checking first and also handle the corner cases gracefully.
Reported-by: Rajendra Nayak rnayak@ti.com Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk.c | 29 +++++++++++++++++++++-------- 1 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 4daacf5..d83a9e0 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -763,25 +763,38 @@ static void clk_calc_subtree(struct clk *clk, unsigned long new_rate) static struct clk *clk_calc_new_rates(struct clk *clk, unsigned long rate) { struct clk *top = clk; - unsigned long best_parent_rate = clk->parent->rate; + unsigned long best_parent_rate; unsigned long new_rate;
- if (!clk->ops->round_rate && !(clk->flags & CLK_SET_RATE_PARENT)) { - clk->new_rate = clk->rate; + /* sanity */ + if (IS_ERR_OR_NULL(clk)) + return NULL; + + /* 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; + } else { + new_rate = clk->ops->round_rate(clk->hw, rate, NULL); + goto out; + } + } + + /* need clk->parent from here on out */ + if (!clk->parent) { + pr_debug("%s: %s has NULL parent\n", __func__, clk->name); return NULL; }
- if (!clk->ops->round_rate && (clk->flags & CLK_SET_RATE_PARENT)) { + if (!clk->ops->round_rate) { top = clk_calc_new_rates(clk->parent, rate); new_rate = clk->new_rate = clk->parent->new_rate;
goto out; }
- if (clk->flags & CLK_SET_RATE_PARENT) - new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate); - else - new_rate = clk->ops->round_rate(clk->hw, rate, NULL); + new_rate = clk->ops->round_rate(clk->hw, rate, &best_parent_rate);
if (best_parent_rate != clk->parent->rate) { top = clk_calc_new_rates(clk->parent, best_parent_rate);
Documentation/clk.txt has some handsome ASCII art outlining which clk_ops are mandatory for a given clock, given the capability of the hardware. Enforce those mandates with sanity checks in __clk_init.
Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index d83a9e0..9924aec 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1202,6 +1202,20 @@ void __clk_init(struct device *dev, struct clk *clk) if (__clk_lookup(clk->name)) goto out;
+ /* check that clk_ops are sane. See Documentation/clk.txt */ + if (clk->ops->set_rate && + !(clk->ops->round_rate && clk->ops->recalc_rate)) { + pr_warning("%s: %s must implement .round_rate & .recalc_rate\n", + __func__, clk->name); + goto out; + } + + if (clk->ops->set_parent && !clk->ops->get_parent) { + pr_warning("%s: %s must implement .get_parent & .set_parent\n", + __func__, clk->name); + goto out; + } + /* throw a WARN if any entries in parent_names are NULL */ for (i = 0; i < clk->num_parents; i++) WARN(!clk->parent_names[i],
On 4/12/2012 6:32 AM, Mike Turquette wrote:
Documentation/clk.txt has some handsome ASCII art outlining which clk_ops are mandatory for a given clock, given the capability of the hardware. Enforce those mandates with sanity checks in __clk_init.
Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com
drivers/clk/clk.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index d83a9e0..9924aec 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1202,6 +1202,20 @@ void __clk_init(struct device *dev, struct clk *clk) if (__clk_lookup(clk->name)) goto out;
- /* check that clk_ops are sane. See Documentation/clk.txt */
- if (clk->ops->set_rate &&
!(clk->ops->round_rate && clk->ops->recalc_rate)) {pr_warning("%s: %s must implement .round_rate & .recalc_rate\n",__func__, clk->name);goto out;- }
- if (clk->ops->set_parent && !clk->ops->get_parent) {
pr_warning("%s: %s must implement .get_parent & .set_parent\n",__func__, clk->name);goto out;- }
- /* throw a WARN if any entries in parent_names are NULL */ for (i = 0; i < clk->num_parents; i++) WARN(!clk->parent_names[i],
Reviewed-by: Viresh Kumar viresh.kumar@st.com
From: Shawn Guo shawn.guo@linaro.org
Change clk_register_mux to use kzalloc, just like what all other basic clk registration functions do.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk-mux.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index c71ad1f..50e0595 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -97,7 +97,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, { struct clk_mux *mux;
- mux = kmalloc(sizeof(struct clk_mux), GFP_KERNEL); + mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
if (!mux) { pr_err("%s: could not allocate mux clk\n", __func__);
On 4/12/2012 6:32 AM, Mike Turquette wrote:
From: Shawn Guo shawn.guo@linaro.org
Change clk_register_mux to use kzalloc, just like what all other basic clk registration functions do.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com
drivers/clk/clk-mux.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index c71ad1f..50e0595 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -97,7 +97,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, { struct clk_mux *mux;
- mux = kmalloc(sizeof(struct clk_mux), GFP_KERNEL);
- mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
if (!mux) { pr_err("%s: could not allocate mux clk\n", __func__);
Reviewed-by: Viresh Kumar viresh.kumar@st.com
From: Shawn Guo shawn.guo@linaro.org
It makes no sense to have EXPORT_SYMBOL_GPL on static functions.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk-divider.c | 3 --- drivers/clk/clk-fixed-rate.c | 1 - drivers/clk/clk-gate.c | 3 --- drivers/clk/clk-mux.c | 2 -- 4 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index d5ac6a7..231cd6e 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -45,7 +45,6 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
return parent_rate / div; } -EXPORT_SYMBOL_GPL(clk_divider_recalc_rate);
/* * The reverse of DIV_ROUND_UP: The maximum number which @@ -117,7 +116,6 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate, return r / div; } } -EXPORT_SYMBOL_GPL(clk_divider_round_rate);
static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate) { @@ -147,7 +145,6 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate)
return 0; } -EXPORT_SYMBOL_GPL(clk_divider_set_rate);
struct clk_ops clk_divider_ops = { .recalc_rate = clk_divider_recalc_rate, diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 90c79fb..651b06f 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -32,7 +32,6 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw, { return to_clk_fixed_rate(hw)->fixed_rate; } -EXPORT_SYMBOL_GPL(clk_fixed_rate_recalc_rate);
struct clk_ops clk_fixed_rate_ops = { .recalc_rate = clk_fixed_rate_recalc_rate, diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index b5902e2..b688f47 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -71,7 +71,6 @@ static int clk_gate_enable(struct clk_hw *hw)
return 0; } -EXPORT_SYMBOL_GPL(clk_gate_enable);
static void clk_gate_disable(struct clk_hw *hw) { @@ -82,7 +81,6 @@ static void clk_gate_disable(struct clk_hw *hw) else clk_gate_clear_bit(gate); } -EXPORT_SYMBOL_GPL(clk_gate_disable);
static int clk_gate_is_enabled(struct clk_hw *hw) { @@ -99,7 +97,6 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
return reg ? 1 : 0; } -EXPORT_SYMBOL_GPL(clk_gate_is_enabled);
struct clk_ops clk_gate_ops = { .enable = clk_gate_enable, diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 50e0595..45cad61 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -55,7 +55,6 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
return val; } -EXPORT_SYMBOL_GPL(clk_mux_get_parent);
static int clk_mux_set_parent(struct clk_hw *hw, u8 index) { @@ -82,7 +81,6 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
return 0; } -EXPORT_SYMBOL_GPL(clk_mux_set_parent);
struct clk_ops clk_mux_ops = { .get_parent = clk_mux_get_parent,
On 4/12/2012 6:32 AM, Mike Turquette wrote:
From: Shawn Guo shawn.guo@linaro.org
It makes no sense to have EXPORT_SYMBOL_GPL on static functions.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com
drivers/clk/clk-divider.c | 3 --- drivers/clk/clk-fixed-rate.c | 1 - drivers/clk/clk-gate.c | 3 --- drivers/clk/clk-mux.c | 2 -- 4 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index d5ac6a7..231cd6e 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -45,7 +45,6 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw, return parent_rate / div; } -EXPORT_SYMBOL_GPL(clk_divider_recalc_rate); /*
- The reverse of DIV_ROUND_UP: The maximum number which
@@ -117,7 +116,6 @@ static long clk_divider_round_rate(struct clk_hw *hw, unsigned long rate, return r / div; } } -EXPORT_SYMBOL_GPL(clk_divider_round_rate); static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate) { @@ -147,7 +145,6 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate) return 0; } -EXPORT_SYMBOL_GPL(clk_divider_set_rate); struct clk_ops clk_divider_ops = { .recalc_rate = clk_divider_recalc_rate, diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 90c79fb..651b06f 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -32,7 +32,6 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw, { return to_clk_fixed_rate(hw)->fixed_rate; } -EXPORT_SYMBOL_GPL(clk_fixed_rate_recalc_rate); struct clk_ops clk_fixed_rate_ops = { .recalc_rate = clk_fixed_rate_recalc_rate, diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index b5902e2..b688f47 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -71,7 +71,6 @@ static int clk_gate_enable(struct clk_hw *hw) return 0; } -EXPORT_SYMBOL_GPL(clk_gate_enable); static void clk_gate_disable(struct clk_hw *hw) { @@ -82,7 +81,6 @@ static void clk_gate_disable(struct clk_hw *hw) else clk_gate_clear_bit(gate); } -EXPORT_SYMBOL_GPL(clk_gate_disable); static int clk_gate_is_enabled(struct clk_hw *hw) { @@ -99,7 +97,6 @@ static int clk_gate_is_enabled(struct clk_hw *hw) return reg ? 1 : 0; } -EXPORT_SYMBOL_GPL(clk_gate_is_enabled); struct clk_ops clk_gate_ops = { .enable = clk_gate_enable, diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 50e0595..45cad61 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -55,7 +55,6 @@ static u8 clk_mux_get_parent(struct clk_hw *hw) return val; } -EXPORT_SYMBOL_GPL(clk_mux_get_parent); static int clk_mux_set_parent(struct clk_hw *hw, u8 index) { @@ -82,7 +81,6 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index) return 0; } -EXPORT_SYMBOL_GPL(clk_mux_set_parent); struct clk_ops clk_mux_ops = { .get_parent = clk_mux_get_parent,
Reviewed-by: Viresh Kumar viresh.kumar@st.com
From: Shawn Guo shawn.guo@linaro.org
The clk_ops of basic clks should have "const" to match the definition in "struct clk" and clk_register prototype.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk-divider.c | 2 +- drivers/clk/clk-fixed-rate.c | 2 +- drivers/clk/clk-gate.c | 2 +- drivers/clk/clk-mux.c | 2 +- include/linux/clk-private.h | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index 231cd6e..b1c4b02 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -146,7 +146,7 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate) return 0; }
-struct clk_ops clk_divider_ops = { +const struct clk_ops clk_divider_ops = { .recalc_rate = clk_divider_recalc_rate, .round_rate = clk_divider_round_rate, .set_rate = clk_divider_set_rate, diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 651b06f..027e477 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -33,7 +33,7 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw, return to_clk_fixed_rate(hw)->fixed_rate; }
-struct clk_ops clk_fixed_rate_ops = { +const struct clk_ops clk_fixed_rate_ops = { .recalc_rate = clk_fixed_rate_recalc_rate, }; EXPORT_SYMBOL_GPL(clk_fixed_rate_ops); diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index b688f47..fe2ff9e 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -98,7 +98,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw) return reg ? 1 : 0; }
-struct clk_ops clk_gate_ops = { +const struct clk_ops clk_gate_ops = { .enable = clk_gate_enable, .disable = clk_gate_disable, .is_enabled = clk_gate_is_enabled, diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 45cad61..5424488 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -82,7 +82,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index) return 0; }
-struct clk_ops clk_mux_ops = { +const struct clk_ops clk_mux_ops = { .get_parent = clk_mux_get_parent, .set_parent = clk_mux_set_parent, }; diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index 5e4312b..5f4ccd7 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -55,7 +55,7 @@ struct clk { * alternative macro for static initialization */
-extern struct clk_ops clk_fixed_rate_ops; +extern const struct clk_ops clk_fixed_rate_ops;
#define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate, \ _fixed_rate_flags) \ @@ -78,7 +78,7 @@ extern struct clk_ops clk_fixed_rate_ops; .flags = _flags, \ };
-extern struct clk_ops clk_gate_ops; +extern const struct clk_ops clk_gate_ops;
#define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr, \ _flags, _reg, _bit_idx, \ @@ -110,7 +110,7 @@ extern struct clk_ops clk_gate_ops; .flags = _flags, \ };
-extern struct clk_ops clk_divider_ops; +extern const struct clk_ops clk_divider_ops;
#define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \ _flags, _reg, _shift, _width, \ @@ -143,7 +143,7 @@ extern struct clk_ops clk_divider_ops; .flags = _flags, \ };
-extern struct clk_ops clk_mux_ops; +extern const struct clk_ops clk_mux_ops;
#define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \ _reg, _shift, _width, \
On 4/12/2012 6:32 AM, Mike Turquette wrote:
From: Shawn Guo shawn.guo@linaro.org
The clk_ops of basic clks should have "const" to match the definition in "struct clk" and clk_register prototype.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com
drivers/clk/clk-divider.c | 2 +- drivers/clk/clk-fixed-rate.c | 2 +- drivers/clk/clk-gate.c | 2 +- drivers/clk/clk-mux.c | 2 +- include/linux/clk-private.h | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index 231cd6e..b1c4b02 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -146,7 +146,7 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate) return 0; } -struct clk_ops clk_divider_ops = { +const struct clk_ops clk_divider_ops = { .recalc_rate = clk_divider_recalc_rate, .round_rate = clk_divider_round_rate, .set_rate = clk_divider_set_rate, diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 651b06f..027e477 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -33,7 +33,7 @@ static unsigned long clk_fixed_rate_recalc_rate(struct clk_hw *hw, return to_clk_fixed_rate(hw)->fixed_rate; } -struct clk_ops clk_fixed_rate_ops = { +const struct clk_ops clk_fixed_rate_ops = { .recalc_rate = clk_fixed_rate_recalc_rate, }; EXPORT_SYMBOL_GPL(clk_fixed_rate_ops); diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index b688f47..fe2ff9e 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -98,7 +98,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw) return reg ? 1 : 0; } -struct clk_ops clk_gate_ops = { +const struct clk_ops clk_gate_ops = { .enable = clk_gate_enable, .disable = clk_gate_disable, .is_enabled = clk_gate_is_enabled, diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 45cad61..5424488 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -82,7 +82,7 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index) return 0; } -struct clk_ops clk_mux_ops = { +const struct clk_ops clk_mux_ops = { .get_parent = clk_mux_get_parent, .set_parent = clk_mux_set_parent, }; diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index 5e4312b..5f4ccd7 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -55,7 +55,7 @@ struct clk {
- alternative macro for static initialization
*/ -extern struct clk_ops clk_fixed_rate_ops; +extern const struct clk_ops clk_fixed_rate_ops; #define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate, \ _fixed_rate_flags) \ @@ -78,7 +78,7 @@ extern struct clk_ops clk_fixed_rate_ops; .flags = _flags, \ }; -extern struct clk_ops clk_gate_ops; +extern const struct clk_ops clk_gate_ops; #define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr, \ _flags, _reg, _bit_idx, \ @@ -110,7 +110,7 @@ extern struct clk_ops clk_gate_ops; .flags = _flags, \ }; -extern struct clk_ops clk_divider_ops; +extern const struct clk_ops clk_divider_ops; #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \ _flags, _reg, _shift, _width, \ @@ -143,7 +143,7 @@ extern struct clk_ops clk_divider_ops; .flags = _flags, \ }; -extern struct clk_ops clk_mux_ops; +extern const struct clk_ops clk_mux_ops; #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \ _reg, _shift, _width, \
Reviewed-by: Viresh Kumar viresh.kumar@st.com
From: Shawn Guo shawn.guo@linaro.org
Besides the static initialization, the clk_ops of basic clks could also be used by particular clk type being subclass of the basic clks.
For example, clk_busy_divider has the same clk_ops as clk_divider, except it has to wait for a busy bit before return success with .set_rate. clk_busy_divider will somehow reuse clk_ops of clk_divider.
Since clk-provider.h is included by clk-private.h, it's safe to move those clk_ops declaration of basic clks form clk-private.h into clk-provider.h, so that implementation of clks like clk_busy_divider above do not need to include clk-private.h to access those clk_ops.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- include/linux/clk-private.h | 8 -------- include/linux/clk-provider.h | 4 ++++ 2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index 5f4ccd7..f19fee0 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -55,8 +55,6 @@ struct clk { * alternative macro for static initialization */
-extern const struct clk_ops clk_fixed_rate_ops; - #define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate, \ _fixed_rate_flags) \ static struct clk _name; \ @@ -78,8 +76,6 @@ extern const struct clk_ops clk_fixed_rate_ops; .flags = _flags, \ };
-extern const struct clk_ops clk_gate_ops; - #define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr, \ _flags, _reg, _bit_idx, \ _gate_flags, _lock) \ @@ -110,8 +106,6 @@ extern const struct clk_ops clk_gate_ops; .flags = _flags, \ };
-extern const struct clk_ops clk_divider_ops; - #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \ _flags, _reg, _shift, _width, \ _divider_flags, _lock) \ @@ -143,8 +137,6 @@ extern const struct clk_ops clk_divider_ops; .flags = _flags, \ };
-extern const struct clk_ops clk_mux_ops; - #define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \ _reg, _shift, _width, \ _mux_flags, _lock) \ diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 5508897..6eb8e5d 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -149,6 +149,7 @@ struct clk_fixed_rate { u8 flags; };
+extern const struct clk_ops clk_fixed_rate_ops; struct clk *clk_register_fixed_rate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, unsigned long fixed_rate); @@ -180,6 +181,7 @@ struct clk_gate {
#define CLK_GATE_SET_TO_DISABLE BIT(0)
+extern const struct clk_ops clk_gate_ops; struct clk *clk_register_gate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 bit_idx, @@ -218,6 +220,7 @@ struct clk_divider { #define CLK_DIVIDER_ONE_BASED BIT(0) #define CLK_DIVIDER_POWER_OF_TWO BIT(1)
+extern const struct clk_ops clk_divider_ops; struct clk *clk_register_divider(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 shift, u8 width, @@ -252,6 +255,7 @@ struct clk_mux { #define CLK_MUX_INDEX_ONE BIT(0) #define CLK_MUX_INDEX_BIT BIT(1)
+extern const struct clk_ops clk_mux_ops; struct clk *clk_register_mux(struct device *dev, const char *name, char **parent_names, u8 num_parents, unsigned long flags, void __iomem *reg, u8 shift, u8 width,
On 4/12/2012 6:32 AM, Mike Turquette wrote:
From: Shawn Guo shawn.guo@linaro.org
Besides the static initialization, the clk_ops of basic clks could also be used by particular clk type being subclass of the basic clks.
For example, clk_busy_divider has the same clk_ops as clk_divider, except it has to wait for a busy bit before return success with .set_rate. clk_busy_divider will somehow reuse clk_ops of clk_divider.
Since clk-provider.h is included by clk-private.h, it's safe to move those clk_ops declaration of basic clks form clk-private.h into clk-provider.h, so that implementation of clks like clk_busy_divider above do not need to include clk-private.h to access those clk_ops.
Signed-off-by: Shawn Guo shawn.guo@linaro.org Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com
include/linux/clk-private.h | 8 -------- include/linux/clk-provider.h | 4 ++++ 2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index 5f4ccd7..f19fee0 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -55,8 +55,6 @@ struct clk {
- alternative macro for static initialization
*/ -extern const struct clk_ops clk_fixed_rate_ops;
#define DEFINE_CLK_FIXED_RATE(_name, _flags, _rate, \ _fixed_rate_flags) \ static struct clk _name; \ @@ -78,8 +76,6 @@ extern const struct clk_ops clk_fixed_rate_ops; .flags = _flags, \ }; -extern const struct clk_ops clk_gate_ops;
#define DEFINE_CLK_GATE(_name, _parent_name, _parent_ptr, \ _flags, _reg, _bit_idx, \ _gate_flags, _lock) \ @@ -110,8 +106,6 @@ extern const struct clk_ops clk_gate_ops; .flags = _flags, \ }; -extern const struct clk_ops clk_divider_ops;
#define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \ _flags, _reg, _shift, _width, \ _divider_flags, _lock) \ @@ -143,8 +137,6 @@ extern const struct clk_ops clk_divider_ops; .flags = _flags, \ }; -extern const struct clk_ops clk_mux_ops;
#define DEFINE_CLK_MUX(_name, _parent_names, _parents, _flags, \ _reg, _shift, _width, \ _mux_flags, _lock) \ diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 5508897..6eb8e5d 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -149,6 +149,7 @@ struct clk_fixed_rate { u8 flags; }; +extern const struct clk_ops clk_fixed_rate_ops; struct clk *clk_register_fixed_rate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, unsigned long fixed_rate); @@ -180,6 +181,7 @@ struct clk_gate { #define CLK_GATE_SET_TO_DISABLE BIT(0) +extern const struct clk_ops clk_gate_ops; struct clk *clk_register_gate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 bit_idx, @@ -218,6 +220,7 @@ struct clk_divider { #define CLK_DIVIDER_ONE_BASED BIT(0) #define CLK_DIVIDER_POWER_OF_TWO BIT(1) +extern const struct clk_ops clk_divider_ops; struct clk *clk_register_divider(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 shift, u8 width, @@ -252,6 +255,7 @@ struct clk_mux { #define CLK_MUX_INDEX_ONE BIT(0) #define CLK_MUX_INDEX_BIT BIT(1) +extern const struct clk_ops clk_mux_ops; struct clk *clk_register_mux(struct device *dev, const char *name, char **parent_names, u8 num_parents, unsigned long flags, void __iomem *reg, u8 shift, u8 width,
Reviewed-by: Viresh Kumar viresh.kumar@st.com
From: Rajendra Nayak rnayak@ti.com
Most users of clk_get_rate() actually assume a non zero return value as a valid rate returned. Returing -EINVAL might confuse such users, so make it instead return zero on error.
Besides the return value of clk_get_rate seems to be 'unsigned long'.
Signed-off-by: Rajendra nayak rnayak@ti.com Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 9924aec..a24b121 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -285,7 +285,7 @@ unsigned long __clk_get_rate(struct clk *clk) unsigned long ret;
if (!clk) { - ret = -EINVAL; + ret = 0; goto out; }
@@ -295,7 +295,7 @@ unsigned long __clk_get_rate(struct clk *clk) goto out;
if (!clk->parent) - ret = -ENODEV; + ret = 0;
out: return ret; @@ -560,7 +560,7 @@ EXPORT_SYMBOL_GPL(clk_enable); * @clk: the clk whose rate is being returned * * Simply returns the cached rate of the clk. Does not query the hardware. If - * clk is NULL then returns -EINVAL. + * clk is NULL then returns 0. */ unsigned long clk_get_rate(struct clk *clk) {
On 4/12/2012 6:32 AM, Mike Turquette wrote:
From: Rajendra Nayak rnayak@ti.com
Most users of clk_get_rate() actually assume a non zero return value as a valid rate returned. Returing -EINVAL might confuse such users, so make it instead return zero on error.
Besides the return value of clk_get_rate seems to be 'unsigned long'.
Signed-off-by: Rajendra nayak rnayak@ti.com Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Viresh Kumar viresh.kumar@st.com
drivers/clk/clk.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 9924aec..a24b121 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -285,7 +285,7 @@ unsigned long __clk_get_rate(struct clk *clk) unsigned long ret; if (!clk) {
ret = -EINVAL;
ret = 0;@@ -295,7 +295,7 @@ unsigned long __clk_get_rate(struct clk *clk) goto out; if (!clk->parent)
ret = -ENODEV;
ret = 0;out: return ret; @@ -560,7 +560,7 @@ EXPORT_SYMBOL_GPL(clk_enable);
- @clk: the clk whose rate is being returned
- Simply returns the cached rate of the clk. Does not query the hardware. If
- clk is NULL then returns -EINVAL.
*/
- clk is NULL then returns 0.
unsigned long clk_get_rate(struct clk *clk) {
Reviewed-by: Viresh Kumar viresh.kumar@st.com
From: Mark Brown broonie@opensource.wolfsonmicro.com
The comment is inaccurate (it actually ends the CONFIG_COMMON_CLK section, there's no else) and given that we've just got a single level of ifdef isn't really needed anyway.
Signed-off-by: Mark Brown broonie@opensource.wolfsonmicro.com Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- include/linux/clk.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/clk.h b/include/linux/clk.h index b025272..c9547d9 100644 --- a/include/linux/clk.h +++ b/include/linux/clk.h @@ -81,7 +81,7 @@ int clk_notifier_register(struct clk *clk, struct notifier_block *nb);
int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb);
-#endif /* !CONFIG_COMMON_CLK */ +#endif
/** * clk_get - lookup and obtain a reference to a clock producer.
From: Mark Brown broonie@opensource.wolfsonmicro.com
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 [mturquette@linaro.org: constified gate] Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk-mux.c | 2 +- drivers/clk/clk.c | 2 +- include/linux/clk-private.h | 2 +- include/linux/clk-provider.h | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index 5424488..bd5e598 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -89,7 +89,7 @@ const 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 a24b121..ddade87 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1328,7 +1328,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 f19fee0..e9c8b98 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 6eb8e5d..8981435 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -176,7 +176,7 @@ struct clk_gate { u8 bit_idx; u8 flags; spinlock_t *lock; - char *parent[1]; + const char *parent[1]; };
#define CLK_GATE_SET_TO_DISABLE BIT(0) @@ -214,7 +214,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) @@ -257,7 +257,7 @@ struct clk_mux {
extern const struct clk_ops 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);
@@ -278,7 +278,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);
This patch cleans up clk_register and solves a few bugs by teaching clk_register and __clk_init to return error codes (instead of just NULL) to better align with the existing clk.h api.
Along with that change this patch also introduces a new behavior whereby clk_register copies the parent_names array, thus allowing platforms to declare their parent_names arrays as __initdata.
Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk.c | 61 +++++++++++++++++++++++++++++++++-------- include/linux/clk-private.h | 4 ++- include/linux/clk-provider.h | 3 +- 3 files changed, 54 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index ddade87..af2bf12 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1185,34 +1185,41 @@ EXPORT_SYMBOL_GPL(clk_set_parent); * very large numbers of clocks that need to be statically initialized. It is * a layering violation to include clk-private.h from any code which implements * a clock's .ops; as such any statically initialized clock data MUST be in a - * separate C file from the logic that implements it's operations. + * separate C file from the logic that implements it's operations. Returns 0 + * on success, otherwise an error code. */ -void __clk_init(struct device *dev, struct clk *clk) +int __clk_init(struct device *dev, struct clk *clk) { - int i; + int i, ret = 0; struct clk *orphan; struct hlist_node *tmp, *tmp2;
if (!clk) - return; + return -EINVAL;
mutex_lock(&prepare_lock);
/* check to see if a clock with this name is already registered */ - if (__clk_lookup(clk->name)) + if (__clk_lookup(clk->name)) { + pr_debug("%s: clk %s already initialized\n", + __func__, clk->name); + ret = -EEXIST; goto out; + }
/* check that clk_ops are sane. See Documentation/clk.txt */ if (clk->ops->set_rate && !(clk->ops->round_rate && clk->ops->recalc_rate)) { pr_warning("%s: %s must implement .round_rate & .recalc_rate\n", __func__, clk->name); + ret = -EINVAL; goto out; }
if (clk->ops->set_parent && !clk->ops->get_parent) { pr_warning("%s: %s must implement .get_parent & .set_parent\n", __func__, clk->name); + ret = -EINVAL; goto out; }
@@ -1308,7 +1315,7 @@ void __clk_init(struct device *dev, struct clk *clk) out: mutex_unlock(&prepare_lock);
- return; + return ret; }
/** @@ -1324,29 +1331,59 @@ out: * clk_register is the primary interface for populating the clock tree with new * clock nodes. It returns a pointer to the newly allocated struct clk which * cannot be dereferenced by driver code but may be used in conjuction with the - * rest of the clock API. + * rest of the clock API. In the event of an error clk_register will return an + * error code; drivers must test for an error code after calling clk_register. */ 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 i, ret = -ENOMEM; struct clk *clk;
clk = kzalloc(sizeof(*clk), GFP_KERNEL); - if (!clk) - return NULL; + if (!clk) { + pr_err("%s: could not allocate clk\n", __func__); + goto fail_out; + }
clk->name = name; clk->ops = ops; clk->hw = hw; clk->flags = flags; - clk->parent_names = parent_names; clk->num_parents = num_parents; hw->clk = clk;
- __clk_init(dev, clk); + /* allocate local copy in case parent_names is __initdata */ + clk->parent_names = kzalloc((sizeof(char*) * num_parents), + GFP_KERNEL); + + if (!clk->parent_names) { + pr_err("%s: could not allocate clk->parent_names\n", __func__); + goto fail_parent_names; + } + + /* copy each string name in case parent_names is __initdata */ + for (i = 0; i < num_parents; i++) { + clk->parent_names[i] = kstrdup(parent_names[i], GFP_KERNEL); + if (!clk->parent_names[i]) { + pr_err("%s: could not copy parent_names\n", __func__); + goto fail_parent_names_copy; + } + } + + ret = __clk_init(dev, clk); + if (!ret) + return clk;
- return clk; +fail_parent_names_copy: + while (--i >= 0) + kfree(clk->parent_names[i]); + kfree(clk->parent_names); +fail_parent_names: + kfree(clk); +fail_out: + return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(clk_register);
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index e9c8b98..e7032fd 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -181,8 +181,10 @@ struct clk { * * It is not necessary to call clk_register if __clk_init is used directly with * statically initialized clock data. + * + * Returns 0 on success, otherwise an error code. */ -void __clk_init(struct device *dev, struct clk *clk); +int __clk_init(struct device *dev, struct clk *clk);
#endif /* CONFIG_COMMON_CLK */ #endif /* CLK_PRIVATE_H */ diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 8981435..97f9fab 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -274,7 +274,8 @@ struct clk *clk_register_mux(struct device *dev, const char *name, * clk_register is the primary interface for populating the clock tree with new * clock nodes. It returns a pointer to the newly allocated struct clk which * cannot be dereferenced by driver code but may be used in conjuction with the - * rest of the clock API. + * rest of the clock API. In the event of an error clk_register will return an + * error code; drivers must test for an error code after calling clk_register. */ struct clk *clk_register(struct device *dev, const char *name, const struct clk_ops *ops, struct clk_hw *hw,
On Wed, Apr 11, 2012 at 06:02:50PM -0700, Mike Turquette wrote:
This patch cleans up clk_register and solves a few bugs by teaching clk_register and __clk_init to return error codes (instead of just NULL) to better align with the existing clk.h api.
Along with that change this patch also introduces a new behavior whereby clk_register copies the parent_names array, thus allowing platforms to declare their parent_names arrays as __initdata.
Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com
drivers/clk/clk.c | 61 +++++++++++++++++++++++++++++++++-------- include/linux/clk-private.h | 4 ++- include/linux/clk-provider.h | 3 +- 3 files changed, 54 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index ddade87..af2bf12 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1185,34 +1185,41 @@ EXPORT_SYMBOL_GPL(clk_set_parent);
- very large numbers of clocks that need to be statically initialized. It is
- a layering violation to include clk-private.h from any code which implements
- a clock's .ops; as such any statically initialized clock data MUST be in a
- separate C file from the logic that implements it's operations.
- separate C file from the logic that implements it's operations. Returns 0
*/
- on success, otherwise an error code.
-void __clk_init(struct device *dev, struct clk *clk) +int __clk_init(struct device *dev, struct clk *clk) {
- int i;
- int i, ret = 0; struct clk *orphan; struct hlist_node *tmp, *tmp2;
if (!clk)
return;
return -EINVAL;mutex_lock(&prepare_lock); /* check to see if a clock with this name is already registered */
- if (__clk_lookup(clk->name))
- if (__clk_lookup(clk->name)) {
pr_debug("%s: clk %s already initialized\n",__func__, clk->name);ret = -EEXIST;- }
/* check that clk_ops are sane. See Documentation/clk.txt */ if (clk->ops->set_rate && !(clk->ops->round_rate && clk->ops->recalc_rate)) { pr_warning("%s: %s must implement .round_rate & .recalc_rate\n", __func__, clk->name);
ret = -EINVAL;if (clk->ops->set_parent && !clk->ops->get_parent) { pr_warning("%s: %s must implement .get_parent & .set_parent\n", __func__, clk->name);
ret = -EINVAL;@@ -1308,7 +1315,7 @@ void __clk_init(struct device *dev, struct clk *clk) out: mutex_unlock(&prepare_lock);
- return;
- return ret;
} /** @@ -1324,29 +1331,59 @@ out:
- clk_register is the primary interface for populating the clock tree with new
- clock nodes. It returns a pointer to the newly allocated struct clk which
- cannot be dereferenced by driver code but may be used in conjuction with the
- rest of the clock API.
- rest of the clock API. In the event of an error clk_register will return an
*/
- error code; drivers must test for an error code after calling clk_register.
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 i, ret = -ENOMEM;
I suggest to move the initialization of ret from here...
struct clk *clk; clk = kzalloc(sizeof(*clk), GFP_KERNEL);
- if (!clk)
return NULL;
- if (!clk) {
pr_err("%s: could not allocate clk\n", __func__);goto fail_out;- }
clk->name = name; clk->ops = ops; clk->hw = hw; clk->flags = flags;
- clk->parent_names = parent_names; clk->num_parents = num_parents; hw->clk = clk;
- __clk_init(dev, clk);
- /* allocate local copy in case parent_names is __initdata */
- clk->parent_names = kzalloc((sizeof(char*) * num_parents),
GFP_KERNEL);- if (!clk->parent_names) {
pr_err("%s: could not allocate clk->parent_names\n", __func__);goto fail_parent_names;- }
- /* copy each string name in case parent_names is __initdata */
... to here.
The rationale is that when this code is changed later someone might use ret above and doesn't remember that the code below expects ret to be initialized with -ENOMEM. Also it's easier to see that the code is correct.
Sascha
- for (i = 0; i < num_parents; i++) {
clk->parent_names[i] = kstrdup(parent_names[i], GFP_KERNEL);if (!clk->parent_names[i]) {pr_err("%s: could not copy parent_names\n", __func__);goto fail_parent_names_copy;}- }
- ret = __clk_init(dev, clk);
- if (!ret)
return clk;
- return clk;
+fail_parent_names_copy:
- while (--i >= 0)
kfree(clk->parent_names[i]);- kfree(clk->parent_names);
+fail_parent_names:
- kfree(clk);
+fail_out:
- return ERR_PTR(ret);
} EXPORT_SYMBOL_GPL(clk_register); diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h index e9c8b98..e7032fd 100644 --- a/include/linux/clk-private.h +++ b/include/linux/clk-private.h @@ -181,8 +181,10 @@ struct clk {
- It is not necessary to call clk_register if __clk_init is used directly with
- statically initialized clock data.
*/
- Returns 0 on success, otherwise an error code.
-void __clk_init(struct device *dev, struct clk *clk); +int __clk_init(struct device *dev, struct clk *clk); #endif /* CONFIG_COMMON_CLK */ #endif /* CLK_PRIVATE_H */ diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 8981435..97f9fab 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -274,7 +274,8 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
- clk_register is the primary interface for populating the clock tree with new
- clock nodes. It returns a pointer to the newly allocated struct clk which
- cannot be dereferenced by driver code but may be used in conjuction with the
- rest of the clock API.
- rest of the clock API. In the event of an error clk_register will return an
*/
- error code; drivers must test for an error code after calling clk_register.
struct clk *clk_register(struct device *dev, const char *name, const struct clk_ops *ops, struct clk_hw *hw, -- 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 Mon, Apr 16, 2012 at 1:30 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Wed, Apr 11, 2012 at 06:02:50PM -0700, Mike Turquette wrote:
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 i, ret = -ENOMEM;
I suggest to move the initialization of ret from here...
struct clk *clk;
clk = kzalloc(sizeof(*clk), GFP_KERNEL);
- if (!clk)
- return NULL;
- if (!clk) {
- pr_err("%s: could not allocate clk\n", __func__);
- goto fail_out;
- }
clk->name = name; clk->ops = ops; clk->hw = hw; clk->flags = flags;
- clk->parent_names = parent_names;
clk->num_parents = num_parents; hw->clk = clk;
- __clk_init(dev, clk);
- /* allocate local copy in case parent_names is __initdata */
- clk->parent_names = kzalloc((sizeof(char*) * num_parents),
- GFP_KERNEL);
- if (!clk->parent_names) {
- pr_err("%s: could not allocate clk->parent_names\n", __func__);
- goto fail_parent_names;
- }
- /* copy each string name in case parent_names is __initdata */
... to here.
The rationale is that when this code is changed later someone might use ret above and doesn't remember that the code below expects ret to be initialized with -ENOMEM. Also it's easier to see that the code is correct.
That is sensible.
Thanks, Mike
Sascha
This patch is the basic clk version of 'clk: core: copy parent_names & return error codes'.
The registration functions are changed to allow the core code to copy the array of strings and allow platforms to declare those arrays as __initdata.
This patch also converts all of the basic clk registration functions to return error codes which better aligns them with the existing clk.h api.
Signed-off-by: Mike Turquette mturquette@linaro.org Cc: Arnd Bergman arnd.bergmann@linaro.org Cc: Olof Johansson olof@lixom.net Cc: Russell King linux@arm.linux.org.uk Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Shawn Guo shawn.guo@freescale.com Cc: Richard Zhao richard.zhao@linaro.org Cc: Saravana Kannan skannan@codeaurora.org Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Andrew Lunn andrew@lunn.ch Cc: Rajendra Nayak rnayak@ti.com Cc: Viresh Kumar viresh.kumar@st.com --- drivers/clk/clk-divider.c | 46 ++++++++++++++++++++++++++--------- drivers/clk/clk-fixed-rate.c | 54 ++++++++++++++++++++++++++--------------- drivers/clk/clk-gate.c | 48 +++++++++++++++++++++++++++---------- drivers/clk/clk-mux.c | 8 +++++- include/linux/clk-provider.h | 2 - 5 files changed, 110 insertions(+), 48 deletions(-)
diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c index b1c4b02..add784b 100644 --- a/drivers/clk/clk-divider.c +++ b/drivers/clk/clk-divider.c @@ -153,16 +153,29 @@ const struct clk_ops clk_divider_ops = { }; EXPORT_SYMBOL_GPL(clk_divider_ops);
+/** + * clk_register_divider - register a divider clock with the clock framework + * @dev: device registering this clock + * @name: name of this clock + * @parent_name: name of clock's parent + * @flags: framework-specific flags + * @reg: register address to adjust divider + * @shift: number of bits to shift the bitfield + * @width: width of the bitfield + * @clk_divider_flags: divider-specific flags for this clock + * @lock: shared register lock for this clock + */ struct clk *clk_register_divider(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags, spinlock_t *lock) { struct clk_divider *div; - struct clk *clk; + struct clk *clk = ERR_PTR(-ENOMEM); + const char *parent_names[1];
+ /* allocate the divider */ div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL); - if (!div) { pr_err("%s: could not allocate divider clk\n", __func__); return NULL; @@ -175,23 +188,32 @@ struct clk *clk_register_divider(struct device *dev, const char *name, div->flags = clk_divider_flags; div->lock = lock;
+ /* allocate the temporary parent_names */ if (parent_name) { - div->parent[0] = kstrdup(parent_name, GFP_KERNEL); - if (!div->parent[0]) - goto out; + parent_names[0] = kstrdup(parent_name, GFP_KERNEL); + if (!parent_names[0]) { + pr_err("%s: could not allocate parent_names\n", + __func__); + goto fail_parent_names; + } }
+ /* register the clock */ clk = clk_register(dev, name, &clk_divider_ops, &div->hw, - div->parent, + (parent_name ? parent_names: NULL), (parent_name ? 1 : 0), flags); - if (clk) - return clk;
-out: - kfree(div->parent[0]); - kfree(div); + /* free the temporary parent_names */ + if (parent_name) + kfree(parent_names[0]); + + if (!IS_ERR(clk)) + goto out;
- return NULL; +fail_parent_names: + kfree(div); +out: + return clk; } diff --git a/drivers/clk/clk-fixed-rate.c b/drivers/clk/clk-fixed-rate.c index 027e477..ecd20ae 100644 --- a/drivers/clk/clk-fixed-rate.c +++ b/drivers/clk/clk-fixed-rate.c @@ -38,44 +38,58 @@ const struct clk_ops clk_fixed_rate_ops = { }; EXPORT_SYMBOL_GPL(clk_fixed_rate_ops);
+/** + * clk_register_fixed_rate - register fixed-rate clock with the clock framework + * @dev: device that is registering this clock + * @name: name of this clock + * @parent_name: name of clock's parent + * @flags: framework-specific flags + * @fixed_rate: non-adjustable clock rate + */ struct clk *clk_register_fixed_rate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, unsigned long fixed_rate) { struct clk_fixed_rate *fixed; - char **parent_names = NULL; - u8 len; + struct clk *clk = ERR_PTR(-ENOMEM); + const char *parent_names[1];
+ /* allocate fixed-rate clock */ fixed = kzalloc(sizeof(struct clk_fixed_rate), GFP_KERNEL); - if (!fixed) { pr_err("%s: could not allocate fixed clk\n", __func__); - return ERR_PTR(-ENOMEM); + goto out; }
/* struct clk_fixed_rate assignments */ fixed->fixed_rate = fixed_rate;
+ /* allocate the temporary parent_names */ if (parent_name) { - parent_names = kmalloc(sizeof(char *), GFP_KERNEL); - - if (! parent_names) - goto out; - - len = sizeof(char) * strlen(parent_name); - - parent_names[0] = kmalloc(len, GFP_KERNEL); - - if (!parent_names[0]) - goto out; - - strncpy(parent_names[0], parent_name, len); + parent_names[0] = kstrdup(parent_name, GFP_KERNEL); + if (!parent_names[0]) { + pr_err("%s: could not allocate parent_names\n", + __func__); + goto fail_parent_names; + } }
-out: - return clk_register(dev, name, + /* register the clock */ + clk = clk_register(dev, name, &clk_fixed_rate_ops, &fixed->hw, - parent_names, + (parent_name ? parent_names : NULL), (parent_name ? 1 : 0), flags); + + /* free the temporary parent_names */ + if (parent_name) + kfree(parent_names[0]); + + if (!IS_ERR(clk)) + goto out; + +fail_parent_names: + kfree(fixed); +out: + return clk; } diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index fe2ff9e..288fb5e 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -105,19 +105,31 @@ const struct clk_ops clk_gate_ops = { }; EXPORT_SYMBOL_GPL(clk_gate_ops);
+/** + * clk_register_gate - register a gate clock with the clock framework + * @dev: device that is registering this clock + * @name: name of this clock + * @parent_name: name of this clock's parent + * @flags: framework-specific flags for this clock + * @reg: register address to control gating of this clock + * @bit_idx: which bit in the register controls gating of this clock + * @clk_gate_flags: gate-specific flags for this clock + * @lock: shared register lock for this clock + */ struct clk *clk_register_gate(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 bit_idx, u8 clk_gate_flags, spinlock_t *lock) { struct clk_gate *gate; - struct clk *clk; + struct clk *clk = ERR_PTR(-ENOMEM); + const char *parent_names[1];
+ /* allocate the gate */ gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL); - if (!gate) { pr_err("%s: could not allocate gated clk\n", __func__); - return NULL; + goto out; }
/* struct clk_gate assignments */ @@ -126,22 +138,32 @@ struct clk *clk_register_gate(struct device *dev, const char *name, gate->flags = clk_gate_flags; gate->lock = lock;
+ /* allocate the temporary parent_names */ if (parent_name) { - gate->parent[0] = kstrdup(parent_name, GFP_KERNEL); - if (!gate->parent[0]) - goto out; + parent_names[0] = kstrdup(parent_name, GFP_KERNEL); + if (!parent_names[0]) { + pr_err("%s: could not allocate parent_names\n", + __func__); + goto fail_parent_names; + } }
+ /* register the clock */ clk = clk_register(dev, name, &clk_gate_ops, &gate->hw, - gate->parent, + (parent_name ? parent_names : NULL), (parent_name ? 1 : 0), flags); - if (clk) - return clk; -out: - kfree(gate->parent[0]); - kfree(gate);
- return NULL; + /* free the temporary parent_names */ + if (parent_name) + kfree(parent_names[0]); + + if (!IS_ERR(clk)) + goto out; + +fail_parent_names: + kfree(gate); +out: + return clk; } diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c index bd5e598..5b237b6 100644 --- a/drivers/clk/clk-mux.c +++ b/drivers/clk/clk-mux.c @@ -94,6 +94,7 @@ struct clk *clk_register_mux(struct device *dev, const char *name, u8 clk_mux_flags, spinlock_t *lock) { struct clk_mux *mux; + struct clk *clk;
mux = kzalloc(sizeof(struct clk_mux), GFP_KERNEL);
@@ -109,6 +110,11 @@ struct clk *clk_register_mux(struct device *dev, const char *name, mux->flags = clk_mux_flags; mux->lock = lock;
- return clk_register(dev, name, &clk_mux_ops, &mux->hw, + clk = clk_register(dev, name, &clk_mux_ops, &mux->hw, parent_names, num_parents, flags); + + if (IS_ERR(clk)) + kfree(mux); + + return clk; } diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 97f9fab..3323d24 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -176,7 +176,6 @@ struct clk_gate { u8 bit_idx; u8 flags; spinlock_t *lock; - const char *parent[1]; };
#define CLK_GATE_SET_TO_DISABLE BIT(0) @@ -214,7 +213,6 @@ struct clk_divider { u8 width; u8 flags; spinlock_t *lock; - const char *parent[1]; };
#define CLK_DIVIDER_ONE_BASED BIT(0)
On Wed, Apr 11, 2012 at 06:02:51PM -0700, Mike Turquette wrote: ...
@@ -175,23 +188,32 @@ struct clk *clk_register_divider(struct device *dev, const char *name, div->flags = clk_divider_flags; div->lock = lock;
- /* allocate the temporary parent_names */ if (parent_name) {
div->parent[0] = kstrdup(parent_name, GFP_KERNEL);if (!div->parent[0])goto out;
parent_names[0] = kstrdup(parent_name, GFP_KERNEL);if (!parent_names[0]) {pr_err("%s: could not allocate parent_names\n",__func__);goto fail_parent_names;}
Why do we need to copy the parent_names here at all? clk_register() has done that for each basic clk.
Regards, Shawn
- /* register the clock */ clk = clk_register(dev, name, &clk_divider_ops, &div->hw,
div->parent,
(parent_name ? parent_names: NULL), (parent_name ? 1 : 0), flags);
- if (clk)
return clk;-out:
- kfree(div->parent[0]);
- kfree(div);
- /* free the temporary parent_names */
- if (parent_name)
kfree(parent_names[0]);- if (!IS_ERR(clk))
goto out;
- return NULL;
+fail_parent_names:
- kfree(div);
+out:
- return clk;
}
On Wed, Apr 11, 2012 at 11:49 PM, Shawn Guo shawn.guo@linaro.org wrote:
On Wed, Apr 11, 2012 at 06:02:51PM -0700, Mike Turquette wrote: ...
@@ -175,23 +188,32 @@ struct clk *clk_register_divider(struct device *dev, const char *name, div->flags = clk_divider_flags; div->lock = lock;
- /* allocate the temporary parent_names */
if (parent_name) {
- div->parent[0] = kstrdup(parent_name, GFP_KERNEL);
- if (!div->parent[0])
- goto out;
- parent_names[0] = kstrdup(parent_name, GFP_KERNEL);
- if (!parent_names[0]) {
- pr_err("%s: could not allocate parent_names\n",
- __func__);
- goto fail_parent_names;
- }
}
Why do we need to copy the parent_names here at all? clk_register() has done that for each basic clk.
Yes, this was a braindead change on my part. I'll remove the kstrdup in my next series (the rest of this patch will stay in).
Thanks, Mike
Regards, Shawn
On 17 April 2012 07:10, Turquette, Mike mturquette@ti.com wrote: ...
Yes, this was a braindead change on my part. I'll remove the kstrdup in my next series (the rest of this patch will stay in).
Do you have an ETA on that? A few platform porting are waiting for a stable branch with all necessary fixup/cleanup integrated to publish the patches.
Regards, Shawn
On Mon, Apr 16, 2012 at 6:46 PM, Shawn Guo shawn.guo@linaro.org wrote:
On 17 April 2012 07:10, Turquette, Mike mturquette@ti.com wrote: ...
Yes, this was a braindead change on my part. I'll remove the kstrdup in my next series (the rest of this patch will stay in).
Do you have an ETA on that? A few platform porting are waiting for a stable branch with all necessary fixup/cleanup integrated to publish the patches.
That is a good question. I think it is worth waiting on Saravana's patch which exposes non-private members of struct clk via struct clk_hw. This will have an effect on both platform clock data and code.
I do not think that there is a point in pushing another series out until I get that in since it will shake things up for platforms trying to convert over. And due to the invasive nature I'm still not sure if this stuff will go into an -rc or some -next branch for 3.5. I don't see the harm in keeping it in a -next branch for 3.5 where all platforms can base on that branch since they will be queuing up for 3.5 anyway.
Regards, Mike
Regards, Shawn
On 17 April 2012 11:50, Turquette, Mike mturquette@ti.com wrote:
That is a good question. I think it is worth waiting on Saravana's patch which exposes non-private members of struct clk via struct clk_hw. This will have an effect on both platform clock data and code.
Saravana,
(*nudge*) (*nudge*) goes to you now :)
Regards, Shawn
On 04/17/2012 12:17 AM, Shawn Guo wrote:
On 17 April 2012 11:50, Turquette, Mikemturquette@ti.com wrote:
That is a good question. I think it is worth waiting on Saravana's patch which exposes non-private members of struct clk via struct clk_hw. This will have an effect on both platform clock data and code.
Saravana,
(*nudge*) (*nudge*) goes to you now :)
Regards, Shawn
Sorry :) Will do soon.
-Saravana
On Tue, April 17, 2012 12:17 am, Shawn Guo wrote:
On 17 April 2012 11:50, Turquette, Mike mturquette@ti.com wrote:
That is a good question. I think it is worth waiting on Saravana's patch which exposes non-private members of struct clk via struct clk_hw. This will have an effect on both platform clock data and code.
Saravana,
(*nudge*) (*nudge*) goes to you now :)
Done! I especially like how it turned out.
-Saravana
On Wed, Apr 11, 2012 at 06:02:51PM -0700, Mike Turquette wrote:
This patch is the basic clk version of 'clk: core: copy parent_names & return error codes'.
The registration functions are changed to allow the core code to copy the array of strings and allow platforms to declare those arrays as __initdata.
This patch also converts all of the basic clk registration functions to return error codes which better aligns them with the existing clk.h api.
- */
struct clk *clk_register_divider(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags, spinlock_t *lock) { struct clk_divider *div;
- struct clk *clk;
- struct clk *clk = ERR_PTR(-ENOMEM);
- const char *parent_names[1];
- /* allocate the divider */ div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL);
- if (!div) { pr_err("%s: could not allocate divider clk\n", __func__); return NULL;
You missed a conversion to ERR_PTR here.
Sascha
On Mon, Apr 16, 2012 at 1:52 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Wed, Apr 11, 2012 at 06:02:51PM -0700, Mike Turquette wrote:
This patch is the basic clk version of 'clk: core: copy parent_names & return error codes'.
The registration functions are changed to allow the core code to copy the array of strings and allow platforms to declare those arrays as __initdata.
This patch also converts all of the basic clk registration functions to return error codes which better aligns them with the existing clk.h api.
- */
struct clk *clk_register_divider(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 shift, u8 width, u8 clk_divider_flags, spinlock_t *lock) { struct clk_divider *div;
- struct clk *clk;
struct clk *clk = ERR_PTR(-ENOMEM);
const char *parent_names[1];
/* allocate the divider */
div = kzalloc(sizeof(struct clk_divider), GFP_KERNEL);
if (!div) { pr_err("%s: could not allocate divider clk\n", __func__); return NULL;
You missed a conversion to ERR_PTR here.
Thanks for the catch.
Mike
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 |
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Apr 11, 2012 at 06:02:38PM -0700, Mike Turquette wrote:
This series collects many of the fixes posted for the recently merged common clock framework as well as some general clean-up. Most of the code classifies as a clean-up moreso than a bug fix; hopefully this is not a problem since the common clk framework is new code pulled for 3.4.
Patches are based on v3.4-rc2 and can be pulled from: git://git.linaro.org/people/mturquette/linux.git v3.4-rc2-clk-fixes
Please let me know I missed any critical fixes that were posted to the list already.
Arnd & Olof, if there are no objections to these changes can this get pulled through the arm-soc tree?
I had a look at this series and except for the comment Shawn already made I am fine with it. I also vote for queuing it during -rc since the framework currently has no users and letting it into -rc will help people like me putting SoC support onto it.
Except the last patch:
Acked-by: Sascha Hauer s.hauer@pengutronix.de
Sascha
On Thursday 12 April 2012, Mike Turquette wrote:
This series collects many of the fixes posted for the recently merged common clock framework as well as some general clean-up. Most of the code classifies as a clean-up moreso than a bug fix; hopefully this is not a problem since the common clk framework is new code pulled for 3.4.
Patches are based on v3.4-rc2 and can be pulled from: git://git.linaro.org/people/mturquette/linux.git v3.4-rc2-clk-fixes
Please let me know I missed any critical fixes that were posted to the list already.
Arnd & Olof, if there are no objections to these changes can this get pulled through the arm-soc tree?
I think pulling it in through the arm-soc tree is still ok, but it's borderline because of the size and patch 13 is probably too big, in addition to the comments that were made there.
Let's pull patches 1 through 12 in to a separate series that we don't mix with the other bug fixes. Mike, please send a pull request with the Acks added in.
Arnd
On Thu, Apr 12, 2012 at 11:14:38AM +0000, Arnd Bergmann wrote:
On Thursday 12 April 2012, Mike Turquette wrote:
This series collects many of the fixes posted for the recently merged common clock framework as well as some general clean-up. Most of the code classifies as a clean-up moreso than a bug fix; hopefully this is not a problem since the common clk framework is new code pulled for 3.4.
Patches are based on v3.4-rc2 and can be pulled from: git://git.linaro.org/people/mturquette/linux.git v3.4-rc2-clk-fixes
Please let me know I missed any critical fixes that were posted to the list already.
Arnd & Olof, if there are no objections to these changes can this get pulled through the arm-soc tree?
I think pulling it in through the arm-soc tree is still ok, but it's borderline because of the size and patch 13 is probably too big, in addition to the comments that were made there.
Let's pull patches 1 through 12 in to a separate series that we don't mix with the other bug fixes. Mike, please send a pull request with the Acks added in.
I just appended 3 more patches to the series. Patches #1 and #2 change the interface between clk core and clk drivers - clk_ops a little bit, (this is something Mike acked a couple of weeks ago, but missed from the series) and patch #3 is a critical bug fix. So unless we can send the whole series for -rc, I'd vote we send none of them for -rc. Instead, we can stabilize it somewhere and ask all the clk driver porting base on that.
Sending part of the cleanup/fixup and leaving the other that could require changes on clk drivers out is a bad idea to me.
On Wed, Apr 11, 2012 at 06:02:38PM -0700, Mike Turquette wrote:
This series collects many of the fixes posted for the recently merged common clock framework as well as some general clean-up. Most of the code classifies as a clean-up moreso than a bug fix; hopefully this is not a problem since the common clk framework is new code pulled for 3.4.
Can you please CC people on your cover letters as well as on the individual patches?
On Fri, Apr 13, 2012 at 2:21 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Apr 11, 2012 at 06:02:38PM -0700, Mike Turquette wrote:
This series collects many of the fixes posted for the recently merged common clock framework as well as some general clean-up. Most of the code classifies as a clean-up moreso than a bug fix; hopefully this is not a problem since the common clk framework is new code pulled for 3.4.
Can you please CC people on your cover letters as well as on the individual patches?
Yes.
Regards, Mike
-
Amit Kucheria -
Andrew Lunn -
Arnd Bergmann -
Mark Brown -
Mike Turquette -
Saravana Kannan -
Sascha Hauer -
Shawn Guo -
Turquette, Mike -
Viresh Kumar