[PATCH v10 3/3] clk: Add rate constraints to clocks
From: Stephen Boyd <hidden>
Date: 2015-01-21 00:47:01
Also in:
linux-mips, linux-omap, lkml
It's looking fairly close. Thanks for keeping up with the review comments. On 01/20, Tomeu Vizoso wrote:
quoted hunk ↗ jump to hunk
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index e867d6a..f241e27 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c@@ -2143,6 +2280,10 @@ struct clk *__clk_register(struct device *dev, struct clk_hw *hw) else clk->owner = NULL; + INIT_HLIST_HEAD(&clk->clks); + + hw->clk = __clk_create_clk(hw, NULL, NULL); + ret = __clk_init(dev, hw->clk); if (ret) return ERR_PTR(ret);
Don't we need to __clk_free_clk() here too?
quoted hunk ↗ jump to hunk
@@ -2151,6 +2292,19 @@ struct clk *__clk_register(struct device *dev, struct clk_hw *hw) } EXPORT_SYMBOL_GPL(__clk_register); +static void __clk_free_clk(struct clk *clk) +{ + struct clk_core *core = clk->core; + + clk_prepare_lock(); + hlist_del(&clk->child_node); + clk_prepare_unlock(); + + kfree(clk); + + clk_core_set_rate(core, core->req_rate);
Is it safe to call this during clock registration? I hope that it will just bail out and do nothing because core->rate == core->req_rate. Maybe we can avoid this given my next comment below.
quoted hunk ↗ jump to hunk
+} + /** * clk_register - allocate a new clock, register it and return an opaque cookie * @dev: device that is registering this clock@@ -2210,12 +2364,14 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw) } } + INIT_HLIST_HEAD(&clk->clks); + hw->clk = __clk_create_clk(hw, NULL, NULL); ret = __clk_init(dev, hw->clk); if (!ret) return hw->clk; - kfree(hw->clk); + __clk_free_clk(hw->clk); fail_parent_names_copy: while (--i >= 0) kfree(clk->parent_names[i]);@@ -2421,7 +2577,7 @@ void __clk_put(struct clk *clk) return; clk_core_put(clk->core); - kfree(clk); + __clk_free_clk(clk);
This doesn't look right. First we drop the core reference here with clk_core_put() and then we call __clk_free_clk() which will go and call clk_core_set_rate() on the clk->core which may or may not exist anymore. I'd think we want to do these steps: 1. Unlink clk from clks list 2. Recalculate rate and set if changed 3. Drop kref on core with clk_core_put() 4. kfree the clk -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project