[PATCH v3 0/2] clk: improve handling of orphan clocks

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stuebner]

Using orphan clocks can introduce strange behaviour as they don't have
rate information at all and also of course don't track 

This v2/v3 takes into account suggestions from Stephen Boyd to not try to
walk the clock tree at runtime but instead keep track of orphan states
on clock tree changes and making it mandatory for everybody from the
start as orphaned clocks should not be used at all.


This fixes an issue on most rk3288 platforms, where some soc-clocks
are supplied by a 32khz clock from an external i2c-chip which often
is only probed later in the boot process and maybe even after the
drivers using these soc-clocks like the tsadc temperature sensor.
In this case the driver using the clock should of course defer probing
until the clock is actually usable.


As this changes the behaviour for orphan clocks, it would of course
benefit from more testing than on my Rockchip boards. To keep the
recipent-list reasonable and not spam to much I selected one (the topmost)
from the get_maintainer output of each drivers/clk entry.
Hopefully some will provide Tested-by-tags :-)


Thanks
Heiko

changes since v2:
adapt to comments from Stephen Boyd
- rename to clk_core_update_orphan_status
- use bools instead of 0/1 for the status
- remove redundant check in clk_is_orphan
changes since v1:
- track orphan status on clock tree changes instead of walking the
  tree on clk_get operations
- make get-deferals mandatory for everybody

Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Alex Elder <elder@linaro.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: kernel@pengutronix.de
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Chao Xie <chao.xie@marvell.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Andrew Bresticker <abrestic@chromium.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Barry Song <baohua@kernel.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
Cc: emilio@elopez.com.ar
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Michal Simek <michal.simek@xilinx.com>


Heiko Stuebner (2):
  clk: track the orphan status of clocks and their children
  clk: prevent orphan clocks from being used

 drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

-- 
2.1.4

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stuebner]

Using orphan clocks can introduce strange behaviour as they don't have
rate information at all and also of course don't track 

This v2/v3 takes into account suggestions from Stephen Boyd to not try to
walk the clock tree at runtime but instead keep track of orphan states
on clock tree changes and making it mandatory for everybody from the
start as orphaned clocks should not be used at all.


This fixes an issue on most rk3288 platforms, where some soc-clocks
are supplied by a 32khz clock from an external i2c-chip which often
is only probed later in the boot process and maybe even after the
drivers using these soc-clocks like the tsadc temperature sensor.
In this case the driver using the clock should of course defer probing
until the clock is actually usable.


As this changes the behaviour for orphan clocks, it would of course
benefit from more testing than on my Rockchip boards. To keep the
recipent-list reasonable and not spam to much I selected one (the topmost)
from the get_maintainer output of each drivers/clk entry.
Hopefully some will provide Tested-by-tags :-)


Thanks
Heiko

changes since v2:
adapt to comments from Stephen Boyd
- rename to clk_core_update_orphan_status
- use bools instead of 0/1 for the status
- remove redundant check in clk_is_orphan
changes since v1:
- track orphan status on clock tree changes instead of walking the
  tree on clk_get operations
- make get-deferals mandatory for everybody

Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Alex Elder <elder@linaro.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: kernel at pengutronix.de
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Chao Xie <chao.xie@marvell.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Andrew Bresticker <abrestic@chromium.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Barry Song <baohua@kernel.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
Cc: emilio at elopez.com.ar
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Michal Simek <michal.simek@xilinx.com>


Heiko Stuebner (2):
  clk: track the orphan status of clocks and their children
  clk: prevent orphan clocks from being used

 drivers/clk/clk.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

-- 
2.1.4

[PATCH v3 1/2] clk: track the orphan status of clocks and their children

[Heiko Stuebner]

While children of orphan clocks are not carried in the orphan-list itself,
they're nevertheless orphans in their own right as they also don't have an
input-rate available. To ease tracking if a clock is an orphan or has an
orphan in its parent path introduce an orphan field into struct clk and
update it and the fields in child-clocks when a clock gets added or removed
from the orphan-list.

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Alex Elder <elder@linaro.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: kernel@pengutronix.de
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Chao Xie <chao.xie@marvell.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Andrew Bresticker <abrestic@chromium.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Barry Song <baohua@kernel.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
Cc: emilio@elopez.com.ar
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 drivers/clk/clk.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b2361d4..341904f 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -62,6 +62,7 @@ struct clk_core {
 	struct clk_core		*new_parent;
 	struct clk_core		*new_child;
 	unsigned long		flags;
+	bool			orphan;
 	unsigned int		enable_count;
 	unsigned int		prepare_count;
 	unsigned long		accuracy;
@@ -1401,18 +1402,40 @@ static int clk_fetch_parent_index(struct clk_core *clk,
 	return -EINVAL;
 }
 
+/*
+ * Update the orphan status of @clk and all its children.
+ */
+static void clk_core_update_orphan_status(struct clk_core *clk, bool is_orphan)
+{
+	struct clk_core *child;
+
+	clk->orphan = is_orphan;
+
+	hlist_for_each_entry(child, &clk->children, child_node)
+		clk_core_update_orphan_status(child, is_orphan);
+}
+
 static void clk_reparent(struct clk_core *clk, struct clk_core *new_parent)
 {
+	bool was_orphan = clk->orphan;
+
 	hlist_del(&clk->child_node);
 
 	if (new_parent) {
+		bool becomes_orphan = new_parent->orphan;
+
 		/* avoid duplicate POST_RATE_CHANGE notifications */
 		if (new_parent->new_child == clk)
 			new_parent->new_child = NULL;
 
 		hlist_add_head(&clk->child_node, &new_parent->children);
+
+		if (was_orphan != becomes_orphan)
+			clk_core_update_orphan_status(clk, becomes_orphan);
 	} else {
 		hlist_add_head(&clk->child_node, &clk_orphan_list);
+		if (!was_orphan)
+			clk_core_update_orphan_status(clk, true);
 	}
 
 	clk->parent = new_parent;
@@ -2302,13 +2325,17 @@ static int __clk_init(struct device *dev, struct clk *clk_user)
 	 * clocks and re-parent any that are children of the clock currently
 	 * being clk_init'd.
 	 */
-	if (clk->parent)
+	if (clk->parent) {
 		hlist_add_head(&clk->child_node,
 				&clk->parent->children);
-	else if (clk->flags & CLK_IS_ROOT)
+		clk->orphan = clk->parent->orphan;
+	} else if (clk->flags & CLK_IS_ROOT) {
 		hlist_add_head(&clk->child_node, &clk_root_list);
-	else
+		clk->orphan = false;
+	} else {
 		hlist_add_head(&clk->child_node, &clk_orphan_list);
+		clk->orphan = true;
+	}
 
 	/*
 	 * Set clk's accuracy.  The preferred method is to use
-- 
2.1.4

[PATCH v3 1/2] clk: track the orphan status of clocks and their children

[Heiko Stuebner]

While children of orphan clocks are not carried in the orphan-list itself,
they're nevertheless orphans in their own right as they also don't have an
input-rate available. To ease tracking if a clock is an orphan or has an
orphan in its parent path introduce an orphan field into struct clk and
update it and the fields in child-clocks when a clock gets added or removed
from the orphan-list.

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Alex Elder <elder@linaro.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: kernel at pengutronix.de
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Chao Xie <chao.xie@marvell.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Andrew Bresticker <abrestic@chromium.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Barry Song <baohua@kernel.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
Cc: emilio at elopez.com.ar
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 drivers/clk/clk.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index b2361d4..341904f 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -62,6 +62,7 @@ struct clk_core {
 	struct clk_core		*new_parent;
 	struct clk_core		*new_child;
 	unsigned long		flags;
+	bool			orphan;
 	unsigned int		enable_count;
 	unsigned int		prepare_count;
 	unsigned long		accuracy;
@@ -1401,18 +1402,40 @@ static int clk_fetch_parent_index(struct clk_core *clk,
 	return -EINVAL;
 }
 
+/*
+ * Update the orphan status of @clk and all its children.
+ */
+static void clk_core_update_orphan_status(struct clk_core *clk, bool is_orphan)
+{
+	struct clk_core *child;
+
+	clk->orphan = is_orphan;
+
+	hlist_for_each_entry(child, &clk->children, child_node)
+		clk_core_update_orphan_status(child, is_orphan);
+}
+
 static void clk_reparent(struct clk_core *clk, struct clk_core *new_parent)
 {
+	bool was_orphan = clk->orphan;
+
 	hlist_del(&clk->child_node);
 
 	if (new_parent) {
+		bool becomes_orphan = new_parent->orphan;
+
 		/* avoid duplicate POST_RATE_CHANGE notifications */
 		if (new_parent->new_child == clk)
 			new_parent->new_child = NULL;
 
 		hlist_add_head(&clk->child_node, &new_parent->children);
+
+		if (was_orphan != becomes_orphan)
+			clk_core_update_orphan_status(clk, becomes_orphan);
 	} else {
 		hlist_add_head(&clk->child_node, &clk_orphan_list);
+		if (!was_orphan)
+			clk_core_update_orphan_status(clk, true);
 	}
 
 	clk->parent = new_parent;
@@ -2302,13 +2325,17 @@ static int __clk_init(struct device *dev, struct clk *clk_user)
 	 * clocks and re-parent any that are children of the clock currently
 	 * being clk_init'd.
 	 */
-	if (clk->parent)
+	if (clk->parent) {
 		hlist_add_head(&clk->child_node,
 				&clk->parent->children);
-	else if (clk->flags & CLK_IS_ROOT)
+		clk->orphan = clk->parent->orphan;
+	} else if (clk->flags & CLK_IS_ROOT) {
 		hlist_add_head(&clk->child_node, &clk_root_list);
-	else
+		clk->orphan = false;
+	} else {
 		hlist_add_head(&clk->child_node, &clk_orphan_list);
+		clk->orphan = true;
+	}
 
 	/*
 	 * Set clk's accuracy.  The preferred method is to use
-- 
2.1.4

[PATCH v3 2/2] clk: prevent orphan clocks from being used

[Heiko Stuebner]

Orphan clocks or children of orphan clocks don't have rate information at
all and can produce strange results if they're allowed to be used and the
parent becomes available later on.

So using the newly introduced orphan status bit defer
__of_clk_get_from_provider calls for orphan clocks.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Alex Elder <elder@linaro.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: kernel@pengutronix.de
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Chao Xie <chao.xie@marvell.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Andrew Bresticker <abrestic@chromium.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Barry Song <baohua@kernel.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
Cc: emilio@elopez.com.ar
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 drivers/clk/clk.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 341904f..27d2d4b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2226,6 +2226,14 @@ bool clk_is_match(const struct clk *p, const struct clk *q)
 }
 EXPORT_SYMBOL_GPL(clk_is_match);
 
+static bool clk_is_orphan(const struct clk *clk)
+{
+	if (!clk)
+		return false;
+
+	return clk->core->orphan;
+}
+
 /**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:	device initializing this clk, placeholder for now
@@ -2968,6 +2976,11 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 		if (provider->node == clkspec->np)
 			clk = provider->get(clkspec, provider->data);
 		if (!IS_ERR(clk)) {
+			if (clk_is_orphan(clk)) {
+				clk = ERR_PTR(-EPROBE_DEFER);
+				break;
+			}
+
 			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
 					       con_id);
 
-- 
2.1.4

[PATCH v3 2/2] clk: prevent orphan clocks from being used

[Heiko Stuebner]

Orphan clocks or children of orphan clocks don't have rate information at
all and can produce strange results if they're allowed to be used and the
parent becomes available later on.

So using the newly introduced orphan status bit defer
__of_clk_get_from_provider calls for orphan clocks.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
Cc: Alex Elder <elder@linaro.org>
Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: kernel at pengutronix.de
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Chao Xie <chao.xie@marvell.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Andrew Bresticker <abrestic@chromium.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Georgi Djakov <georgi.djakov@linaro.org>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Barry Song <baohua@kernel.org>
Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Viresh Kumar <viresh.linux@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
Cc: emilio at elopez.com.ar
Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Michal Simek <michal.simek@xilinx.com>
---
 drivers/clk/clk.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 341904f..27d2d4b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2226,6 +2226,14 @@ bool clk_is_match(const struct clk *p, const struct clk *q)
 }
 EXPORT_SYMBOL_GPL(clk_is_match);
 
+static bool clk_is_orphan(const struct clk *clk)
+{
+	if (!clk)
+		return false;
+
+	return clk->core->orphan;
+}
+
 /**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:	device initializing this clk, placeholder for now
@@ -2968,6 +2976,11 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 		if (provider->node == clkspec->np)
 			clk = provider->get(clkspec, provider->data);
 		if (!IS_ERR(clk)) {
+			if (clk_is_orphan(clk)) {
+				clk = ERR_PTR(-EPROBE_DEFER);
+				break;
+			}
+
 			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
 					       con_id);
 
-- 
2.1.4

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stefan Wahren]

Hi Heiko,

> Heiko Stuebner <heiko@sntech.de> hat am 22. April 2015 um 22:53 geschrieben:
>
>
> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
>
> [...]
>
>
> As this changes the behaviour for orphan clocks, it would of course
> benefit from more testing than on my Rockchip boards. To keep the
> recipent-list reasonable and not spam to much I selected one (the topmost)
> from the get_maintainer output of each drivers/clk entry.
> Hopefully some will provide Tested-by-tags :-)
>

excuse me for my naive question, but what kind of tests do you expect (beside
applying the patches)?

Stefan

>
> Thanks
> Heiko
>

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stefan Wahren]

Hi Heiko,

> Heiko Stuebner <heiko@sntech.de> hat am 22. April 2015 um 22:53 geschrieben:
>
>
> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
>
> [...]
>
>
> As this changes the behaviour for orphan clocks, it would of course
> benefit from more testing than on my Rockchip boards. To keep the
> recipent-list reasonable and not spam to much I selected one (the topmost)
> from the get_maintainer output of each drivers/clk entry.
> Hopefully some will provide Tested-by-tags :-)
>

excuse me for my naive question, but what kind of tests do you expect (beside
applying the patches)?

Stefan

>
> Thanks
> Heiko
>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Hi Stefan,

Am Samstag, 25. April 2015, 14:23:39 schrieb Stefan Wahren:
> > Heiko Stuebner <heiko@sntech.de> hat am 22. April 2015 um 22:53
> > geschrieben:
> > 
> > 
> > Using orphan clocks can introduce strange behaviour as they don't have
> > rate information at all and also of course don't track
> > 
> > [...]
> > 
> > 
> > As this changes the behaviour for orphan clocks, it would of course
> > benefit from more testing than on my Rockchip boards. To keep the
> > recipent-list reasonable and not spam to much I selected one (the topmost)
> > from the get_maintainer output of each drivers/clk entry.
> > Hopefully some will provide Tested-by-tags :-)
> 
> excuse me for my naive question, but what kind of tests do you expect
> (beside applying the patches)?

just a "everything that worked before still works" :-)

Using orphaned clocks should already produce strange issues most of the time - 
for example I see clk_disable mismatches when suspending a rk3288 board, 
before this patchset.

But nevertheless we now disallow the usage of orphaned clocks completely while 
before it was possible to knowingly/unknowingly use them.

And while hopefully most drivers should handle an EPROBE_DEFER from clk_get 
just fine, there may still be one or two lurking around that would need fixing 
then ;-)


Heiko

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Hi Stefan,

Am Samstag, 25. April 2015, 14:23:39 schrieb Stefan Wahren:
> > Heiko Stuebner <heiko@sntech.de> hat am 22. April 2015 um 22:53
> > geschrieben:
> > 
> > 
> > Using orphan clocks can introduce strange behaviour as they don't have
> > rate information at all and also of course don't track
> > 
> > [...]
> > 
> > 
> > As this changes the behaviour for orphan clocks, it would of course
> > benefit from more testing than on my Rockchip boards. To keep the
> > recipent-list reasonable and not spam to much I selected one (the topmost)
> > from the get_maintainer output of each drivers/clk entry.
> > Hopefully some will provide Tested-by-tags :-)
> 
> excuse me for my naive question, but what kind of tests do you expect
> (beside applying the patches)?

just a "everything that worked before still works" :-)

Using orphaned clocks should already produce strange issues most of the time - 
for example I see clk_disable mismatches when suspending a rk3288 board, 
before this patchset.

But nevertheless we now disallow the usage of orphaned clocks completely while 
before it was possible to knowingly/unknowingly use them.

And while hopefully most drivers should handle an EPROBE_DEFER from clk_get 
just fine, there may still be one or two lurking around that would need fixing 
then ;-)


Heiko

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Robert Jarzmik]

Heiko Stuebner <heiko@sntech.de> writes:

> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
Ok, everything works as before on some pxa platforms.

As pxa clocks support is not even fully mainline, this test is not the best you
could get.

Cheers.

-- 
Robert

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Robert Jarzmik]

Heiko Stuebner <heiko@sntech.de> writes:

> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
Ok, everything works as before on some pxa platforms.

As pxa clocks support is not even fully mainline, this test is not the best you
could get.

Cheers.

-- 
Robert

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Robert Jarzmik]

Heiko Stuebner <heiko@sntech.de> writes:

> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track
Ok, everything works as before on some pxa platforms.

As pxa clocks support is not even fully mainline, this test is not the best you
could get.

Cheers.

-- 
Robert

Re: [PATCH v3 1/2] clk: track the orphan status of clocks and their children

[Stephen Boyd]

On 04/22, Heiko Stuebner wrote:
> @@ -1401,18 +1402,40 @@ static int clk_fetch_parent_index(struct clk_core *clk,
>  	return -EINVAL;
>  }
>  
> +/*
> + * Update the orphan status of @clk and all its children.
> + */
> +static void clk_core_update_orphan_status(struct clk_core *clk, bool is_orphan)

Missed s/clk/core/ here. I did it, no need to resend.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 1/2] clk: track the orphan status of clocks and their children

[Stephen Boyd]

On 04/22, Heiko Stuebner wrote:
> @@ -1401,18 +1402,40 @@ static int clk_fetch_parent_index(struct clk_core *clk,
>  	return -EINVAL;
>  }
>  
> +/*
> + * Update the orphan status of @clk and all its children.
> + */
> +static void clk_core_update_orphan_status(struct clk_core *clk, bool is_orphan)

Missed s/clk/core/ here. I did it, no need to resend.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 2/2] clk: prevent orphan clocks from being used

[Stephen Boyd]

On 04/22, Heiko Stuebner wrote:
> Orphan clocks or children of orphan clocks don't have rate information at
> all and can produce strange results if they're allowed to be used and the
> parent becomes available later on.
> 
> So using the newly introduced orphan status bit defer
> __of_clk_get_from_provider calls for orphan clocks.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Alex Elder <elder@linaro.org>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: kernel@pengutronix.de
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Chao Xie <chao.xie@marvell.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Andrew Bresticker <abrestic@chromium.org>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Georgi Djakov <georgi.djakov@linaro.org>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Barry Song <baohua@kernel.org>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Viresh Kumar <viresh.linux@gmail.com>
> Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
> Cc: emilio@elopez.com.ar
> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Michal Simek <michal.simek@xilinx.com>

Applied to clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 2/2] clk: prevent orphan clocks from being used

[Stephen Boyd]

On 04/22, Heiko Stuebner wrote:
> Orphan clocks or children of orphan clocks don't have rate information at
> all and can produce strange results if they're allowed to be used and the
> parent becomes available later on.
> 
> So using the newly introduced orphan status bit defer
> __of_clk_get_from_provider calls for orphan clocks.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Cc: Boris Brezillon <boris.brezillon@free-electrons.com>
> Cc: Alex Elder <elder@linaro.org>
> Cc: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Max Filippov <jcmvbkbc@gmail.com>
> Cc: kernel at pengutronix.de
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Chao Xie <chao.xie@marvell.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Andrew Bresticker <abrestic@chromium.org>
> Cc: Robert Jarzmik <robert.jarzmik@free.fr>
> Cc: Georgi Djakov <georgi.djakov@linaro.org>
> Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Barry Song <baohua@kernel.org>
> Cc: Dinh Nguyen <dinguyen@opensource.altera.com>
> Cc: Viresh Kumar <viresh.linux@gmail.com>
> Cc: Gabriel FERNANDEZ <gabriel.fernandez@st.com>
> Cc: emilio at elopez.com.ar
> Cc: Peter De Schrijver <pdeschrijver@nvidia.com>
> Cc: Tero Kristo <t-kristo@ti.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Michal Simek <michal.simek@xilinx.com>

Applied to clk-next.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 04/22, Heiko Stuebner wrote:
> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track 
> 
> This v2/v3 takes into account suggestions from Stephen Boyd to not try to
> walk the clock tree at runtime but instead keep track of orphan states
> on clock tree changes and making it mandatory for everybody from the
> start as orphaned clocks should not be used at all.
> 
> 
> This fixes an issue on most rk3288 platforms, where some soc-clocks
> are supplied by a 32khz clock from an external i2c-chip which often
> is only probed later in the boot process and maybe even after the
> drivers using these soc-clocks like the tsadc temperature sensor.
> In this case the driver using the clock should of course defer probing
> until the clock is actually usable.
> 
> 
> As this changes the behaviour for orphan clocks, it would of course
> benefit from more testing than on my Rockchip boards. To keep the
> recipent-list reasonable and not spam to much I selected one (the topmost)
> from the get_maintainer output of each drivers/clk entry.
> Hopefully some will provide Tested-by-tags :-)
> 

<grumble> I don't see any Tested-by: tags yet </grumble>. I've
put these two patches on a separate branch "defer-orphans" and
pushed it to clk-next so we can give it some more exposure.

Unfortunately this doesn't solve the orphan problem for non-OF
providers. What if we did the orphan check in __clk_create_clk()
instead and returned an error pointer for orphans? I suspect that
will solve all cases, right?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 04/22, Heiko Stuebner wrote:
> Using orphan clocks can introduce strange behaviour as they don't have
> rate information at all and also of course don't track 
> 
> This v2/v3 takes into account suggestions from Stephen Boyd to not try to
> walk the clock tree at runtime but instead keep track of orphan states
> on clock tree changes and making it mandatory for everybody from the
> start as orphaned clocks should not be used at all.
> 
> 
> This fixes an issue on most rk3288 platforms, where some soc-clocks
> are supplied by a 32khz clock from an external i2c-chip which often
> is only probed later in the boot process and maybe even after the
> drivers using these soc-clocks like the tsadc temperature sensor.
> In this case the driver using the clock should of course defer probing
> until the clock is actually usable.
> 
> 
> As this changes the behaviour for orphan clocks, it would of course
> benefit from more testing than on my Rockchip boards. To keep the
> recipent-list reasonable and not spam to much I selected one (the topmost)
> from the get_maintainer output of each drivers/clk entry.
> Hopefully some will provide Tested-by-tags :-)
> 

<grumble> I don't see any Tested-by: tags yet </grumble>. I've
put these two patches on a separate branch "defer-orphans" and
pushed it to clk-next so we can give it some more exposure.

Unfortunately this doesn't solve the orphan problem for non-OF
providers. What if we did the orphan check in __clk_create_clk()
instead and returned an error pointer for orphans? I suspect that
will solve all cases, right?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
> On 04/22, Heiko Stuebner wrote:
> > Using orphan clocks can introduce strange behaviour as they don't have
> > rate information at all and also of course don't track
> > 
> > This v2/v3 takes into account suggestions from Stephen Boyd to not try to
> > walk the clock tree at runtime but instead keep track of orphan states
> > on clock tree changes and making it mandatory for everybody from the
> > start as orphaned clocks should not be used at all.
> > 
> > 
> > This fixes an issue on most rk3288 platforms, where some soc-clocks
> > are supplied by a 32khz clock from an external i2c-chip which often
> > is only probed later in the boot process and maybe even after the
> > drivers using these soc-clocks like the tsadc temperature sensor.
> > In this case the driver using the clock should of course defer probing
> > until the clock is actually usable.
> > 
> > 
> > As this changes the behaviour for orphan clocks, it would of course
> > benefit from more testing than on my Rockchip boards. To keep the
> > recipent-list reasonable and not spam to much I selected one (the topmost)
> > from the get_maintainer output of each drivers/clk entry.
> > Hopefully some will provide Tested-by-tags :-)
> 
> <grumble> I don't see any Tested-by: tags yet </grumble>. I've
> put these two patches on a separate branch "defer-orphans" and
> pushed it to clk-next so we can give it some more exposure.
> 
> Unfortunately this doesn't solve the orphan problem for non-OF
> providers. What if we did the orphan check in __clk_create_clk()
> instead and returned an error pointer for orphans? I suspect that
> will solve all cases, right?

hmm, clk_register also uses __clk_create_clk, which in turn would prevent
registering orphan-clocks at all, I'd think.
As on my platform I'm dependant on orphan clocks (the soc-level clock gets
registerted as part of the big clock controller way before the i2c-based
supplying clock), I'd rather not touch this :-) .

Instead I guess we could hook it less deep into clk_get_sys, like in the
following patch?


------------ 8< ------------------------- >8 -----------------
From: Heiko Stuebner <heiko@sntech.de>
Date: Fri, 1 May 2015 21:50:46 +0200
Subject: [PATCH] clk: prevent orphan access on non-devicetree platforms too

The orphan-check in __of_clk_get_from_provider only prevents orphan-access
on devicetree platforms. To bring non-dt platforms to the same level of
functionality let clk_get_sys (called from clk_get for non-dt platforms)
also check for orphans and return -EPROBE_DEFER in that case.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/clk.c    | 2 +-
 drivers/clk/clk.h    | 5 +++++
 drivers/clk/clkdev.c | 5 +++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 36d1a01..167d0bf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2220,7 +2220,7 @@ static inline void clk_debug_unregister(struct clk_core *core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
+bool clk_is_orphan(const struct clk *clk)
 {
 	if (!clk)
 		return false;
diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
index 00b35a1..b8a6061 100644
--- a/drivers/clk/clk.h
+++ b/drivers/clk/clk.h
@@ -20,6 +20,7 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 			     const char *con_id);
 void __clk_free_clk(struct clk *clk);
+bool clk_is_orphan(const struct clk *clk);
 #else
 /* All these casts to avoid ifdefs in clkdev... */
 static inline struct clk *
@@ -32,5 +33,9 @@ static struct clk_hw *__clk_get_hw(struct clk *clk)
 {
 	return (struct clk_hw *)clk;
 }
+static inline bool clk_is_orphan(const struct clk *clk)
+{
+	return false;
+}
 
 #endif
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 1fcb6ef..ad96775 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -177,6 +177,11 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id)
 	if (!cl)
 		goto out;
 
+	if (clk_is_orphan(cl->clk)) {
+		clk = ERR_PTR(-EPROBE_DEFER);
+		goto out;
+	}
+
 	clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id);
 	if (IS_ERR(clk))
 		goto out;
-- 
2.1.4

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
> On 04/22, Heiko Stuebner wrote:
> > Using orphan clocks can introduce strange behaviour as they don't have
> > rate information at all and also of course don't track
> > 
> > This v2/v3 takes into account suggestions from Stephen Boyd to not try to
> > walk the clock tree at runtime but instead keep track of orphan states
> > on clock tree changes and making it mandatory for everybody from the
> > start as orphaned clocks should not be used at all.
> > 
> > 
> > This fixes an issue on most rk3288 platforms, where some soc-clocks
> > are supplied by a 32khz clock from an external i2c-chip which often
> > is only probed later in the boot process and maybe even after the
> > drivers using these soc-clocks like the tsadc temperature sensor.
> > In this case the driver using the clock should of course defer probing
> > until the clock is actually usable.
> > 
> > 
> > As this changes the behaviour for orphan clocks, it would of course
> > benefit from more testing than on my Rockchip boards. To keep the
> > recipent-list reasonable and not spam to much I selected one (the topmost)
> > from the get_maintainer output of each drivers/clk entry.
> > Hopefully some will provide Tested-by-tags :-)
> 
> <grumble> I don't see any Tested-by: tags yet </grumble>. I've
> put these two patches on a separate branch "defer-orphans" and
> pushed it to clk-next so we can give it some more exposure.
> 
> Unfortunately this doesn't solve the orphan problem for non-OF
> providers. What if we did the orphan check in __clk_create_clk()
> instead and returned an error pointer for orphans? I suspect that
> will solve all cases, right?

hmm, clk_register also uses __clk_create_clk, which in turn would prevent
registering orphan-clocks at all, I'd think.
As on my platform I'm dependant on orphan clocks (the soc-level clock gets
registerted as part of the big clock controller way before the i2c-based
supplying clock), I'd rather not touch this :-) .

Instead I guess we could hook it less deep into clk_get_sys, like in the
following patch?


------------ 8< ------------------------- >8 -----------------
From: Heiko Stuebner <heiko@sntech.de>
Date: Fri, 1 May 2015 21:50:46 +0200
Subject: [PATCH] clk: prevent orphan access on non-devicetree platforms too

The orphan-check in __of_clk_get_from_provider only prevents orphan-access
on devicetree platforms. To bring non-dt platforms to the same level of
functionality let clk_get_sys (called from clk_get for non-dt platforms)
also check for orphans and return -EPROBE_DEFER in that case.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 drivers/clk/clk.c    | 2 +-
 drivers/clk/clk.h    | 5 +++++
 drivers/clk/clkdev.c | 5 +++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 36d1a01..167d0bf 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2220,7 +2220,7 @@ static inline void clk_debug_unregister(struct clk_core *core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
+bool clk_is_orphan(const struct clk *clk)
 {
 	if (!clk)
 		return false;
diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
index 00b35a1..b8a6061 100644
--- a/drivers/clk/clk.h
+++ b/drivers/clk/clk.h
@@ -20,6 +20,7 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 			     const char *con_id);
 void __clk_free_clk(struct clk *clk);
+bool clk_is_orphan(const struct clk *clk);
 #else
 /* All these casts to avoid ifdefs in clkdev... */
 static inline struct clk *
@@ -32,5 +33,9 @@ static struct clk_hw *__clk_get_hw(struct clk *clk)
 {
 	return (struct clk_hw *)clk;
 }
+static inline bool clk_is_orphan(const struct clk *clk)
+{
+	return false;
+}
 
 #endif
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 1fcb6ef..ad96775 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -177,6 +177,11 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id)
 	if (!cl)
 		goto out;
 
+	if (clk_is_orphan(cl->clk)) {
+		clk = ERR_PTR(-EPROBE_DEFER);
+		goto out;
+	}
+
 	clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id);
 	if (IS_ERR(clk))
 		goto out;
-- 
2.1.4

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/01/15 12:59, Heiko Stübner wrote:
> Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
>> On 04/22, Heiko Stuebner wrote:
>>> Using orphan clocks can introduce strange behaviour as they don't have
>>> rate information at all and also of course don't track
>>>
>>> This v2/v3 takes into account suggestions from Stephen Boyd to not try to
>>> walk the clock tree at runtime but instead keep track of orphan states
>>> on clock tree changes and making it mandatory for everybody from the
>>> start as orphaned clocks should not be used at all.
>>>
>>>
>>> This fixes an issue on most rk3288 platforms, where some soc-clocks
>>> are supplied by a 32khz clock from an external i2c-chip which often
>>> is only probed later in the boot process and maybe even after the
>>> drivers using these soc-clocks like the tsadc temperature sensor.
>>> In this case the driver using the clock should of course defer probing
>>> until the clock is actually usable.
>>>
>>>
>>> As this changes the behaviour for orphan clocks, it would of course
>>> benefit from more testing than on my Rockchip boards. To keep the
>>> recipent-list reasonable and not spam to much I selected one (the topmost)
>>> from the get_maintainer output of each drivers/clk entry.
>>> Hopefully some will provide Tested-by-tags :-)
>> <grumble> I don't see any Tested-by: tags yet </grumble>. I've
>> put these two patches on a separate branch "defer-orphans" and
>> pushed it to clk-next so we can give it some more exposure.
>>
>> Unfortunately this doesn't solve the orphan problem for non-OF
>> providers. What if we did the orphan check in __clk_create_clk()
>> instead and returned an error pointer for orphans? I suspect that
>> will solve all cases, right?
> hmm, clk_register also uses __clk_create_clk, which in turn would prevent
> registering orphan-clocks at all, I'd think.
> As on my platform I'm dependant on orphan clocks (the soc-level clock gets
> registerted as part of the big clock controller way before the i2c-based
> supplying clock), I'd rather not touch this :-) .

Have no fear! We should just change clk_register() to call a
__clk_create_clk() type function that doesn't check for orphan status.

>
> Instead I guess we could hook it less deep into clk_get_sys, like in the
> following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only patch?

This also brings up an existing problem with clk_unregister() where
orphaned clocks are sitting out there useable by drivers when their
parent is unregistered. That code could use some work to atomically
switch all the orphaned clocks over to use the nodrv_ops.

----8<-----

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 30d45c657a07..1d23daa42dd2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct clk_core *core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
-{
-	if (!clk)
-		return false;
-
-	return clk->core->orphan;
-}
-
 /**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:	device initializing this clk, placeholder for now
@@ -2420,15 +2412,11 @@ out:
 	return ret;
 }
 
-struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
-			     const char *con_id)
+static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
+				     const char *con_id)
 {
 	struct clk *clk;
 
-	/* This is to allow this function to be chained to others */
-	if (!hw || IS_ERR(hw))
-		return (struct clk *) hw;
-
 	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
 	if (!clk)
 		return ERR_PTR(-ENOMEM);
@@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 	return clk;
 }
 
+struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
+			     const char *con_id)
+{
+	/* This is to allow this function to be chained to others */
+	if (!hw || IS_ERR(hw))
+		return (struct clk *) hw;
+
+	if (hw->core->orphan)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return clk_hw_create_clk(hw, dev_id, con_id);
+}
+
 void __clk_free_clk(struct clk *clk)
 {
 	clk_prepare_lock();
@@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 
 	INIT_HLIST_HEAD(&core->clks);
 
-	hw->clk = __clk_create_clk(hw, NULL, NULL);
+	hw->clk = clk_hw_create_clk(hw, NULL, NULL);
 	if (IS_ERR(hw->clk)) {
 		ret = PTR_ERR(hw->clk);
 		goto fail_parent_names_copy;
@@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 		if (provider->node == clkspec->np)
 			clk = provider->get(clkspec, provider->data);
 		if (!IS_ERR(clk)) {
-			if (clk_is_orphan(clk)) {
-				clk = ERR_PTR(-EPROBE_DEFER);
-				break;
-			}
 
 			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
 					       con_id);


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/01/15 12:59, Heiko St?bner wrote:
> Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
>> On 04/22, Heiko Stuebner wrote:
>>> Using orphan clocks can introduce strange behaviour as they don't have
>>> rate information at all and also of course don't track
>>>
>>> This v2/v3 takes into account suggestions from Stephen Boyd to not try to
>>> walk the clock tree at runtime but instead keep track of orphan states
>>> on clock tree changes and making it mandatory for everybody from the
>>> start as orphaned clocks should not be used at all.
>>>
>>>
>>> This fixes an issue on most rk3288 platforms, where some soc-clocks
>>> are supplied by a 32khz clock from an external i2c-chip which often
>>> is only probed later in the boot process and maybe even after the
>>> drivers using these soc-clocks like the tsadc temperature sensor.
>>> In this case the driver using the clock should of course defer probing
>>> until the clock is actually usable.
>>>
>>>
>>> As this changes the behaviour for orphan clocks, it would of course
>>> benefit from more testing than on my Rockchip boards. To keep the
>>> recipent-list reasonable and not spam to much I selected one (the topmost)
>>> from the get_maintainer output of each drivers/clk entry.
>>> Hopefully some will provide Tested-by-tags :-)
>> <grumble> I don't see any Tested-by: tags yet </grumble>. I've
>> put these two patches on a separate branch "defer-orphans" and
>> pushed it to clk-next so we can give it some more exposure.
>>
>> Unfortunately this doesn't solve the orphan problem for non-OF
>> providers. What if we did the orphan check in __clk_create_clk()
>> instead and returned an error pointer for orphans? I suspect that
>> will solve all cases, right?
> hmm, clk_register also uses __clk_create_clk, which in turn would prevent
> registering orphan-clocks at all, I'd think.
> As on my platform I'm dependant on orphan clocks (the soc-level clock gets
> registerted as part of the big clock controller way before the i2c-based
> supplying clock), I'd rather not touch this :-) .

Have no fear! We should just change clk_register() to call a
__clk_create_clk() type function that doesn't check for orphan status.

>
> Instead I guess we could hook it less deep into clk_get_sys, like in the
> following patch?

It looks like it will work at least, but still I'd prefer to keep the
orphan check contained to clk.c. How about this compile tested only patch?

This also brings up an existing problem with clk_unregister() where
orphaned clocks are sitting out there useable by drivers when their
parent is unregistered. That code could use some work to atomically
switch all the orphaned clocks over to use the nodrv_ops.

----8<-----

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 30d45c657a07..1d23daa42dd2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct clk_core *core)
 }
 #endif
 
-static bool clk_is_orphan(const struct clk *clk)
-{
-	if (!clk)
-		return false;
-
-	return clk->core->orphan;
-}
-
 /**
  * __clk_init - initialize the data structures in a struct clk
  * @dev:	device initializing this clk, placeholder for now
@@ -2420,15 +2412,11 @@ out:
 	return ret;
 }
 
-struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
-			     const char *con_id)
+static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
+				     const char *con_id)
 {
 	struct clk *clk;
 
-	/* This is to allow this function to be chained to others */
-	if (!hw || IS_ERR(hw))
-		return (struct clk *) hw;
-
 	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
 	if (!clk)
 		return ERR_PTR(-ENOMEM);
@@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
 	return clk;
 }
 
+struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
+			     const char *con_id)
+{
+	/* This is to allow this function to be chained to others */
+	if (!hw || IS_ERR(hw))
+		return (struct clk *) hw;
+
+	if (hw->core->orphan)
+		return ERR_PTR(-EPROBE_DEFER);
+
+	return clk_hw_create_clk(hw, dev_id, con_id);
+}
+
 void __clk_free_clk(struct clk *clk)
 {
 	clk_prepare_lock();
@@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 
 	INIT_HLIST_HEAD(&core->clks);
 
-	hw->clk = __clk_create_clk(hw, NULL, NULL);
+	hw->clk = clk_hw_create_clk(hw, NULL, NULL);
 	if (IS_ERR(hw->clk)) {
 		ret = PTR_ERR(hw->clk);
 		goto fail_parent_names_copy;
@@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec,
 		if (provider->node == clkspec->np)
 			clk = provider->get(clkspec, provider->data);
 		if (!IS_ERR(clk)) {
-			if (clk_is_orphan(clk)) {
-				clk = ERR_PTR(-EPROBE_DEFER);
-				break;
-			}
 
 			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
 					       con_id);


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> On 05/01/15 12:59, Heiko Stübner wrote:
> > Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
> >> On 04/22, Heiko Stuebner wrote:
> >>> Using orphan clocks can introduce strange behaviour as they don't have
> >>> rate information at all and also of course don't track
> >>> 
> >>> This v2/v3 takes into account suggestions from Stephen Boyd to not try
> >>> to
> >>> walk the clock tree at runtime but instead keep track of orphan states
> >>> on clock tree changes and making it mandatory for everybody from the
> >>> start as orphaned clocks should not be used at all.
> >>> 
> >>> 
> >>> This fixes an issue on most rk3288 platforms, where some soc-clocks
> >>> are supplied by a 32khz clock from an external i2c-chip which often
> >>> is only probed later in the boot process and maybe even after the
> >>> drivers using these soc-clocks like the tsadc temperature sensor.
> >>> In this case the driver using the clock should of course defer probing
> >>> until the clock is actually usable.
> >>> 
> >>> 
> >>> As this changes the behaviour for orphan clocks, it would of course
> >>> benefit from more testing than on my Rockchip boards. To keep the
> >>> recipent-list reasonable and not spam to much I selected one (the
> >>> topmost)
> >>> from the get_maintainer output of each drivers/clk entry.
> >>> Hopefully some will provide Tested-by-tags :-)
> >> 
> >> <grumble> I don't see any Tested-by: tags yet </grumble>. I've
> >> put these two patches on a separate branch "defer-orphans" and
> >> pushed it to clk-next so we can give it some more exposure.
> >> 
> >> Unfortunately this doesn't solve the orphan problem for non-OF
> >> providers. What if we did the orphan check in __clk_create_clk()
> >> instead and returned an error pointer for orphans? I suspect that
> >> will solve all cases, right?
> > 
> > hmm, clk_register also uses __clk_create_clk, which in turn would prevent
> > registering orphan-clocks at all, I'd think.
> > As on my platform I'm dependant on orphan clocks (the soc-level clock gets
> > registerted as part of the big clock controller way before the i2c-based
> > supplying clock), I'd rather not touch this :-) .
> 
> Have no fear! We should just change clk_register() to call a
> __clk_create_clk() type function that doesn't check for orphan status.

ok :-D


> > Instead I guess we could hook it less deep into clk_get_sys, like in the
> > following patch?
> 
> It looks like it will work at least, but still I'd prefer to keep the
> orphan check contained to clk.c. How about this compile tested only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
looks the same and it also still defers nicely in the scenario I needed it 
for. The implementation also looks nice - and of course much more compact than 
my check in two places :-) . I don't know if you want to put this as follow-up 
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> This also brings up an existing problem with clk_unregister() where
> orphaned clocks are sitting out there useable by drivers when their
> parent is unregistered. That code could use some work to atomically
> switch all the orphaned clocks over to use the nodrv_ops.

Not sure I understand this correctly yet, but when these children get 
orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
back if the parent reappears.

So I guess we would need to store the original ops in secondary property of 
struct clk_core and I guess simply bind the ops-switch to the orphan state 
update?


> 
> ----8<-----
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 30d45c657a07..1d23daa42dd2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct
> clk_core *core) }
>  #endif
> 
> -static bool clk_is_orphan(const struct clk *clk)
> -{
> -	if (!clk)
> -		return false;
> -
> -	return clk->core->orphan;
> -}
> -
>  /**
>   * __clk_init - initialize the data structures in a struct clk
>   * @dev:	device initializing this clk, placeholder for now
> @@ -2420,15 +2412,11 @@ out:
>  	return ret;
>  }
> 
> -struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
> -			     const char *con_id)
> +static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
> +				     const char *con_id)
>  {
>  	struct clk *clk;
> 
> -	/* This is to allow this function to be chained to others */
> -	if (!hw || IS_ERR(hw))
> -		return (struct clk *) hw;
> -
>  	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
>  	if (!clk)
>  		return ERR_PTR(-ENOMEM);
> @@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const
> char *dev_id, return clk;
>  }
> 
> +struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
> +			     const char *con_id)
> +{
> +	/* This is to allow this function to be chained to others */
> +	if (!hw || IS_ERR(hw))
> +		return (struct clk *) hw;
> +
> +	if (hw->core->orphan)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return clk_hw_create_clk(hw, dev_id, con_id);
> +}
> +
>  void __clk_free_clk(struct clk *clk)
>  {
>  	clk_prepare_lock();
> @@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct
> clk_hw *hw)
> 
>  	INIT_HLIST_HEAD(&core->clks);
> 
> -	hw->clk = __clk_create_clk(hw, NULL, NULL);
> +	hw->clk = clk_hw_create_clk(hw, NULL, NULL);
>  	if (IS_ERR(hw->clk)) {
>  		ret = PTR_ERR(hw->clk);
>  		goto fail_parent_names_copy;
> @@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct
> of_phandle_args *clkspec, if (provider->node == clkspec->np)
>  			clk = provider->get(clkspec, provider->data);
>  		if (!IS_ERR(clk)) {
> -			if (clk_is_orphan(clk)) {
> -				clk = ERR_PTR(-EPROBE_DEFER);
> -				break;
> -			}
> 
>  			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
>  					       con_id);

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> On 05/01/15 12:59, Heiko St?bner wrote:
> > Am Donnerstag, 30. April 2015, 17:19:01 schrieb Stephen Boyd:
> >> On 04/22, Heiko Stuebner wrote:
> >>> Using orphan clocks can introduce strange behaviour as they don't have
> >>> rate information at all and also of course don't track
> >>> 
> >>> This v2/v3 takes into account suggestions from Stephen Boyd to not try
> >>> to
> >>> walk the clock tree at runtime but instead keep track of orphan states
> >>> on clock tree changes and making it mandatory for everybody from the
> >>> start as orphaned clocks should not be used at all.
> >>> 
> >>> 
> >>> This fixes an issue on most rk3288 platforms, where some soc-clocks
> >>> are supplied by a 32khz clock from an external i2c-chip which often
> >>> is only probed later in the boot process and maybe even after the
> >>> drivers using these soc-clocks like the tsadc temperature sensor.
> >>> In this case the driver using the clock should of course defer probing
> >>> until the clock is actually usable.
> >>> 
> >>> 
> >>> As this changes the behaviour for orphan clocks, it would of course
> >>> benefit from more testing than on my Rockchip boards. To keep the
> >>> recipent-list reasonable and not spam to much I selected one (the
> >>> topmost)
> >>> from the get_maintainer output of each drivers/clk entry.
> >>> Hopefully some will provide Tested-by-tags :-)
> >> 
> >> <grumble> I don't see any Tested-by: tags yet </grumble>. I've
> >> put these two patches on a separate branch "defer-orphans" and
> >> pushed it to clk-next so we can give it some more exposure.
> >> 
> >> Unfortunately this doesn't solve the orphan problem for non-OF
> >> providers. What if we did the orphan check in __clk_create_clk()
> >> instead and returned an error pointer for orphans? I suspect that
> >> will solve all cases, right?
> > 
> > hmm, clk_register also uses __clk_create_clk, which in turn would prevent
> > registering orphan-clocks at all, I'd think.
> > As on my platform I'm dependant on orphan clocks (the soc-level clock gets
> > registerted as part of the big clock controller way before the i2c-based
> > supplying clock), I'd rather not touch this :-) .
> 
> Have no fear! We should just change clk_register() to call a
> __clk_create_clk() type function that doesn't check for orphan status.

ok :-D


> > Instead I guess we could hook it less deep into clk_get_sys, like in the
> > following patch?
> 
> It looks like it will work at least, but still I'd prefer to keep the
> orphan check contained to clk.c. How about this compile tested only patch?

I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
looks the same and it also still defers nicely in the scenario I needed it 
for. The implementation also looks nice - and of course much more compact than 
my check in two places :-) . I don't know if you want to put this as follow-up 
on top or fold it into the original orphan-check, so in any case

Tested-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> This also brings up an existing problem with clk_unregister() where
> orphaned clocks are sitting out there useable by drivers when their
> parent is unregistered. That code could use some work to atomically
> switch all the orphaned clocks over to use the nodrv_ops.

Not sure I understand this correctly yet, but when these children get 
orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
back if the parent reappears.

So I guess we would need to store the original ops in secondary property of 
struct clk_core and I guess simply bind the ops-switch to the orphan state 
update?


> 
> ----8<-----
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 30d45c657a07..1d23daa42dd2 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2221,14 +2221,6 @@ static inline void clk_debug_unregister(struct
> clk_core *core) }
>  #endif
> 
> -static bool clk_is_orphan(const struct clk *clk)
> -{
> -	if (!clk)
> -		return false;
> -
> -	return clk->core->orphan;
> -}
> -
>  /**
>   * __clk_init - initialize the data structures in a struct clk
>   * @dev:	device initializing this clk, placeholder for now
> @@ -2420,15 +2412,11 @@ out:
>  	return ret;
>  }
> 
> -struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
> -			     const char *con_id)
> +static struct clk *clk_hw_create_clk(struct clk_hw *hw, const char *dev_id,
> +				     const char *con_id)
>  {
>  	struct clk *clk;
> 
> -	/* This is to allow this function to be chained to others */
> -	if (!hw || IS_ERR(hw))
> -		return (struct clk *) hw;
> -
>  	clk = kzalloc(sizeof(*clk), GFP_KERNEL);
>  	if (!clk)
>  		return ERR_PTR(-ENOMEM);
> @@ -2445,6 +2433,19 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const
> char *dev_id, return clk;
>  }
> 
> +struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id,
> +			     const char *con_id)
> +{
> +	/* This is to allow this function to be chained to others */
> +	if (!hw || IS_ERR(hw))
> +		return (struct clk *) hw;
> +
> +	if (hw->core->orphan)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	return clk_hw_create_clk(hw, dev_id, con_id);
> +}
> +
>  void __clk_free_clk(struct clk *clk)
>  {
>  	clk_prepare_lock();
> @@ -2511,7 +2512,7 @@ struct clk *clk_register(struct device *dev, struct
> clk_hw *hw)
> 
>  	INIT_HLIST_HEAD(&core->clks);
> 
> -	hw->clk = __clk_create_clk(hw, NULL, NULL);
> +	hw->clk = clk_hw_create_clk(hw, NULL, NULL);
>  	if (IS_ERR(hw->clk)) {
>  		ret = PTR_ERR(hw->clk);
>  		goto fail_parent_names_copy;
> @@ -2958,10 +2959,6 @@ struct clk *__of_clk_get_from_provider(struct
> of_phandle_args *clkspec, if (provider->node == clkspec->np)
>  			clk = provider->get(clkspec, provider->data);
>  		if (!IS_ERR(clk)) {
> -			if (clk_is_orphan(clk)) {
> -				clk = ERR_PTR(-EPROBE_DEFER);
> -				break;
> -			}
> 
>  			clk = __clk_create_clk(__clk_get_hw(clk), dev_id,
>  					       con_id);

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/01/15 15:07, Heiko Stübner wrote:
> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>
>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>> following patch?
>> It looks like it will work at least, but still I'd prefer to keep the
>> orphan check contained to clk.c. How about this compile tested only patch?
> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
> looks the same and it also still defers nicely in the scenario I needed it 
> for. The implementation also looks nice - and of course much more compact than 
> my check in two places :-) . I don't know if you want to put this as follow-up 
> on top or fold it into the original orphan-check, so in any case
>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.

>
>
>> This also brings up an existing problem with clk_unregister() where
>> orphaned clocks are sitting out there useable by drivers when their
>> parent is unregistered. That code could use some work to atomically
>> switch all the orphaned clocks over to use the nodrv_ops.
> Not sure I understand this correctly yet, but when these children get 
> orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
> back if the parent reappears.
>
> So I guess we would need to store the original ops in secondary property of 
> struct clk_core and I guess simply bind the ops-switch to the orphan state 
> update?

Yep. We'll need to store away the original ops in case we need to put
them back. Don't feel obligated to fix this either. It would certainly
be nice if someone tried to fix this case at some point, but it's not
like things are any worse off right now.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/01/15 15:07, Heiko St?bner wrote:
> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>
>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>> following patch?
>> It looks like it will work at least, but still I'd prefer to keep the
>> orphan check contained to clk.c. How about this compile tested only patch?
> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree 
> looks the same and it also still defers nicely in the scenario I needed it 
> for. The implementation also looks nice - and of course much more compact than 
> my check in two places :-) . I don't know if you want to put this as follow-up 
> on top or fold it into the original orphan-check, so in any case
>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
my patch and a note that it's based on an earlier patch from you.

>
>
>> This also brings up an existing problem with clk_unregister() where
>> orphaned clocks are sitting out there useable by drivers when their
>> parent is unregistered. That code could use some work to atomically
>> switch all the orphaned clocks over to use the nodrv_ops.
> Not sure I understand this correctly yet, but when these children get 
> orphaned, switched to the clk_nodrv_ops, they won't get their original ops 
> back if the parent reappears.
>
> So I guess we would need to store the original ops in secondary property of 
> struct clk_core and I guess simply bind the ops-switch to the orphan state 
> update?

Yep. We'll need to store away the original ops in case we need to put
them back. Don't feel obligated to fix this either. It would certainly
be nice if someone tried to fix this case at some point, but it's not
like things are any worse off right now.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Tero Kristo]

On 05/02/2015 02:40 AM, Stephen Boyd wrote:
> On 05/01/15 15:07, Heiko Stübner wrote:
>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>
>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>> following patch?
>>> It looks like it will work at least, but still I'd prefer to keep the
>>> orphan check contained to clk.c. How about this compile tested only patch?
>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> looks the same and it also still defers nicely in the scenario I needed it
>> for. The implementation also looks nice - and of course much more compact than
>> my check in two places :-) . I don't know if you want to put this as follow-up
>> on top or fold it into the original orphan-check, so in any case
>>
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> my patch and a note that it's based on an earlier patch from you.

FWIW, just gave a try for these two patches on all TI boards I have 
access to.

Tested-by: Tero Kristo <t-kristo@ti.com>

I didn't try your evolved patch though, as you don't seem to have made 
your mind yet.

-Tero

>
>>
>>
>>> This also brings up an existing problem with clk_unregister() where
>>> orphaned clocks are sitting out there useable by drivers when their
>>> parent is unregistered. That code could use some work to atomically
>>> switch all the orphaned clocks over to use the nodrv_ops.
>> Not sure I understand this correctly yet, but when these children get
>> orphaned, switched to the clk_nodrv_ops, they won't get their original ops
>> back if the parent reappears.
>>
>> So I guess we would need to store the original ops in secondary property of
>> struct clk_core and I guess simply bind the ops-switch to the orphan state
>> update?
>
> Yep. We'll need to store away the original ops in case we need to put
> them back. Don't feel obligated to fix this either. It would certainly
> be nice if someone tried to fix this case at some point, but it's not
> like things are any worse off right now.
>

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Tero Kristo]

On 05/02/2015 02:40 AM, Stephen Boyd wrote:
> On 05/01/15 15:07, Heiko St?bner wrote:
>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>
>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>> following patch?
>>> It looks like it will work at least, but still I'd prefer to keep the
>>> orphan check contained to clk.c. How about this compile tested only patch?
>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> looks the same and it also still defers nicely in the scenario I needed it
>> for. The implementation also looks nice - and of course much more compact than
>> my check in two places :-) . I don't know if you want to put this as follow-up
>> on top or fold it into the original orphan-check, so in any case
>>
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> my patch and a note that it's based on an earlier patch from you.

FWIW, just gave a try for these two patches on all TI boards I have 
access to.

Tested-by: Tero Kristo <t-kristo@ti.com>

I didn't try your evolved patch though, as you don't seem to have made 
your mind yet.

-Tero

>
>>
>>
>>> This also brings up an existing problem with clk_unregister() where
>>> orphaned clocks are sitting out there useable by drivers when their
>>> parent is unregistered. That code could use some work to atomically
>>> switch all the orphaned clocks over to use the nodrv_ops.
>> Not sure I understand this correctly yet, but when these children get
>> orphaned, switched to the clk_nodrv_ops, they won't get their original ops
>> back if the parent reappears.
>>
>> So I guess we would need to store the original ops in secondary property of
>> struct clk_core and I guess simply bind the ops-switch to the orphan state
>> update?
>
> Yep. We'll need to store away the original ops in case we need to put
> them back. Don't feel obligated to fix this either. It would certainly
> be nice if someone tried to fix this case at some point, but it's not
> like things are any worse off right now.
>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/01/15 15:07, Heiko Stübner wrote:
>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>
>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>> following patch?
>>> It looks like it will work at least, but still I'd prefer to keep the
>>> orphan check contained to clk.c. How about this compile tested only patch?
>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> looks the same and it also still defers nicely in the scenario I needed it
>> for. The implementation also looks nice - and of course much more compact than
>> my check in two places :-) . I don't know if you want to put this as follow-up
>> on top or fold it into the original orphan-check, so in any case
>>
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> my patch and a note that it's based on an earlier patch from you.

It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot failures
for sunxi in today's linux-next[1] were bisected down to that patch.

I confirmed that reverting that commit on top of next/master gets
sunxi booting again.

Kevin

[1] http://kernelci.org/boot/all/job/next/kernel/next-20150507/

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/01/15 15:07, Heiko St=C3=BCbner wrote:
>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>
>>>> Instead I guess we could hook it less deep into clk_get_sys, like in t=
he
>>>> following patch?
>>> It looks like it will work at least, but still I'd prefer to keep the
>>> orphan check contained to clk.c. How about this compile tested only pat=
ch?
>> I gave this a spin on my rk3288-firefly board. It still boots, the clock=
 tree
>> looks the same and it also still defers nicely in the scenario I needed =
it
>> for. The implementation also looks nice - and of course much more compac=
t than
>> my check in two places :-) . I don't know if you want to put this as fol=
low-up
>> on top or fold it into the original orphan-check, so in any case
>>
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> my patch and a note that it's based on an earlier patch from you.

It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot failures
for sunxi in today's linux-next[1] were bisected down to that patch.

I confirmed that reverting that commit on top of next/master gets
sunxi booting again.

Kevin

[1] http://kernelci.org/boot/all/job/next/kernel/next-20150507/

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/01/15 15:07, Heiko St?bner wrote:
>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>
>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>> following patch?
>>> It looks like it will work at least, but still I'd prefer to keep the
>>> orphan check contained to clk.c. How about this compile tested only patch?
>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> looks the same and it also still defers nicely in the scenario I needed it
>> for. The implementation also looks nice - and of course much more compact than
>> my check in two places :-) . I don't know if you want to put this as follow-up
>> on top or fold it into the original orphan-check, so in any case
>>
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>
> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> my patch and a note that it's based on an earlier patch from you.

It appears this has landed in linux-next in the form of 882667c1fcf1
clk: prevent orphan clocks from being used.  A bunch of boot failures
for sunxi in today's linux-next[1] were bisected down to that patch.

I confirmed that reverting that commit on top of next/master gets
sunxi booting again.

Kevin

[1] http://kernelci.org/boot/all/job/next/kernel/next-20150507/

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/07/15 01:22, Tero Kristo wrote:
> On 05/02/2015 02:40 AM, Stephen Boyd wrote:
>> On 05/01/15 15:07, Heiko Stübner wrote:
>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>
>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
>>>>> in the
>>>>> following patch?
>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>> orphan check contained to clk.c. How about this compile tested only
>>>> patch?
>>> I gave this a spin on my rk3288-firefly board. It still boots, the
>>> clock tree
>>> looks the same and it also still defers nicely in the scenario I
>>> needed it
>>> for. The implementation also looks nice - and of course much more
>>> compact than
>>> my check in two places :-) . I don't know if you want to put this as
>>> follow-up
>>> on top or fold it into the original orphan-check, so in any case
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>
>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> my patch and a note that it's based on an earlier patch from you.
>
> FWIW, just gave a try for these two patches on all TI boards I have
> access to.
>
> Tested-by: Tero Kristo <t-kristo@ti.com>
>
> I didn't try your evolved patch though, as you don't seem to have made
> your mind yet.
>

Thanks. Can you try the evolved patch? It's in linux-next now as commit
882667c1fcf1, and it seems to at least break sunxi boot. I'd be
interested if it broke TI boards.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/07/15 01:22, Tero Kristo wrote:
> On 05/02/2015 02:40 AM, Stephen Boyd wrote:
>> On 05/01/15 15:07, Heiko St?bner wrote:
>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>
>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
>>>>> in the
>>>>> following patch?
>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>> orphan check contained to clk.c. How about this compile tested only
>>>> patch?
>>> I gave this a spin on my rk3288-firefly board. It still boots, the
>>> clock tree
>>> looks the same and it also still defers nicely in the scenario I
>>> needed it
>>> for. The implementation also looks nice - and of course much more
>>> compact than
>>> my check in two places :-) . I don't know if you want to put this as
>>> follow-up
>>> on top or fold it into the original orphan-check, so in any case
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>
>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> my patch and a note that it's based on an earlier patch from you.
>
> FWIW, just gave a try for these two patches on all TI boards I have
> access to.
>
> Tested-by: Tero Kristo <t-kristo@ti.com>
>
> I didn't try your evolved patch though, as you don't seem to have made
> your mind yet.
>

Thanks. Can you try the evolved patch? It's in linux-next now as commit
882667c1fcf1, and it seems to at least break sunxi boot. I'd be
interested if it broke TI boards.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/07/15 08:17, Kevin Hilman wrote:
> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 05/01/15 15:07, Heiko Stübner wrote:
>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>
>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>>> following patch?
>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>> orphan check contained to clk.c. How about this compile tested only patch?
>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>>> looks the same and it also still defers nicely in the scenario I needed it
>>> for. The implementation also looks nice - and of course much more compact than
>>> my check in two places :-) . I don't know if you want to put this as follow-up
>>> on top or fold it into the original orphan-check, so in any case
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> my patch and a note that it's based on an earlier patch from you.
> It appears this has landed in linux-next in the form of 882667c1fcf1
> clk: prevent orphan clocks from being used.  A bunch of boot failures
> for sunxi in today's linux-next[1] were bisected down to that patch.
>
> I confirmed that reverting that commit on top of next/master gets
> sunxi booting again.
>
>

Thanks for the report. I've removed the two clk orphan patches from
clk-next. Would it be possible to try with next-20150507 and
clk_ignore_unused on the command line? Also we can try to see if
critical clocks aren't being forced on by applying this patch and
looking for clk_get() failures

------8<------

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e1e2bd189b6..d88585b680bb 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1347,6 +1347,9 @@ static void __init sunxi_init_clocks(const char *clocks[], int nclocks)
 
 		if (!IS_ERR(clk))
 			clk_prepare_enable(clk);
+		else
+			pr_err("Failed to enable critical clock %s\n",
+					clocks[i]);
 	}
 }
 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/07/15 08:17, Kevin Hilman wrote:
> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> On 05/01/15 15:07, Heiko St?bner wrote:
>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>
>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>>> following patch?
>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>> orphan check contained to clk.c. How about this compile tested only patch?
>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>>> looks the same and it also still defers nicely in the scenario I needed it
>>> for. The implementation also looks nice - and of course much more compact than
>>> my check in two places :-) . I don't know if you want to put this as follow-up
>>> on top or fold it into the original orphan-check, so in any case
>>>
>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> my patch and a note that it's based on an earlier patch from you.
> It appears this has landed in linux-next in the form of 882667c1fcf1
> clk: prevent orphan clocks from being used.  A bunch of boot failures
> for sunxi in today's linux-next[1] were bisected down to that patch.
>
> I confirmed that reverting that commit on top of next/master gets
> sunxi booting again.
>
>

Thanks for the report. I've removed the two clk orphan patches from
clk-next. Would it be possible to try with next-20150507 and
clk_ignore_unused on the command line? Also we can try to see if
critical clocks aren't being forced on by applying this patch and
looking for clk_get() failures

------8<------

diff --git a/drivers/clk/sunxi/clk-sunxi.c b/drivers/clk/sunxi/clk-sunxi.c
index 7e1e2bd189b6..d88585b680bb 100644
--- a/drivers/clk/sunxi/clk-sunxi.c
+++ b/drivers/clk/sunxi/clk-sunxi.c
@@ -1347,6 +1347,9 @@ static void __init sunxi_init_clocks(const char *clocks[], int nclocks)
 
 		if (!IS_ERR(clk))
 			clk_prepare_enable(clk);
+		else
+			pr_err("Failed to enable critical clock %s\n",
+					clocks[i]);
 	}
 }
 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>> On 05/01/15 15:07, Heiko Stübner wrote:
>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>
>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>>>> following patch?
>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>>>> looks the same and it also still defers nicely in the scenario I needed it
>>>> for. The implementation also looks nice - and of course much more compact than
>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>>>> on top or fold it into the original orphan-check, so in any case
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>> my patch and a note that it's based on an earlier patch from you.
>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>
>> I confirmed that reverting that commit on top of next/master gets
>> sunxi booting again.
>>
>>
>
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

That doesn't help.  I tried on cubieboard2 and bananapi.

> Also we can try to see if
> critical clocks aren't being forced on by applying this patch and
> looking for clk_get() failures

>From cubieboard2, there's a few that look rather important:

[    0.000000] Additional per-CPU info printed with stalls.
[    0.000000] Build-time adjustment of leaf fanout to 32.
[    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
[    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
[    0.000000] Failed to enable critical clock cpu
[    0.000000] Failed to enable critical clock pll5_ddr
[    0.000000] Failed to enable critical clock ahb_sdram
[    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).

Kevin

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrot=
e:
>>> On 05/01/15 15:07, Heiko St=C3=BCbner wrote:
>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>
>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in=
 the
>>>>>> following patch?
>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>> orphan check contained to clk.c. How about this compile tested only p=
atch?
>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clo=
ck tree
>>>> looks the same and it also still defers nicely in the scenario I neede=
d it
>>>> for. The implementation also looks nice - and of course much more comp=
act than
>>>> my check in two places :-) . I don't know if you want to put this as f=
ollow-up
>>>> on top or fold it into the original orphan-check, so in any case
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it wit=
h
>>> my patch and a note that it's based on an earlier patch from you.
>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>
>> I confirmed that reverting that commit on top of next/master gets
>> sunxi booting again.
>>
>>
>
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

That doesn't help.  I tried on cubieboard2 and bananapi.

> Also we can try to see if
> critical clocks aren't being forced on by applying this patch and
> looking for clk_get() failures

>From cubieboard2, there's a few that look rather important:

[    0.000000] Additional per-CPU info printed with stalls.
[    0.000000] Build-time adjustment of leaf fanout to 32.
[    0.000000] RCU restricting CPUs from NR_CPUS=3D16 to nr_cpu_ids=3D2.
[    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=3D32, nr_cpu_ids=
=3D2
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
[    0.000000] Failed to enable critical clock cpu
[    0.000000] Failed to enable critical clock pll5_ddr
[    0.000000] Failed to enable critical clock ahb_sdram
[    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).

Kevin

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>> On 05/01/15 15:07, Heiko St?bner wrote:
>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>
>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>>>> following patch?
>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>>>> looks the same and it also still defers nicely in the scenario I needed it
>>>> for. The implementation also looks nice - and of course much more compact than
>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>>>> on top or fold it into the original orphan-check, so in any case
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>> my patch and a note that it's based on an earlier patch from you.
>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>
>> I confirmed that reverting that commit on top of next/master gets
>> sunxi booting again.
>>
>>
>
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

That doesn't help.  I tried on cubieboard2 and bananapi.

> Also we can try to see if
> critical clocks aren't being forced on by applying this patch and
> looking for clk_get() failures

>From cubieboard2, there's a few that look rather important:

[    0.000000] Additional per-CPU info printed with stalls.
[    0.000000] Build-time adjustment of leaf fanout to 32.
[    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
[    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb at 01c20054
[    0.000000] Failed to enable critical clock cpu
[    0.000000] Failed to enable critical clock pll5_ddr
[    0.000000] Failed to enable critical clock ahb_sdram
[    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).

Kevin

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/07, Kevin Hilman wrote:
> On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 05/07/15 08:17, Kevin Hilman wrote:
> >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >>> On 05/01/15 15:07, Heiko Stübner wrote:
> >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>>
> >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >>>>>> following patch?
> >>>>> It looks like it will work at least, but still I'd prefer to keep the
> >>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >>>> looks the same and it also still defers nicely in the scenario I needed it
> >>>> for. The implementation also looks nice - and of course much more compact than
> >>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >>>> on top or fold it into the original orphan-check, so in any case
> >>>>
> >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >>> my patch and a note that it's based on an earlier patch from you.
> >> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> for sunxi in today's linux-next[1] were bisected down to that patch.
> >>
> >> I confirmed that reverting that commit on top of next/master gets
> >> sunxi booting again.
> >>
> >>
> >
> > Thanks for the report. I've removed the two clk orphan patches from
> > clk-next. Would it be possible to try with next-20150507 and
> > clk_ignore_unused on the command line?
> 
> That doesn't help.  I tried on cubieboard2 and bananapi.

Thanks for trying.

> 
> > Also we can try to see if
> > critical clocks aren't being forced on by applying this patch and
> > looking for clk_get() failures
> 
> From cubieboard2, there's a few that look rather important:
> 
> [    0.000000] Additional per-CPU info printed with stalls.
> [    0.000000] Build-time adjustment of leaf fanout to 32.
> [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> [    0.000000] NR_IRQS:16 nr_irqs:16 16
> [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> [    0.000000] Failed to enable critical clock cpu
> [    0.000000] Failed to enable critical clock pll5_ddr
> [    0.000000] Failed to enable critical clock ahb_sdram
> [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).

Ok. So it seems we need to come up with some solution to the
"critical clocks" problem that doesn't require the individual
clock drivers to call clk_prepare_enable().

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/07, Kevin Hilman wrote:
> On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 05/07/15 08:17, Kevin Hilman wrote:
> >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >>> On 05/01/15 15:07, Heiko St?bner wrote:
> >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>>
> >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >>>>>> following patch?
> >>>>> It looks like it will work at least, but still I'd prefer to keep the
> >>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >>>> looks the same and it also still defers nicely in the scenario I needed it
> >>>> for. The implementation also looks nice - and of course much more compact than
> >>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >>>> on top or fold it into the original orphan-check, so in any case
> >>>>
> >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >>> my patch and a note that it's based on an earlier patch from you.
> >> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> for sunxi in today's linux-next[1] were bisected down to that patch.
> >>
> >> I confirmed that reverting that commit on top of next/master gets
> >> sunxi booting again.
> >>
> >>
> >
> > Thanks for the report. I've removed the two clk orphan patches from
> > clk-next. Would it be possible to try with next-20150507 and
> > clk_ignore_unused on the command line?
> 
> That doesn't help.  I tried on cubieboard2 and bananapi.

Thanks for trying.

> 
> > Also we can try to see if
> > critical clocks aren't being forced on by applying this patch and
> > looking for clk_get() failures
> 
> From cubieboard2, there's a few that look rather important:
> 
> [    0.000000] Additional per-CPU info printed with stalls.
> [    0.000000] Build-time adjustment of leaf fanout to 32.
> [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> [    0.000000] NR_IRQS:16 nr_irqs:16 16
> [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb at 01c20054
> [    0.000000] Failed to enable critical clock cpu
> [    0.000000] Failed to enable critical clock pll5_ddr
> [    0.000000] Failed to enable critical clock ahb_sdram
> [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).

Ok. So it seems we need to come up with some solution to the
"critical clocks" problem that doesn't require the individual
clock drivers to call clk_prepare_enable().

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Sascha Hauer]

On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> On 05/07, Kevin Hilman wrote:
> > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > > On 05/07/15 08:17, Kevin Hilman wrote:
> > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > >>> On 05/01/15 15:07, Heiko Stübner wrote:
> > >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > >>>>
> > >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> > >>>>>> following patch?
> > >>>>> It looks like it will work at least, but still I'd prefer to keep the
> > >>>>> orphan check contained to clk.c. How about this compile tested only patch?
> > >>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> > >>>> looks the same and it also still defers nicely in the scenario I needed it
> > >>>> for. The implementation also looks nice - and of course much more compact than
> > >>>> my check in two places :-) . I don't know if you want to put this as follow-up
> > >>>> on top or fold it into the original orphan-check, so in any case
> > >>>>
> > >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> > >>> my patch and a note that it's based on an earlier patch from you.
> > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > >>
> > >> I confirmed that reverting that commit on top of next/master gets
> > >> sunxi booting again.
> > >>
> > >>
> > >
> > > Thanks for the report. I've removed the two clk orphan patches from
> > > clk-next. Would it be possible to try with next-20150507 and
> > > clk_ignore_unused on the command line?
> > 
> > That doesn't help.  I tried on cubieboard2 and bananapi.
> 
> Thanks for trying.
> 
> > 
> > > Also we can try to see if
> > > critical clocks aren't being forced on by applying this patch and
> > > looking for clk_get() failures
> > 
> > From cubieboard2, there's a few that look rather important:
> > 
> > [    0.000000] Additional per-CPU info printed with stalls.
> > [    0.000000] Build-time adjustment of leaf fanout to 32.
> > [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> > [    0.000000] NR_IRQS:16 nr_irqs:16 16
> > [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> > [    0.000000] Failed to enable critical clock cpu
> > [    0.000000] Failed to enable critical clock pll5_ddr
> > [    0.000000] Failed to enable critical clock ahb_sdram
> > [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).
> 
> Ok. So it seems we need to come up with some solution to the
> "critical clocks" problem that doesn't require the individual
> clock drivers to call clk_prepare_enable().

I'm getting more and more unsure if we can really handle the complexity
we get by allowing to register orphaned clocks. On one hand we can't
handle the orphaned clocks properly when we do a clk_prepare/enable on
them, on the other hand we run into trouble when we forbid to
prepare/enable them. The fact that clocks can become orphans by
reparenting them makes it even more complicated.
Maybe allowing orphans is something that has to be revisited.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Sascha Hauer]

On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> On 05/07, Kevin Hilman wrote:
> > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > > On 05/07/15 08:17, Kevin Hilman wrote:
> > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > >>> On 05/01/15 15:07, Heiko St?bner wrote:
> > >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > >>>>
> > >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> > >>>>>> following patch?
> > >>>>> It looks like it will work at least, but still I'd prefer to keep the
> > >>>>> orphan check contained to clk.c. How about this compile tested only patch?
> > >>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> > >>>> looks the same and it also still defers nicely in the scenario I needed it
> > >>>> for. The implementation also looks nice - and of course much more compact than
> > >>>> my check in two places :-) . I don't know if you want to put this as follow-up
> > >>>> on top or fold it into the original orphan-check, so in any case
> > >>>>
> > >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> > >>> my patch and a note that it's based on an earlier patch from you.
> > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > >>
> > >> I confirmed that reverting that commit on top of next/master gets
> > >> sunxi booting again.
> > >>
> > >>
> > >
> > > Thanks for the report. I've removed the two clk orphan patches from
> > > clk-next. Would it be possible to try with next-20150507 and
> > > clk_ignore_unused on the command line?
> > 
> > That doesn't help.  I tried on cubieboard2 and bananapi.
> 
> Thanks for trying.
> 
> > 
> > > Also we can try to see if
> > > critical clocks aren't being forced on by applying this patch and
> > > looking for clk_get() failures
> > 
> > From cubieboard2, there's a few that look rather important:
> > 
> > [    0.000000] Additional per-CPU info printed with stalls.
> > [    0.000000] Build-time adjustment of leaf fanout to 32.
> > [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
> > [    0.000000] NR_IRQS:16 nr_irqs:16 16
> > [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb at 01c20054
> > [    0.000000] Failed to enable critical clock cpu
> > [    0.000000] Failed to enable critical clock pll5_ddr
> > [    0.000000] Failed to enable critical clock ahb_sdram
> > [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).
> 
> Ok. So it seems we need to come up with some solution to the
> "critical clocks" problem that doesn't require the individual
> clock drivers to call clk_prepare_enable().

I'm getting more and more unsure if we can really handle the complexity
we get by allowing to register orphaned clocks. On one hand we can't
handle the orphaned clocks properly when we do a clk_prepare/enable on
them, on the other hand we run into trouble when we forbid to
prepare/enable them. The fact that clocks can become orphans by
reparenting them makes it even more complicated.
Maybe allowing orphans is something that has to be revisited.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
> On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> > On 05/07, Kevin Hilman wrote:
> > > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> 
wrote:
> > > > On 05/07/15 08:17, Kevin Hilman wrote:
> > > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> 
wrote:
> > > >>> On 05/01/15 15:07, Heiko Stübner wrote:
> > > >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
> > > >>>>>> in the
> > > >>>>>> following patch?
> > > >>>>> 
> > > >>>>> It looks like it will work at least, but still I'd prefer to keep
> > > >>>>> the
> > > >>>>> orphan check contained to clk.c. How about this compile tested
> > > >>>>> only patch?
> > > >>>> 
> > > >>>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > > >>>> clock tree looks the same and it also still defers nicely in the
> > > >>>> scenario I needed it for. The implementation also looks nice - and
> > > >>>> of course much more compact than my check in two places :-) . I
> > > >>>> don't know if you want to put this as follow-up on top or fold it
> > > >>>> into the original orphan-check, so in any case
> > > >>>> 
> > > >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > > >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > > >>> 
> > > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > > >>> with
> > > >>> my patch and a note that it's based on an earlier patch from you.
> > > >> 
> > > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > > >> 
> > > >> I confirmed that reverting that commit on top of next/master gets
> > > >> sunxi booting again.
> > > > 
> > > > Thanks for the report. I've removed the two clk orphan patches from
> > > > clk-next. Would it be possible to try with next-20150507 and
> > > > clk_ignore_unused on the command line?
> > > 
> > > That doesn't help.  I tried on cubieboard2 and bananapi.
> > 
> > Thanks for trying.
> > 
> > > > Also we can try to see if
> > > > critical clocks aren't being forced on by applying this patch and
> > > > looking for clk_get() failures
> > > 
> > > From cubieboard2, there's a few that look rather important:
> > > 
> > > [    0.000000] Additional per-CPU info printed with stalls.
> > > [    0.000000] Build-time adjustment of leaf fanout to 32.
> > > [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > > [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32,
> > > nr_cpu_ids=2
> > > [    0.000000] NR_IRQS:16 nr_irqs:16 16
> > > [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> > > [    0.000000] Failed to enable critical clock cpu
> > > [    0.000000] Failed to enable critical clock pll5_ddr
> > > [    0.000000] Failed to enable critical clock ahb_sdram
> > > [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).
> > 
> > Ok. So it seems we need to come up with some solution to the
> > "critical clocks" problem that doesn't require the individual
> > clock drivers to call clk_prepare_enable().
> 
> I'm getting more and more unsure if we can really handle the complexity
> we get by allowing to register orphaned clocks. On one hand we can't
> handle the orphaned clocks properly when we do a clk_prepare/enable on
> them, on the other hand we run into trouble when we forbid to
> prepare/enable them. The fact that clocks can become orphans by
> reparenting them makes it even more complicated.
> Maybe allowing orphans is something that has to be revisited.

hmm, I don't see it this drastic. I was expecting a lot more fallout from 
changing the behaviour of orphaned clocks over all arches using the CCF.

>From the kernelci-boards only the Sunxi-ones seem to have been affected at all 
and also only because they need a "regulator-always-on" equivalent in the CCF, 
which currently hijacks prepare/enable inside the clock driver to achive this.

On the Rockchip clocks we have something similar, with the only difference that 
it is done after all clocks are registered and does not need to access the 3 
orphans we have at this point due to their source clock coming from an 
external i2c-connected chip that gets probed a lot later.

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
> On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> > On 05/07, Kevin Hilman wrote:
> > > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> 
wrote:
> > > > On 05/07/15 08:17, Kevin Hilman wrote:
> > > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> 
wrote:
> > > >>> On 05/01/15 15:07, Heiko St?bner wrote:
> > > >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
> > > >>>>>> in the
> > > >>>>>> following patch?
> > > >>>>> 
> > > >>>>> It looks like it will work at least, but still I'd prefer to keep
> > > >>>>> the
> > > >>>>> orphan check contained to clk.c. How about this compile tested
> > > >>>>> only patch?
> > > >>>> 
> > > >>>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > > >>>> clock tree looks the same and it also still defers nicely in the
> > > >>>> scenario I needed it for. The implementation also looks nice - and
> > > >>>> of course much more compact than my check in two places :-) . I
> > > >>>> don't know if you want to put this as follow-up on top or fold it
> > > >>>> into the original orphan-check, so in any case
> > > >>>> 
> > > >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > > >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > > >>> 
> > > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > > >>> with
> > > >>> my patch and a note that it's based on an earlier patch from you.
> > > >> 
> > > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > > >> 
> > > >> I confirmed that reverting that commit on top of next/master gets
> > > >> sunxi booting again.
> > > > 
> > > > Thanks for the report. I've removed the two clk orphan patches from
> > > > clk-next. Would it be possible to try with next-20150507 and
> > > > clk_ignore_unused on the command line?
> > > 
> > > That doesn't help.  I tried on cubieboard2 and bananapi.
> > 
> > Thanks for trying.
> > 
> > > > Also we can try to see if
> > > > critical clocks aren't being forced on by applying this patch and
> > > > looking for clk_get() failures
> > > 
> > > From cubieboard2, there's a few that look rather important:
> > > 
> > > [    0.000000] Additional per-CPU info printed with stalls.
> > > [    0.000000] Build-time adjustment of leaf fanout to 32.
> > > [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > > [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32,
> > > nr_cpu_ids=2
> > > [    0.000000] NR_IRQS:16 nr_irqs:16 16
> > > [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb at 01c20054
> > > [    0.000000] Failed to enable critical clock cpu
> > > [    0.000000] Failed to enable critical clock pll5_ddr
> > > [    0.000000] Failed to enable critical clock ahb_sdram
> > > [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).
> > 
> > Ok. So it seems we need to come up with some solution to the
> > "critical clocks" problem that doesn't require the individual
> > clock drivers to call clk_prepare_enable().
> 
> I'm getting more and more unsure if we can really handle the complexity
> we get by allowing to register orphaned clocks. On one hand we can't
> handle the orphaned clocks properly when we do a clk_prepare/enable on
> them, on the other hand we run into trouble when we forbid to
> prepare/enable them. The fact that clocks can become orphans by
> reparenting them makes it even more complicated.
> Maybe allowing orphans is something that has to be revisited.

hmm, I don't see it this drastic. I was expecting a lot more fallout from 
changing the behaviour of orphaned clocks over all arches using the CCF.

>From the kernelci-boards only the Sunxi-ones seem to have been affected at all 
and also only because they need a "regulator-always-on" equivalent in the CCF, 
which currently hijacks prepare/enable inside the clock driver to achive this.

On the Rockchip clocks we have something similar, with the only difference that 
it is done after all clocks are registered and does not need to access the 3 
orphans we have at this point due to their source clock coming from an 
external i2c-connected chip that gets probed a lot later.

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Sascha Hauer]

On Fri, May 08, 2015 at 11:30:19AM +0200, Heiko Stübner wrote:
> Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
> > On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> > > On 05/07, Kevin Hilman wrote:
> > > > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> 
> wrote:
> > > > > On 05/07/15 08:17, Kevin Hilman wrote:
> > > > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> 
> wrote:
> > > > >>> On 05/01/15 15:07, Heiko Stübner wrote:
> > > > >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > > >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
> > > > >>>>>> in the
> > > > >>>>>> following patch?
> > > > >>>>> 
> > > > >>>>> It looks like it will work at least, but still I'd prefer to keep
> > > > >>>>> the
> > > > >>>>> orphan check contained to clk.c. How about this compile tested
> > > > >>>>> only patch?
> > > > >>>> 
> > > > >>>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > > > >>>> clock tree looks the same and it also still defers nicely in the
> > > > >>>> scenario I needed it for. The implementation also looks nice - and
> > > > >>>> of course much more compact than my check in two places :-) . I
> > > > >>>> don't know if you want to put this as follow-up on top or fold it
> > > > >>>> into the original orphan-check, so in any case
> > > > >>>> 
> > > > >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > > > >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > > > >>> 
> > > > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > > > >>> with
> > > > >>> my patch and a note that it's based on an earlier patch from you.
> > > > >> 
> > > > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > > > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > > > >> 
> > > > >> I confirmed that reverting that commit on top of next/master gets
> > > > >> sunxi booting again.
> > > > > 
> > > > > Thanks for the report. I've removed the two clk orphan patches from
> > > > > clk-next. Would it be possible to try with next-20150507 and
> > > > > clk_ignore_unused on the command line?
> > > > 
> > > > That doesn't help.  I tried on cubieboard2 and bananapi.
> > > 
> > > Thanks for trying.
> > > 
> > > > > Also we can try to see if
> > > > > critical clocks aren't being forced on by applying this patch and
> > > > > looking for clk_get() failures
> > > > 
> > > > From cubieboard2, there's a few that look rather important:
> > > > 
> > > > [    0.000000] Additional per-CPU info printed with stalls.
> > > > [    0.000000] Build-time adjustment of leaf fanout to 32.
> > > > [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > > > [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32,
> > > > nr_cpu_ids=2
> > > > [    0.000000] NR_IRQS:16 nr_irqs:16 16
> > > > [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb@01c20054
> > > > [    0.000000] Failed to enable critical clock cpu
> > > > [    0.000000] Failed to enable critical clock pll5_ddr
> > > > [    0.000000] Failed to enable critical clock ahb_sdram
> > > > [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).
> > > 
> > > Ok. So it seems we need to come up with some solution to the
> > > "critical clocks" problem that doesn't require the individual
> > > clock drivers to call clk_prepare_enable().
> > 
> > I'm getting more and more unsure if we can really handle the complexity
> > we get by allowing to register orphaned clocks. On one hand we can't
> > handle the orphaned clocks properly when we do a clk_prepare/enable on
> > them, on the other hand we run into trouble when we forbid to
> > prepare/enable them. The fact that clocks can become orphans by
> > reparenting them makes it even more complicated.
> > Maybe allowing orphans is something that has to be revisited.
> 
> hmm, I don't see it this drastic. I was expecting a lot more fallout from 
> changing the behaviour of orphaned clocks over all arches using the CCF.
> 
> From the kernelci-boards only the Sunxi-ones seem to have been affected at all 
> and also only because they need a "regulator-always-on" equivalent in the CCF, 
> which currently hijacks prepare/enable inside the clock driver to achive this.

Mediatek has the same case. Here we needs some clock to stay always on and
these clocks get registered before the PLLs they depend on. This means
they can't be enabled right after registration.

Keeping the prepare/enable count correct and syncing the software state
with the hardware state is made quite complicated with orphaned clocks
and I suspect more bugs here. Even now it's hard to move in the
clock framework without breaking other stuff, this won't get easier with
more bug fixes.

So I think asking what allowing orphaned clocks really buys us is valid.
Of course we would need to find a way to get the initialisation order
straight which will probably cause some pain.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Sascha Hauer]

On Fri, May 08, 2015 at 11:30:19AM +0200, Heiko St?bner wrote:
> Am Freitag, 8. Mai 2015, 10:13:55 schrieb Sascha Hauer:
> > On Thu, May 07, 2015 at 11:53:18PM -0700, Stephen Boyd wrote:
> > > On 05/07, Kevin Hilman wrote:
> > > > On Thu, May 7, 2015 at 2:03 PM, Stephen Boyd <sboyd@codeaurora.org> 
> wrote:
> > > > > On 05/07/15 08:17, Kevin Hilman wrote:
> > > > >> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> 
> wrote:
> > > > >>> On 05/01/15 15:07, Heiko St?bner wrote:
> > > > >>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > > > >>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
> > > > >>>>>> in the
> > > > >>>>>> following patch?
> > > > >>>>> 
> > > > >>>>> It looks like it will work at least, but still I'd prefer to keep
> > > > >>>>> the
> > > > >>>>> orphan check contained to clk.c. How about this compile tested
> > > > >>>>> only patch?
> > > > >>>> 
> > > > >>>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > > > >>>> clock tree looks the same and it also still defers nicely in the
> > > > >>>> scenario I needed it for. The implementation also looks nice - and
> > > > >>>> of course much more compact than my check in two places :-) . I
> > > > >>>> don't know if you want to put this as follow-up on top or fold it
> > > > >>>> into the original orphan-check, so in any case
> > > > >>>> 
> > > > >>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > > > >>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > > > >>> 
> > > > >>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > > > >>> with
> > > > >>> my patch and a note that it's based on an earlier patch from you.
> > > > >> 
> > > > >> It appears this has landed in linux-next in the form of 882667c1fcf1
> > > > >> clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > > >> for sunxi in today's linux-next[1] were bisected down to that patch.
> > > > >> 
> > > > >> I confirmed that reverting that commit on top of next/master gets
> > > > >> sunxi booting again.
> > > > > 
> > > > > Thanks for the report. I've removed the two clk orphan patches from
> > > > > clk-next. Would it be possible to try with next-20150507 and
> > > > > clk_ignore_unused on the command line?
> > > > 
> > > > That doesn't help.  I tried on cubieboard2 and bananapi.
> > > 
> > > Thanks for trying.
> > > 
> > > > > Also we can try to see if
> > > > > critical clocks aren't being forced on by applying this patch and
> > > > > looking for clk_get() failures
> > > > 
> > > > From cubieboard2, there's a few that look rather important:
> > > > 
> > > > [    0.000000] Additional per-CPU info printed with stalls.
> > > > [    0.000000] Build-time adjustment of leaf fanout to 32.
> > > > [    0.000000] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=2.
> > > > [    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32,
> > > > nr_cpu_ids=2
> > > > [    0.000000] NR_IRQS:16 nr_irqs:16 16
> > > > [    0.000000] clk: couldn't get parent clock 0 for /clocks/ahb at 01c20054
> > > > [    0.000000] Failed to enable critical clock cpu
> > > > [    0.000000] Failed to enable critical clock pll5_ddr
> > > > [    0.000000] Failed to enable critical clock ahb_sdram
> > > > [    0.000000] Architected cp15 timer(s) running at 24.00MHz (virt).
> > > 
> > > Ok. So it seems we need to come up with some solution to the
> > > "critical clocks" problem that doesn't require the individual
> > > clock drivers to call clk_prepare_enable().
> > 
> > I'm getting more and more unsure if we can really handle the complexity
> > we get by allowing to register orphaned clocks. On one hand we can't
> > handle the orphaned clocks properly when we do a clk_prepare/enable on
> > them, on the other hand we run into trouble when we forbid to
> > prepare/enable them. The fact that clocks can become orphans by
> > reparenting them makes it even more complicated.
> > Maybe allowing orphans is something that has to be revisited.
> 
> hmm, I don't see it this drastic. I was expecting a lot more fallout from 
> changing the behaviour of orphaned clocks over all arches using the CCF.
> 
> From the kernelci-boards only the Sunxi-ones seem to have been affected at all 
> and also only because they need a "regulator-always-on" equivalent in the CCF, 
> which currently hijacks prepare/enable inside the clock driver to achive this.

Mediatek has the same case. Here we needs some clock to stay always on and
these clocks get registered before the PLLs they depend on. This means
they can't be enabled right after registration.

Keeping the prepare/enable count correct and syncing the software state
with the hardware state is made quite complicated with orphaned clocks
and I suspect more bugs here. Even now it's hard to move in the
clock framework without breaking other stuff, this won't get easier with
more bug fixes.

So I think asking what allowing orphaned clocks really buys us is valid.
Of course we would need to find a way to get the initialisation order
straight which will probably cause some pain.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]

On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
> > On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> On 05/01/15 15:07, Heiko Stübner wrote:
> >>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>
> >>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >>>>> following patch?
> >>>> It looks like it will work at least, but still I'd prefer to keep the
> >>>> orphan check contained to clk.c. How about this compile tested only patch?
> >>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >>> looks the same and it also still defers nicely in the scenario I needed it
> >>> for. The implementation also looks nice - and of course much more compact than
> >>> my check in two places :-) . I don't know if you want to put this as follow-up
> >>> on top or fold it into the original orphan-check, so in any case
> >>>
> >>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> my patch and a note that it's based on an earlier patch from you.
> > It appears this has landed in linux-next in the form of 882667c1fcf1
> > clk: prevent orphan clocks from being used.  A bunch of boot failures
> > for sunxi in today's linux-next[1] were bisected down to that patch.
> >
> > I confirmed that reverting that commit on top of next/master gets
> > sunxi booting again.
> >
> >
> 
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

This makes it work, but it's not really an option.

> Also we can try to see if critical clocks aren't being forced on by
> applying this patch and looking for clk_get() failures

And that shows that the CPU and DDR clocks are not protected, which
obviously is pretty mad.

I've mass converted all our probing code to use OF_CLK_DECLARE, and
make things work again.

http://code.bulix.org/5goa5j-88345?raw

Is this an acceptable solution?

We were already moving to this, I'm not really fond of doing this like
that, but I guess this whole debacle makes it necessary.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> On 05/07/15 08:17, Kevin Hilman wrote:
> > On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> On 05/01/15 15:07, Heiko St?bner wrote:
> >>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>
> >>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >>>>> following patch?
> >>>> It looks like it will work at least, but still I'd prefer to keep the
> >>>> orphan check contained to clk.c. How about this compile tested only patch?
> >>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >>> looks the same and it also still defers nicely in the scenario I needed it
> >>> for. The implementation also looks nice - and of course much more compact than
> >>> my check in two places :-) . I don't know if you want to put this as follow-up
> >>> on top or fold it into the original orphan-check, so in any case
> >>>
> >>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> my patch and a note that it's based on an earlier patch from you.
> > It appears this has landed in linux-next in the form of 882667c1fcf1
> > clk: prevent orphan clocks from being used.  A bunch of boot failures
> > for sunxi in today's linux-next[1] were bisected down to that patch.
> >
> > I confirmed that reverting that commit on top of next/master gets
> > sunxi booting again.
> >
> >
> 
> Thanks for the report. I've removed the two clk orphan patches from
> clk-next. Would it be possible to try with next-20150507 and
> clk_ignore_unused on the command line?

This makes it work, but it's not really an option.

> Also we can try to see if critical clocks aren't being forced on by
> applying this patch and looking for clk_get() failures

And that shows that the CPU and DDR clocks are not protected, which
obviously is pretty mad.

I've mass converted all our probing code to use OF_CLK_DECLARE, and
make things work again.

http://code.bulix.org/5goa5j-88345?raw

Is this an acceptable solution?

We were already moving to this, I'm not really fond of doing this like
that, but I guess this whole debacle makes it necessary.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150508/634989ca/attachment.sig>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Tero Kristo]

On 05/07/2015 09:18 PM, Stephen Boyd wrote:
> On 05/07/15 01:22, Tero Kristo wrote:
>> On 05/02/2015 02:40 AM, Stephen Boyd wrote:
>>> On 05/01/15 15:07, Heiko Stübner wrote:
>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>
>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
>>>>>> in the
>>>>>> following patch?
>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>> orphan check contained to clk.c. How about this compile tested only
>>>>> patch?
>>>> I gave this a spin on my rk3288-firefly board. It still boots, the
>>>> clock tree
>>>> looks the same and it also still defers nicely in the scenario I
>>>> needed it
>>>> for. The implementation also looks nice - and of course much more
>>>> compact than
>>>> my check in two places :-) . I don't know if you want to put this as
>>>> follow-up
>>>> on top or fold it into the original orphan-check, so in any case
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>>
>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>> my patch and a note that it's based on an earlier patch from you.
>>
>> FWIW, just gave a try for these two patches on all TI boards I have
>> access to.
>>
>> Tested-by: Tero Kristo <t-kristo@ti.com>
>>
>> I didn't try your evolved patch though, as you don't seem to have made
>> your mind yet.
>>
>
> Thanks. Can you try the evolved patch? It's in linux-next now as commit
> 882667c1fcf1, and it seems to at least break sunxi boot. I'd be
> interested if it broke TI boards.

Just tried it out, boots fine on all these:

   : Board           : Boot commit                    log
  1: am335x-evm      : PASS 4.1.0-rc2-next-20150507   am335x-evm.txt
  2: am335x-evmsk    : PASS 4.1.0-rc2-next-20150507   am335x-sk.txt
  3: am3517-evm      : PASS 4.1.0-rc2-next-20150507   am3517-evm.txt
  4: am43x-epos-evm  : PASS 4.1.0-rc2-next-20150507   am43xx-epos.txt
  5: am437x-gp-evm   : PASS 4.1.0-rc2-next-20150507   am43xx-gpevm.txt
  6: am57xx-evm      : PASS 4.1.0-rc2-next-20150507   am57xx-evm.txt
  7: omap3-beagle-xm : PASS 4.1.0-rc2-next-20150507   beagleboard.txt
  8: omap3-beagle    : PASS 4.1.0-rc2-next-20150507 
beagleboard-vanilla.txt
  9: am335x-boneblack: PASS 4.1.0-rc2-next-20150507   beaglebone-black.txt
10: am335x-bone     : PASS 4.1.0-rc2-next-20150507   beaglebone.txt
11: dra7xx-evm      : PASS 4.1.0-rc2-next-20150507   dra7xx-evm.txt
12: omap3-n900      : PASS 4.1.0-rc2-next-20150507   n900.txt
13: omap5-uevm      : PASS 4.1.0-rc2-next-20150507   omap5-evm.txt
14: omap4-panda-es  : PASS 4.1.0-rc2-next-20150507   pandaboard-es.txt
15: omap4-panda     : PASS 4.1.0-rc2-next-20150507   pandaboard-vanilla.txt
16: omap2430-sdp    : PASS 4.1.0-rc2-next-20150507   sdp2430.txt
17: omap3430-sdp    : PASS 4.1.0-rc2-next-20150507   sdp3430.txt
18: omap4-sdp-es23plus: PASS 4.1.0-rc2-next-20150507   sdp4430.txt
TOTAL = 18 boards, Booted Boards = 18, No Boot boards = 0

TI boards do not have any orphan clocks.

-Tero

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Tero Kristo]

On 05/07/2015 09:18 PM, Stephen Boyd wrote:
> On 05/07/15 01:22, Tero Kristo wrote:
>> On 05/02/2015 02:40 AM, Stephen Boyd wrote:
>>> On 05/01/15 15:07, Heiko St?bner wrote:
>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>
>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like
>>>>>> in the
>>>>>> following patch?
>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>> orphan check contained to clk.c. How about this compile tested only
>>>>> patch?
>>>> I gave this a spin on my rk3288-firefly board. It still boots, the
>>>> clock tree
>>>> looks the same and it also still defers nicely in the scenario I
>>>> needed it
>>>> for. The implementation also looks nice - and of course much more
>>>> compact than
>>>> my check in two places :-) . I don't know if you want to put this as
>>>> follow-up
>>>> on top or fold it into the original orphan-check, so in any case
>>>>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>>
>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>> my patch and a note that it's based on an earlier patch from you.
>>
>> FWIW, just gave a try for these two patches on all TI boards I have
>> access to.
>>
>> Tested-by: Tero Kristo <t-kristo@ti.com>
>>
>> I didn't try your evolved patch though, as you don't seem to have made
>> your mind yet.
>>
>
> Thanks. Can you try the evolved patch? It's in linux-next now as commit
> 882667c1fcf1, and it seems to at least break sunxi boot. I'd be
> interested if it broke TI boards.

Just tried it out, boots fine on all these:

   : Board           : Boot commit                    log
  1: am335x-evm      : PASS 4.1.0-rc2-next-20150507   am335x-evm.txt
  2: am335x-evmsk    : PASS 4.1.0-rc2-next-20150507   am335x-sk.txt
  3: am3517-evm      : PASS 4.1.0-rc2-next-20150507   am3517-evm.txt
  4: am43x-epos-evm  : PASS 4.1.0-rc2-next-20150507   am43xx-epos.txt
  5: am437x-gp-evm   : PASS 4.1.0-rc2-next-20150507   am43xx-gpevm.txt
  6: am57xx-evm      : PASS 4.1.0-rc2-next-20150507   am57xx-evm.txt
  7: omap3-beagle-xm : PASS 4.1.0-rc2-next-20150507   beagleboard.txt
  8: omap3-beagle    : PASS 4.1.0-rc2-next-20150507 
beagleboard-vanilla.txt
  9: am335x-boneblack: PASS 4.1.0-rc2-next-20150507   beaglebone-black.txt
10: am335x-bone     : PASS 4.1.0-rc2-next-20150507   beaglebone.txt
11: dra7xx-evm      : PASS 4.1.0-rc2-next-20150507   dra7xx-evm.txt
12: omap3-n900      : PASS 4.1.0-rc2-next-20150507   n900.txt
13: omap5-uevm      : PASS 4.1.0-rc2-next-20150507   omap5-evm.txt
14: omap4-panda-es  : PASS 4.1.0-rc2-next-20150507   pandaboard-es.txt
15: omap4-panda     : PASS 4.1.0-rc2-next-20150507   pandaboard-vanilla.txt
16: omap2430-sdp    : PASS 4.1.0-rc2-next-20150507   sdp2430.txt
17: omap3430-sdp    : PASS 4.1.0-rc2-next-20150507   sdp3430.txt
18: omap4-sdp-es23plus: PASS 4.1.0-rc2-next-20150507   sdp4430.txt
TOTAL = 18 boards, Booted Boards = 18, No Boot boards = 0

TI boards do not have any orphan clocks.

-Tero

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/08/15 03:02, Maxime Ripard wrote:
> On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> On 05/07/15 08:17, Kevin Hilman wrote:
>>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> On 05/01/15 15:07, Heiko Stübner wrote:
>>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>>
>>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>>>>> following patch?
>>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>>>>> looks the same and it also still defers nicely in the scenario I needed it
>>>>> for. The implementation also looks nice - and of course much more compact than
>>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>>>>> on top or fold it into the original orphan-check, so in any case
>>>>>
>>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>>> my patch and a note that it's based on an earlier patch from you.
>>> It appears this has landed in linux-next in the form of 882667c1fcf1
>>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>>
>>> I confirmed that reverting that commit on top of next/master gets
>>> sunxi booting again.
>>>
>>>
>> Thanks for the report. I've removed the two clk orphan patches from
>> clk-next. Would it be possible to try with next-20150507 and
>> clk_ignore_unused on the command line?
> This makes it work, but it's not really an option.
>

Hmm.. I thought it didn't fix it for Kevin. Confused.

>> Also we can try to see if critical clocks aren't being forced on by
>> applying this patch and looking for clk_get() failures
> And that shows that the CPU and DDR clocks are not protected, which
> obviously is pretty mad.
>
> I've mass converted all our probing code to use OF_CLK_DECLARE, and
> make things work again.
>
> http://code.bulix.org/5goa5j-88345?raw
>
> Is this an acceptable solution?
>
> We were already moving to this, I'm not really fond of doing this like
> that, but I guess this whole debacle makes it necessary.
>

I wonder why we can't switch out the clk_ops on the affected platforms +
clocks to be read-only (at least for the enable/disable part)? That
would fix it just the same right? I wasn't around for the original
discussion regarding this always-on stuff so perhaps I've missed something.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 05/08/15 03:02, Maxime Ripard wrote:
> On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> On 05/07/15 08:17, Kevin Hilman wrote:
>>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> On 05/01/15 15:07, Heiko St?bner wrote:
>>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>>>>>
>>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>>>>>>> following patch?
>>>>>> It looks like it will work at least, but still I'd prefer to keep the
>>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>>>>> looks the same and it also still defers nicely in the scenario I needed it
>>>>> for. The implementation also looks nice - and of course much more compact than
>>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>>>>> on top or fold it into the original orphan-check, so in any case
>>>>>
>>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>>>> my patch and a note that it's based on an earlier patch from you.
>>> It appears this has landed in linux-next in the form of 882667c1fcf1
>>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>>> for sunxi in today's linux-next[1] were bisected down to that patch.
>>>
>>> I confirmed that reverting that commit on top of next/master gets
>>> sunxi booting again.
>>>
>>>
>> Thanks for the report. I've removed the two clk orphan patches from
>> clk-next. Would it be possible to try with next-20150507 and
>> clk_ignore_unused on the command line?
> This makes it work, but it's not really an option.
>

Hmm.. I thought it didn't fix it for Kevin. Confused.

>> Also we can try to see if critical clocks aren't being forced on by
>> applying this patch and looking for clk_get() failures
> And that shows that the CPU and DDR clocks are not protected, which
> obviously is pretty mad.
>
> I've mass converted all our probing code to use OF_CLK_DECLARE, and
> make things work again.
>
> http://code.bulix.org/5goa5j-88345?raw
>
> Is this an acceptable solution?
>
> We were already moving to this, I'm not really fond of doing this like
> that, but I guess this whole debacle makes it necessary.
>

I wonder why we can't switch out the clk_ops on the affected platforms +
clocks to be read-only (at least for the enable/disable part)? That
would fix it just the same right? I wasn't around for the original
discussion regarding this always-on stuff so perhaps I've missed something.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3331 bytes --]

On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> On 05/08/15 03:02, Maxime Ripard wrote:
> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >>>> On 05/01/15 15:07, Heiko Stübner wrote:
> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>>>
> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >>>>>>> following patch?
> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >>>>> looks the same and it also still defers nicely in the scenario I needed it
> >>>>> for. The implementation also looks nice - and of course much more compact than
> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >>>>> on top or fold it into the original orphan-check, so in any case
> >>>>>
> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >>>> my patch and a note that it's based on an earlier patch from you.
> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >>>
> >>> I confirmed that reverting that commit on top of next/master gets
> >>> sunxi booting again.
> >>>
> >>>
> >> Thanks for the report. I've removed the two clk orphan patches from
> >> clk-next. Would it be possible to try with next-20150507 and
> >> clk_ignore_unused on the command line?
> > This makes it work, but it's not really an option.
> >
> 
> Hmm.. I thought it didn't fix it for Kevin. Confused.

I'm too, but it does fix things here.

> >> Also we can try to see if critical clocks aren't being forced on by
> >> applying this patch and looking for clk_get() failures
> > And that shows that the CPU and DDR clocks are not protected, which
> > obviously is pretty mad.
> >
> > I've mass converted all our probing code to use OF_CLK_DECLARE, and
> > make things work again.
> >
> > http://code.bulix.org/5goa5j-88345?raw
> >
> > Is this an acceptable solution?
> >
> > We were already moving to this, I'm not really fond of doing this like
> > that, but I guess this whole debacle makes it necessary.
> >
> 
> I wonder why we can't switch out the clk_ops on the affected platforms +
> clocks to be read-only (at least for the enable/disable part)? That
> would fix it just the same right? I wasn't around for the original
> discussion regarding this always-on stuff so perhaps I've missed something.

We're using clk-gate, so that would require to recode our own
driver. That change seemed less invasive, while fixing the issue and
ensuring that we wouldn't have any orphan clock, which seems to be
hunted down these days...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> On 05/08/15 03:02, Maxime Ripard wrote:
> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >>>> On 05/01/15 15:07, Heiko St?bner wrote:
> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >>>>>
> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >>>>>>> following patch?
> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >>>>> looks the same and it also still defers nicely in the scenario I needed it
> >>>>> for. The implementation also looks nice - and of course much more compact than
> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >>>>> on top or fold it into the original orphan-check, so in any case
> >>>>>
> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >>>> my patch and a note that it's based on an earlier patch from you.
> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >>>
> >>> I confirmed that reverting that commit on top of next/master gets
> >>> sunxi booting again.
> >>>
> >>>
> >> Thanks for the report. I've removed the two clk orphan patches from
> >> clk-next. Would it be possible to try with next-20150507 and
> >> clk_ignore_unused on the command line?
> > This makes it work, but it's not really an option.
> >
> 
> Hmm.. I thought it didn't fix it for Kevin. Confused.

I'm too, but it does fix things here.

> >> Also we can try to see if critical clocks aren't being forced on by
> >> applying this patch and looking for clk_get() failures
> > And that shows that the CPU and DDR clocks are not protected, which
> > obviously is pretty mad.
> >
> > I've mass converted all our probing code to use OF_CLK_DECLARE, and
> > make things work again.
> >
> > http://code.bulix.org/5goa5j-88345?raw
> >
> > Is this an acceptable solution?
> >
> > We were already moving to this, I'm not really fond of doing this like
> > that, but I guess this whole debacle makes it necessary.
> >
> 
> I wonder why we can't switch out the clk_ops on the affected platforms +
> clocks to be read-only (at least for the enable/disable part)? That
> would fix it just the same right? I wasn't around for the original
> discussion regarding this always-on stuff so perhaps I've missed something.

We're using clk-gate, so that would require to recode our own
driver. That change seemed less invasive, while fixing the issue and
ensuring that we wouldn't have any orphan clock, which seems to be
hunted down these days...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150513/34503f2b/attachment-0001.sig>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

Maxime Ripard <maxime.ripard@free-electrons.com> writes:

> On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> On 05/08/15 03:02, Maxime Ripard wrote:
>> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> >>>> On 05/01/15 15:07, Heiko Stübner wrote:
>> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >>>>>
>> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>> >>>>>>> following patch?
>> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
>> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> >>>>> looks the same and it also still defers nicely in the scenario I needed it
>> >>>>> for. The implementation also looks nice - and of course much more compact than
>> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>> >>>>> on top or fold it into the original orphan-check, so in any case
>> >>>>>
>> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> >>>> my patch and a note that it's based on an earlier patch from you.
>> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >>>
>> >>> I confirmed that reverting that commit on top of next/master gets
>> >>> sunxi booting again.
>> >>>
>> >>>
>> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> clk-next. Would it be possible to try with next-20150507 and
>> >> clk_ignore_unused on the command line?
>> > This makes it work, but it's not really an option.
>> >
>> 
>> Hmm.. I thought it didn't fix it for Kevin. Confused.
>
> I'm too, but it does fix things here.

To be more precise on what I tested.  I used next-20150507 and tested on
4 different sunxi platforms.  First test was "normal" commandline,
second was with clk_ignore_unused appended:

  - cubie: fail, fail
  - cubie2: fail, fail
  - bananpi: fail, pass
  - cubietruck: fail, pass

So it seems to have some effect, but by itself, doesn't fix the issue.

Kevin

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

Maxime Ripard <maxime.ripard@free-electrons.com> writes:

> On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> On 05/08/15 03:02, Maxime Ripard wrote:
>> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> =
wrote:
>> >>>> On 05/01/15 15:07, Heiko St=C3=BCbner wrote:
>> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >>>>>
>> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, lik=
e in the
>> >>>>>>> following patch?
>> >>>>>> It looks like it will work at least, but still I'd prefer to keep=
 the
>> >>>>>> orphan check contained to clk.c. How about this compile tested on=
ly patch?
>> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the=
 clock tree
>> >>>>> looks the same and it also still defers nicely in the scenario I n=
eeded it
>> >>>>> for. The implementation also looks nice - and of course much more =
compact than
>> >>>>> my check in two places :-) . I don't know if you want to put this =
as follow-up
>> >>>>> on top or fold it into the original orphan-check, so in any case
>> >>>>>
>> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it=
 with
>> >>>> my patch and a note that it's based on an earlier patch from you.
>> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >>>
>> >>> I confirmed that reverting that commit on top of next/master gets
>> >>> sunxi booting again.
>> >>>
>> >>>
>> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> clk-next. Would it be possible to try with next-20150507 and
>> >> clk_ignore_unused on the command line?
>> > This makes it work, but it's not really an option.
>> >
>>=20
>> Hmm.. I thought it didn't fix it for Kevin. Confused.
>
> I'm too, but it does fix things here.

To be more precise on what I tested.  I used next-20150507 and tested on
4 different sunxi platforms.  First test was "normal" commandline,
second was with clk_ignore_unused appended:

  - cubie: fail, fail
  - cubie2: fail, fail
  - bananpi: fail, pass
  - cubietruck: fail, pass

So it seems to have some effect, but by itself, doesn't fix the issue.

Kevin

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

Maxime Ripard <maxime.ripard@free-electrons.com> writes:

> On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> On 05/08/15 03:02, Maxime Ripard wrote:
>> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> >>>> On 05/01/15 15:07, Heiko St?bner wrote:
>> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >>>>>
>> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>> >>>>>>> following patch?
>> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
>> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> >>>>> looks the same and it also still defers nicely in the scenario I needed it
>> >>>>> for. The implementation also looks nice - and of course much more compact than
>> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>> >>>>> on top or fold it into the original orphan-check, so in any case
>> >>>>>
>> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> >>>> my patch and a note that it's based on an earlier patch from you.
>> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >>>
>> >>> I confirmed that reverting that commit on top of next/master gets
>> >>> sunxi booting again.
>> >>>
>> >>>
>> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> clk-next. Would it be possible to try with next-20150507 and
>> >> clk_ignore_unused on the command line?
>> > This makes it work, but it's not really an option.
>> >
>> 
>> Hmm.. I thought it didn't fix it for Kevin. Confused.
>
> I'm too, but it does fix things here.

To be more precise on what I tested.  I used next-20150507 and tested on
4 different sunxi platforms.  First test was "normal" commandline,
second was with clk_ignore_unused appended:

  - cubie: fail, fail
  - cubie2: fail, fail
  - bananpi: fail, pass
  - cubietruck: fail, pass

So it seems to have some effect, but by itself, doesn't fix the issue.

Kevin

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3013 bytes --]

On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
> 
> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> >> On 05/08/15 03:02, Maxime Ripard wrote:
> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> >>>> On 05/01/15 15:07, Heiko Stübner wrote:
> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >> >>>>>
> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >> >>>>>>> following patch?
> >> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
> >> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >> >>>>> looks the same and it also still defers nicely in the scenario I needed it
> >> >>>>> for. The implementation also looks nice - and of course much more compact than
> >> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >> >>>>> on top or fold it into the original orphan-check, so in any case
> >> >>>>>
> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> >>>> my patch and a note that it's based on an earlier patch from you.
> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >> >>>
> >> >>> I confirmed that reverting that commit on top of next/master gets
> >> >>> sunxi booting again.
> >> >>>
> >> >>>
> >> >> Thanks for the report. I've removed the two clk orphan patches from
> >> >> clk-next. Would it be possible to try with next-20150507 and
> >> >> clk_ignore_unused on the command line?
> >> > This makes it work, but it's not really an option.
> >> >
> >> 
> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
> >
> > I'm too, but it does fix things here.
> 
> To be more precise on what I tested.  I used next-20150507 and tested on
> 4 different sunxi platforms.  First test was "normal" commandline,
> second was with clk_ignore_unused appended:
> 
>   - cubie: fail, fail
>   - cubie2: fail, fail
>   - bananpi: fail, pass
>   - cubietruck: fail, pass
> 
> So it seems to have some effect, but by itself, doesn't fix the issue.

It's very odd, I actually tried with a cubie2 here...

I'm booting on an initramfs and not MMC though, but I can't see how
that can be related to our issue...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
> 
> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> >> On 05/08/15 03:02, Maxime Ripard wrote:
> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> >>>> On 05/01/15 15:07, Heiko St?bner wrote:
> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >> >>>>>
> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >> >>>>>>> following patch?
> >> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
> >> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >> >>>>> looks the same and it also still defers nicely in the scenario I needed it
> >> >>>>> for. The implementation also looks nice - and of course much more compact than
> >> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >> >>>>> on top or fold it into the original orphan-check, so in any case
> >> >>>>>
> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> >>>> my patch and a note that it's based on an earlier patch from you.
> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >> >>>
> >> >>> I confirmed that reverting that commit on top of next/master gets
> >> >>> sunxi booting again.
> >> >>>
> >> >>>
> >> >> Thanks for the report. I've removed the two clk orphan patches from
> >> >> clk-next. Would it be possible to try with next-20150507 and
> >> >> clk_ignore_unused on the command line?
> >> > This makes it work, but it's not really an option.
> >> >
> >> 
> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
> >
> > I'm too, but it does fix things here.
> 
> To be more precise on what I tested.  I used next-20150507 and tested on
> 4 different sunxi platforms.  First test was "normal" commandline,
> second was with clk_ignore_unused appended:
> 
>   - cubie: fail, fail
>   - cubie2: fail, fail
>   - bananpi: fail, pass
>   - cubietruck: fail, pass
> 
> So it seems to have some effect, but by itself, doesn't fix the issue.

It's very odd, I actually tried with a cubie2 here...

I'm booting on an initramfs and not MMC though, but I can't see how
that can be related to our issue...

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150513/f7533541/attachment-0001.sig>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
>> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
>>
>> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> >> On 05/08/15 03:02, Maxime Ripard wrote:
>> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> >> >>>> On 05/01/15 15:07, Heiko Stübner wrote:
>> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >> >>>>>
>> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>> >> >>>>>>> following patch?
>> >> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
>> >> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> >> >>>>> looks the same and it also still defers nicely in the scenario I needed it
>> >> >>>>> for. The implementation also looks nice - and of course much more compact than
>> >> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>> >> >>>>> on top or fold it into the original orphan-check, so in any case
>> >> >>>>>
>> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> >> >>>> my patch and a note that it's based on an earlier patch from you.
>> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >> >>>
>> >> >>> I confirmed that reverting that commit on top of next/master gets
>> >> >>> sunxi booting again.
>> >> >>>
>> >> >>>
>> >> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> >> clk-next. Would it be possible to try with next-20150507 and
>> >> >> clk_ignore_unused on the command line?
>> >> > This makes it work, but it's not really an option.
>> >> >
>> >>
>> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
>> >
>> > I'm too, but it does fix things here.
>>
>> To be more precise on what I tested.  I used next-20150507 and tested on
>> 4 different sunxi platforms.  First test was "normal" commandline,
>> second was with clk_ignore_unused appended:
>>
>>   - cubie: fail, fail
>>   - cubie2: fail, fail
>>   - bananpi: fail, pass
>>   - cubietruck: fail, pass
>>
>> So it seems to have some effect, but by itself, doesn't fix the issue.
>
> It's very odd, I actually tried with a cubie2 here...
>
> I'm booting on an initramfs and not MMC though, but I can't see how
> that can be related to our issue...

I'm booting an initramfs too.

Kevin

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
>> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
>>
>> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> >> On 05/08/15 03:02, Maxime Ripard wrote:
>> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.or=
g> wrote:
>> >> >>>> On 05/01/15 15:07, Heiko St=C3=BCbner wrote:
>> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >> >>>>>
>> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, =
like in the
>> >> >>>>>>> following patch?
>> >> >>>>>> It looks like it will work at least, but still I'd prefer to k=
eep the
>> >> >>>>>> orphan check contained to clk.c. How about this compile tested=
 only patch?
>> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, =
the clock tree
>> >> >>>>> looks the same and it also still defers nicely in the scenario =
I needed it
>> >> >>>>> for. The implementation also looks nice - and of course much mo=
re compact than
>> >> >>>>> my check in two places :-) . I don't know if you want to put th=
is as follow-up
>> >> >>>>> on top or fold it into the original orphan-check, so in any cas=
e
>> >> >>>>>
>> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing=
 it with
>> >> >>>> my patch and a note that it's based on an earlier patch from you=
.
>> >> >>> It appears this has landed in linux-next in the form of 882667c1f=
cf1
>> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot fail=
ures
>> >> >>> for sunxi in today's linux-next[1] were bisected down to that pat=
ch.
>> >> >>>
>> >> >>> I confirmed that reverting that commit on top of next/master gets
>> >> >>> sunxi booting again.
>> >> >>>
>> >> >>>
>> >> >> Thanks for the report. I've removed the two clk orphan patches fro=
m
>> >> >> clk-next. Would it be possible to try with next-20150507 and
>> >> >> clk_ignore_unused on the command line?
>> >> > This makes it work, but it's not really an option.
>> >> >
>> >>
>> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
>> >
>> > I'm too, but it does fix things here.
>>
>> To be more precise on what I tested.  I used next-20150507 and tested on
>> 4 different sunxi platforms.  First test was "normal" commandline,
>> second was with clk_ignore_unused appended:
>>
>>   - cubie: fail, fail
>>   - cubie2: fail, fail
>>   - bananpi: fail, pass
>>   - cubietruck: fail, pass
>>
>> So it seems to have some effect, but by itself, doesn't fix the issue.
>
> It's very odd, I actually tried with a cubie2 here...
>
> I'm booting on an initramfs and not MMC though, but I can't see how
> that can be related to our issue...

I'm booting an initramfs too.

Kevin

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Kevin Hilman]

On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
>> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
>>
>> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
>> >> On 05/08/15 03:02, Maxime Ripard wrote:
>> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
>> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
>> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> >> >>>> On 05/01/15 15:07, Heiko St?bner wrote:
>> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
>> >> >>>>>
>> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
>> >> >>>>>>> following patch?
>> >> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
>> >> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
>> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
>> >> >>>>> looks the same and it also still defers nicely in the scenario I needed it
>> >> >>>>> for. The implementation also looks nice - and of course much more compact than
>> >> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
>> >> >>>>> on top or fold it into the original orphan-check, so in any case
>> >> >>>>>
>> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
>> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
>> >> >>>> my patch and a note that it's based on an earlier patch from you.
>> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
>> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
>> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
>> >> >>>
>> >> >>> I confirmed that reverting that commit on top of next/master gets
>> >> >>> sunxi booting again.
>> >> >>>
>> >> >>>
>> >> >> Thanks for the report. I've removed the two clk orphan patches from
>> >> >> clk-next. Would it be possible to try with next-20150507 and
>> >> >> clk_ignore_unused on the command line?
>> >> > This makes it work, but it's not really an option.
>> >> >
>> >>
>> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
>> >
>> > I'm too, but it does fix things here.
>>
>> To be more precise on what I tested.  I used next-20150507 and tested on
>> 4 different sunxi platforms.  First test was "normal" commandline,
>> second was with clk_ignore_unused appended:
>>
>>   - cubie: fail, fail
>>   - cubie2: fail, fail
>>   - bananpi: fail, pass
>>   - cubietruck: fail, pass
>>
>> So it seems to have some effect, but by itself, doesn't fix the issue.
>
> It's very odd, I actually tried with a cubie2 here...
>
> I'm booting on an initramfs and not MMC though, but I can't see how
> that can be related to our issue...

I'm booting an initramfs too.

Kevin

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 3395 bytes --]

On Wed, May 13, 2015 at 01:44:50PM -0700, Kevin Hilman wrote:
> On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
> >> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
> >>
> >> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> >> >> On 05/08/15 03:02, Maxime Ripard wrote:
> >> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> >> >>>> On 05/01/15 15:07, Heiko Stübner wrote:
> >> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >> >> >>>>>
> >> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >> >> >>>>>>> following patch?
> >> >> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
> >> >> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >> >> >>>>> looks the same and it also still defers nicely in the scenario I needed it
> >> >> >>>>> for. The implementation also looks nice - and of course much more compact than
> >> >> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >> >> >>>>> on top or fold it into the original orphan-check, so in any case
> >> >> >>>>>
> >> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> >> >>>> my patch and a note that it's based on an earlier patch from you.
> >> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >> >> >>>
> >> >> >>> I confirmed that reverting that commit on top of next/master gets
> >> >> >>> sunxi booting again.
> >> >> >>>
> >> >> >>>
> >> >> >> Thanks for the report. I've removed the two clk orphan patches from
> >> >> >> clk-next. Would it be possible to try with next-20150507 and
> >> >> >> clk_ignore_unused on the command line?
> >> >> > This makes it work, but it's not really an option.
> >> >> >
> >> >>
> >> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
> >> >
> >> > I'm too, but it does fix things here.
> >>
> >> To be more precise on what I tested.  I used next-20150507 and tested on
> >> 4 different sunxi platforms.  First test was "normal" commandline,
> >> second was with clk_ignore_unused appended:
> >>
> >>   - cubie: fail, fail
> >>   - cubie2: fail, fail
> >>   - bananpi: fail, pass
> >>   - cubietruck: fail, pass
> >>
> >> So it seems to have some effect, but by itself, doesn't fix the issue.
> >
> > It's very odd, I actually tried with a cubie2 here...
> >
> > I'm booting on an initramfs and not MMC though, but I can't see how
> > that can be related to our issue...
> 
> I'm booting an initramfs too.

Then I don't know :)

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

On Wed, May 13, 2015 at 01:44:50PM -0700, Kevin Hilman wrote:
> On Wed, May 13, 2015 at 1:14 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > On Wed, May 13, 2015 at 07:33:54AM -0700, Kevin Hilman wrote:
> >> Maxime Ripard <maxime.ripard@free-electrons.com> writes:
> >>
> >> > On Tue, May 12, 2015 at 03:35:50PM -0700, Stephen Boyd wrote:
> >> >> On 05/08/15 03:02, Maxime Ripard wrote:
> >> >> > On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> >> >> >> On 05/07/15 08:17, Kevin Hilman wrote:
> >> >> >>> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> >> >>>> On 05/01/15 15:07, Heiko St?bner wrote:
> >> >> >>>>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> >> >> >>>>>
> >> >> >>>>>>> Instead I guess we could hook it less deep into clk_get_sys, like in the
> >> >> >>>>>>> following patch?
> >> >> >>>>>> It looks like it will work at least, but still I'd prefer to keep the
> >> >> >>>>>> orphan check contained to clk.c. How about this compile tested only patch?
> >> >> >>>>> I gave this a spin on my rk3288-firefly board. It still boots, the clock tree
> >> >> >>>>> looks the same and it also still defers nicely in the scenario I needed it
> >> >> >>>>> for. The implementation also looks nice - and of course much more compact than
> >> >> >>>>> my check in two places :-) . I don't know if you want to put this as follow-up
> >> >> >>>>> on top or fold it into the original orphan-check, so in any case
> >> >> >>>>>
> >> >> >>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> >> >> >>>>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> >> >> >>>> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it with
> >> >> >>>> my patch and a note that it's based on an earlier patch from you.
> >> >> >>> It appears this has landed in linux-next in the form of 882667c1fcf1
> >> >> >>> clk: prevent orphan clocks from being used.  A bunch of boot failures
> >> >> >>> for sunxi in today's linux-next[1] were bisected down to that patch.
> >> >> >>>
> >> >> >>> I confirmed that reverting that commit on top of next/master gets
> >> >> >>> sunxi booting again.
> >> >> >>>
> >> >> >>>
> >> >> >> Thanks for the report. I've removed the two clk orphan patches from
> >> >> >> clk-next. Would it be possible to try with next-20150507 and
> >> >> >> clk_ignore_unused on the command line?
> >> >> > This makes it work, but it's not really an option.
> >> >> >
> >> >>
> >> >> Hmm.. I thought it didn't fix it for Kevin. Confused.
> >> >
> >> > I'm too, but it does fix things here.
> >>
> >> To be more precise on what I tested.  I used next-20150507 and tested on
> >> 4 different sunxi platforms.  First test was "normal" commandline,
> >> second was with clk_ignore_unused appended:
> >>
> >>   - cubie: fail, fail
> >>   - cubie2: fail, fail
> >>   - bananpi: fail, pass
> >>   - cubietruck: fail, pass
> >>
> >> So it seems to have some effect, but by itself, doesn't fix the issue.
> >
> > It's very odd, I actually tried with a cubie2 here...
> >
> > I'm booting on an initramfs and not MMC though, but I can't see how
> > that can be related to our issue...
> 
> I'm booting an initramfs too.

Then I don't know :)

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150513/31eee124/attachment.sig>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Hi Maxime, Stephen,

Am Freitag, 8. Mai 2015, 12:02:47 schrieb Maxime Ripard:
> On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> > On 05/07/15 08:17, Kevin Hilman wrote:
> > > On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> 
wrote:
> > >> On 05/01/15 15:07, Heiko Stübner wrote:
> > >>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > >>>>> Instead I guess we could hook it less deep into clk_get_sys, like in
> > >>>>> the
> > >>>>> following patch?
> > >>>> 
> > >>>> It looks like it will work at least, but still I'd prefer to keep the
> > >>>> orphan check contained to clk.c. How about this compile tested only
> > >>>> patch?
> > >>> 
> > >>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > >>> clock tree looks the same and it also still defers nicely in the
> > >>> scenario I needed it for. The implementation also looks nice - and of
> > >>> course much more compact than my check in two places :-) . I don't
> > >>> know if you want to put this as follow-up on top or fold it into the
> > >>> original orphan-check, so in any case
> > >>> 
> > >>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > >> 
> > >> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > >> with
> > >> my patch and a note that it's based on an earlier patch from you.
> > > 
> > > It appears this has landed in linux-next in the form of 882667c1fcf1
> > > clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > for sunxi in today's linux-next[1] were bisected down to that patch.
> > > 
> > > I confirmed that reverting that commit on top of next/master gets
> > > sunxi booting again.
> > 
> > Thanks for the report. I've removed the two clk orphan patches from
> > clk-next. Would it be possible to try with next-20150507 and
> > clk_ignore_unused on the command line?
> 
> This makes it work, but it's not really an option.
> 
> > Also we can try to see if critical clocks aren't being forced on by
> > applying this patch and looking for clk_get() failures
> 
> And that shows that the CPU and DDR clocks are not protected, which
> obviously is pretty mad.
> 
> I've mass converted all our probing code to use OF_CLK_DECLARE, and
> make things work again.
> 
> http://code.bulix.org/5goa5j-88345?raw
> 
> Is this an acceptable solution?
> 
> We were already moving to this, I'm not really fond of doing this like
> that, but I guess this whole debacle makes it necessary.


did this lead anywhere meanwhile.

Last I remember the change to orphan handling made sunxi fail, but I'm still 
hoping to get this usable at some point :-)


Thanks
Heiko

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Hi Maxime, Stephen,

Am Freitag, 8. Mai 2015, 12:02:47 schrieb Maxime Ripard:
> On Thu, May 07, 2015 at 02:03:57PM -0700, Stephen Boyd wrote:
> > On 05/07/15 08:17, Kevin Hilman wrote:
> > > On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sboyd@codeaurora.org> 
wrote:
> > >> On 05/01/15 15:07, Heiko St?bner wrote:
> > >>> Am Freitag, 1. Mai 2015, 13:52:47 schrieb Stephen Boyd:
> > >>>>> Instead I guess we could hook it less deep into clk_get_sys, like in
> > >>>>> the
> > >>>>> following patch?
> > >>>> 
> > >>>> It looks like it will work at least, but still I'd prefer to keep the
> > >>>> orphan check contained to clk.c. How about this compile tested only
> > >>>> patch?
> > >>> 
> > >>> I gave this a spin on my rk3288-firefly board. It still boots, the
> > >>> clock tree looks the same and it also still defers nicely in the
> > >>> scenario I needed it for. The implementation also looks nice - and of
> > >>> course much more compact than my check in two places :-) . I don't
> > >>> know if you want to put this as follow-up on top or fold it into the
> > >>> original orphan-check, so in any case
> > >>> 
> > >>> Tested-by: Heiko Stuebner <heiko@sntech.de>
> > >>> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > >> 
> > >> Thanks. I'm leaning towards tossing your patch 2/2 and replacing it
> > >> with
> > >> my patch and a note that it's based on an earlier patch from you.
> > > 
> > > It appears this has landed in linux-next in the form of 882667c1fcf1
> > > clk: prevent orphan clocks from being used.  A bunch of boot failures
> > > for sunxi in today's linux-next[1] were bisected down to that patch.
> > > 
> > > I confirmed that reverting that commit on top of next/master gets
> > > sunxi booting again.
> > 
> > Thanks for the report. I've removed the two clk orphan patches from
> > clk-next. Would it be possible to try with next-20150507 and
> > clk_ignore_unused on the command line?
> 
> This makes it work, but it's not really an option.
> 
> > Also we can try to see if critical clocks aren't being forced on by
> > applying this patch and looking for clk_get() failures
> 
> And that shows that the CPU and DDR clocks are not protected, which
> obviously is pretty mad.
> 
> I've mass converted all our probing code to use OF_CLK_DECLARE, and
> make things work again.
> 
> http://code.bulix.org/5goa5j-88345?raw
> 
> Is this an acceptable solution?
> 
> We were already moving to this, I'm not really fond of doing this like
> that, but I guess this whole debacle makes it necessary.


did this lead anywhere meanwhile.

Last I remember the change to orphan handling made sunxi fail, but I'm still 
hoping to get this usable at some point :-)


Thanks
Heiko

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1230 bytes --]

Hi Heiko,

On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
> > > Also we can try to see if critical clocks aren't being forced on by
> > > applying this patch and looking for clk_get() failures
> > 
> > And that shows that the CPU and DDR clocks are not protected, which
> > obviously is pretty mad.
> > 
> > I've mass converted all our probing code to use OF_CLK_DECLARE, and
> > make things work again.
> > 
> > http://code.bulix.org/5goa5j-88345?raw
> > 
> > Is this an acceptable solution?
> > 
> > We were already moving to this, I'm not really fond of doing this like
> > that, but I guess this whole debacle makes it necessary.
> 
> 
> did this lead anywhere meanwhile.
> 
> Last I remember the change to orphan handling made sunxi fail, but
> I'm still hoping to get this usable at some point :-)

To be honest, I don't know what the current status is. I haven't get
any news since that mail.

I started to move a significant portion of our clocks to
CLK_OF_DECLARE, but not all of them are (which probably make the
situation worse for the time being).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Maxime Ripard]

Hi Heiko,

On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko St?bner wrote:
> > > Also we can try to see if critical clocks aren't being forced on by
> > > applying this patch and looking for clk_get() failures
> > 
> > And that shows that the CPU and DDR clocks are not protected, which
> > obviously is pretty mad.
> > 
> > I've mass converted all our probing code to use OF_CLK_DECLARE, and
> > make things work again.
> > 
> > http://code.bulix.org/5goa5j-88345?raw
> > 
> > Is this an acceptable solution?
> > 
> > We were already moving to this, I'm not really fond of doing this like
> > that, but I guess this whole debacle makes it necessary.
> 
> 
> did this lead anywhere meanwhile.
> 
> Last I remember the change to orphan handling made sunxi fail, but
> I'm still hoping to get this usable at some point :-)

To be honest, I don't know what the current status is. I haven't get
any news since that mail.

I started to move a significant portion of our clocks to
CLK_OF_DECLARE, but not all of them are (which probably make the
situation worse for the time being).

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150730/6f3d5af2/attachment.sig>

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 07/30, Maxime Ripard wrote:
> On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
> > 
> > did this lead anywhere meanwhile.
> > 
> > Last I remember the change to orphan handling made sunxi fail, but
> > I'm still hoping to get this usable at some point :-)
> 
> To be honest, I don't know what the current status is. I haven't get
> any news since that mail.
> 
> I started to move a significant portion of our clocks to
> CLK_OF_DECLARE, but not all of them are (which probably make the
> situation worse for the time being).
> 

Sorry, I think we're still waiting for the critical clocks stuff
to settle down so that Sunxi can use that instead of calling
clk_prepare_enable() in init code. For now, I've put the first
patch of the two into clk-next so that we can have proper orphan
tracking. Hopefully we resolve critical clocks soon so that we
can merge the defer part.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Stephen Boyd]

On 07/30, Maxime Ripard wrote:
> On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko St?bner wrote:
> > 
> > did this lead anywhere meanwhile.
> > 
> > Last I remember the change to orphan handling made sunxi fail, but
> > I'm still hoping to get this usable at some point :-)
> 
> To be honest, I don't know what the current status is. I haven't get
> any news since that mail.
> 
> I started to move a significant portion of our clocks to
> CLK_OF_DECLARE, but not all of them are (which probably make the
> situation worse for the time being).
> 

Sorry, I think we're still waiting for the critical clocks stuff
to settle down so that Sunxi can use that instead of calling
clk_prepare_enable() in init code. For now, I've put the first
patch of the two into clk-next so that we can have proper orphan
tracking. Hopefully we resolve critical clocks soon so that we
can merge the defer part.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Dienstag, 11. August 2015, 15:34:09 schrieb Stephen Boyd:
> On 07/30, Maxime Ripard wrote:
> > On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko Stübner wrote:
> > > did this lead anywhere meanwhile.
> > > 
> > > Last I remember the change to orphan handling made sunxi fail, but
> > > I'm still hoping to get this usable at some point :-)
> > 
> > To be honest, I don't know what the current status is. I haven't get
> > any news since that mail.
> > 
> > I started to move a significant portion of our clocks to
> > CLK_OF_DECLARE, but not all of them are (which probably make the
> > situation worse for the time being).
> 
> Sorry, I think we're still waiting for the critical clocks stuff
> to settle down so that Sunxi can use that instead of calling
> clk_prepare_enable() in init code. For now, I've put the first
> patch of the two into clk-next so that we can have proper orphan
> tracking. Hopefully we resolve critical clocks soon so that we
> can merge the defer part.

great :-)

For the defer-part you came up with a better solution than mine anyway, if I 
remember correctly.

[PATCH v3 0/2] clk: improve handling of orphan clocks

[Heiko Stübner]

Am Dienstag, 11. August 2015, 15:34:09 schrieb Stephen Boyd:
> On 07/30, Maxime Ripard wrote:
> > On Mon, Jul 27, 2015 at 10:57:33AM +0200, Heiko St?bner wrote:
> > > did this lead anywhere meanwhile.
> > > 
> > > Last I remember the change to orphan handling made sunxi fail, but
> > > I'm still hoping to get this usable at some point :-)
> > 
> > To be honest, I don't know what the current status is. I haven't get
> > any news since that mail.
> > 
> > I started to move a significant portion of our clocks to
> > CLK_OF_DECLARE, but not all of them are (which probably make the
> > situation worse for the time being).
> 
> Sorry, I think we're still waiting for the critical clocks stuff
> to settle down so that Sunxi can use that instead of calling
> clk_prepare_enable() in init code. For now, I've put the first
> patch of the two into clk-next so that we can have proper orphan
> tracking. Hopefully we resolve critical clocks soon so that we
> can merge the defer part.

great :-)

For the defer-part you came up with a better solution than mine anyway, if I 
remember correctly.