[PATCH 0/2] some minor clk fixes

Mike,

These are a couple of fixes I found while testing my
OMAP clock port.

regards,
Rajendra

Rajendra Nayak (2):
  clk: cache parent clocks only for muxes
  clk: Allow late cache allocation for clk->parents

 drivers/clk/clk.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

caching parent clocks makes sense only when a clock has more
than one parent (mux clocks).
Avoid doing this for every other clock.

Signed-off-by: Rajendra Nayak <redacted>
---
 drivers/clk/clk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 687b00d..40568e9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1229,7 +1229,7 @@ int __clk_init(struct device *dev, struct clk *clk)
 	 * If clk->parents is not NULL we skip this entire block.  This allows
 	 * for clock drivers to statically initialize clk->parents.
 	 */
-	if (clk->num_parents && !clk->parents) {
+	if ((clk->num_parents > 1) && !clk->parents) {
 		clk->parents = kmalloc((sizeof(struct clk*) * clk->num_parents),
 				GFP_KERNEL);
 		/*

-- 
1.7.1

Parent clocks for muxes are cached in clk->parents to
avoid frequent lookups, however the cache allocation happens
only during clock registeration and later clk_set_parent()
assumes a cache space available and allocated.

This is not entirely true for platforms which do early clock
registerations wherein the cache allocation using kzalloc
could fail during clock registeration.

Allow cache allocation to happen later as part of clk_set_parent()
to help such cases and avoid crashes assuming a cache being
available.

While here also replace existing kmalloc() with kzalloc()
in the file.

Signed-off-by: Rajendra Nayak <redacted>
---
 drivers/clk/clk.c |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 40568e9..0d67745 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -999,7 +999,7 @@ static struct clk *__clk_init_parent(struct clk *clk)
 
 	if (!clk->parents)
 		clk->parents =
-			kmalloc((sizeof(struct clk*) * clk->num_parents),
+			kzalloc((sizeof(struct clk*) * clk->num_parents),
 					GFP_KERNEL);
 
 	if (!clk->parents)

@@ -1065,9 +1065,13 @@ static int __clk_set_parent(struct clk *clk, struct clk *parent)
 	old_parent = clk->parent;
 
 	/* find index of new parent clock using cached parent ptrs */
-	for (i = 0; i < clk->num_parents; i++)
-		if (clk->parents[i] == parent)
-			break;
+	if (clk->parents)
+		for (i = 0; i < clk->num_parents; i++)
+			if (clk->parents[i] == parent)
+				break;
+	else
+		clk->parents = kzalloc((sizeof(struct clk*) * clk->num_parents),
+								GFP_KERNEL);
 
 	/*
 	 * find index of new parent clock using string name comparison

@@ -1076,7 +1080,8 @@ static int __clk_set_parent(struct clk *clk, struct clk *parent)
 	if (i == clk->num_parents)
 		for (i = 0; i < clk->num_parents; i++)
 			if (!strcmp(clk->parent_names[i], parent->name)) {
-				clk->parents[i] = __clk_lookup(parent->name);
+				if (clk->parents)
+					clk->parents[i] = __clk_lookup(parent->name);
 				break;
 			}
 

@@ -1230,7 +1235,7 @@ int __clk_init(struct device *dev, struct clk *clk)
 	 * for clock drivers to statically initialize clk->parents.
 	 */
 	if ((clk->num_parents > 1) && !clk->parents) {
-		clk->parents = kmalloc((sizeof(struct clk*) * clk->num_parents),
+		clk->parents = kzalloc((sizeof(struct clk*) * clk->num_parents),
 				GFP_KERNEL);
 		/*
 		 * __clk_lookup returns NULL for parents that have not been

-- 
1.7.1

On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:

You don't need the additional parens here.  Please learn the C precedence
rules.  Additional unnecessary parens can make expressions much harder
to read.

On 20120606-11:47, Russell King - ARM Linux wrote:

On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:

quoted

caching parent clocks makes sense only when a clock has more
than one parent (mux clocks).
Avoid doing this for every other clock.

Signed-off-by: Rajendra Nayak <redacted>
---
 drivers/clk/clk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 687b00d..40568e9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1229,7 +1229,7 @@ int __clk_init(struct device *dev, struct clk *clk)
 	 * If clk->parents is not NULL we skip this entire block.  This allows
 	 * for clock drivers to statically initialize clk->parents.
 	 */
-	if (clk->num_parents && !clk->parents) {
+	if ((clk->num_parents > 1) && !clk->parents) {

You don't need the additional parens here.  Please learn the C precedence
rules.  Additional unnecessary parens can make expressions much harder
to read.

Rajendra,

I've taken this patch into clk-next for testing.  I've fixed up the
extra parens locally, so no need for a resend.

Regards,
Mike

On 20120606-14:41, Rajendra Nayak wrote:

Parent clocks for muxes are cached in clk->parents to
avoid frequent lookups, however the cache allocation happens
only during clock registeration and later clk_set_parent()
assumes a cache space available and allocated.

This is not entirely true for platforms which do early clock
registerations wherein the cache allocation using kzalloc
could fail during clock registeration.

Allow cache allocation to happen later as part of clk_set_parent()
to help such cases and avoid crashes assuming a cache being
available.

While here also replace existing kmalloc() with kzalloc()
in the file.

Signed-off-by: Rajendra Nayak <redacted>

I've taken this patch into clk-next for testing.

Thanks,
Mike

On Wednesday 13 June 2012 12:34 AM, Mike Turquette wrote:

On 20120606-11:47, Russell King - ARM Linux wrote:

quoted

On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:

quoted

caching parent clocks makes sense only when a clock has more
than one parent (mux clocks).
Avoid doing this for every other clock.

Signed-off-by: Rajendra Nayak<redacted>
---
  drivers/clk/clk.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 687b00d..40568e9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -1229,7 +1229,7 @@ int __clk_init(struct device *dev, struct clk *clk)
  	 * If clk->parents is not NULL we skip this entire block.  This allows
  	 * for clock drivers to statically initialize clk->parents.
  	 */
-	if (clk->num_parents&&  !clk->parents) {
+	if ((clk->num_parents>  1)&&  !clk->parents) {

You don't need the additional parens here.  Please learn the C precedence
rules.  Additional unnecessary parens can make expressions much harder
to read.

Rajendra,

I've taken this patch into clk-next for testing.  I've fixed up the
extra parens locally, so no need for a resend.

Thanks Mike.

Regards,
Mike