[PATCH] clk: define and export __clk_get_debug for providers

[PATCH] clk: define and export __clk_get_debug for providers

[Peter De Schrijver]

Define and export a new function __clk_get_debug which returns the debugfs
directory entry pointer. This can be used by clock providers to add debugfs
entries which are not related to a specific clock type. Examples include the
ability to measure the rate of a clock. It can also be used by modules to
create new debugfs entries. This is useful if you want to expose features
for testing which can potentially cause system instability such as allowing
to change a clock's rate from userspace.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/clk.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8b73ede..5fe7942 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -652,6 +652,15 @@ unsigned long __clk_get_flags(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(__clk_get_flags);
 
+struct dentry *__clk_get_dentry(struct clk *clk)
+{
+	if (!clk)
+		return NULL;
+
+	return clk->dentry;
+}
+EXPORT_SYMBOL_GPL(__clk_get_dentry);
+
 bool __clk_is_prepared(struct clk *clk)
 {
 	int ret;
-- 
1.7.7.rc0.72.g4b5ea.dirty

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Arnd Bergmann]

On Wednesday 25 June 2014 19:26:31 Peter De Schrijver wrote:
> Define and export a new function __clk_get_debug which returns the debugfs
> directory entry pointer. This can be used by clock providers to add debugfs
> entries which are not related to a specific clock type. Examples include the
> ability to measure the rate of a clock. It can also be used by modules to
> create new debugfs entries. This is useful if you want to expose features
> for testing which can potentially cause system instability such as allowing
> to change a clock's rate from userspace.
> 
> Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> 

Just an idea, but would it be better to add a pair of functions to add/remove
debugfs files rather than exposing the dentry itself?

	Arnd

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Mike Turquette]

Quoting Arnd Bergmann (2014-06-25 11:07:37)
> On Wednesday 25 June 2014 19:26:31 Peter De Schrijver wrote:
> > Define and export a new function __clk_get_debug which returns the debugfs
> > directory entry pointer. This can be used by clock providers to add debugfs
> > entries which are not related to a specific clock type. Examples include the
> > ability to measure the rate of a clock. It can also be used by modules to
> > create new debugfs entries. This is useful if you want to expose features
> > for testing which can potentially cause system instability such as allowing
> > to change a clock's rate from userspace.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > 
> 
> Just an idea, but would it be better to add a pair of functions to add/remove
> debugfs files rather than exposing the dentry itself?

Arnd,

Yes, accessor functions are much better than exposing struct clk
internals.

Peter,

Just FYI, I'm trying to reverse the trend of prepending double
underscores for functions that are used by clock providers. That stuff
started out small and sort of grew out of control ;-)

Regards,
Mike

> 
>         Arnd

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Tomeu Vizoso]

On 25 June 2014 20:23, Mike Turquette <mturquette@linaro.org> wrote:
>
> Peter,
>
> Just FYI, I'm trying to reverse the trend of prepending double
> underscores for functions that are used by clock providers. That stuff
> started out small and sort of grew out of control ;-)

I'm looking at rebasing Rabin's patches for per-user clks, and I have
had to add a few variants for providers, so they can be called with
clk_core instead. Have been prepending underscores for those internal
variants, but in some cases I had to also append a _internal suffix
because there were already variants without locking.

What convention were you thinking of for distinguishing provider-only
variants for e.g. clk_set_rate?

Regards,

Tomeu

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Peter De Schrijver]

On Wed, Jun 25, 2014 at 08:23:49PM +0200, Mike Turquette wrote:
> Quoting Arnd Bergmann (2014-06-25 11:07:37)
> > On Wednesday 25 June 2014 19:26:31 Peter De Schrijver wrote:
> > > Define and export a new function __clk_get_debug which returns the debugfs
> > > directory entry pointer. This can be used by clock providers to add debugfs
> > > entries which are not related to a specific clock type. Examples include the
> > > ability to measure the rate of a clock. It can also be used by modules to
> > > create new debugfs entries. This is useful if you want to expose features
> > > for testing which can potentially cause system instability such as allowing
> > > to change a clock's rate from userspace.
> > > 
> > > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > > 
> > 
> > Just an idea, but would it be better to add a pair of functions to add/remove
> > debugfs files rather than exposing the dentry itself?
> 
> Arnd,
> 
> Yes, accessor functions are much better than exposing struct clk
> internals.
> 
> Peter,
> 
> Just FYI, I'm trying to reverse the trend of prepending double
> underscores for functions that are used by clock providers. That stuff
> started out small and sort of grew out of control ;-)
> 

Ah. I was just following the existing pattern here :)

Cheers,

Peter.

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Peter De Schrijver]

On Wed, Jun 25, 2014 at 08:07:37PM +0200, Arnd Bergmann wrote:
> On Wednesday 25 June 2014 19:26:31 Peter De Schrijver wrote:
> > Define and export a new function __clk_get_debug which returns the debugfs
> > directory entry pointer. This can be used by clock providers to add debugfs
> > entries which are not related to a specific clock type. Examples include the
> > ability to measure the rate of a clock. It can also be used by modules to
> > create new debugfs entries. This is useful if you want to expose features
> > for testing which can potentially cause system instability such as allowing
> > to change a clock's rate from userspace.
> > 
> > Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
> > 
> 
> Just an idea, but would it be better to add a pair of functions to add/remove
> debugfs files rather than exposing the dentry itself?
> 

Ok. Will do that.

Cheers,

Peter.

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Mike Turquette]

On Thu, Jun 26, 2014 at 12:37 AM, Tomeu Vizoso
<tomeu.vizoso@collabora.com> wrote:
> On 25 June 2014 20:23, Mike Turquette <mturquette@linaro.org> wrote:
>>
>> Peter,
>>
>> Just FYI, I'm trying to reverse the trend of prepending double
>> underscores for functions that are used by clock providers. That stuff
>> started out small and sort of grew out of control ;-)
>
> I'm looking at rebasing Rabin's patches for per-user clks, and I have
> had to add a few variants for providers, so they can be called with
> clk_core instead. Have been prepending underscores for those internal
> variants, but in some cases I had to also append a _internal suffix
> because there were already variants without locking.
>
> What convention were you thinking of for distinguishing provider-only
> variants for e.g. clk_set_rate?

Hmm. Now we need a convention. Damn. ;-)

I'm open to suggestions. I think prepending a single underscore makes
great sense for a lock-free inner function. And some number of
underscores (1 or 2) makes sense for static functions not exposed to
clock providers. I kind of like using 2 underscores for the static
functions so that it is easier to differentiate them.

For any new functions you create with unique names that are exposed to
clock providers, do not prepend any underscore at all. It's just
another public function in clk-provider.h.

If any taste makers out there have suggestions I am happy to listen.

Regards,
Mike

>
> Regards,
>
> Tomeu

Re: [PATCH] clk: define and export __clk_get_debug for providers

[Peter De Schrijver]

On Thu, Jun 26, 2014 at 03:22:01PM +0200, Peter De Schrijver wrote:
> Define and export a new function __clk_get_debug which returns the debugfs
> directory entry pointer. This can be used by clock providers to add debugfs
> entries which are not related to a specific clock type. Examples include the
> ability to measure the rate of a clock. It can also be used by modules to
> create new debugfs entries. This is useful if you want to expose features
> for testing which can potentially cause system instability such as allowing
> to change a clock's rate from userspace.

Oops. Ignore this :)

[PATCH] clk: define and export __clk_get_debug for providers

[Peter De Schrijver]

Define and export a new function __clk_get_debug which returns the debugfs
directory entry pointer. This can be used by clock providers to add debugfs
entries which are not related to a specific clock type. Examples include the
ability to measure the rate of a clock. It can also be used by modules to
create new debugfs entries. This is useful if you want to expose features
for testing which can potentially cause system instability such as allowing
to change a clock's rate from userspace.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/clk.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8b73ede..5fe7942 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -652,6 +652,15 @@ unsigned long __clk_get_flags(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(__clk_get_flags);
 
+struct dentry *__clk_get_dentry(struct clk *clk)
+{
+	if (!clk)
+		return NULL;
+
+	return clk->dentry;
+}
+EXPORT_SYMBOL_GPL(__clk_get_dentry);
+
 bool __clk_is_prepared(struct clk *clk)
 {
 	int ret;
-- 
1.7.7.rc0.72.g4b5ea.dirty

[PATCH] clk: define and export __clk_get_debug for providers

[Peter De Schrijver]

Define and export a new function __clk_get_debug which returns the debugfs
directory entry pointer. This can be used by clock providers to add debugfs
entries which are not related to a specific clock type. Examples include the
ability to measure the rate of a clock. It can also be used by modules to
create new debugfs entries. This is useful if you want to expose features
for testing which can potentially cause system instability such as allowing
to change a clock's rate from userspace.

Signed-off-by: Peter De Schrijver <pdeschrijver@nvidia.com>
---
 drivers/clk/clk.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 8b73ede..5fe7942 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -652,6 +652,15 @@ unsigned long __clk_get_flags(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(__clk_get_flags);
 
+struct dentry *__clk_get_dentry(struct clk *clk)
+{
+	if (!clk)
+		return NULL;
+
+	return clk->dentry;
+}
+EXPORT_SYMBOL_GPL(__clk_get_dentry);
+
 bool __clk_is_prepared(struct clk *clk)
 {
 	int ret;
-- 
1.7.7.rc0.72.g4b5ea.dirty