* [PATCH 00/16] clk: a collection of various clean-ups and improvements
@ 2015-11-30 7:34 Masahiro Yamada
2015-11-30 7:34 ` [PATCH 01/16] clk: remove unused first argument of __clk_init() Masahiro Yamada
` (15 more replies)
0 siblings, 16 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
Many refactoring, with detection of circular parent looping.
Masahiro Yamada (16):
clk: remove unused first argument of __clk_init()
clk: change the argument of __clk_init() into pointer to clk_core
clk: rename __clk_init() into __clk_core_init()
clk: remove unnecessary !core->parents conditional
clk: change sizeof(struct clk *) to sizeof(*core->parents)
clk: move core->parents allocation to clk_register()
clk: simplify clk_core_get_parent_by_index()
clk: drop the initial core->parents look-ups from __clk_core_init()
clk: replace pr_warn() with pr_err() for fatal cases
clk: move checking .git_parent to __clk_core_init()
clk: simplify __clk_init_parent()
clk: avoid circular clock topology
clk: walk the orphan clock list more simply
clk: make sure parent is not NULL in clk_fetch_parent_index()
clk: simplify clk_fetch_parent_index() function
clk: slightly optimize clk_core_set_parent()
drivers/clk/clk.c | 214 ++++++++++++++++++++++--------------------------------
1 file changed, 85 insertions(+), 129 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 01/16] clk: remove unused first argument of __clk_init()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 02/16] clk: change the argument of __clk_init() into pointer to clk_core Masahiro Yamada
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
The "struct device *dev" is not used at all in this function.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 6d9cd05..65530e9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2286,13 +2286,12 @@ static inline void clk_debug_unregister(struct clk_core *core)
/**
* __clk_init - initialize the data structures in a struct clk
- * @dev: device initializing this clk, placeholder for now
* @clk: clk being initialized
*
* Initializes the lists in struct clk_core, queries the hardware for the
* parent and rate and sets them both.
*/
-static int __clk_init(struct device *dev, struct clk *clk_user)
+static int __clk_init(struct clk *clk_user)
{
int i, ret = 0;
struct clk_core *orphan;
@@ -2575,7 +2574,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
goto fail_parent_names_copy;
}
- ret = __clk_init(dev, hw->clk);
+ ret = __clk_init(hw->clk);
if (!ret)
return hw->clk;
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 02/16] clk: change the argument of __clk_init() into pointer to clk_core
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
2015-11-30 7:34 ` [PATCH 01/16] clk: remove unused first argument of __clk_init() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 03/16] clk: rename __clk_init() into __clk_core_init() Masahiro Yamada
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
The argument clk_user is used only for the clk_user->core. The rest
of this function only takes care of clk_core.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 65530e9..8c8ba91 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2285,25 +2285,22 @@ static inline void clk_debug_unregister(struct clk_core *core)
#endif
/**
- * __clk_init - initialize the data structures in a struct clk
- * @clk: clk being initialized
+ * __clk_init - initialize the data structures in a struct clk_core
+ * @core: clk_core being initialized
*
* Initializes the lists in struct clk_core, queries the hardware for the
* parent and rate and sets them both.
*/
-static int __clk_init(struct clk *clk_user)
+static int __clk_init(struct clk_core *core)
{
int i, ret = 0;
struct clk_core *orphan;
struct hlist_node *tmp2;
- struct clk_core *core;
unsigned long rate;
- if (!clk_user)
+ if (!core)
return -EINVAL;
- core = clk_user->core;
-
clk_prepare_lock();
/* check to see if a clock with this name is already registered */
@@ -2574,7 +2571,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
goto fail_parent_names_copy;
}
- ret = __clk_init(hw->clk);
+ ret = __clk_init(core);
if (!ret)
return hw->clk;
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 03/16] clk: rename __clk_init() into __clk_core_init()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
2015-11-30 7:34 ` [PATCH 01/16] clk: remove unused first argument of __clk_init() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 02/16] clk: change the argument of __clk_init() into pointer to clk_core Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 04/16] clk: remove unnecessary !core->parents conditional Masahiro Yamada
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
Now this function takes clk_core as its argument. __clk_core_init()
would be more suitable for the name of this function.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8c8ba91..36373d3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2285,13 +2285,13 @@ static inline void clk_debug_unregister(struct clk_core *core)
#endif
/**
- * __clk_init - initialize the data structures in a struct clk_core
+ * __clk_core_init - initialize the data structures in a struct clk_core
* @core: clk_core being initialized
*
* Initializes the lists in struct clk_core, queries the hardware for the
* parent and rate and sets them both.
*/
-static int __clk_init(struct clk_core *core)
+static int __clk_core_init(struct clk_core *core)
{
int i, ret = 0;
struct clk_core *orphan;
@@ -2571,7 +2571,7 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
goto fail_parent_names_copy;
}
- ret = __clk_init(core);
+ ret = __clk_core_init(core);
if (!ret)
return hw->clk;
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 04/16] clk: remove unnecessary !core->parents conditional
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (2 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 03/16] clk: rename __clk_init() into __clk_core_init() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 05/16] clk: change sizeof(struct clk *) to sizeof(*core->parents) Masahiro Yamada
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
This if-block has been here since the introduction of the common
clock framework. Now no clock drivers are statically initialized.
core->parent is always NULL at this point. Drop the redundant
check and the confusing comment.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 36373d3..66c6f34 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2348,11 +2348,8 @@ static int __clk_core_init(struct clk_core *core)
* in to clk_init during early boot; thus any access to core->parents[]
* must always check for a NULL pointer and try to populate it if
* necessary.
- *
- * If core->parents is not NULL we skip this entire block. This allows
- * for clock drivers to statically initialize core->parents.
*/
- if (core->num_parents > 1 && !core->parents) {
+ if (core->num_parents > 1) {
core->parents = kcalloc(core->num_parents, sizeof(struct clk *),
GFP_KERNEL);
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 05/16] clk: change sizeof(struct clk *) to sizeof(*core->parents)
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (3 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 04/16] clk: remove unnecessary !core->parents conditional Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 06/16] clk: move core->parents allocation to clk_register() Masahiro Yamada
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
Now, the clock parent is not "struct clk *", but "struct clk_core *".
Of course, the size of a pointer is always same, but strictly speaking,
sizeof(struct clk *) should be sizeof(struct clk_core *) here.
This mismatch happened when we split the structure into struct clk
and struct clk_core. For the potential possibility of future renaming,
sizeof(*core->parents) would be better.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 66c6f34..0f80c69 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1069,7 +1069,7 @@ static int clk_fetch_parent_index(struct clk_core *core,
if (!core->parents) {
core->parents = kcalloc(core->num_parents,
- sizeof(struct clk *), GFP_KERNEL);
+ sizeof(*core->parents), GFP_KERNEL);
if (!core->parents)
return -ENOMEM;
}
@@ -1702,7 +1702,7 @@ static struct clk_core *__clk_init_parent(struct clk_core *core)
if (!core->parents)
core->parents =
- kcalloc(core->num_parents, sizeof(struct clk *),
+ kcalloc(core->num_parents, sizeof(*core->parents),
GFP_KERNEL);
ret = clk_core_get_parent_by_index(core, index);
@@ -2350,8 +2350,8 @@ static int __clk_core_init(struct clk_core *core)
* necessary.
*/
if (core->num_parents > 1) {
- core->parents = kcalloc(core->num_parents, sizeof(struct clk *),
- GFP_KERNEL);
+ core->parents = kcalloc(core->num_parents,
+ sizeof(*core->parents), GFP_KERNEL);
/*
* clk_core_lookup returns NULL for parents that have not been
* clk_init'd; thus any access to clk->parents[] must check
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 06/16] clk: move core->parents allocation to clk_register()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (4 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 05/16] clk: change sizeof(struct clk *) to sizeof(*core->parents) Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 07/16] clk: simplify clk_core_get_parent_by_index() Masahiro Yamada
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
Currently, __clk_core_init() allows failure of the kcalloc() for the
core->parents. So, clk_fetch_parent_index() and __clk_init_parent()
also try to allocate core->parents in case it has not been allocated
yet. Scattering memory allocation here and there makes things
complicated.
Like other clk_core members, allocate core->parents in clk_register()
and let it fail in case of memory shortage. If we cannot allocate
such a small piece of memory, the system is already insane. There is
no point to postpone the memory allocation.
Also, allocate core->parents regardless of core->num_parents. We want
it even if core->num_parents == 1 because clk_fetch_parent_index()
might be called against the clk_core with a single parent.
If core->num_parents == 0, core->parents is set to ZERO_SIZE_PTR. It
is harmless because no access happens to core->parents in such a case.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 51 +++++++++++++++++++--------------------------------
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0f80c69..e05084e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1067,13 +1067,6 @@ static int clk_fetch_parent_index(struct clk_core *core,
{
int i;
- if (!core->parents) {
- core->parents = kcalloc(core->num_parents,
- sizeof(*core->parents), GFP_KERNEL);
- if (!core->parents)
- return -ENOMEM;
- }
-
/*
* find index of new parent clock using cached parent ptrs,
* or if not yet cached, use string name comparison and cache
@@ -1700,11 +1693,6 @@ static struct clk_core *__clk_init_parent(struct clk_core *core)
index = core->ops->get_parent(core->hw);
- if (!core->parents)
- core->parents =
- kcalloc(core->num_parents, sizeof(*core->parents),
- GFP_KERNEL);
-
ret = clk_core_get_parent_by_index(core, index);
out:
@@ -2343,26 +2331,15 @@ static int __clk_core_init(struct clk_core *core)
__func__, core->name);
/*
- * Allocate an array of struct clk *'s to avoid unnecessary string
- * look-ups of clk's possible parents. This can fail for clocks passed
- * in to clk_init during early boot; thus any access to core->parents[]
- * must always check for a NULL pointer and try to populate it if
- * necessary.
+ * clk_core_lookup returns NULL for parents that have not been
+ * clk_init'd; thus any access to clk->parents[] must check
+ * for a NULL pointer. We can always perform lazy lookups for
+ * missing parents later on.
*/
- if (core->num_parents > 1) {
- core->parents = kcalloc(core->num_parents,
- sizeof(*core->parents), GFP_KERNEL);
- /*
- * clk_core_lookup returns NULL for parents that have not been
- * clk_init'd; thus any access to clk->parents[] must check
- * for a NULL pointer. We can always perform lazy lookups for
- * missing parents later on.
- */
- if (core->parents)
- for (i = 0; i < core->num_parents; i++)
- core->parents[i] =
- clk_core_lookup(core->parent_names[i]);
- }
+ if (core->parents)
+ for (i = 0; i < core->num_parents; i++)
+ core->parents[i] =
+ clk_core_lookup(core->parent_names[i]);
core->parent = __clk_init_parent(core);
@@ -2560,12 +2537,20 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
}
}
+ /* avoid unnecessary string look-ups of clk_core's possible parents. */
+ core->parents = kcalloc(core->num_parents, sizeof(*core->parents),
+ GFP_KERNEL);
+ if (!core->parents) {
+ ret = -ENOMEM;
+ goto fail_parents;
+ };
+
INIT_HLIST_HEAD(&core->clks);
hw->clk = __clk_create_clk(hw, NULL, NULL);
if (IS_ERR(hw->clk)) {
ret = PTR_ERR(hw->clk);
- goto fail_parent_names_copy;
+ goto fail_parents;
}
ret = __clk_core_init(core);
@@ -2575,6 +2560,8 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
__clk_free_clk(hw->clk);
hw->clk = NULL;
+fail_parents:
+ kfree(core->parents);
fail_parent_names_copy:
while (--i >= 0)
kfree_const(core->parent_names[i]);
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 07/16] clk: simplify clk_core_get_parent_by_index()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (5 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 06/16] clk: move core->parents allocation to clk_register() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 08/16] clk: drop the initial core->parents look-ups from __clk_core_init() Masahiro Yamada
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
Drop the "if (!core->parents)" case and refactor the function a bit
because core->parents is always allocated. (Strictly speaking, it is
ZERO_SIZE_PTR if core->num_parents == 0, but such a case is omitted
by the if-conditional above.)
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e05084e..f2758c4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -350,13 +350,12 @@ static struct clk_core *clk_core_get_parent_by_index(struct clk_core *core,
{
if (!core || index >= core->num_parents)
return NULL;
- else if (!core->parents)
- return clk_core_lookup(core->parent_names[index]);
- else if (!core->parents[index])
- return core->parents[index] =
- clk_core_lookup(core->parent_names[index]);
- else
- return core->parents[index];
+
+ if (!core->parents[index])
+ core->parents[index] =
+ clk_core_lookup(core->parent_names[index]);
+
+ return core->parents[index];
}
struct clk_hw *
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 08/16] clk: drop the initial core->parents look-ups from __clk_core_init()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (6 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 07/16] clk: simplify clk_core_get_parent_by_index() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 09/16] clk: replace pr_warn() with pr_err() for fatal cases Masahiro Yamada
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
The core->parents is a cache to save expensive clock parent look-ups.
It will be filled as needed later. We do not have to do it here.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f2758c4..43fb329 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2329,17 +2329,6 @@ static int __clk_core_init(struct clk_core *core)
"%s: invalid NULL in %s's .parent_names\n",
__func__, core->name);
- /*
- * clk_core_lookup returns NULL for parents that have not been
- * clk_init'd; thus any access to clk->parents[] must check
- * for a NULL pointer. We can always perform lazy lookups for
- * missing parents later on.
- */
- if (core->parents)
- for (i = 0; i < core->num_parents; i++)
- core->parents[i] =
- clk_core_lookup(core->parent_names[i]);
-
core->parent = __clk_init_parent(core);
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 09/16] clk: replace pr_warn() with pr_err() for fatal cases
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (7 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 08/16] clk: drop the initial core->parents look-ups from __clk_core_init() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 10/16] clk: move checking .git_parent to __clk_core_init() Masahiro Yamada
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
These three cases let clk_register() fail. They should be considered
as error messages.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 43fb329..cd96442 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2302,22 +2302,22 @@ static int __clk_core_init(struct clk_core *core)
if (core->ops->set_rate &&
!((core->ops->round_rate || core->ops->determine_rate) &&
core->ops->recalc_rate)) {
- pr_warning("%s: %s must implement .round_rate or .determine_rate in addition to .recalc_rate\n",
- __func__, core->name);
+ pr_err("%s: %s must implement .round_rate or .determine_rate in addition to .recalc_rate\n",
+ __func__, core->name);
ret = -EINVAL;
goto out;
}
if (core->ops->set_parent && !core->ops->get_parent) {
- pr_warning("%s: %s must implement .get_parent & .set_parent\n",
- __func__, core->name);
+ pr_err("%s: %s must implement .get_parent & .set_parent\n",
+ __func__, core->name);
ret = -EINVAL;
goto out;
}
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
- pr_warn("%s: %s must implement .set_parent & .set_rate\n",
+ pr_err("%s: %s must implement .set_parent & .set_rate\n",
__func__, core->name);
ret = -EINVAL;
goto out;
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 10/16] clk: move checking .git_parent to __clk_core_init()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (8 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 09/16] clk: replace pr_warn() with pr_err() for fatal cases Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 11/16] clk: simplify __clk_init_parent() Masahiro Yamada
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
The .git_parent is mandatory for multi-parent clocks. Move the check
to __clk_core_init(), like other callback checkings.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index cd96442..486f6d4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1677,13 +1677,6 @@ static struct clk_core *__clk_init_parent(struct clk_core *core)
goto out;
}
- if (!core->ops->get_parent) {
- WARN(!core->ops->get_parent,
- "%s: multi-parent clocks must implement .get_parent\n",
- __func__);
- goto out;
- }
-
/*
* Do our best to cache parent clocks in core->parents. This prevents
* unnecessary and expensive lookups. We don't set core->parent here;
@@ -2315,6 +2308,11 @@ static int __clk_core_init(struct clk_core *core)
goto out;
}
+ if (core->num_parents > 1 && !core->ops->get_parent) {
+ pr_err("%s: %s must implement .get_parent as it has multi parents\n",
+ __func__, core->name);
+ }
+
if (core->ops->set_rate_and_parent &&
!(core->ops->set_parent && core->ops->set_rate)) {
pr_err("%s: %s must implement .set_parent & .set_rate\n",
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 11/16] clk: simplify __clk_init_parent()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (9 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 10/16] clk: move checking .git_parent to __clk_core_init() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 12/16] clk: avoid circular clock topology Masahiro Yamada
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
The translation from the index into clk_core is done by
clk_core_get_parent_by_index(). The if-block for num_parents == 1
case is duplicating the code in the clk_core_get_parent_by_index().
Drop the "if (num_parents == 1)" from the special case. Instead,
set the index to zero if .get_parent() is missing.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 38 ++++----------------------------------
1 file changed, 4 insertions(+), 34 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 486f6d4..ef6fedb 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1651,44 +1651,14 @@ struct clk *clk_get_parent(struct clk *clk)
}
EXPORT_SYMBOL_GPL(clk_get_parent);
-/*
- * .get_parent is mandatory for clocks with multiple possible parents. It is
- * optional for single-parent clocks. Always call .get_parent if it is
- * available and WARN if it is missing for multi-parent clocks.
- *
- * For single-parent clocks without .get_parent, first check to see if the
- * .parents array exists, and if so use it to avoid an expensive tree
- * traversal. If .parents does not exist then walk the tree.
- */
static struct clk_core *__clk_init_parent(struct clk_core *core)
{
- struct clk_core *ret = NULL;
- u8 index;
-
- /* handle the trivial cases */
+ u8 index = 0;
- if (!core->num_parents)
- goto out;
-
- if (core->num_parents == 1) {
- if (IS_ERR_OR_NULL(core->parent))
- core->parent = clk_core_lookup(core->parent_names[0]);
- ret = core->parent;
- goto out;
- }
+ if (core->ops->get_parent)
+ index = core->ops->get_parent(core->hw);
- /*
- * Do our best to cache parent clocks in core->parents. This prevents
- * unnecessary and expensive lookups. We don't set core->parent here;
- * that is done by the calling function.
- */
-
- index = core->ops->get_parent(core->hw);
-
- ret = clk_core_get_parent_by_index(core, index);
-
-out:
- return ret;
+ return clk_core_get_parent_by_index(core, index);
}
static void clk_core_reparent(struct clk_core *core,
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 12/16] clk: avoid circular clock topology
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (10 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 11/16] clk: simplify __clk_init_parent() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 13/16] clk: walk the orphan clock list more simply Masahiro Yamada
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
Currently, clk_register() never checks a circular parent looping,
but clock providers could register such an insane clock topology.
For example, "clk_a" could have "clk_b" as a parent, and vice versa.
In this case, clk_core_reparent() creates a circular parent list
and __clk_recalc_accuracies() calls itself recursively forever.
The core infrastructure should be kind enough to bail out, showing
an appropriate error message in such a case. This helps to easily
find a bug in clock providers. (uh, I made such a silly mistake
when I was implementing my clock providers first. I was upset
because the kernel did not respond, without any error message.)
This commit adds a new helper function, __clk_is_ancestor(). It
returns true if the second argument is a possible ancestor of the
first one. If a clock core is a possible ancestor of itself, it
would make a loop when it were registered. That should be detected
as an error.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 40 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ef6fedb..a1d046c 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2235,6 +2235,38 @@ static inline void clk_debug_unregister(struct clk_core *core)
#endif
/**
+ * __clk_is_ancestor - check if a clk_core is a possible ancestor of another
+ * @core: clock core
+ * @ancestor: ancestor clock core
+ *
+ * Returns true if there is a possibility that @ancestor can be an ancestor
+ * of @core, false otherwise.
+ *
+ * This function can be used against @core or @ancestor that has not been
+ * registered yet.
+ */
+static bool __clk_is_ancestor(struct clk_core *core, struct clk_core *ancestor)
+{
+ struct clk_core *parent;
+ int i;
+
+ for (i = 0; i < core->num_parents; i++) {
+ parent = clk_core_get_parent_by_index(core, i);
+ /*
+ * If ancestor has not been added to clk_{root,orphan}_list
+ * yet, clk_core_lookup() cannot find it. If parent is NULL,
+ * compare the name strings, too.
+ */
+ if ((parent && (parent == ancestor ||
+ __clk_is_ancestor(parent, ancestor))) ||
+ (!parent && !strcmp(core->parent_names[i], ancestor->name)))
+ return true;
+ }
+
+ return false;
+}
+
+/**
* __clk_core_init - initialize the data structures in a struct clk_core
* @core: clk_core being initialized
*
@@ -2297,6 +2329,14 @@ static int __clk_core_init(struct clk_core *core)
"%s: invalid NULL in %s's .parent_names\n",
__func__, core->name);
+ /* If core is an ancestor of itself, it would make a loop. */
+ if (__clk_is_ancestor(core, core)) {
+ pr_err("%s: %s would create circular parent\n", __func__,
+ core->name);
+ ret = -EINVAL;
+ goto out;
+ }
+
core->parent = __clk_init_parent(core);
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 13/16] clk: walk the orphan clock list more simply
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (11 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 12/16] clk: avoid circular clock topology Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index() Masahiro Yamada
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
This loop can be much simpler. If a new parent is available for
orphan clocks, __clk_init_parent(orphan) can detect it.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 21 ++++++---------------
1 file changed, 6 insertions(+), 15 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index a1d046c..e3c6559 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2402,24 +2402,15 @@ static int __clk_core_init(struct clk_core *core)
core->rate = core->req_rate = rate;
/*
- * walk the list of orphan clocks and reparent any that are children of
- * this clock
+ * walk the list of orphan clocks and reparent any that newly finds a
+ * parent.
*/
hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
- if (orphan->num_parents && orphan->ops->get_parent) {
- i = orphan->ops->get_parent(orphan->hw);
- if (i >= 0 && i < orphan->num_parents &&
- !strcmp(core->name, orphan->parent_names[i]))
- clk_core_reparent(orphan, core);
- continue;
- }
+ struct clk_core *parent = __clk_init_parent(orphan);
- for (i = 0; i < orphan->num_parents; i++)
- if (!strcmp(core->name, orphan->parent_names[i])) {
- clk_core_reparent(orphan, core);
- break;
- }
- }
+ if (parent)
+ clk_core_reparent(orphan, parent);
+ }
/*
* optional platform-specific magic
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (12 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 13/16] clk: walk the orphan clock list more simply Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 15/16] clk: simplify clk_fetch_parent_index() function Masahiro Yamada
2015-11-30 7:34 ` [PATCH 16/16] clk: slightly optimize clk_core_set_parent() Masahiro Yamada
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
If parent is given with NULL, clk_fetch_parent_index() could return
a positive index value.
Currently, parent is checked by the callers of this function, but
it would be safer to do it in this function.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e3c6559..50b9b3d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1066,6 +1066,9 @@ static int clk_fetch_parent_index(struct clk_core *core,
{
int i;
+ if (parent)
+ return -EINVAL;
+
/*
* find index of new parent clock using cached parent ptrs,
* or if not yet cached, use string name comparison and cache
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 15/16] clk: simplify clk_fetch_parent_index() function
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (13 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index() Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
2015-11-30 7:34 ` [PATCH 16/16] clk: slightly optimize clk_core_set_parent() Masahiro Yamada
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
The clk_core_get_parent_by_index can be used as a helper function
to simplify the implementation of clk_fetch_parent_index().
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 50b9b3d..8a6a33b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1069,23 +1069,9 @@ static int clk_fetch_parent_index(struct clk_core *core,
if (parent)
return -EINVAL;
- /*
- * find index of new parent clock using cached parent ptrs,
- * or if not yet cached, use string name comparison and cache
- * them now to avoid future calls to clk_core_lookup.
- */
- for (i = 0; i < core->num_parents; i++) {
- if (core->parents[i] == parent)
- return i;
-
- if (core->parents[i])
- continue;
-
- if (!strcmp(core->parent_names[i], parent->name)) {
- core->parents[i] = clk_core_lookup(parent->name);
+ for (i = 0; i < core->num_parents; i++)
+ if (clk_core_get_parent_by_index(core, i) == parent)
return i;
- }
- }
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 16/16] clk: slightly optimize clk_core_set_parent()
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
` (14 preceding siblings ...)
2015-11-30 7:34 ` [PATCH 15/16] clk: simplify clk_fetch_parent_index() function Masahiro Yamada
@ 2015-11-30 7:34 ` Masahiro Yamada
15 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2015-11-30 7:34 UTC (permalink / raw)
To: linux-clk; +Cc: Masahiro Yamada, Stephen Boyd, Michael Turquette, linux-kernel
If clk_fetch_parent_index() fails, p_rate is unused. Move the
assignment after the error checking.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/clk/clk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8a6a33b..479a754 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1730,13 +1730,13 @@ static int clk_core_set_parent(struct clk_core *core, struct clk_core *parent)
/* try finding the new parent index */
if (parent) {
p_index = clk_fetch_parent_index(core, parent);
- p_rate = parent->rate;
if (p_index < 0) {
pr_debug("%s: clk %s can not be parent of clk %s\n",
__func__, parent->name, core->name);
ret = p_index;
goto out;
}
+ p_rate = parent->rate;
}
/* propagate PRE_RATE_CHANGE notifications */
--
1.9.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-11-30 7:37 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-30 7:34 [PATCH 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada
2015-11-30 7:34 ` [PATCH 01/16] clk: remove unused first argument of __clk_init() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 02/16] clk: change the argument of __clk_init() into pointer to clk_core Masahiro Yamada
2015-11-30 7:34 ` [PATCH 03/16] clk: rename __clk_init() into __clk_core_init() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 04/16] clk: remove unnecessary !core->parents conditional Masahiro Yamada
2015-11-30 7:34 ` [PATCH 05/16] clk: change sizeof(struct clk *) to sizeof(*core->parents) Masahiro Yamada
2015-11-30 7:34 ` [PATCH 06/16] clk: move core->parents allocation to clk_register() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 07/16] clk: simplify clk_core_get_parent_by_index() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 08/16] clk: drop the initial core->parents look-ups from __clk_core_init() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 09/16] clk: replace pr_warn() with pr_err() for fatal cases Masahiro Yamada
2015-11-30 7:34 ` [PATCH 10/16] clk: move checking .git_parent to __clk_core_init() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 11/16] clk: simplify __clk_init_parent() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 12/16] clk: avoid circular clock topology Masahiro Yamada
2015-11-30 7:34 ` [PATCH 13/16] clk: walk the orphan clock list more simply Masahiro Yamada
2015-11-30 7:34 ` [PATCH 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index() Masahiro Yamada
2015-11-30 7:34 ` [PATCH 15/16] clk: simplify clk_fetch_parent_index() function Masahiro Yamada
2015-11-30 7:34 ` [PATCH 16/16] clk: slightly optimize clk_core_set_parent() Masahiro Yamada
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.