[PATCH 0/2] some minor clk fixes

[PATCH 0/2] some minor clk fixes

[Rajendra Nayak]

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

[PATCH 0/2] some minor clk fixes

[Rajendra Nayak]

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

[PATCH 1/2] clk: cache parent clocks only for muxes

[Rajendra Nayak]

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 <rnayak@ti.com>
---
 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

[PATCH 1/2] clk: cache parent clocks only for muxes

[Rajendra Nayak]

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 <rnayak@ti.com>
---
 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

[PATCH 1/2] clk: cache parent clocks only for muxes

[Russell King - ARM Linux]

On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:
> 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 <rnayak@ti.com>
> ---
>  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.

Re: [PATCH 1/2] clk: cache parent clocks only for muxes

[Russell King - ARM Linux]

On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:
> 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 <rnayak@ti.com>
> ---
>  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.

[PATCH 1/2] clk: cache parent clocks only for muxes

[Mike Turquette]

On 20120606-11:47, Russell King - ARM Linux wrote:
> On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:
> > 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 <rnayak@ti.com>
> > ---
> >  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

Re: [PATCH 1/2] clk: cache parent clocks only for muxes

[Mike Turquette]

On 20120606-11:47, Russell King - ARM Linux wrote:
> On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:
> > 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 <rnayak@ti.com>
> > ---
> >  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

[PATCH 1/2] clk: cache parent clocks only for muxes

[Rajendra Nayak]

On Wednesday 13 June 2012 12:34 AM, Mike Turquette wrote:
> On 20120606-11:47, Russell King - ARM Linux wrote:
>> On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:
>>> 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<rnayak@ti.com>
>>> ---
>>>   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

Re: [PATCH 1/2] clk: cache parent clocks only for muxes

[Rajendra Nayak]

On Wednesday 13 June 2012 12:34 AM, Mike Turquette wrote:
> On 20120606-11:47, Russell King - ARM Linux wrote:
>> On Wed, Jun 06, 2012 at 02:41:30PM +0530, Rajendra Nayak wrote:
>>> 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<rnayak@ti.com>
>>> ---
>>>   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

[PATCH 2/2] clk: Allow late cache allocation for clk->parents

[Rajendra Nayak]

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 <rnayak@ti.com>
---
 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

[PATCH 2/2] clk: Allow late cache allocation for clk->parents

[Rajendra Nayak]

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 <rnayak@ti.com>
---
 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

[PATCH 2/2] clk: Allow late cache allocation for clk->parents

[Mike Turquette]

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 <rnayak@ti.com>

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

Thanks,
Mike

Re: [PATCH 2/2] clk: Allow late cache allocation for clk->parents

[Mike Turquette]

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 <rnayak@ti.com>

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

Thanks,
Mike