Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

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

24 views
Skip to first unread message

Heiko Stübner

unread,
Apr 12, 2015, 7:40:05 AM4/12/15
to
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 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 orphan 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.


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

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

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

--
2.1.4


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Heiko Stübner

unread,
Apr 12, 2015, 7:40:05 AM4/12/15
to
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 <sb...@codeaurora.org>
Signed-off-by: Heiko Stuebner <he...@sntech.de>
---
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 f85c8e2..a9fa5ab 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;
@@ -1433,18 +1434,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_update_orphan_status(struct clk_core *clk, int is_orphan)
+{
+ struct clk_core *child;
+
+ clk->orphan = is_orphan;
+
+ hlist_for_each_entry(child, &clk->children, child_node)
+ clk_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_update_orphan_status(clk, becomes_orphan);
} else {
hlist_add_head(&clk->child_node, &clk_orphan_list);
+ if (!was_orphan)
+ clk_update_orphan_status(clk, 1);
}

clk->parent = new_parent;
@@ -2348,13 +2371,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 = 0;
+ } else {
hlist_add_head(&clk->child_node, &clk_orphan_list);
+ clk->orphan = 1;
+ }

/*
* Set clk's accuracy. The preferred method is to use

Heiko Stübner

unread,
Apr 12, 2015, 7:40:06 AM4/12/15
to
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 <he...@sntech.de>
---
drivers/clk/clk.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a9fa5ab..b977701 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2272,6 +2272,17 @@ 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;
+
+ if (!clk->core)
+ return false;
+
+ return clk->core->orphan;
+}
+
/**
* __clk_init - initialize the data structures in a struct clk
* @dev: device initializing this clk, placeholder for now
@@ -3009,6 +3020,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);

Stephen Boyd

unread,
Apr 20, 2015, 6:30:06 PM4/20/15
to
On 04/12/15 04:37, Heiko Stübner wrote:
> 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 <sb...@codeaurora.org>
> Signed-off-by: Heiko Stuebner <he...@sntech.de>

Just nitpicks. Otherwise it looks good to me.

> ---
> 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 f85c8e2..a9fa5ab 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;
> @@ -1433,18 +1434,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_update_orphan_status(struct clk_core *clk, int is_orphan)

Please s/clk/core/ here because it's a new function (yeah we've been
slow and haven't updated the whole file to do this consistently). Also
use bool for is_orphan.
Please use true here.

> }
>
> clk->parent = new_parent;
> @@ -2348,13 +2371,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 = 0;
> + } else {
> hlist_add_head(&clk->child_node, &clk_orphan_list);
> + clk->orphan = 1;

Please use true/false here.

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

Stephen Boyd

unread,
Apr 20, 2015, 6:40:06 PM4/20/15
to
On 04/12/15 04:38, Heiko Stübner 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 <he...@sntech.de>
> ---
> drivers/clk/clk.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index a9fa5ab..b977701 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2272,6 +2272,17 @@ 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;
> +
> + if (!clk->core)
> + return false;

Looks like a useless check. clk->core better be there.

> +
> + return clk->core->orphan;
> +}
> +
> /**
> * __clk_init - initialize the data structures in a struct clk
> * @dev: device initializing this clk, placeholder for now
> @@ -3009,6 +3020,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);
>

Otherwise looks good. We really need to test it on lots of platforms though.

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

Heiko Stuebner

unread,
Apr 22, 2015, 5:00:05 PM4/22/15
to
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 <he...@sntech.de>
Cc: Boris Brezillon <boris.b...@free-electrons.com>
Cc: Alex Elder <el...@linaro.org>
Cc: Alexandre Belloni <alexandr...@free-electrons.com>
Cc: Stephen Warren <swa...@wwwdotorg.org>
Cc: Max Filippov <jcmv...@gmail.com>
Cc: ker...@pengutronix.de
Cc: Zhangfei Gao <zhangf...@linaro.org>
Cc: Santosh Shilimkar <ssan...@kernel.org>
Cc: Chao Xie <chao...@marvell.com>
Cc: Jason Cooper <ja...@lakedaemon.net>
Cc: Stefan Wahren <stefan...@i2se.com>
Cc: Andrew Bresticker <abre...@chromium.org>
Cc: Robert Jarzmik <robert....@free.fr>
Cc: Georgi Djakov <georgi...@linaro.org>
Cc: Sylwester Nawrocki <s.naw...@samsung.com>
Cc: Geert Uytterhoeven <geert+...@glider.be>
Cc: Barry Song <bao...@kernel.org>
Cc: Dinh Nguyen <ding...@opensource.altera.com>
Cc: Viresh Kumar <viresh...@gmail.com>
Cc: Gabriel FERNANDEZ <gabriel....@st.com>
Cc: emi...@elopez.com.ar
Cc: Peter De Schrijver <pdesch...@nvidia.com>
Cc: Tero Kristo <t-kr...@ti.com>
Cc: Ulf Hansson <ulf.h...@linaro.org>
Cc: Pawel Moll <pawel...@arm.com>
Cc: Michal Simek <michal...@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

Heiko Stuebner

unread,
Apr 22, 2015, 5:00:06 PM4/22/15
to
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 <sb...@codeaurora.org>
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

Heiko Stuebner

unread,
Apr 22, 2015, 5:00:06 PM4/22/15
to
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

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(-)

Stefan Wahren

unread,
Apr 25, 2015, 8:30:05 AM4/25/15
to
Hi Heiko,

> Heiko Stuebner <he...@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

Heiko Stübner

unread,
Apr 25, 2015, 9:50:05 AM4/25/15
to
Hi Stefan,

Am Samstag, 25. April 2015, 14:23:39 schrieb Stefan Wahren:
> > Heiko Stuebner <he...@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 ;-)

Robert Jarzmik

unread,
Apr 26, 2015, 4:00:06 PM4/26/15
to
Heiko Stuebner <he...@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

Stephen Boyd

unread,
Apr 30, 2015, 7:30:06 PM4/30/15
to
Applied to clk-next.

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

Stephen Boyd

unread,
Apr 30, 2015, 7:30:06 PM4/30/15
to
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

Stephen Boyd

unread,
Apr 30, 2015, 8:20:06 PM4/30/15
to
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

Heiko Stübner

unread,
May 1, 2015, 4:10:06 PM5/1/15
to
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 <he...@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 <he...@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

Stephen Boyd

unread,
May 1, 2015, 5:00:06 PM5/1/15
to
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

Heiko Stübner

unread,
May 1, 2015, 6:10:06 PM5/1/15
to
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 <he...@sntech.de>
Reviewed-by: Heiko Stuebner <he...@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?

Stephen Boyd

unread,
May 1, 2015, 7:50:05 PM5/1/15
to
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 <he...@sntech.de>
> Reviewed-by: Heiko Stuebner <he...@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

Tero Kristo

unread,
May 7, 2015, 4:30:07 AM5/7/15
to
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 <he...@sntech.de>
>> Reviewed-by: Heiko Stuebner <he...@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-kr...@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.
>

--

Kevin Hilman

unread,
May 7, 2015, 11:20:07 AM5/7/15
to
On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sb...@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 <he...@sntech.de>
>> Reviewed-by: Heiko Stuebner <he...@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/

Stephen Boyd

unread,
May 7, 2015, 2:20:05 PM5/7/15
to
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

Stephen Boyd

unread,
May 7, 2015, 5:10:06 PM5/7/15
to
On 05/07/15 08:17, Kevin Hilman wrote:
> On Fri, May 1, 2015 at 4:40 PM, Stephen Boyd <sb...@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 <he...@sntech.de>
>>> Reviewed-by: Heiko Stuebner <he...@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

Kevin Hilman

unread,
May 7, 2015, 8:30:05 PM5/7/15
to
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

Stephen Boyd

unread,
May 8, 2015, 3:00:04 AM5/8/15
to
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

Sascha Hauer

unread,
May 8, 2015, 4:20:05 AM5/8/15
to
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 |

Heiko Stübner

unread,
May 8, 2015, 5:40:06 AM5/8/15
to
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.

Sascha Hauer

unread,
May 8, 2015, 6:00:07 AM5/8/15
to
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 |

Maxime Ripard

unread,
May 8, 2015, 6:10:04 AM5/8/15
to
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
signature.asc

Tero Kristo

unread,
May 8, 2015, 7:50:05 AM5/8/15
to
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

Stephen Boyd

unread,
May 12, 2015, 6:40:06 PM5/12/15
to
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.

Maxime Ripard

unread,
May 13, 2015, 9:10:05 AM5/13/15
to
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...
signature.asc

Kevin Hilman

unread,
May 13, 2015, 10:40:07 AM5/13/15
to
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

Maxime Ripard

unread,
May 13, 2015, 4:20:08 PM5/13/15
to
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...
signature.asc

Kevin Hilman

unread,
May 13, 2015, 4:50:05 PM5/13/15
to
I'm booting an initramfs too.

Maxime Ripard

unread,
May 13, 2015, 5:00:07 PM5/13/15
to
Then I don't know :)
signature.asc

Heiko Stübner

unread,
Jul 27, 2015, 5:00:07 AM7/27/15
to
Hi Maxime, Stephen,
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

Maxime Ripard

unread,
Jul 30, 2015, 6:20:07 AM7/30/15
to
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).
signature.asc

Stephen Boyd

unread,
Aug 11, 2015, 6:40:06 PM8/11/15
to
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

Heiko Stübner

unread,
Aug 12, 2015, 4:30:07 AM8/12/15
to
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.
0 new messages