Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1298586 > unrolled thread

[RESEND PATCH v2 00/16] clk: a collection of various clean-ups and improvements

Back to article view | Back to linux.kernel

Contents

  [RESEND PATCH v2 00/16] clk: a collection of various clean-ups and improvements Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 15/16] clk: simplify clk_fetch_parent_index() function Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 09/16] clk: replace pr_warn() with pr_err() for fatal cases Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 04/16] clk: remove unnecessary !core->parents conditional Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 02/16] clk: change the argument of __clk_init() into pointer to clk_core Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 05/16] clk: change sizeof(struct clk *) to sizeof(*core->parents) Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 11/16] clk: simplify __clk_init_parent() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 12/16] clk: avoid circular clock topology Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 16/16] clk: slightly optimize clk_core_set_parent() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 06/16] clk: move core->parents allocation to clk_register() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 03/16] clk: rename __clk_init() into __clk_core_init() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 10/16] clk: move checking .git_parent to __clk_core_init() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 08/16] clk: drop the initial core->parents look-ups from __clk_core_init() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100
    [RESEND PATCH v2 07/16] clk: simplify clk_core_get_parent_by_index() Masahiro Yamada <yamada.masahiro@socionext.com> - 2015-12-28 11:30 +0100

#1298586 — [RESEND PATCH v2 00/16] clk: a collection of various clean-ups and improvements

Many refactoring, with detection of circular parent looping.


Changes in v2:
  - Fix a bug.  Return -EINVAL when parent _is_ NULL.

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

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

[toc] | [next] | [standalone]

#1298587 — [RESEND PATCH v2 15/16] clk: simplify clk_fetch_parent_index() function

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

Changes in v2: None

 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 151cbe8..5b9aec0 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

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

[toc] | [prev] | [next] | [standalone]

#1298588 — [RESEND PATCH v2 09/16] clk: replace pr_warn() with pr_err() for fatal cases

These three cases let clk_register() fail.  They should be considered
as error messages.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v2: None

 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 3215b2b..e6e10f5 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

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

[toc] | [prev] | [next] | [standalone]

#1298589 — [RESEND PATCH v2 04/16] clk: remove unnecessary !core->parents conditional

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

Changes in v2: None

 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 41179f3..30c0bbc 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

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

[toc] | [prev] | [next] | [standalone]

#1298590 — [RESEND PATCH v2 02/16] clk: change the argument of __clk_init() into pointer to clk_core

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

Changes in v2: None

 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 897e5ae..623838f 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

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

[toc] | [prev] | [next] | [standalone]

#1298591 — [RESEND PATCH v2 14/16] clk: make sure parent is not NULL in clk_fetch_parent_index()

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

Changes in v2:
  - Fix a bug.  Return -EINVAL when parent _is_ NULL.

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

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efd093a..151cbe8 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

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

[toc] | [prev] | [next] | [standalone]

#1298593 — [RESEND PATCH v2 05/16] clk: change sizeof(struct clk *) to sizeof(*core->parents)

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

Changes in v2: None

 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 30c0bbc..98745c3 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

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

[toc] | [prev] | [next] | [standalone]

#1298594 — [RESEND PATCH v2 11/16] clk: simplify __clk_init_parent()

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

Changes in v2: None

 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 4ad0a36..03b87d8 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

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

[toc] | [prev] | [next] | [standalone]

#1298595 — [RESEND PATCH v2 12/16] clk: avoid circular clock topology

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

Changes in v2: None

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

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 03b87d8..a4f66fe 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

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

[toc] | [prev] | [next] | [standalone]

#1298596 — [RESEND PATCH v2 16/16] clk: slightly optimize clk_core_set_parent()

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

Changes in v2: None

 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 5b9aec0..e81301b 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

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

[toc] | [prev] | [next] | [standalone]

#1298597 — [RESEND PATCH v2 06/16] clk: move core->parents allocation to clk_register()

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

Changes in v2: None

 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 98745c3..b5be02a 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

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

[toc] | [prev] | [next] | [standalone]

#1298598 — [RESEND PATCH v2 03/16] clk: rename __clk_init() into __clk_core_init()

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

Changes in v2: None

 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 623838f..41179f3 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

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

[toc] | [prev] | [next] | [standalone]

#1298599 — [RESEND PATCH v2 10/16] clk: move checking .git_parent to __clk_core_init()

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

Changes in v2: None

 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 e6e10f5..4ad0a36 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

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

[toc] | [prev] | [next] | [standalone]

#1298600 — [RESEND PATCH v2 08/16] clk: drop the initial core->parents look-ups from __clk_core_init()

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

Changes in v2: None

 drivers/clk/clk.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index ca7849a..3215b2b 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

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

[toc] | [prev] | [next] | [standalone]

#1298601 — [RESEND PATCH v2 07/16] clk: simplify clk_core_get_parent_by_index()

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

Changes in v2: None

 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 b5be02a..ca7849a 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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web