From 1bd37a46770017e89943769112c5f09e5a7b24c1 Mon Sep 17 00:00:00 2001 From: Leonard Crestez Date: Tue, 2 Jul 2019 16:27:09 +0300 Subject: clk: Add clk_min/max_rate entries in debugfs Add two files to expose min/max clk rates as determined by clk_core_get_boundaries, taking all consumer requests into account. This information does not appear to be otherwise exposed to userspace. Signed-off-by: Leonard Crestez Link: https://lkml.kernel.org/r/68e96af2df96512300604d797ade2088d7e6e496.1562073871.git.leonard.crestez@nxp.com [sboyd@kernel.org: Drop if statements for JSON printing] Signed-off-by: Stephen Boyd --- drivers/clk/clk.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c0990703ce54..a7cee0fac071 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2896,15 +2896,21 @@ DEFINE_SHOW_ATTRIBUTE(clk_summary); static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) { + unsigned long min_rate, max_rate; + if (!c) return; + clk_core_get_boundaries(c, &min_rate, &max_rate); + /* This should be JSON format, i.e. elements separated with a comma */ seq_printf(s, "\"%s\": { ", c->name); seq_printf(s, "\"enable_count\": %d,", c->enable_count); seq_printf(s, "\"prepare_count\": %d,", c->prepare_count); seq_printf(s, "\"protect_count\": %d,", c->protect_count); seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c)); + seq_printf(s, "\"min_rate\": %lu,", min_rate); + seq_printf(s, "\"max_rate\": %lu,", max_rate); seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c)); seq_printf(s, "\"phase\": %d,", clk_core_get_phase(c)); seq_printf(s, "\"duty_cycle\": %u", @@ -3064,6 +3070,34 @@ static int clk_duty_cycle_show(struct seq_file *s, void *data) } DEFINE_SHOW_ATTRIBUTE(clk_duty_cycle); +static int clk_min_rate_show(struct seq_file *s, void *data) +{ + struct clk_core *core = s->private; + unsigned long min_rate, max_rate; + + clk_prepare_lock(); + clk_core_get_boundaries(core, &min_rate, &max_rate); + clk_prepare_unlock(); + seq_printf(s, "%lu\n", min_rate); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(clk_min_rate); + +static int clk_max_rate_show(struct seq_file *s, void *data) +{ + struct clk_core *core = s->private; + unsigned long min_rate, max_rate; + + clk_prepare_lock(); + clk_core_get_boundaries(core, &min_rate, &max_rate); + clk_prepare_unlock(); + seq_printf(s, "%lu\n", max_rate); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(clk_max_rate); + static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) { struct dentry *root; @@ -3075,6 +3109,8 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) core->dentry = root; debugfs_create_ulong("clk_rate", 0444, root, &core->rate); + debugfs_create_file("clk_min_rate", 0444, root, core, &clk_min_rate_fops); + debugfs_create_file("clk_max_rate", 0444, root, core, &clk_max_rate_fops); debugfs_create_ulong("clk_accuracy", 0444, root, &core->accuracy); debugfs_create_u32("clk_phase", 0444, root, &core->phase); debugfs_create_file("clk_flags", 0444, root, core, &clk_flags_fops); -- cgit v1.2.3 From 9f7767226083d51c0674f1b71ac52daea6778b84 Mon Sep 17 00:00:00 2001 From: Leonard Crestez Date: Tue, 2 Jul 2019 16:27:10 +0300 Subject: clk: Assert prepare_lock in clk_core_get_boundaries This function iterates the clk consumer list on clk_core so it must be called under prepare_lock. This is already done by all callers but add a lockdep assert to check anyway. Signed-off-by: Leonard Crestez Link: https://lkml.kernel.org/r/29453ee8e820457d87a8faf9d496390e59c6826f.1562073871.git.leonard.crestez@nxp.com Signed-off-by: Stephen Boyd --- drivers/clk/clk.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index a7cee0fac071..991fb3a62bda 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -593,6 +593,8 @@ static void clk_core_get_boundaries(struct clk_core *core, { struct clk *clk_user; + lockdep_assert_held(&prepare_lock); + *min_rate = core->min_rate; *max_rate = core->max_rate; -- cgit v1.2.3 From 1ccc0ddf046a0197f2f9acca02a64da10aa6112d Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Mon, 1 Jul 2019 22:20:40 +0200 Subject: clk: Use seq_puts() in possible_parent_show() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A string which did not contain a data format specification should be put into a sequence. Thus use the corresponding function “seq_puts”. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Stephen Boyd --- drivers/clk/clk.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 991fb3a62bda..b6f50db759a4 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3021,15 +3021,15 @@ static void possible_parent_show(struct seq_file *s, struct clk_core *core, */ parent = clk_core_get_parent_by_index(core, i); if (parent) - seq_printf(s, "%s", parent->name); + seq_puts(s, parent->name); else if (core->parents[i].name) - seq_printf(s, "%s", core->parents[i].name); + seq_puts(s, core->parents[i].name); else if (core->parents[i].fw_name) seq_printf(s, "<%s>(fw)", core->parents[i].fw_name); else if (core->parents[i].index >= 0) - seq_printf(s, "%s", - of_clk_get_parent_name(core->of_node, - core->parents[i].index)); + seq_puts(s, + of_clk_get_parent_name(core->of_node, + core->parents[i].index)); else seq_puts(s, "(missing)"); -- cgit v1.2.3 From 0214f33c4e0e1f5a9a9c3f4881310c61713294e6 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Wed, 31 Jul 2019 12:35:17 -0700 Subject: clk: Overwrite clk_hw::init with NULL during clk_register() We don't want clk provider drivers to use the init structure after clk registration time, but we leave a dangling reference to it by means of clk_hw::init. Let's overwrite the member with NULL during clk_register() so that this can't be used anymore after registration time. Cc: Bjorn Andersson Cc: Doug Anderson Signed-off-by: Stephen Boyd Link: https://lkml.kernel.org/r/20190731193517.237136-10-sboyd@kernel.org Reviewed-by: Sylwester Nawrocki --- drivers/clk/clk.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c0990703ce54..efac620264a2 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3484,9 +3484,9 @@ static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist) return 0; } -static int clk_core_populate_parent_map(struct clk_core *core) +static int clk_core_populate_parent_map(struct clk_core *core, + const struct clk_init_data *init) { - const struct clk_init_data *init = core->hw->init; u8 num_parents = init->num_parents; const char * const *parent_names = init->parent_names; const struct clk_hw **parent_hws = init->parent_hws; @@ -3566,6 +3566,14 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) { int ret; struct clk_core *core; + const struct clk_init_data *init = hw->init; + + /* + * The init data is not supposed to be used outside of registration path. + * Set it to NULL so that provider drivers can't use it either and so that + * we catch use of hw->init early on in the core. + */ + hw->init = NULL; core = kzalloc(sizeof(*core), GFP_KERNEL); if (!core) { @@ -3573,17 +3581,17 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) goto fail_out; } - core->name = kstrdup_const(hw->init->name, GFP_KERNEL); + core->name = kstrdup_const(init->name, GFP_KERNEL); if (!core->name) { ret = -ENOMEM; goto fail_name; } - if (WARN_ON(!hw->init->ops)) { + if (WARN_ON(!init->ops)) { ret = -EINVAL; goto fail_ops; } - core->ops = hw->init->ops; + core->ops = init->ops; if (dev && pm_runtime_enabled(dev)) core->rpm_enabled = true; @@ -3592,13 +3600,13 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw) if (dev && dev->driver) core->owner = dev->driver->owner; core->hw = hw; - core->flags = hw->init->flags; - core->num_parents = hw->init->num_parents; + core->flags = init->flags; + core->num_parents = init->num_parents; core->min_rate = 0; core->max_rate = ULONG_MAX; hw->core = core; - ret = clk_core_populate_parent_map(core); + ret = clk_core_populate_parent_map(core, init); if (ret) goto fail_parents; -- cgit v1.2.3 From ef13e55c27e128b6d84aa939dcc565b1d2ee7724 Mon Sep 17 00:00:00 2001 From: Rishi Gupta Date: Sat, 17 Aug 2019 12:05:59 +0530 Subject: clk: Remove extraneous 'for' word in comments An extra 'for' word is grammatically incorrect in the comment 'verifying ops for multi-parent clks'. This commit removes this extra for word. Signed-off-by: Rishi Gupta Link: https://lkml.kernel.org/r/1566023759-7880-1-git-send-email-gupt21@gmail.com Signed-off-by: Stephen Boyd --- drivers/clk/clk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c0990703ce54..bea50eee9e8c 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2437,7 +2437,7 @@ static int clk_core_set_parent_nolock(struct clk_core *core, if (core->parent == parent) return 0; - /* verify ops for for multi-parent clks */ + /* verify ops for multi-parent clks */ if (core->num_parents > 1 && !core->ops->set_parent) return -EPERM; -- cgit v1.2.3 From 226fd70209451428c563977f5ce022a41d077110 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 26 Aug 2019 14:20:42 -0700 Subject: clk: Document of_parse_clkspec() some more The return value of of_parse_clkspec() is peculiar. If the function is called with a NULL argument for 'name' it will return -ENOENT, but if it's called with a non-NULL argument for 'name' it will return -EINVAL. This peculiarity is documented by commit 5c56dfe63b6e ("clk: Add comment about __of_clk_get_by_name() error values"). Let's further document this function so that it's clear what the return value is and how to use the arguments to parse clk specifiers. Cc: Phil Edworthy Signed-off-by: Stephen Boyd Link: https://lkml.kernel.org/r/20190826212042.48642-1-sboyd@kernel.org Reviewed-by: Phil Edworthy --- drivers/clk/clk.c | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index bea50eee9e8c..4649e036eed2 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -4316,12 +4316,43 @@ void devm_of_clk_del_provider(struct device *dev) } EXPORT_SYMBOL(devm_of_clk_del_provider); -/* - * Beware the return values when np is valid, but no clock provider is found. - * If name == NULL, the function returns -ENOENT. - * If name != NULL, the function returns -EINVAL. This is because - * of_parse_phandle_with_args() is called even if of_property_match_string() - * returns an error. +/** + * of_parse_clkspec() - Parse a DT clock specifier for a given device node + * @np: device node to parse clock specifier from + * @index: index of phandle to parse clock out of. If index < 0, @name is used + * @name: clock name to find and parse. If name is NULL, the index is used + * @out_args: Result of parsing the clock specifier + * + * Parses a device node's "clocks" and "clock-names" properties to find the + * phandle and cells for the index or name that is desired. The resulting clock + * specifier is placed into @out_args, or an errno is returned when there's a + * parsing error. The @index argument is ignored if @name is non-NULL. + * + * Example: + * + * phandle1: clock-controller@1 { + * #clock-cells = <2>; + * } + * + * phandle2: clock-controller@2 { + * #clock-cells = <1>; + * } + * + * clock-consumer@3 { + * clocks = <&phandle1 1 2 &phandle2 3>; + * clock-names = "name1", "name2"; + * } + * + * To get a device_node for `clock-controller@2' node you may call this + * function a few different ways: + * + * of_parse_clkspec(clock-consumer@3, -1, "name2", &args); + * of_parse_clkspec(clock-consumer@3, 1, NULL, &args); + * of_parse_clkspec(clock-consumer@3, 1, "name2", &args); + * + * Return: 0 upon successfully parsing the clock specifier. Otherwise, -ENOENT + * if @name is NULL or -EINVAL if @name is non-NULL and it can't be found in + * the "clock-names" property of @np. */ static int of_parse_clkspec(const struct device_node *np, int index, const char *name, struct of_phandle_args *out_args) -- cgit v1.2.3 From bdcf1dc253248542537a742ae1e7ccafdd03f2d3 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Wed, 28 Aug 2019 11:19:59 -0700 Subject: clk: Evict unregistered clks from parent caches We leave a dangling pointer in each clk_core::parents array that has an unregistered clk as a potential parent when that clk_core pointer is freed by clk{_hw}_unregister(). It is impossible for the true parent of a clk to be set with clk_set_parent() once the dangling pointer is left in the cache because we compare parent pointers in clk_fetch_parent_index() instead of checking for a matching clk name or clk_hw pointer. Before commit ede77858473a ("clk: Remove global clk traversal on fetch parent index"), we would check clk_hw pointers, which has a higher chance of being the same between registration and unregistration, but it can still be allocated and freed by the clk provider. In fact, this has been a long standing problem since commit da0f0b2c3ad2 ("clk: Correct lookup logic in clk_fetch_parent_index()") where we stopped trying to compare clk names and skipped over entries in the cache that weren't NULL. There are good (performance) reasons to not do the global tree lookup in cases where the cache holds dangling pointers to parents that have been unregistered. Let's take the performance hit on the uncommon registration path instead. Loop through all the clk_core::parents arrays when a clk is unregistered and set the entry to NULL when the parent cache entry and clk being unregistered are the same pointer. This will fix this problem and avoid the overhead for the "normal" case. Based on a patch by Bjorn Andersson. Fixes: da0f0b2c3ad2 ("clk: Correct lookup logic in clk_fetch_parent_index()") Reviewed-by: Bjorn Andersson Tested-by: Sai Prakash Ranjan Signed-off-by: Stephen Boyd Link: https://lkml.kernel.org/r/20190828181959.204401-1-sboyd@kernel.org --- drivers/clk/clk.c | 42 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c0990703ce54..f9076c74bf0d 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -37,6 +37,12 @@ static HLIST_HEAD(clk_root_list); static HLIST_HEAD(clk_orphan_list); static LIST_HEAD(clk_notifier_list); +static struct hlist_head *all_lists[] = { + &clk_root_list, + &clk_orphan_list, + NULL, +}; + /*** private data structures ***/ struct clk_parent_map { @@ -2833,12 +2839,6 @@ static int inited = 0; static DEFINE_MUTEX(clk_debug_lock); static HLIST_HEAD(clk_debug_list); -static struct hlist_head *all_lists[] = { - &clk_root_list, - &clk_orphan_list, - NULL, -}; - static struct hlist_head *orphan_list[] = { &clk_orphan_list, NULL, @@ -3737,6 +3737,34 @@ static const struct clk_ops clk_nodrv_ops = { .set_parent = clk_nodrv_set_parent, }; +static void clk_core_evict_parent_cache_subtree(struct clk_core *root, + struct clk_core *target) +{ + int i; + struct clk_core *child; + + for (i = 0; i < root->num_parents; i++) + if (root->parents[i].core == target) + root->parents[i].core = NULL; + + hlist_for_each_entry(child, &root->children, child_node) + clk_core_evict_parent_cache_subtree(child, target); +} + +/* Remove this clk from all parent caches */ +static void clk_core_evict_parent_cache(struct clk_core *core) +{ + struct hlist_head **lists; + struct clk_core *root; + + lockdep_assert_held(&prepare_lock); + + for (lists = all_lists; *lists; lists++) + hlist_for_each_entry(root, *lists, child_node) + clk_core_evict_parent_cache_subtree(root, core); + +} + /** * clk_unregister - unregister a currently registered clock * @clk: clock to unregister @@ -3775,6 +3803,8 @@ void clk_unregister(struct clk *clk) clk_core_set_parent_nolock(child, NULL); } + clk_core_evict_parent_cache(clk->core); + hlist_del_init(&clk->core->child_node); if (clk->core->prepare_count) -- cgit v1.2.3 From 7f4804665b58ced5d09848785d835af0f7a51b3e Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 26 Aug 2019 16:47:29 -0700 Subject: clk: Drop !clk checks in debugfs dumping These recursive functions have checks for !clk being passed in, but the callers are always looping through lists and therefore the pointers can't be NULL. Drop the checks to simplify the code. Signed-off-by: Stephen Boyd Link: https://lkml.kernel.org/r/20190826234729.145593-1-sboyd@kernel.org --- drivers/clk/clk.c | 12 ------------ 1 file changed, 12 deletions(-) (limited to 'drivers/clk/clk.c') diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index b6f50db759a4..7783c25fb407 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2849,9 +2849,6 @@ static struct hlist_head *orphan_list[] = { static void clk_summary_show_one(struct seq_file *s, struct clk_core *c, int level) { - if (!c) - return; - seq_printf(s, "%*s%-*s %7d %8d %8d %11lu %10lu %5d %6d\n", level * 3 + 1, "", 30 - level * 3, c->name, @@ -2866,9 +2863,6 @@ static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c, { struct clk_core *child; - if (!c) - return; - clk_summary_show_one(s, c, level); hlist_for_each_entry(child, &c->children, child_node) @@ -2900,9 +2894,6 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) { unsigned long min_rate, max_rate; - if (!c) - return; - clk_core_get_boundaries(c, &min_rate, &max_rate); /* This should be JSON format, i.e. elements separated with a comma */ @@ -2923,9 +2914,6 @@ static void clk_dump_subtree(struct seq_file *s, struct clk_core *c, int level) { struct clk_core *child; - if (!c) - return; - clk_dump_one(s, c, level); hlist_for_each_entry(child, &c->children, child_node) { -- cgit v1.2.3