Message-Id: <20190801171209.234546-1-sboyd@kernel.org>
Date:   Thu,  1 Aug 2019 10:12:09 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
        Taniya Das <tdas@...eaurora.org>,
        Jerome Brunet <jbrunet@...libre.com>,
        Chen-Yu Tsai <wens@...e.org>
Subject: [PATCH] clk: Fix falling back to legacy parent string matching

Calls to clk_core_get() will return ERR_PTR(-EINVAL) if we've started
migrating a clk driver to use the DT based style of specifying parents
but we haven't made any DT updates yet. This happens when we pass a
non-NULL value as the 'name' argument of of_parse_clkspec(). That
function returns -EINVAL in such a situation, instead of -ENOENT like we
expected. The return value comes back up to clk_core_fill_parent_index()
which proceeds to skip calling clk_core_lookup() because the error
pointer isn't equal to -ENOENT, it's -EINVAL.

Furthermore, we'll blindly overwrite the error pointer returned by
clk_core_get() with NULL when there isn't a legacy .name member
specified in the parent map. This isn't too bad right now because we
don't really care to differentiate NULL from an error, but in the future
we should only try to do a legacy lookup if we know we might find
something so that DT lookups that fail don't try to lookup based on
strings when there isn't any string to match, hiding the error.

Fix both these problems so that clk provider drivers can use the new
style of parent mapping without having to also update their DT at the
same time. This patch is based on an earlier patch from Taniya Das which
checked for -EINVAL in addition to -ENOENT return values.

Fixes: 601b6e93304a ("clk: Allow parents to be specified via clkspec index")
Cc: Taniya Das <tdas@...eaurora.org>
Cc: Jerome Brunet <jbrunet@...libre.com>
Cc: Chen-Yu Tsai <wens@...e.org>
Reported-by: Taniya Das <tdas@...eaurora.org>
Signed-off-by: Stephen Boyd <sboyd@...nel.org>
---
 drivers/clk/clk.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c0990703ce54..6587a70c271c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -355,8 +355,9 @@ static struct clk_core *clk_core_lookup(const char *name)
  *      };
  *
  * Returns: -ENOENT when the provider can't be found or the clk doesn't
- * exist in the provider. -EINVAL when the name can't be found. NULL when the
- * provider knows about the clk but it isn't provided on this system.
+ * exist in the provider or the name can't be found in the DT node or
+ * in a clkdev lookup. NULL when the provider knows about the clk but it
+ * isn't provided on this system.
  * A valid clk_core pointer when the clk can be found in the provider.
  */
 static struct clk_core *clk_core_get(struct clk_core *core, u8 p_index)
@@ -374,9 +375,9 @@ static struct clk_core *clk_core_get(struct clk_core *core, u8 p_index)
 	/*
 	 * If the DT search above couldn't find the provider or the provider
 	 * didn't know about this clk, fallback to looking up via clkdev based
-	 * clk_lookups
+	 * clk_lookups.
 	 */
-	if (PTR_ERR(hw) == -ENOENT && name)
+	if (IS_ERR(hw) && name)
 		hw = clk_find_hw(dev_id, name);
 
 	if (IS_ERR(hw))
@@ -401,7 +402,7 @@ static void clk_core_fill_parent_index(struct clk_core *core, u8 index)
 			parent = ERR_PTR(-EPROBE_DEFER);
 	} else {
 		parent = clk_core_get(core, index);
-		if (IS_ERR(parent) && PTR_ERR(parent) == -ENOENT)
+		if (IS_ERR(parent) && entry->name)
 			parent = clk_core_lookup(entry->name);
 	}
 
-- 
Sent by a computer through tubes

Powered by blists - more mailing lists