[01/25] clk: Fix debugfs clk_possible_parents for clks without parent string names

Commit Message

Chen-Yu Tsai May 20, 2019, 8:03 a.m. UTC

From: Chen-Yu Tsai <[e-mail]>

Following the commit fc0c209c147f ("clk: Allow parents to be specified
without string names"), the parent name string is not always populated.

Instead, fetch the parents clk_core struct using the appropriate helper,
and read its name directly.

Fixes: fc0c209c147f ("clk: Allow parents to be specified without string names")
Signed-off-by: Chen-Yu Tsai <[e-mail]>
---
 drivers/clk/clk.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Stephen Boyd June 7, 2019, 6:14 p.m. UTC | #1

Quoting Chen-Yu Tsai (2019-05-20 01:03:57)
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index aa51756fd4d6..bdb077ba59b9 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3000,12 +3000,16 @@ DEFINE_SHOW_ATTRIBUTE(clk_flags);
>  static int possible_parents_show(struct seq_file *s, void *data)
>  {
>         struct clk_core *core = s->private;
> +       struct clk_core *parent;
>         int i;
>  
> -       for (i = 0; i < core->num_parents - 1; i++)
> -               seq_printf(s, "%s ", core->parents[i].name);
> +       for (i = 0; i < core->num_parents - 1; i++) {
> +               parent = clk_core_get_parent_by_index(core, i);
> +               seq_printf(s, "%s ", parent ? parent->name : NULL);
> +       }
>  
> -       seq_printf(s, "%s\n", core->parents[i].name);
> +       parent = clk_core_get_parent_by_index(core, i);
> +       seq_printf(s, "%s", parent ? parent->name : NULL);

What do we do if the parent won't appear on this system, but we've
listed it as a possible parent with the '.fw_name' string? Is that not a
"possible" parent because it won't ever appear?

I'm mostly saying that we should try to detect these corner cases and
print something besides NULL. Maybe we could even print the fallback
'.name' if clk_core_get_parent_by_index() fails (and '.name' isn't
NULL).  Then we're left with the case where even the fallback '.name' is
NULL, and we can print something like "<fw_name> (fw)" to indicate that
we're waiting for the kernel to probe a provider for that parent, but
the firmware name is <fw_name> and that's all we know.

Patch

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index aa51756fd4d6..bdb077ba59b9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3000,12 +3000,16 @@  DEFINE_SHOW_ATTRIBUTE(clk_flags);
 static int possible_parents_show(struct seq_file *s, void *data)
 {
 	struct clk_core *core = s->private;
+	struct clk_core *parent;
 	int i;
 
-	for (i = 0; i < core->num_parents - 1; i++)
-		seq_printf(s, "%s ", core->parents[i].name);
+	for (i = 0; i < core->num_parents - 1; i++) {
+		parent = clk_core_get_parent_by_index(core, i);
+		seq_printf(s, "%s ", parent ? parent->name : NULL);
+	}
 
-	seq_printf(s, "%s\n", core->parents[i].name);
+	parent = clk_core_get_parent_by_index(core, i);
+	seq_printf(s, "%s", parent ? parent->name : NULL);
 
 	return 0;
 }