diff mbox

[RFC,v3,1/2] clk: samsung: Add a clock lookup function

Message ID 1427730803-28635-2-git-send-email-javier.martinez@collabora.co.uk (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Javier Martinez Canillas March 30, 2015, 3:53 p.m. UTC
The Samsung helpers functions to register clocks, add the clock instance
returned by the common clock framework to a lookup table that is used by
OF to lookup the clocks.

But this table could also be useful to clock drivers if they need to get
a clock instance since the helper functions don't return them.

The common clock framework __clk_lookup() function from the clk provider
API could be used by drivers as well. But it's more efficient to use the
Samsung specific lookup table that returns the clock instance in constant
time, than using the __clk_lookup() function that uses the clock name as
an index so it has a linear search time.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/clk/samsung/clk.c | 6 ++++++
 drivers/clk/samsung/clk.h | 3 +++
 2 files changed, 9 insertions(+)

Comments

Tomasz Figa March 30, 2015, 4:02 p.m. UTC | #1
Hi Javier,

2015-03-31 0:53 GMT+09:00 Javier Martinez Canillas
<javier.martinez@collabora.co.uk>:
> The Samsung helpers functions to register clocks, add the clock instance
> returned by the common clock framework to a lookup table that is used by
> OF to lookup the clocks.
>
> But this table could also be useful to clock drivers if they need to get
> a clock instance since the helper functions don't return them.
>
> The common clock framework __clk_lookup() function from the clk provider
> API could be used by drivers as well. But it's more efficient to use the
> Samsung specific lookup table that returns the clock instance in constant
> time, than using the __clk_lookup() function that uses the clock name as
> an index so it has a linear search time.

Is this really something we should be concerned about? If so, maybe
the generic look-up function should be rewritten to use something
faster, such as tree or hash table?

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Martinez Canillas March 30, 2015, 4:08 p.m. UTC | #2
Hello Tomasz,

On 03/30/2015 06:02 PM, Tomasz Figa wrote:
> Hi Javier,
> 
> 2015-03-31 0:53 GMT+09:00 Javier Martinez Canillas
> <javier.martinez@collabora.co.uk>:
>> The Samsung helpers functions to register clocks, add the clock instance
>> returned by the common clock framework to a lookup table that is used by
>> OF to lookup the clocks.
>>
>> But this table could also be useful to clock drivers if they need to get
>> a clock instance since the helper functions don't return them.
>>
>> The common clock framework __clk_lookup() function from the clk provider
>> API could be used by drivers as well. But it's more efficient to use the
>> Samsung specific lookup table that returns the clock instance in constant
>> time, than using the __clk_lookup() function that uses the clock name as
>> an index so it has a linear search time.
> 
> Is this really something we should be concerned about? If so, maybe
> the generic look-up function should be rewritten to use something
> faster, such as tree or hash table?
>

I don't performance is a big issue here. I just thought that since the
lookup table is already filled by the driver and the lookup function
is one line, we could use that instead to get the performance benefit.

But I don't mind to drop this patch and use the generic lookup function
from the CCF API if that is preferred.
 
> Best regards,
> Tomasz
>

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Turquette March 31, 2015, 1:40 a.m. UTC | #3
Quoting Javier Martinez Canillas (2015-03-30 09:08:40)
> Hello Tomasz,
> 
> On 03/30/2015 06:02 PM, Tomasz Figa wrote:
> > Hi Javier,
> > 
> > 2015-03-31 0:53 GMT+09:00 Javier Martinez Canillas
> > <javier.martinez@collabora.co.uk>:
> >> The Samsung helpers functions to register clocks, add the clock instance
> >> returned by the common clock framework to a lookup table that is used by
> >> OF to lookup the clocks.
> >>
> >> But this table could also be useful to clock drivers if they need to get
> >> a clock instance since the helper functions don't return them.
> >>
> >> The common clock framework __clk_lookup() function from the clk provider
> >> API could be used by drivers as well. But it's more efficient to use the
> >> Samsung specific lookup table that returns the clock instance in constant
> >> time, than using the __clk_lookup() function that uses the clock name as
> >> an index so it has a linear search time.
> > 
> > Is this really something we should be concerned about? If so, maybe
> > the generic look-up function should be rewritten to use something
> > faster, such as tree or hash table?
> >
> 
> I don't performance is a big issue here. I just thought that since the
> lookup table is already filled by the driver and the lookup function
> is one line, we could use that instead to get the performance benefit.
> 
> But I don't mind to drop this patch and use the generic lookup function
> from the CCF API if that is preferred.

Hello,

I am not a fan of __clk_lookup and I don't like to see it used more and
more outside of drivers/clk/clk.c. You mentioned that performance wasn't
really the problem here.  The real method for a driver to get a clock is
with clk_get(). Any reason to not use that?

Regards,
Mike

>  
> > Best regards,
> > Tomasz
> >
> 
> Best regards,
> Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index 9e1f88c04fd4..3b2868a70774 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -96,6 +96,12 @@  void samsung_clk_add_lookup(struct samsung_clk_provider *ctx, struct clk *clk,
 		ctx->clk_data.clks[id] = clk;
 }
 
+struct clk *samsung_clk_lookup(struct samsung_clk_provider *ctx,
+				unsigned int id)
+{
+	return ctx->clk_data.clks ? ctx->clk_data.clks[id] : NULL;
+}
+
 /* register a list of aliases */
 void __init samsung_clk_register_alias(struct samsung_clk_provider *ctx,
 				struct samsung_clock_alias *list,
diff --git a/drivers/clk/samsung/clk.h b/drivers/clk/samsung/clk.h
index e4c75383cea7..ad04220bd733 100644
--- a/drivers/clk/samsung/clk.h
+++ b/drivers/clk/samsung/clk.h
@@ -368,6 +368,9 @@  extern void __init samsung_clk_of_register_fixed_ext(
 extern void samsung_clk_add_lookup(struct samsung_clk_provider *ctx,
 			struct clk *clk, unsigned int id);
 
+extern struct clk *samsung_clk_lookup(struct samsung_clk_provider *ctx,
+			unsigned int id);
+
 extern void samsung_clk_register_alias(struct samsung_clk_provider *ctx,
 			struct samsung_clock_alias *list,
 			unsigned int nr_clk);