[PATCH v4 0/9] Rewrite clk parent handling

[PATCH v4 0/9] Rewrite clk parent handling

[Stephen Boyd]

There are a couple warts with clk parent handling in the common clk
framework. This patch series addresses one of those warts, parent-child
linkages. We use strings for all parent-child linkages, and this leads
to poorly written code that extracts clk names from struct clk pointers
and makes clk provider drivers use clk consumer APIs. I've converted the
fixed factor clk and I'm looking at converting users of other "basic"
clk types in follow-up patches so we can start moving drivers away from
string matching in the clk namespace.

Changes from v3:
 * Split clkdev patch into two
 * New patch to let dev_of_node() accept NULL
 * Use dev_of_node() in of_clk_hw_register() to avoid NULL deref

Changes from v2:
 * Dropped patches that got merged into v5.1-rc1
 * Introduced a new function of_clk_hw_register()
 * Introduced 'index' into clk_parent_data structure 
 * Converted fixed-factor basic type to new design
 * Fixed some bugs in a recent patch to clkdev, leading to
   an essential API for clkdev based lookups working

Changes from v1:
 * Dropped new get_parent_hw, we'll pick it up in a later series
 * Rebased to v5.0-rc6 to avoid conflicts with clk-fixes
 * Renamed 'fallback' to 'name' and 'name' to 'fw_name' in parent map
 * Made sure that clk_hw_get_parent_by_index() never sees an error pointer
   so that we don't mistakenly try to defer an error pointer
 * Fixed index passing mistake on of_clk_get_hw_from_clkspec()
 * Copy over all the data from parent maps and free it correctly too

Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>

Stephen Boyd (9):
  clkdev: Hold clocks_mutex while iterating clocks list
  clkdev: Move clk creation outside of 'clocks_mutex'
  clk: Prepare for clk registration API that uses DT nodes
  driver core: Let dev_of_node() accept a NULL dev
  clk: Add of_clk_hw_register() API for early clk drivers
  clk: Allow parents to be specified without string names
  clk: Look for parents with clkdev based clk_lookups
  clk: Allow parents to be specified via clkspec index
  clk: fixed-factor: Let clk framework find parent

 drivers/clk/clk-fixed-factor.c |  53 ++++--
 drivers/clk/clk.c              | 326 +++++++++++++++++++++++++--------
 drivers/clk/clk.h              |   2 +
 drivers/clk/clkdev.c           |  30 +--
 include/linux/clk-provider.h   |  22 +++
 include/linux/device.h         |   2 +-
 6 files changed, 327 insertions(+), 108 deletions(-)


base-commit: 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b
-- 
Sent by a computer through tubes

[PATCH v4 1/9] clkdev: Hold clocks_mutex while iterating clocks list

[Stephen Boyd]

We recently introduced a change to support devm clk lookups. That change
introduced a code-path that used clk_find() without holding the
'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
list and so we need to prevent the list from being modified at the same
time. Do this by holding the mutex and checking to make sure it's held
while iterating the list.

Note, we don't really care if the lookup is freed after we find it with
clk_find() because we're just doing a pointer comparison, but if we did
care we would need to keep holding the mutex while we dereference the
clk_lookup pointer.

Fixes: 3eee6c7d119c ("clkdev: add managed clkdev lookup registration")
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

I plan to take this through clk-fixes for v5.1-rc series.

 drivers/clk/clkdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 8c4435c53f09..6e787cc9e5b9 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -46,6 +46,8 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	if (con_id)
 		best_possible += 1;
 
+	lockdep_assert_held(&clocks_mutex);
+
 	list_for_each_entry(p, &clocks, node) {
 		match = 0;
 		if (p->dev_id) {
@@ -402,7 +404,10 @@ void devm_clk_release_clkdev(struct device *dev, const char *con_id,
 	struct clk_lookup *cl;
 	int rval;
 
+	mutex_lock(&clocks_mutex);
 	cl = clk_find(dev_id, con_id);
+	mutex_unlock(&clocks_mutex);
+
 	WARN_ON(!cl);
 	rval = devres_release(dev, devm_clkdev_release,
 			      devm_clk_match_clkdev, cl);
-- 
Sent by a computer through tubes

Re: [PATCH v4 1/9] clkdev: Hold clocks_mutex while iterating clocks list

[Matti Vaittinen]

On Fri, Apr 12, 2019 at 11:31:42AM -0700, Stephen Boyd wrote:
> We recently introduced a change to support devm clk lookups. That change
> introduced a code-path that used clk_find() without holding the
> 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> list and so we need to prevent the list from being modified at the same
> time. Do this by holding the mutex and checking to make sure it's held
> while iterating the list.
> 
> Note, we don't really care if the lookup is freed after we find it with
> clk_find() because we're just doing a pointer comparison, but if we did
> care we would need to keep holding the mutex while we dereference the
> clk_lookup pointer.
> 
> Fixes: 3eee6c7d119c ("clkdev: add managed clkdev lookup registration")
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
Acked-By: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>

Re: [PATCH v4 1/9] clkdev: Hold clocks_mutex while iterating clocks list

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:42)
> We recently introduced a change to support devm clk lookups. That change
> introduced a code-path that used clk_find() without holding the
> 'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
> list and so we need to prevent the list from being modified at the same
> time. Do this by holding the mutex and checking to make sure it's held
> while iterating the list.
> 
> Note, we don't really care if the lookup is freed after we find it with
> clk_find() because we're just doing a pointer comparison, but if we did
> care we would need to keep holding the mutex while we dereference the
> clk_lookup pointer.
> 
> Fixes: 3eee6c7d119c ("clkdev: add managed clkdev lookup registration")
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-fixes

[PATCH v4 2/9] clkdev: Move clk creation outside of 'clocks_mutex'

[Stephen Boyd]

We don't need to hold the 'clocks_mutex' here when we're creating a clk
pointer from a clk_lookup structure. Instead, we just need to make sure
that the lookup doesn't go away while we dereference the lookup pointer
to extract the clk_hw pointer out of it. Let's move things around
slightly so that we have a new function to get the clk_hw out of the
lookup with the right locking and then chain the two together for what
used to be __clk_get_sys().

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clkdev.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6e787cc9e5b9..6f65bde696da 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -72,25 +72,26 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	return cl;
 }
 
-static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
-				 const char *con_id)
+static struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *cl;
-	struct clk *clk = NULL;
+	struct clk_hw *hw = ERR_PTR(-ENOENT);
 
 	mutex_lock(&clocks_mutex);
-
 	cl = clk_find(dev_id, con_id);
-	if (!cl)
-		goto out;
-
-	clk = clk_hw_create_clk(dev, cl->clk_hw, dev_id, con_id);
-	if (IS_ERR(clk))
-		cl = NULL;
-out:
+	if (cl)
+		hw = cl->clk_hw;
 	mutex_unlock(&clocks_mutex);
 
-	return cl ? clk : ERR_PTR(-ENOENT);
+	return hw;
+}
+
+static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
+				 const char *con_id)
+{
+	struct clk_hw *hw = clk_find_hw(dev_id, con_id);
+
+	return clk_hw_create_clk(dev, hw, dev_id, con_id);
 }
 
 struct clk *clk_get_sys(const char *dev_id, const char *con_id)
-- 
Sent by a computer through tubes

Re: [PATCH v4 2/9] clkdev: Move clk creation outside of 'clocks_mutex'

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:43)
> We don't need to hold the 'clocks_mutex' here when we're creating a clk
> pointer from a clk_lookup structure. Instead, we just need to make sure
> that the lookup doesn't go away while we dereference the lookup pointer
> to extract the clk_hw pointer out of it. Let's move things around
> slightly so that we have a new function to get the clk_hw out of the
> lookup with the right locking and then chain the two together for what
> used to be __clk_get_sys().
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 3/9] clk: Prepare for clk registration API that uses DT nodes

[Stephen Boyd]

Split out the body of the clk_register() function so it can be shared
between the different types of registration APIs (DT, device).

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 96053a96fe2f..d27775a73e67 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3313,18 +3313,7 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
 	return clk;
 }
 
-/**
- * clk_register - allocate a new clock, register it and return an opaque cookie
- * @dev: device that is registering this clock
- * @hw: link to hardware-specific clock data
- *
- * clk_register is the primary interface for populating the clock tree with new
- * clock nodes.  It returns a pointer to the newly allocated struct clk which
- * cannot be dereferenced by driver code but may be used in conjunction with the
- * rest of the clock API.  In the event of an error clk_register will return an
- * error code; drivers must test for an error code after calling clk_register.
- */
-struct clk *clk_register(struct device *dev, struct clk_hw *hw)
+static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
 {
 	int i, ret;
 	struct clk_core *core;
@@ -3426,6 +3415,22 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 fail_out:
 	return ERR_PTR(ret);
 }
+
+/**
+ * clk_register - allocate a new clock, register it and return an opaque cookie
+ * @dev: device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * clk_register is the primary interface for populating the clock tree with new
+ * clock nodes.  It returns a pointer to the newly allocated struct clk which
+ * cannot be dereferenced by driver code but may be used in conjunction with the
+ * rest of the clock API.  In the event of an error clk_register will return an
+ * error code; drivers must test for an error code after calling clk_register.
+ */
+struct clk *clk_register(struct device *dev, struct clk_hw *hw)
+{
+	return __clk_register(dev, hw);
+}
 EXPORT_SYMBOL_GPL(clk_register);
 
 /**
@@ -3440,7 +3445,7 @@ EXPORT_SYMBOL_GPL(clk_register);
  */
 int clk_hw_register(struct device *dev, struct clk_hw *hw)
 {
-	return PTR_ERR_OR_ZERO(clk_register(dev, hw));
+	return PTR_ERR_OR_ZERO(__clk_register(dev, hw));
 }
 EXPORT_SYMBOL_GPL(clk_hw_register);
 
-- 
Sent by a computer through tubes

Re: [PATCH v4 3/9] clk: Prepare for clk registration API that uses DT nodes

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:44)
> Split out the body of the clk_register() function so it can be shared
> between the different types of registration APIs (DT, device).
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 4/9] driver core: Let dev_of_node() accept a NULL dev

[Stephen Boyd]

We'd like to chain this in places where the 'dev' argument might be
NULL. Let this function take a NULL 'dev' so this can work.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---

Please ack/review so I can take this through clk tree.

 include/linux/device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index b425a7ee04ce..0370dd0b3ae7 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1231,7 +1231,7 @@ static inline void device_lock_assert(struct device *dev)
 
 static inline struct device_node *dev_of_node(struct device *dev)
 {
-	if (!IS_ENABLED(CONFIG_OF))
+	if (!IS_ENABLED(CONFIG_OF) || !dev)
 		return NULL;
 	return dev->of_node;
 }
-- 
Sent by a computer through tubes

Re: [PATCH v4 4/9] driver core: Let dev_of_node() accept a NULL dev

[Greg Kroah-Hartman]

On Fri, Apr 12, 2019 at 11:31:45AM -0700, Stephen Boyd wrote:
> We'd like to chain this in places where the 'dev' argument might be
> NULL. Let this function take a NULL 'dev' so this can work.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
> 
> Please ack/review so I can take this through clk tree.
> 
>  include/linux/device.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/device.h b/include/linux/device.h
> index b425a7ee04ce..0370dd0b3ae7 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1231,7 +1231,7 @@ static inline void device_lock_assert(struct device *dev)
>  
>  static inline struct device_node *dev_of_node(struct device *dev)
>  {
> -	if (!IS_ENABLED(CONFIG_OF))
> +	if (!IS_ENABLED(CONFIG_OF) || !dev)
>  		return NULL;
>  	return dev->of_node;
>  }

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v4 4/9] driver core: Let dev_of_node() accept a NULL dev

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:45)
> We'd like to chain this in places where the 'dev' argument might be
> NULL. Let this function take a NULL 'dev' so this can work.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 5/9] clk: Add of_clk_hw_register() API for early clk drivers

[Stephen Boyd]

In some circumstances drivers register clks early and don't have access
to a struct device because the device model isn't initialized yet. Add
an API to let drivers register clks associated with a struct device_node
so that these drivers can participate in getting parent clks through DT.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c            | 26 +++++++++++++++++++++++---
 include/linux/clk-provider.h |  1 +
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index d27775a73e67..ffa63ddcd408 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -45,6 +45,7 @@ struct clk_core {
 	struct clk_hw		*hw;
 	struct module		*owner;
 	struct device		*dev;
+	struct device_node	*of_node;
 	struct clk_core		*parent;
 	const char		**parent_names;
 	struct clk_core		**parents;
@@ -3313,7 +3314,8 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
 	return clk;
 }
 
-static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
+static struct clk *
+__clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 {
 	int i, ret;
 	struct clk_core *core;
@@ -3339,6 +3341,7 @@ static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
 	if (dev && pm_runtime_enabled(dev))
 		core->rpm_enabled = true;
 	core->dev = dev;
+	core->of_node = np;
 	if (dev && dev->driver)
 		core->owner = dev->driver->owner;
 	core->hw = hw;
@@ -3429,7 +3432,7 @@ static struct clk *__clk_register(struct device *dev, struct clk_hw *hw)
  */
 struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 {
-	return __clk_register(dev, hw);
+	return __clk_register(dev, dev_of_node(dev), hw);
 }
 EXPORT_SYMBOL_GPL(clk_register);
 
@@ -3445,10 +3448,27 @@ EXPORT_SYMBOL_GPL(clk_register);
  */
 int clk_hw_register(struct device *dev, struct clk_hw *hw)
 {
-	return PTR_ERR_OR_ZERO(__clk_register(dev, hw));
+	return PTR_ERR_OR_ZERO(__clk_register(dev, dev_of_node(dev), hw));
 }
 EXPORT_SYMBOL_GPL(clk_hw_register);
 
+/*
+ * of_clk_hw_register - register a clk_hw and return an error code
+ * @node: device_node of device that is registering this clock
+ * @hw: link to hardware-specific clock data
+ *
+ * of_clk_hw_register() is the primary interface for populating the clock tree
+ * with new clock nodes when a struct device is not available, but a struct
+ * device_node is. It returns an integer equal to zero indicating success or
+ * less than zero indicating failure. Drivers must test for an error code after
+ * calling of_clk_hw_register().
+ */
+int of_clk_hw_register(struct device_node *node, struct clk_hw *hw)
+{
+	return PTR_ERR_OR_ZERO(__clk_register(NULL, node, hw));
+}
+EXPORT_SYMBOL_GPL(of_clk_hw_register);
+
 /* Free memory allocated for a clock. */
 static void __clk_release(struct kref *ref)
 {
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index b7cf80a71293..7d2d97e15b76 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -773,6 +773,7 @@ struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw);
 
 int __must_check clk_hw_register(struct device *dev, struct clk_hw *hw);
 int __must_check devm_clk_hw_register(struct device *dev, struct clk_hw *hw);
+int __must_check of_clk_hw_register(struct device_node *node, struct clk_hw *hw);
 
 void clk_unregister(struct clk *clk);
 void devm_clk_unregister(struct device *dev, struct clk *clk);
-- 
Sent by a computer through tubes

Re: [PATCH v4 5/9] clk: Add of_clk_hw_register() API for early clk drivers

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:46)
> In some circumstances drivers register clks early and don't have access
> to a struct device because the device model isn't initialized yet. Add
> an API to let drivers register clks associated with a struct device_node
> so that these drivers can participate in getting parent clks through DT.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 6/9] clk: Allow parents to be specified without string names

[Stephen Boyd]

The common clk framework is lacking in ability to describe the clk
topology without specifying strings for every possible parent-child
link. There are a few drawbacks to the current approach:

 1) String comparisons are used for everything, including describing
 topologies that are 'local' to a single clock controller.

 2) clk providers (e.g. i2c clk drivers) need to create globally unique
 clk names to avoid collisions in the clk namespace, leading to awkward
 name generation code in various clk drivers.

 3) DT bindings may not fully describe the clk topology and linkages
 between clk controllers because drivers can easily rely on globally unique
 strings to describe connections between clks.

This leads to confusing DT bindings, complicated clk name generation
code, and inefficient string comparisons during clk registration just so
that the clk framework can detect the topology of the clk tree.
Furthermore, some drivers call clk_get() and then __clk_get_name() to
extract the globally unique clk name just so they can specify the parent
of the clk they're registering. We have of_clk_parent_fill() but that
mostly only works for single clks registered from a DT node, which isn't
the norm. Let's simplify this all by introducing two new ways of
specifying clk parents.

The first method is an array of pointers to clk_hw structures
corresponding to the parents at that index. This works for clks that are
registered when we have access to all the clk_hw pointers for the
parents.

The second method is a mix of clk_hw pointers and strings of local and
global parent clk names. If the .fw_name member of the map is set we'll
look for that clk by performing a DT based lookup of the device the clk
is registered with and the .name specified in the map. If that fails,
we'll fallback to the .name member and perform a global clk name lookup
like we've always done before.

Using either one of these new methods is entirely optional. Existing
drivers will continue to work, and they can migrate to this new approach
as they see fit. Eventually, we'll want to get rid of the 'parent_names'
array in struct clk_init_data and use one of these new methods instead.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c            | 262 ++++++++++++++++++++++++++---------
 include/linux/clk-provider.h |  19 +++
 2 files changed, 219 insertions(+), 62 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ffa63ddcd408..de49a7c4214b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -39,6 +39,13 @@ static LIST_HEAD(clk_notifier_list);
 
 /***    private data structures    ***/
 
+struct clk_parent_map {
+	const struct clk_hw	*hw;
+	struct clk_core		*core;
+	const char	 	*fw_name;
+	const char	 	*name;
+};
+
 struct clk_core {
 	const char		*name;
 	const struct clk_ops	*ops;
@@ -47,8 +54,7 @@ struct clk_core {
 	struct device		*dev;
 	struct device_node	*of_node;
 	struct clk_core		*parent;
-	const char		**parent_names;
-	struct clk_core		**parents;
+	struct clk_parent_map	*parents;
 	u8			num_parents;
 	u8			new_parent_index;
 	unsigned long		rate;
@@ -317,17 +323,92 @@ static struct clk_core *clk_core_lookup(const char *name)
 	return NULL;
 }
 
+/**
+ * clk_core_get - Find the parent of a clk using a clock specifier in DT
+ * @core: clk to find parent of
+ * @name: name to search for in 'clock-names' of device providing clk
+ *
+ * This is the preferred method for clk providers to find the parent of a
+ * clk when that parent is external to the clk controller. The parent_names
+ * array is indexed and treated as a local name matching a string in the device
+ * node's 'clock-names' property. This allows clk providers to use their own
+ * namespace instead of looking for a globally unique parent string.
+ *
+ * For example the following DT snippet would allow a clock registered by the
+ * clock-controller@c001 that has a clk_init_data::parent_data array
+ * with 'xtal' in the 'name' member to find the clock provided by the
+ * clock-controller@f00abcd without needing to get the globally unique name of
+ * the xtal clk.
+ *
+ * 	parent: clock-controller@f00abcd {
+ * 		reg = <0xf00abcd 0xabcd>;
+ * 		#clock-cells = <0>;
+ * 	};
+ *
+ * 	clock-controller@c001 {
+ * 		reg = <0xc001 0xf00d>;
+ * 		clocks = <&parent>;
+ * 		clock-names = "xtal";
+ * 		#clock-cells = <1>;
+ * 	};
+ *
+ * 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.
+ * 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, const char *name)
+{
+	struct clk_hw *hw;
+	struct device_node *np = core->of_node;
+
+	if (!np)
+		return ERR_PTR(-ENOENT);
+
+	/* TODO: Support clkdev clk_lookups */
+	hw = of_clk_get_hw(np, -1, name);
+	if (IS_ERR_OR_NULL(hw))
+		return ERR_CAST(hw);
+
+	return hw->core;
+}
+
+static void clk_core_fill_parent_index(struct clk_core *core, u8 index)
+{
+	struct clk_parent_map *entry = &core->parents[index];
+	struct clk_core *parent = ERR_PTR(-ENOENT);
+
+	if (entry->hw) {
+		parent = entry->hw->core;
+		/*
+		 * We have a direct reference but it isn't registered yet? Orphan it
+		 * and let clk_reparent() update the orphan status when the parent
+		 * is registered.
+		 */
+		if (!parent)
+			parent = ERR_PTR(-EPROBE_DEFER);
+	} else {
+		if (entry->fw_name)
+			parent = clk_core_get(core, entry->fw_name);
+		if (IS_ERR(parent) && PTR_ERR(parent) == -ENOENT)
+			parent = clk_core_lookup(entry->name);
+	}
+
+	/* Only cache it if it's not an error */
+	if (!IS_ERR(parent))
+		entry->core = parent;
+}
+
 static struct clk_core *clk_core_get_parent_by_index(struct clk_core *core,
 							 u8 index)
 {
-	if (!core || index >= core->num_parents)
+	if (!core || index >= core->num_parents || !core->parents)
 		return NULL;
 
-	if (!core->parents[index])
-		core->parents[index] =
-				clk_core_lookup(core->parent_names[index]);
+	if (!core->parents[index].core)
+		clk_core_fill_parent_index(core, index);
 
-	return core->parents[index];
+	return core->parents[index].core;
 }
 
 struct clk_hw *
@@ -1520,15 +1601,15 @@ static int clk_fetch_parent_index(struct clk_core *core,
 		return -EINVAL;
 
 	for (i = 0; i < core->num_parents; i++) {
-		if (core->parents[i] == parent)
+		if (core->parents[i].core == parent)
 			return i;
 
-		if (core->parents[i])
+		if (core->parents[i].core)
 			continue;
 
 		/* Fallback to comparing globally unique names */
-		if (!strcmp(parent->name, core->parent_names[i])) {
-			core->parents[i] = parent;
+		if (!strcmp(parent->name, core->parents[i].name)) {
+			core->parents[i].core = parent;
 			return i;
 		}
 	}
@@ -2294,6 +2375,7 @@ void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent)
 bool clk_has_parent(struct clk *clk, struct clk *parent)
 {
 	struct clk_core *core, *parent_core;
+	int i;
 
 	/* NULL clocks should be nops, so return success if either is NULL. */
 	if (!clk || !parent)
@@ -2306,8 +2388,11 @@ bool clk_has_parent(struct clk *clk, struct clk *parent)
 	if (core->parent == parent_core)
 		return true;
 
-	return match_string(core->parent_names, core->num_parents,
-			    parent_core->name) >= 0;
+	for (i = 0; i < core->num_parents; i++)
+		if (!strcmp(core->parents[i].name, parent_core->name))
+			return true;
+
+	return false;
 }
 EXPORT_SYMBOL_GPL(clk_has_parent);
 
@@ -2890,9 +2975,9 @@ static int possible_parents_show(struct seq_file *s, void *data)
 	int i;
 
 	for (i = 0; i < core->num_parents - 1; i++)
-		seq_printf(s, "%s ", core->parent_names[i]);
+		seq_printf(s, "%s ", core->parents[i].name);
 
-	seq_printf(s, "%s\n", core->parent_names[i]);
+	seq_printf(s, "%s\n", core->parents[i].name);
 
 	return 0;
 }
@@ -3026,7 +3111,7 @@ static inline void clk_debug_unregister(struct clk_core *core)
  */
 static int __clk_core_init(struct clk_core *core)
 {
-	int i, ret;
+	int ret;
 	struct clk_core *orphan;
 	struct hlist_node *tmp2;
 	unsigned long rate;
@@ -3080,12 +3165,6 @@ static int __clk_core_init(struct clk_core *core)
 		goto out;
 	}
 
-	/* throw a WARN if any entries in parent_names are NULL */
-	for (i = 0; i < core->num_parents; i++)
-		WARN(!core->parent_names[i],
-				"%s: invalid NULL in %s's .parent_names\n",
-				__func__, core->name);
-
 	core->parent = __clk_init_parent(core);
 
 	/*
@@ -3314,10 +3393,102 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
 	return clk;
 }
 
+static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
+{
+	const char *dst;
+
+	if (!src) {
+		if (must_exist)
+			return -EINVAL;
+		return 0;
+	}
+
+	*dst_p = dst = kstrdup_const(src, GFP_KERNEL);
+	if (!dst)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int clk_core_populate_parent_map(struct clk_core *core)
+{
+	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;
+	const struct clk_parent_data *parent_data = init->parent_data;
+	int i, ret = 0;
+	struct clk_parent_map *parents, *parent;
+
+	if (!num_parents)
+		return 0;
+
+	/*
+	 * Avoid unnecessary string look-ups of clk_core's possible parents by
+	 * having a cache of names/clk_hw pointers to clk_core pointers.
+	 */
+	parents = kcalloc(num_parents, sizeof(*parents), GFP_KERNEL);
+	core->parents = parents;
+	if (!parents)
+		return -ENOMEM;
+
+	/* Copy everything over because it might be __initdata */
+	for (i = 0, parent = parents; i < num_parents; i++, parent++) {
+		if (parent_names) {
+			/* throw a WARN if any entries are NULL */
+			WARN(!parent_names[i],
+				"%s: invalid NULL in %s's .parent_names\n",
+				__func__, core->name);
+			ret = clk_cpy_name(&parent->name, parent_names[i],
+					   true);
+		} else if (parent_data) {
+			parent->hw = parent_data[i].hw;
+			ret = clk_cpy_name(&parent->fw_name,
+					   parent_data[i].fw_name, false);
+			if (!ret)
+				ret = clk_cpy_name(&parent->name,
+						   parent_data[i].name,
+						   false);
+		} else if (parent_hws) {
+			parent->hw = parent_hws[i];
+		} else {
+			ret = -EINVAL;
+			WARN(1, "Must specify parents if num_parents > 0\n");
+		}
+
+		if (ret) {
+			do {
+				kfree_const(parents[i].name);
+				kfree_const(parents[i].fw_name);
+			} while (--i >= 0);
+			kfree(parents);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static void clk_core_free_parent_map(struct clk_core *core)
+{
+	int i = core->num_parents;
+
+	if (!core->num_parents)
+		return;
+
+	while (--i >= 0) {
+		kfree_const(core->parents[i].name);
+		kfree_const(core->parents[i].fw_name);
+	}
+
+	kfree(core->parents);
+}
+
 static struct clk *
 __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 {
-	int i, ret;
+	int ret;
 	struct clk_core *core;
 
 	core = kzalloc(sizeof(*core), GFP_KERNEL);
@@ -3351,33 +3522,9 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	core->max_rate = ULONG_MAX;
 	hw->core = core;
 
-	/* allocate local copy in case parent_names is __initdata */
-	core->parent_names = kcalloc(core->num_parents, sizeof(char *),
-					GFP_KERNEL);
-
-	if (!core->parent_names) {
-		ret = -ENOMEM;
-		goto fail_parent_names;
-	}
-
-
-	/* copy each string name in case parent_names is __initdata */
-	for (i = 0; i < core->num_parents; i++) {
-		core->parent_names[i] = kstrdup_const(hw->init->parent_names[i],
-						GFP_KERNEL);
-		if (!core->parent_names[i]) {
-			ret = -ENOMEM;
-			goto fail_parent_names_copy;
-		}
-	}
-
-	/* avoid unnecessary string look-ups of clk_core's possible parents. */
-	core->parents = kcalloc(core->num_parents, sizeof(*core->parents),
-				GFP_KERNEL);
-	if (!core->parents) {
-		ret = -ENOMEM;
+	ret = clk_core_populate_parent_map(core);
+	if (ret)
 		goto fail_parents;
-	};
 
 	INIT_HLIST_HEAD(&core->clks);
 
@@ -3388,7 +3535,7 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	hw->clk = alloc_clk(core, NULL, NULL);
 	if (IS_ERR(hw->clk)) {
 		ret = PTR_ERR(hw->clk);
-		goto fail_parents;
+		goto fail_create_clk;
 	}
 
 	clk_core_link_consumer(hw->core, hw->clk);
@@ -3404,13 +3551,9 @@ __clk_register(struct device *dev, struct device_node *np, struct clk_hw *hw)
 	free_clk(hw->clk);
 	hw->clk = NULL;
 
+fail_create_clk:
+	clk_core_free_parent_map(core);
 fail_parents:
-	kfree(core->parents);
-fail_parent_names_copy:
-	while (--i >= 0)
-		kfree_const(core->parent_names[i]);
-	kfree(core->parent_names);
-fail_parent_names:
 fail_ops:
 	kfree_const(core->name);
 fail_name:
@@ -3473,15 +3616,10 @@ EXPORT_SYMBOL_GPL(of_clk_hw_register);
 static void __clk_release(struct kref *ref)
 {
 	struct clk_core *core = container_of(ref, struct clk_core, ref);
-	int i = core->num_parents;
 
 	lockdep_assert_held(&prepare_lock);
 
-	kfree(core->parents);
-	while (--i >= 0)
-		kfree_const(core->parent_names[i]);
-
-	kfree(core->parent_names);
+	clk_core_free_parent_map(core);
 	kfree_const(core->name);
 	kfree(core);
 }
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 7d2d97e15b76..95d5a9d4e8c3 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -250,6 +250,18 @@ struct clk_ops {
 	void		(*debug_init)(struct clk_hw *hw, struct dentry *dentry);
 };
 
+/**
+ * struct clk_parent_data - clk parent information
+ * @hw: parent clk_hw pointer (used for clk providers with internal clks)
+ * @fw_name: parent name local to provider registering clk
+ * @name: globally unique parent name (used as a fallback)
+ */
+struct clk_parent_data {
+	const struct clk_hw 	*hw;
+	const char		*fw_name;
+	const char		*name;
+};
+
 /**
  * struct clk_init_data - holds init data that's common to all clocks and is
  * shared between the clock provider and the common clock framework.
@@ -257,13 +269,20 @@ struct clk_ops {
  * @name: clock name
  * @ops: operations this clock supports
  * @parent_names: array of string names for all possible parents
+ * @parent_data: array of parent data for all possible parents (when some
+ *               parents are external to the clk controller)
+ * @parent_hws: array of pointers to all possible parents (when all parents
+ *              are internal to the clk controller)
  * @num_parents: number of possible parents
  * @flags: framework-level hints and quirks
  */
 struct clk_init_data {
 	const char		*name;
 	const struct clk_ops	*ops;
+	/* Only one of the following three should be assigned */
 	const char		* const *parent_names;
+	const struct clk_parent_data	*parent_data;
+	const struct clk_hw		**parent_hws;
 	u8			num_parents;
 	unsigned long		flags;
 };
-- 
Sent by a computer through tubes

Re: [PATCH v4 6/9] clk: Allow parents to be specified without string names

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:47)
> The common clk framework is lacking in ability to describe the clk
> topology without specifying strings for every possible parent-child
> link. There are a few drawbacks to the current approach:
> 
>  1) String comparisons are used for everything, including describing
>  topologies that are 'local' to a single clock controller.
> 
>  2) clk providers (e.g. i2c clk drivers) need to create globally unique
>  clk names to avoid collisions in the clk namespace, leading to awkward
>  name generation code in various clk drivers.
> 
>  3) DT bindings may not fully describe the clk topology and linkages
>  between clk controllers because drivers can easily rely on globally unique
>  strings to describe connections between clks.
> 
> This leads to confusing DT bindings, complicated clk name generation
> code, and inefficient string comparisons during clk registration just so
> that the clk framework can detect the topology of the clk tree.
> Furthermore, some drivers call clk_get() and then __clk_get_name() to
> extract the globally unique clk name just so they can specify the parent
> of the clk they're registering. We have of_clk_parent_fill() but that
> mostly only works for single clks registered from a DT node, which isn't
> the norm. Let's simplify this all by introducing two new ways of
> specifying clk parents.
> 
> The first method is an array of pointers to clk_hw structures
> corresponding to the parents at that index. This works for clks that are
> registered when we have access to all the clk_hw pointers for the
> parents.
> 
> The second method is a mix of clk_hw pointers and strings of local and
> global parent clk names. If the .fw_name member of the map is set we'll
> look for that clk by performing a DT based lookup of the device the clk
> is registered with and the .name specified in the map. If that fails,
> we'll fallback to the .name member and perform a global clk name lookup
> like we've always done before.
> 
> Using either one of these new methods is entirely optional. Existing
> drivers will continue to work, and they can migrate to this new approach
> as they see fit. Eventually, we'll want to get rid of the 'parent_names'
> array in struct clk_init_data and use one of these new methods instead.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 7/9] clk: Look for parents with clkdev based clk_lookups

[Stephen Boyd]

In addition to looking for DT based parents, support clkdev based
clk_lookups. This should allow non-DT based clk drivers to participate
in the parent lookup process.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c    | 27 ++++++++++++++++++---------
 drivers/clk/clk.h    |  2 ++
 drivers/clk/clkdev.c |  2 +-
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index de49a7c4214b..42f29fd6bfd8 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -324,14 +324,15 @@ static struct clk_core *clk_core_lookup(const char *name)
 }
 
 /**
- * clk_core_get - Find the parent of a clk using a clock specifier in DT
+ * clk_core_get - Find the clk_core parent of a clk
  * @core: clk to find parent of
- * @name: name to search for in 'clock-names' of device providing clk
+ * @name: name to search for
  *
  * This is the preferred method for clk providers to find the parent of a
  * clk when that parent is external to the clk controller. The parent_names
  * array is indexed and treated as a local name matching a string in the device
- * node's 'clock-names' property. This allows clk providers to use their own
+ * node's 'clock-names' property or as the 'con_id' matching the device's
+ * dev_name() in a clk_lookup. This allows clk providers to use their own
  * namespace instead of looking for a globally unique parent string.
  *
  * For example the following DT snippet would allow a clock registered by the
@@ -359,15 +360,23 @@ static struct clk_core *clk_core_lookup(const char *name)
  */
 static struct clk_core *clk_core_get(struct clk_core *core, const char *name)
 {
-	struct clk_hw *hw;
+	struct clk_hw *hw = ERR_PTR(-ENOENT);
+	struct device *dev = core->dev;
+	const char *dev_id = dev ? dev_name(dev) : NULL;
 	struct device_node *np = core->of_node;
 
-	if (!np)
-		return ERR_PTR(-ENOENT);
+	if (np)
+		hw = of_clk_get_hw(np, -1, name);
 
-	/* TODO: Support clkdev clk_lookups */
-	hw = of_clk_get_hw(np, -1, name);
-	if (IS_ERR_OR_NULL(hw))
+	/*
+	 * 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
+	 */
+	if (PTR_ERR(hw) == -ENOENT)
+		hw = clk_find_hw(dev_id, name);
+
+	if (IS_ERR(hw))
 		return ERR_CAST(hw);
 
 	return hw->core;
diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
index 553f531cc232..d8400d623b34 100644
--- a/drivers/clk/clk.h
+++ b/drivers/clk/clk.h
@@ -19,6 +19,8 @@ static inline struct clk_hw *of_clk_get_hw(struct device_node *np,
 }
 #endif
 
+struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id);
+
 #ifdef CONFIG_COMMON_CLK
 struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
 			      const char *dev_id, const char *con_id);
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 6f65bde696da..2afc8df8acff 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -72,7 +72,7 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	return cl;
 }
 
-static struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id)
+struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *cl;
 	struct clk_hw *hw = ERR_PTR(-ENOENT);
-- 
Sent by a computer through tubes

Re: [PATCH v4 7/9] clk: Look for parents with clkdev based clk_lookups

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:48)
> In addition to looking for DT based parents, support clkdev based
> clk_lookups. This should allow non-DT based clk drivers to participate
> in the parent lookup process.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 8/9] clk: Allow parents to be specified via clkspec index

[Stephen Boyd]

Some clk providers are simple DT nodes that only have a 'clocks'
property without having an associated 'clock-names' property. In these
cases, we want to let these clk providers point to their parent clks
without having to dereference the 'clocks' property at probe time to
figure out the parent's globally unique clk name. Let's add an 'index'
property to the parent_data structure so that clk providers can indicate
that their parent is a particular index in the 'clocks' DT property.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk.c            | 18 +++++++++++-------
 include/linux/clk-provider.h |  2 ++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 42f29fd6bfd8..65fe50b139ea 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -44,6 +44,7 @@ struct clk_parent_map {
 	struct clk_core		*core;
 	const char	 	*fw_name;
 	const char	 	*name;
+	int			index;
 };
 
 struct clk_core {
@@ -326,7 +327,8 @@ static struct clk_core *clk_core_lookup(const char *name)
 /**
  * clk_core_get - Find the clk_core parent of a clk
  * @core: clk to find parent of
- * @name: name to search for
+ * @name: name to search for (if string based)
+ * @index: index to use for search (if DT index based)
  *
  * This is the preferred method for clk providers to find the parent of a
  * clk when that parent is external to the clk controller. The parent_names
@@ -358,22 +360,23 @@ static struct clk_core *clk_core_lookup(const char *name)
  * 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, const char *name)
+static struct clk_core *clk_core_get(struct clk_core *core, const char *name,
+				     int index)
 {
 	struct clk_hw *hw = ERR_PTR(-ENOENT);
 	struct device *dev = core->dev;
 	const char *dev_id = dev ? dev_name(dev) : NULL;
 	struct device_node *np = core->of_node;
 
-	if (np)
-		hw = of_clk_get_hw(np, -1, name);
+	if (np && index >= 0)
+		hw = of_clk_get_hw(np, index, name);
 
 	/*
 	 * 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
 	 */
-	if (PTR_ERR(hw) == -ENOENT)
+	if (PTR_ERR(hw) == -ENOENT && name)
 		hw = clk_find_hw(dev_id, name);
 
 	if (IS_ERR(hw))
@@ -397,8 +400,7 @@ static void clk_core_fill_parent_index(struct clk_core *core, u8 index)
 		if (!parent)
 			parent = ERR_PTR(-EPROBE_DEFER);
 	} else {
-		if (entry->fw_name)
-			parent = clk_core_get(core, entry->fw_name);
+		parent = clk_core_get(core, entry->fw_name, entry->index);
 		if (IS_ERR(parent) && PTR_ERR(parent) == -ENOENT)
 			parent = clk_core_lookup(entry->name);
 	}
@@ -3443,6 +3445,7 @@ static int clk_core_populate_parent_map(struct clk_core *core)
 
 	/* Copy everything over because it might be __initdata */
 	for (i = 0, parent = parents; i < num_parents; i++, parent++) {
+		parent->index = -1;
 		if (parent_names) {
 			/* throw a WARN if any entries are NULL */
 			WARN(!parent_names[i],
@@ -3452,6 +3455,7 @@ static int clk_core_populate_parent_map(struct clk_core *core)
 					   true);
 		} else if (parent_data) {
 			parent->hw = parent_data[i].hw;
+			parent->index = parent_data[i].index;
 			ret = clk_cpy_name(&parent->fw_name,
 					   parent_data[i].fw_name, false);
 			if (!ret)
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 95d5a9d4e8c3..12fef24b740f 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -255,11 +255,13 @@ struct clk_ops {
  * @hw: parent clk_hw pointer (used for clk providers with internal clks)
  * @fw_name: parent name local to provider registering clk
  * @name: globally unique parent name (used as a fallback)
+ * @index: parent index local to provider registering clk (if @fw_name absent)
  */
 struct clk_parent_data {
 	const struct clk_hw 	*hw;
 	const char		*fw_name;
 	const char		*name;
+	int			index;
 };
 
 /**
-- 
Sent by a computer through tubes

Re: [PATCH v4 8/9] clk: Allow parents to be specified via clkspec index

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:49)
> Some clk providers are simple DT nodes that only have a 'clocks'
> property without having an associated 'clock-names' property. In these
> cases, we want to let these clk providers point to their parent clks
> without having to dereference the 'clocks' property at probe time to
> figure out the parent's globally unique clk name. Let's add an 'index'
> property to the parent_data structure so that clk providers can indicate
> that their parent is a particular index in the 'clocks' DT property.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

[PATCH v4 9/9] clk: fixed-factor: Let clk framework find parent

[Stephen Boyd]

Convert this driver to a more modern way of specifying parents now that
we have a way to specify clk parents by DT index. This lets us nicely
avoid a problem where a parent clk name isn't know because the parent
clk hasn't been registered yet.

Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
 drivers/clk/clk-fixed-factor.c | 53 +++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 241b3f8c61a9..67cc7e515e42 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -64,12 +64,14 @@ const struct clk_ops clk_fixed_factor_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_fixed_factor_ops);
 
-struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
-		const char *name, const char *parent_name, unsigned long flags,
-		unsigned int mult, unsigned int div)
+static struct clk_hw *
+__clk_hw_register_fixed_factor(struct device *dev, struct device_node *np,
+		const char *name, const char *parent_name, int index,
+		unsigned long flags, unsigned int mult, unsigned int div)
 {
 	struct clk_fixed_factor *fix;
 	struct clk_init_data init;
+	struct clk_parent_data pdata = { .index = index };
 	struct clk_hw *hw;
 	int ret;
 
@@ -85,11 +87,17 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
 	init.name = name;
 	init.ops = &clk_fixed_factor_ops;
 	init.flags = flags | CLK_IS_BASIC;
-	init.parent_names = &parent_name;
+	if (parent_name)
+		init.parent_names = &parent_name;
+	else
+		init.parent_data = &pdata;
 	init.num_parents = 1;
 
 	hw = &fix->hw;
-	ret = clk_hw_register(dev, hw);
+	if (dev)
+		ret = clk_hw_register(dev, hw);
+	else
+		ret = of_clk_hw_register(np, hw);
 	if (ret) {
 		kfree(fix);
 		hw = ERR_PTR(ret);
@@ -97,6 +105,14 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
 
 	return hw;
 }
+
+struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
+		const char *name, const char *parent_name, unsigned long flags,
+		unsigned int mult, unsigned int div)
+{
+	return __clk_hw_register_fixed_factor(dev, NULL, name, parent_name, -1, flags,
+					      mult, div);
+}
 EXPORT_SYMBOL_GPL(clk_hw_register_fixed_factor);
 
 struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
@@ -143,11 +159,10 @@ static const struct of_device_id set_rate_parent_matches[] = {
 	{ /* Sentinel */ },
 };
 
-static struct clk *_of_fixed_factor_clk_setup(struct device_node *node)
+static struct clk_hw *_of_fixed_factor_clk_setup(struct device_node *node)
 {
-	struct clk *clk;
+	struct clk_hw *hw;
 	const char *clk_name = node->name;
-	const char *parent_name;
 	unsigned long flags = 0;
 	u32 div, mult;
 	int ret;
@@ -165,30 +180,28 @@ static struct clk *_of_fixed_factor_clk_setup(struct device_node *node)
 	}
 
 	of_property_read_string(node, "clock-output-names", &clk_name);
-	parent_name = of_clk_get_parent_name(node, 0);
 
 	if (of_match_node(set_rate_parent_matches, node))
 		flags |= CLK_SET_RATE_PARENT;
 
-	clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
-					mult, div);
-	if (IS_ERR(clk)) {
+	hw = __clk_hw_register_fixed_factor(NULL, node, clk_name, NULL, 0,
+					    flags, mult, div);
+	if (IS_ERR(hw)) {
 		/*
-		 * If parent clock is not registered, registration would fail.
 		 * Clear OF_POPULATED flag so that clock registration can be
 		 * attempted again from probe function.
 		 */
 		of_node_clear_flag(node, OF_POPULATED);
-		return clk;
+		return ERR_CAST(hw);
 	}
 
-	ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	ret = of_clk_add_hw_provider(node, of_clk_hw_simple_get, hw);
 	if (ret) {
-		clk_unregister(clk);
+		clk_hw_unregister_fixed_factor(hw);
 		return ERR_PTR(ret);
 	}
 
-	return clk;
+	return hw;
 }
 
 /**
@@ -203,17 +216,17 @@ CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clock",
 
 static int of_fixed_factor_clk_remove(struct platform_device *pdev)
 {
-	struct clk *clk = platform_get_drvdata(pdev);
+	struct clk_hw *clk = platform_get_drvdata(pdev);
 
 	of_clk_del_provider(pdev->dev.of_node);
-	clk_unregister_fixed_factor(clk);
+	clk_hw_unregister_fixed_factor(clk);
 
 	return 0;
 }
 
 static int of_fixed_factor_clk_probe(struct platform_device *pdev)
 {
-	struct clk *clk;
+	struct clk_hw *clk;
 
 	/*
 	 * This function is not executed when of_fixed_factor_clk_setup
-- 
Sent by a computer through tubes

Re: [PATCH v4 9/9] clk: fixed-factor: Let clk framework find parent

[Stephen Boyd]

Quoting Stephen Boyd (2019-04-12 11:31:50)
> Convert this driver to a more modern way of specifying parents now that
> we have a way to specify clk parents by DT index. This lets us nicely
> avoid a problem where a parent clk name isn't know because the parent
> clk hasn't been registered yet.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next

Re: [PATCH v4 9/9] clk: fixed-factor: Let clk framework find parent

[Guenter Roeck]

Hi,

On Fri, Apr 12, 2019 at 11:31:50AM -0700, Stephen Boyd wrote:
> Convert this driver to a more modern way of specifying parents now that
> we have a way to specify clk parents by DT index. This lets us nicely
> avoid a problem where a parent clk name isn't know because the parent
> clk hasn't been registered yet.
> 
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>

This patch causes a substantial number of crashes of qemu boot tests in -next.

Failed tests: 
	arm:versatilepb:versatile_defconfig:aeabi:pci:scsi:mem128:versatile-pb:rootfs 
	arm:versatilepb:versatile_defconfig:aeabi:pci:mem128:versatile-pb:initrd 
	arm:versatileab:versatile_defconfig:mem128:versatile-ab:initrd 
	arm:beagle:multi_v7_defconfig:sd:mem256:omap3-beagle:rootfs 
	arm:beaglexm:multi_v7_defconfig:sd:mem512:omap3-beagle-xm:rootfs 
	arm:overo:multi_v7_defconfig:sd:mem256:omap3-overo-tobi:rootfs 
	arm:realview-pb-a8:realview_defconfig:realview_pb:mem512:arm-realview-pba8:initrd 
	arm:realview-pbx-a9:realview_defconfig:realview_pb:arm-realview-pbx-a9:initrd 
	arm:realview-eb:realview_defconfig:realview_eb:mem512:arm-realview-eb:initrd 
	arm:realview-eb-mpcore:realview_defconfig:realview_eb:mem512:arm-realview-eb-11mp-ctrevb:initrd 
	arm:integratorcp:integrator_defconfig:mem128:integratorcp:initrd 
	arm:mps2-an385:mps2_defconfig:mps2-an385:initrd

Most of the time the crash happens too early to generate a log,
but here is one:

[    0.000000] [<2100bd59>] (unwind_backtrace) from [<2100b11f>] (show_stack+0xb/0xc)
[    0.000000] [<2100b11f>] (show_stack) from [<211b2d27>] (Ldiv0_64+0x9/0x1a)
[    0.000000] [<211b2d27>] (Ldiv0_64) from [<21038e87>] (clocks_calc_max_nsecs+0x1d/0x62)
[    0.000000] [<21038e87>] (clocks_calc_max_nsecs) from [<21038fb1>] (__clocksource_update_freq_scale+0xe5/0x11c)
[    0.000000] [<21038fb1>] (__clocksource_update_freq_scale) from [<21038ff1>] (__clocksource_register_scale+0x9/0x40)
[    0.000000] [<21038ff1>] (__clocksource_register_scale) from [<212a8713>] (mps2_timer_init+0xaf/0x29c)
[    0.000000] [<212a8713>] (mps2_timer_init) from [<212a85b1>] (timer_probe+0x49/0x80)
[    0.000000] [<212a85b1>] (timer_probe) from [<2129d639>] (start_kernel+0x1c5/0x2f4)
[    0.000000] [<2129d639>] (start_kernel) from [<00000000>] (  (null))
[    0.000000] clocksource: mps2-clksrc: mask: 0xffffffff max_cycles: 0x0, max_idle_ns: 0 ns
[    0.000000] Division by zero in kernel.

Reverting the crash fixes the problem. Bisect log attached.

Guenter

---
# bad: [76c938fcaa4b4a5d8f05fa907925d5043834964e] Add linux-next specific files for 20190423
# good: [085b7755808aa11f78ab9377257e1dad2e6fa4bb] Linux 5.1-rc6
git bisect start 'HEAD' 'v5.1-rc6'
# bad: [ed04f675fa2c22316d7b57bea1258a18a47537ea] Merge remote-tracking branch 'crypto/master'
git bisect bad ed04f675fa2c22316d7b57bea1258a18a47537ea
# bad: [f66d30ddc658fb37848e8e6297b1e658fa297e79] Merge remote-tracking branch 'hid/for-next'
git bisect bad f66d30ddc658fb37848e8e6297b1e658fa297e79
# good: [24523334fd0feef03f3dc42487c158c233455676] Merge remote-tracking branch 'tegra/for-next'
git bisect good 24523334fd0feef03f3dc42487c158c233455676
# bad: [4d5d5f95d0ef4ba470287a941d06600889760ab7] Merge remote-tracking branch 'btrfs-kdave/for-next'
git bisect bad 4d5d5f95d0ef4ba470287a941d06600889760ab7
# bad: [c8040e3c8ab0870b3dfa502cc931258fc04709c6] Merge remote-tracking branch 's390/features'
git bisect bad c8040e3c8ab0870b3dfa502cc931258fc04709c6
# bad: [4209fc3374cfa572aa2defb8ecafe94a9db3c7d4] Merge remote-tracking branch 'csky/linux-next'
git bisect bad 4209fc3374cfa572aa2defb8ecafe94a9db3c7d4
# good: [21eb35a1ae4db08d32e2b5a8d9fe476c16056511] Merge commit 'tags/clk-fixes-for-linus^0' into clk-next
git bisect good 21eb35a1ae4db08d32e2b5a8d9fe476c16056511
# good: [3f644cdb2351fe21cded6ee1e5c13ea7905c3a64] Merge branch 'clk-zynq' into clk-next
git bisect good 3f644cdb2351fe21cded6ee1e5c13ea7905c3a64
# bad: [0db9597d81d918605d4d36c87ab140228fe14150] Merge remote-tracking branch 'clk-samsung/for-next'
git bisect bad 0db9597d81d918605d4d36c87ab140228fe14150
# bad: [e04cb6e358cbcdce56cda317725131252ecf6ccd] Merge branch 'clk-parent-rewrite-1' into clk-next
git bisect bad e04cb6e358cbcdce56cda317725131252ecf6ccd
# good: [89a5ddcc799d5d7dbcf6197b79dafc1dc9f997f5] clk: Add of_clk_hw_register() API for early clk drivers
git bisect good 89a5ddcc799d5d7dbcf6197b79dafc1dc9f997f5
# good: [dde4eff47c82c52a72af333d9e55370eee6d95d6] clk: Look for parents with clkdev based clk_lookups
git bisect good dde4eff47c82c52a72af333d9e55370eee6d95d6
# bad: [ecbf3f1795fda56122632c1d024cfd0d3f4fe353] clk: fixed-factor: Let clk framework find parent
git bisect bad ecbf3f1795fda56122632c1d024cfd0d3f4fe353
# good: [601b6e93304a65f8f7c37168763ab9ba5b195ce5] clk: Allow parents to be specified via clkspec index
git bisect good 601b6e93304a65f8f7c37168763ab9ba5b195ce5
# first bad commit: [ecbf3f1795fda56122632c1d024cfd0d3f4fe353] clk: fixed-factor: Let clk framework find parent

Re: [PATCH v4 9/9] clk: fixed-factor: Let clk framework find parent

[Stephen Boyd]

Quoting Guenter Roeck (2019-04-23 11:09:22)
> Hi,
> 
> On Fri, Apr 12, 2019 at 11:31:50AM -0700, Stephen Boyd wrote:
> > Convert this driver to a more modern way of specifying parents now that
> > we have a way to specify clk parents by DT index. This lets us nicely
> > avoid a problem where a parent clk name isn't know because the parent
> > clk hasn't been registered yet.
> > 
> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > Cc: Jerome Brunet <jbrunet@baylibre.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> > Cc: Chen-Yu Tsai <wens@csie.org>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> 
> This patch causes a substantial number of crashes of qemu boot tests in -next.
> 
> Failed tests: 
>         arm:versatilepb:versatile_defconfig:aeabi:pci:scsi:mem128:versatile-pb:rootfs 
>         arm:versatilepb:versatile_defconfig:aeabi:pci:mem128:versatile-pb:initrd 
>         arm:versatileab:versatile_defconfig:mem128:versatile-ab:initrd 
>         arm:beagle:multi_v7_defconfig:sd:mem256:omap3-beagle:rootfs 
>         arm:beaglexm:multi_v7_defconfig:sd:mem512:omap3-beagle-xm:rootfs 
>         arm:overo:multi_v7_defconfig:sd:mem256:omap3-overo-tobi:rootfs 
>         arm:realview-pb-a8:realview_defconfig:realview_pb:mem512:arm-realview-pba8:initrd 
>         arm:realview-pbx-a9:realview_defconfig:realview_pb:arm-realview-pbx-a9:initrd 
>         arm:realview-eb:realview_defconfig:realview_eb:mem512:arm-realview-eb:initrd 
>         arm:realview-eb-mpcore:realview_defconfig:realview_eb:mem512:arm-realview-eb-11mp-ctrevb:initrd 
>         arm:integratorcp:integrator_defconfig:mem128:integratorcp:initrd 
>         arm:mps2-an385:mps2_defconfig:mps2-an385:initrd
> 
> Most of the time the crash happens too early to generate a log,
> but here is one:
> 
> [    0.000000] [<2100bd59>] (unwind_backtrace) from [<2100b11f>] (show_stack+0xb/0xc)
> [    0.000000] [<2100b11f>] (show_stack) from [<211b2d27>] (Ldiv0_64+0x9/0x1a)
> [    0.000000] [<211b2d27>] (Ldiv0_64) from [<21038e87>] (clocks_calc_max_nsecs+0x1d/0x62)
> [    0.000000] [<21038e87>] (clocks_calc_max_nsecs) from [<21038fb1>] (__clocksource_update_freq_scale+0xe5/0x11c)
> [    0.000000] [<21038fb1>] (__clocksource_update_freq_scale) from [<21038ff1>] (__clocksource_register_scale+0x9/0x40)
> [    0.000000] [<21038ff1>] (__clocksource_register_scale) from [<212a8713>] (mps2_timer_init+0xaf/0x29c)
> [    0.000000] [<212a8713>] (mps2_timer_init) from [<212a85b1>] (timer_probe+0x49/0x80)
> [    0.000000] [<212a85b1>] (timer_probe) from [<2129d639>] (start_kernel+0x1c5/0x2f4)
> [    0.000000] [<2129d639>] (start_kernel) from [<00000000>] (  (null))
> [    0.000000] clocksource: mps2-clksrc: mask: 0xffffffff max_cycles: 0x0, max_idle_ns: 0 ns
> [    0.000000] Division by zero in kernel.
> 
> Reverting the crash fixes the problem. Bisect log attached.
> 

Thanks for the report. This was bisected yesterday by kernelci.org (see
https://lkml.kernel.org/r/5cbe596c.1c69fb81.e252.b9d0@mx.google.com for
more details). Can you try the latest version of clk-next and see if it
fixes the early crashes? The one-liner patch I attached in that thread
should be all you need.

It would be even better for kernelci to find the offending patch like
you've done here and reply to the patch on the mailing list.

Finally, can you share your qemu recipe? I can pull it into my testing
and integration workflow so that this doesn't happen again.

Re: [PATCH v4 9/9] clk: fixed-factor: Let clk framework find parent

[Guenter Roeck]

On Tue, Apr 23, 2019 at 11:22:57AM -0700, Stephen Boyd wrote:
> Quoting Guenter Roeck (2019-04-23 11:09:22)
> > Hi,
> > 
> > On Fri, Apr 12, 2019 at 11:31:50AM -0700, Stephen Boyd wrote:
> > > Convert this driver to a more modern way of specifying parents now that
> > > we have a way to specify clk parents by DT index. This lets us nicely
> > > avoid a problem where a parent clk name isn't know because the parent
> > > clk hasn't been registered yet.
> > > 
> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Cc: Jerome Brunet <jbrunet@baylibre.com>
> > > Cc: Russell King <linux@armlinux.org.uk>
> > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> > > Cc: Chen-Yu Tsai <wens@csie.org>
> > > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > 
> > This patch causes a substantial number of crashes of qemu boot tests in -next.
> > 
> > Failed tests: 
> >         arm:versatilepb:versatile_defconfig:aeabi:pci:scsi:mem128:versatile-pb:rootfs 
> >         arm:versatilepb:versatile_defconfig:aeabi:pci:mem128:versatile-pb:initrd 
> >         arm:versatileab:versatile_defconfig:mem128:versatile-ab:initrd 
> >         arm:beagle:multi_v7_defconfig:sd:mem256:omap3-beagle:rootfs 
> >         arm:beaglexm:multi_v7_defconfig:sd:mem512:omap3-beagle-xm:rootfs 
> >         arm:overo:multi_v7_defconfig:sd:mem256:omap3-overo-tobi:rootfs 
> >         arm:realview-pb-a8:realview_defconfig:realview_pb:mem512:arm-realview-pba8:initrd 
> >         arm:realview-pbx-a9:realview_defconfig:realview_pb:arm-realview-pbx-a9:initrd 
> >         arm:realview-eb:realview_defconfig:realview_eb:mem512:arm-realview-eb:initrd 
> >         arm:realview-eb-mpcore:realview_defconfig:realview_eb:mem512:arm-realview-eb-11mp-ctrevb:initrd 
> >         arm:integratorcp:integrator_defconfig:mem128:integratorcp:initrd 
> >         arm:mps2-an385:mps2_defconfig:mps2-an385:initrd
> > 
> > Most of the time the crash happens too early to generate a log,
> > but here is one:
> > 
> > [    0.000000] [<2100bd59>] (unwind_backtrace) from [<2100b11f>] (show_stack+0xb/0xc)
> > [    0.000000] [<2100b11f>] (show_stack) from [<211b2d27>] (Ldiv0_64+0x9/0x1a)
> > [    0.000000] [<211b2d27>] (Ldiv0_64) from [<21038e87>] (clocks_calc_max_nsecs+0x1d/0x62)
> > [    0.000000] [<21038e87>] (clocks_calc_max_nsecs) from [<21038fb1>] (__clocksource_update_freq_scale+0xe5/0x11c)
> > [    0.000000] [<21038fb1>] (__clocksource_update_freq_scale) from [<21038ff1>] (__clocksource_register_scale+0x9/0x40)
> > [    0.000000] [<21038ff1>] (__clocksource_register_scale) from [<212a8713>] (mps2_timer_init+0xaf/0x29c)
> > [    0.000000] [<212a8713>] (mps2_timer_init) from [<212a85b1>] (timer_probe+0x49/0x80)
> > [    0.000000] [<212a85b1>] (timer_probe) from [<2129d639>] (start_kernel+0x1c5/0x2f4)
> > [    0.000000] [<2129d639>] (start_kernel) from [<00000000>] (  (null))
> > [    0.000000] clocksource: mps2-clksrc: mask: 0xffffffff max_cycles: 0x0, max_idle_ns: 0 ns
> > [    0.000000] Division by zero in kernel.
> > 
> > Reverting the crash fixes the problem. Bisect log attached.
> > 
> 
> Thanks for the report. This was bisected yesterday by kernelci.org (see
> https://lkml.kernel.org/r/5cbe596c.1c69fb81.e252.b9d0@mx.google.com for
> more details). Can you try the latest version of clk-next and see if it
> fixes the early crashes? The one-liner patch I attached in that thread
> should be all you need.
> 
Yes, it done. Thanks for taking care of it.

> It would be even better for kernelci to find the offending patch like
> you've done here and reply to the patch on the mailing list.
> 
> Finally, can you share your qemu recipe? I can pull it into my testing
> and integration workflow so that this doesn't happen again.
> 

It is all available at https://github.com/groeck/linux-build-test/.
You would need an out-of-tree version of qemu to test mps2-an385, though;
upstream qemu rejected the patches necessary to make it work for testing
linux kernel images.

Guenter

Re: [PATCH v4 0/9] Rewrite clk parent handling

[Jeffrey Hugo]

On 4/12/2019 12:31 PM, Stephen Boyd wrote:
> There are a couple warts with clk parent handling in the common clk
> framework. This patch series addresses one of those warts, parent-child
> linkages. We use strings for all parent-child linkages, and this leads
> to poorly written code that extracts clk names from struct clk pointers
> and makes clk provider drivers use clk consumer APIs. I've converted the
> fixed factor clk and I'm looking at converting users of other "basic"
> clk types in follow-up patches so we can start moving drivers away from
> string matching in the clk namespace.
> 
> Changes from v3:
>   * Split clkdev patch into two
>   * New patch to let dev_of_node() accept NULL
>   * Use dev_of_node() in of_clk_hw_register() to avoid NULL deref
> 
> Changes from v2:
>   * Dropped patches that got merged into v5.1-rc1
>   * Introduced a new function of_clk_hw_register()
>   * Introduced 'index' into clk_parent_data structure
>   * Converted fixed-factor basic type to new design
>   * Fixed some bugs in a recent patch to clkdev, leading to
>     an essential API for clkdev based lookups working
> 
> Changes from v1:
>   * Dropped new get_parent_hw, we'll pick it up in a later series
>   * Rebased to v5.0-rc6 to avoid conflicts with clk-fixes
>   * Renamed 'fallback' to 'name' and 'name' to 'fw_name' in parent map
>   * Made sure that clk_hw_get_parent_by_index() never sees an error pointer
>     so that we don't mistakenly try to defer an error pointer
>   * Fixed index passing mistake on of_clk_get_hw_from_clkspec()
>   * Copy over all the data from parent maps and free it correctly too
> 
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jeffrey Hugo <jhugo@codeaurora.org>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Russell King <linux@armlinux.org.uk>
> 
> Stephen Boyd (9):
>    clkdev: Hold clocks_mutex while iterating clocks list
>    clkdev: Move clk creation outside of 'clocks_mutex'
>    clk: Prepare for clk registration API that uses DT nodes
>    driver core: Let dev_of_node() accept a NULL dev
>    clk: Add of_clk_hw_register() API for early clk drivers
>    clk: Allow parents to be specified without string names
>    clk: Look for parents with clkdev based clk_lookups
>    clk: Allow parents to be specified via clkspec index
>    clk: fixed-factor: Let clk framework find parent
> 
>   drivers/clk/clk-fixed-factor.c |  53 ++++--
>   drivers/clk/clk.c              | 326 +++++++++++++++++++++++++--------
>   drivers/clk/clk.h              |   2 +
>   drivers/clk/clkdev.c           |  30 +--
>   include/linux/clk-provider.h   |  22 +++
>   include/linux/device.h         |   2 +-
>   6 files changed, 327 insertions(+), 108 deletions(-)
> 
> 
> base-commit: 9e98c678c2d6ae3a17cb2de55d17f69dddaa231b
> 

Tested-by: Jeffrey Hugo <jhugo@codeaurora.org>

-- 
Jeffrey Hugo
Qualcomm Datacenter Technologies as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.