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 <[email protected]>
---
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 <[email protected]>
---
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:
> 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 <[email protected]>
> ---
> 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.
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 <[email protected]>
> > ---
> > 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 <[email protected]>
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:
>> 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<[email protected]>
>>> ---
>>> 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