lkml.org 
[lkml]   [2011]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/4] clk: Implement clk_set_rate
    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 |


    \
     
     \ /
      Last update: 2011-05-20 14:27    [W:0.053 / U:0.860 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site