From: Sascha Hauer <s.hauer@pengutronix.de>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/4] clk: Implement clk_set_rate
Date: Fri, 20 May 2011 12:25:42 +0000 [thread overview]
Message-ID: <20110520122542.GB10931@pengutronix.de> (raw)
In-Reply-To: <1305876469.326620.351525457111.2.gpush@pororo>
On Fri, May 20, 2011 at 03:27:49PM +0800, Jeremy Kerr wrote:
> Implemenent clk_set_rate by adding a set_rate callback to clk_hw_ops,
> and core code to handle propagation of rate changes up and down the
> clock tree.
>
> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>
> ---
> drivers/clk/clk.c | 74 ++++++++++++++++++++++++++++++++++++++++----
> include/linux/clk.h | 12 +++++++
> 2 files changed, 80 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ad90a90..3a4d70e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -21,6 +21,8 @@ struct clk {
> unsigned int enable_count;
> unsigned int prepare_count;
> struct clk *parent;
> + struct hlist_head children;
> + struct hlist_node child_node;
> unsigned long rate;
> };
>
> @@ -176,10 +178,61 @@ long clk_round_rate(struct clk *clk, unsigned long rate)
> }
> EXPORT_SYMBOL_GPL(clk_round_rate);
>
> +/*
> + * clk_recalc_rates - Given a clock (with a recently updated clk->rate),
> + * notify its children that the rate may need to be recalculated, using
> + * ops->recalc_rate().
> + */
> +static void clk_recalc_rates(struct clk *clk)
> +{
> + struct hlist_node *tmp;
> + struct clk *child;
> +
> + if (clk->ops->recalc_rate)
> + clk->rate = clk->ops->recalc_rate(clk->hw);
> +
> + hlist_for_each_entry(child, tmp, &clk->children, child_node)
> + clk_recalc_rates(child);
> +}
> +
> int clk_set_rate(struct clk *clk, unsigned long rate)
> {
> - /* not yet implemented */
> - return -ENOSYS;
> + unsigned long parent_rate, new_rate;
> + int ret;
> +
> + if (!clk->ops->set_rate)
> + return -ENOSYS;
> +
> + new_rate = rate;
> +
> + /* prevent racing with updates to the clock topology */
> + mutex_lock(&prepare_lock);
> +
> +propagate:
> + ret = clk->ops->set_rate(clk->hw, new_rate, &parent_rate);
> +
> + if (ret < 0)
> + return ret;
Should be 'goto out_recalc' instead.
If the first ops->set_rate succeedes but then the parent fails the rate
already has changed. I can't see whether this becomes a problem when
clk_set_rate fails but still changes the rate, but in any way we have to
call clk_recalc_rates() before leaving to keep the tree in sync with the
hardware. Also mutex_unlock is missing.
> +
> + /* ops->set_rate may require the parent's rate to change (to
> + * parent_rate), we need to propagate the set_rate call to the
> + * parent.
> + */
> + if (ret = CLK_SET_RATE_PROPAGATE) {
> + new_rate = parent_rate;
> + clk = clk->parent;
> + goto propagate;
> + }
> +
> + /* If successful (including propagation to the parent clock(s)),
> + * recalculate the rates of the clock, including children. We'll be
> + * calling this on the 'parent-most' clock that we propagated to.
> + */
> + clk_recalc_rates(clk);
> +
> + mutex_unlock(&prepare_lock);
> +
> + return 0;
> }
> EXPORT_SYMBOL_GPL(clk_set_rate);
>
> @@ -213,16 +266,25 @@ struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
> clk->hw = hw;
> hw->clk = clk;
>
> - /* Query the hardware for parent and initial rate */
> + /* Query the hardware for parent and initial rate. We may alter
> + * the clock topology, making this clock available from the parent's
> + * children list. So, we need to protect against concurrent
> + * accesses through set_rate
> + */
> + mutex_lock(&prepare_lock);
>
> - if (clk->ops->get_parent)
> - /* We don't to lock against prepare/enable here, as
> - * the clock is not yet accessible from anywhere */
> + if (clk->ops->get_parent) {
> clk->parent = clk->ops->get_parent(clk->hw);
> + if (clk->parent)
> + hlist_add_head(&clk->child_node,
> + &clk->parent->children);
> + }
>
> if (clk->ops->recalc_rate)
> clk->rate = clk->ops->recalc_rate(clk->hw);
>
> + mutex_unlock(&prepare_lock);
> +
>
> return clk;
> }
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 93ff870..e0969d2 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -58,6 +58,12 @@ struct clk_hw {
> * parent. Currently only called when the clock is first
> * registered.
> *
> + * @set_rate Change the rate of this clock. If this callback returns
> + * CLK_SET_RATE_PROPAGATE, the rate change will be propagated to
> + * the parent clock (which may propagate again). The requested
> + * rate of the parent is passed back from the callback in the
> + * second 'unsigned long *' argument.
> + *
> * The clk_enable/clk_disable and clk_prepare/clk_unprepare pairs allow
> * implementations to split any work between atomic (enable) and sleepable
> * (prepare) contexts. If a clock requires sleeping code to be turned on, this
> @@ -76,9 +82,15 @@ struct clk_hw_ops {
> void (*disable)(struct clk_hw *);
> unsigned long (*recalc_rate)(struct clk_hw *);
> long (*round_rate)(struct clk_hw *, unsigned long);
> + int (*set_rate)(struct clk_hw *,
> + unsigned long, unsigned long *);
> struct clk * (*get_parent)(struct clk_hw *);
> };
>
> +enum {
> + CLK_SET_RATE_PROPAGATE = 1,
> +};
> +
> /**
> * clk_prepare - prepare clock for atomic enabling.
> *
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
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 |
next prev parent reply other threads:[~2011-05-20 12:25 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-20 7:27 [PATCH 0/4] Add a generic struct clk Jeremy Kerr
2011-05-20 7:27 ` [PATCH 4/4] clk: Add simple gated clock Jeremy Kerr
2011-05-20 11:37 ` Arnd Bergmann
2011-05-20 22:19 ` Rob Herring
2011-05-20 7:27 ` [PATCH 2/4] clk: Implement clk_set_rate Jeremy Kerr
2011-05-20 12:25 ` Sascha Hauer [this message]
2011-05-24 7:59 ` Colin Cross
2011-05-25 19:03 ` Sascha Hauer
[not found] ` <1306373867.2875.162.camel@pororo>
2011-05-26 6:54 ` Sascha Hauer
2011-05-30 5:05 ` Mike Frysinger
2011-05-20 7:27 ` [PATCH 3/4] clk: Add fixed-rate clock Jeremy Kerr
2011-05-30 5:01 ` Mike Frysinger
2011-05-30 5:02 ` Mike Frysinger
2011-05-20 7:27 ` [PATCH 1/4] clk: Add a generic clock infrastructure Jeremy Kerr
2011-05-20 11:59 ` Sascha Hauer
2011-05-20 13:25 ` Thomas Gleixner
2011-05-20 13:36 ` Sascha Hauer
2011-05-23 23:55 ` Colin Cross
2011-05-24 7:02 ` Sascha Hauer
2011-05-24 7:51 ` Colin Cross
2011-05-24 8:38 ` Sascha Hauer
2011-05-25 11:22 ` Richard Zhao
2011-05-25 11:43 ` Thomas Gleixner
2011-05-24 4:30 ` viresh kumar
2011-05-25 10:47 ` Richard Zhao
2011-05-30 5:00 ` Mike Frysinger
2011-05-23 23:12 ` [PATCH 0/4] Add a generic struct clk Colin Cross
2011-05-24 6:26 ` Sascha Hauer
2011-05-24 7:31 ` Colin Cross
2011-05-24 8:09 ` Sascha Hauer
2011-05-24 19:41 ` Colin Cross
2011-05-25 2:32 ` Richard Zhao
2011-05-25 6:23 ` Sascha Hauer
2011-05-25 7:51 ` Thomas Gleixner
2011-05-27 14:39 ` Mark Brown
2011-05-24 17:22 ` Richard Zhao
2011-05-24 17:52 ` Colin Cross
2011-05-25 2:08 ` Richard Zhao
2011-05-30 5:20 ` Mike Frysinger
2011-07-10 9:09 ` Mark Brown
2011-07-10 9:50 ` Russell King - ARM Linux
2011-07-10 10:00 ` Russell King - ARM Linux
2011-07-10 11:27 ` Mark Brown
2011-07-10 11:52 ` Russell King - ARM Linux
2011-07-11 2:49 ` Jeremy Kerr
2011-07-11 3:57 ` Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110520122542.GB10931@pengutronix.de \
--to=s.hauer@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).