[RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Masahiro Yamada]

The clock consumer, drivers/video/fbdev/simplefb.c, includes
<linux/clk-provider.h> just for calling of_clk_get_parent_count().
This is ugly.

Looking at simplefb_clocks_get(), of_clk_get_parent_count() seems
useful for clock consumers as well as for clock providers.

Unfortunately, we do not have a good home for declarations shared
between consumers and providers.

Create a new header <linux/__clk.h>, and move it over to it.  This
header must be included via <linux/clk.h> or <linux/clk-provider.h>
(this is why it is prefixed with double-underscore).  Add #error
so the build terminates if it is included directly.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/video/fbdev/simplefb.c |  1 -
 include/linux/__clk.h          | 31 +++++++++++++++++++++++++++++++
 include/linux/clk-provider.h   |  7 ++-----
 include/linux/clk.h            |  2 ++
 4 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 include/linux/__clk.h

diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index a3c44ec..17f0aec 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -27,7 +27,6 @@
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
-#include <linux/clk-provider.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/parser.h>
diff --git a/include/linux/__clk.h b/include/linux/__clk.h
new file mode 100644
index 0000000..a8b86bf
--- /dev/null
+++ b/include/linux/__clk.h
@@ -0,0 +1,31 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX___CLK_H
+#define __LINUX___CLK_H
+
+/* This header contains stuff shared between consumers and providers. */
+
+#if !defined(__LINUX_CLK_H) && !defined(__LINUX_CLK_PROVIDER_H)
+#error "Please don't include <linux/__clk.h> directly, include <linux/clk.h> or <linux/clk-provider.h> instead."
+#endif
+
+struct device_node;
+
+#if defined(CONFIG_COMMON_CLK) && defined(CONFIG_OF)
+
+unsigned int of_clk_get_parent_count(struct device_node *np);
+
+#else
+
+static inline unsigned int of_clk_get_parent_count(struct device_node *np)
+{
+	return 0;
+}
+
+#endif
+
+#endif /* __LINUX___CLK_H */
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5100ec1..cd9bca1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -14,6 +14,8 @@
 #include <linux/io.h>
 #include <linux/of.h>
 
+#include "__clk.h"
+
 #ifdef CONFIG_COMMON_CLK
 
 /*
@@ -823,7 +825,6 @@ struct clk_hw *of_clk_hw_simple_get(struct of_phandle_args *clkspec,
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
 struct clk_hw *of_clk_hw_onecell_get(struct of_phandle_args *clkspec,
 				     void *data);
-unsigned int of_clk_get_parent_count(struct device_node *np);
 int of_clk_parent_fill(struct device_node *np, const char **parents,
 		       unsigned int size);
 const char *of_clk_get_parent_name(struct device_node *np, int index);
@@ -868,10 +869,6 @@ of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
 {
 	return ERR_PTR(-ENOENT);
 }
-static inline unsigned int of_clk_get_parent_count(struct device_node *np)
-{
-	return 0;
-}
 static inline int of_clk_parent_fill(struct device_node *np,
 				     const char **parents, unsigned int size)
 {
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 12c96d9..83d24cb 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -16,6 +16,8 @@
 #include <linux/kernel.h>
 #include <linux/notifier.h>
 
+#include "__clk.h"
+
 struct device;
 struct clk;
 struct device_node;
-- 
2.7.4

[RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Masahiro Yamada]

The clock consumer, drivers/video/fbdev/simplefb.c, includes
<linux/clk-provider.h> just for calling of_clk_get_parent_count().
This is ugly.

Looking at simplefb_clocks_get(), of_clk_get_parent_count() seems
useful for clock consumers as well as for clock providers.

Unfortunately, we do not have a good home for declarations shared
between consumers and providers.

Create a new header <linux/__clk.h>, and move it over to it.  This
header must be included via <linux/clk.h> or <linux/clk-provider.h>
(this is why it is prefixed with double-underscore).  Add #error
so the build terminates if it is included directly.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 drivers/video/fbdev/simplefb.c |  1 -
 include/linux/__clk.h          | 31 +++++++++++++++++++++++++++++++
 include/linux/clk-provider.h   |  7 ++-----
 include/linux/clk.h            |  2 ++
 4 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 include/linux/__clk.h

diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
index a3c44ec..17f0aec 100644
--- a/drivers/video/fbdev/simplefb.c
+++ b/drivers/video/fbdev/simplefb.c
@@ -27,7 +27,6 @@
 #include <linux/platform_data/simplefb.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
-#include <linux/clk-provider.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/parser.h>
diff --git a/include/linux/__clk.h b/include/linux/__clk.h
new file mode 100644
index 0000000..a8b86bf
--- /dev/null
+++ b/include/linux/__clk.h
@@ -0,0 +1,31 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __LINUX___CLK_H
+#define __LINUX___CLK_H
+
+/* This header contains stuff shared between consumers and providers. */
+
+#if !defined(__LINUX_CLK_H) && !defined(__LINUX_CLK_PROVIDER_H)
+#error "Please don't include <linux/__clk.h> directly, include <linux/clk.h> or <linux/clk-provider.h> instead."
+#endif
+
+struct device_node;
+
+#if defined(CONFIG_COMMON_CLK) && defined(CONFIG_OF)
+
+unsigned int of_clk_get_parent_count(struct device_node *np);
+
+#else
+
+static inline unsigned int of_clk_get_parent_count(struct device_node *np)
+{
+	return 0;
+}
+
+#endif
+
+#endif /* __LINUX___CLK_H */
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 5100ec1..cd9bca1 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -14,6 +14,8 @@
 #include <linux/io.h>
 #include <linux/of.h>
 
+#include "__clk.h"
+
 #ifdef CONFIG_COMMON_CLK
 
 /*
@@ -823,7 +825,6 @@ struct clk_hw *of_clk_hw_simple_get(struct of_phandle_args *clkspec,
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
 struct clk_hw *of_clk_hw_onecell_get(struct of_phandle_args *clkspec,
 				     void *data);
-unsigned int of_clk_get_parent_count(struct device_node *np);
 int of_clk_parent_fill(struct device_node *np, const char **parents,
 		       unsigned int size);
 const char *of_clk_get_parent_name(struct device_node *np, int index);
@@ -868,10 +869,6 @@ of_clk_hw_onecell_get(struct of_phandle_args *clkspec, void *data)
 {
 	return ERR_PTR(-ENOENT);
 }
-static inline unsigned int of_clk_get_parent_count(struct device_node *np)
-{
-	return 0;
-}
 static inline int of_clk_parent_fill(struct device_node *np,
 				     const char **parents, unsigned int size)
 {
diff --git a/include/linux/clk.h b/include/linux/clk.h
index 12c96d9..83d24cb 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -16,6 +16,8 @@
 #include <linux/kernel.h>
 #include <linux/notifier.h>
 
+#include "__clk.h"
+
 struct device;
 struct clk;
 struct device_node;
-- 
2.7.4

Re: [RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Stephen Boyd]

On 10/05, Masahiro Yamada wrote:
> The clock consumer, drivers/video/fbdev/simplefb.c, includes
> <linux/clk-provider.h> just for calling of_clk_get_parent_count().
> This is ugly.
> 
> Looking at simplefb_clocks_get(), of_clk_get_parent_count() seems
> useful for clock consumers as well as for clock providers.
> 
> Unfortunately, we do not have a good home for declarations shared
> between consumers and providers.
> 
> Create a new header <linux/__clk.h>, and move it over to it.  This
> header must be included via <linux/clk.h> or <linux/clk-provider.h>
> (this is why it is prefixed with double-underscore).  Add #error
> so the build terminates if it is included directly.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

How about we add a get_all_the_clks_for_the_device() sort of API
that uses the bulk clk code but also allocates the array by
reading the number of clks from DT? Please think of a better name
of course. We can figure out how to handle non-DT platforms if we
need to. With clkdev we could probably handle it with some sort
of lookup table search given a device name. Don't add that part
until we have a user though. I assume simplefb is all DT
platforms?

Either way, it looks like what we really want here is a way to
get every clk for a device and not look at the details. USB has a
similar case, which I think Shawn Guo/Dong Aisheng was trying to
add an OF based bulk clk_get() API called of_clk_bulk_get()
for[1]. If this get all clks API works there too then we should
use it. If it can be non-DT specific, even better.

[1] http://lkml.kernel.org/r/1506415441-4435-1-git-send-email-aisheng.dong@nxp.com

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Stephen Boyd]

On 10/05, Masahiro Yamada wrote:
> The clock consumer, drivers/video/fbdev/simplefb.c, includes
> <linux/clk-provider.h> just for calling of_clk_get_parent_count().
> This is ugly.
> 
> Looking at simplefb_clocks_get(), of_clk_get_parent_count() seems
> useful for clock consumers as well as for clock providers.
> 
> Unfortunately, we do not have a good home for declarations shared
> between consumers and providers.
> 
> Create a new header <linux/__clk.h>, and move it over to it.  This
> header must be included via <linux/clk.h> or <linux/clk-provider.h>
> (this is why it is prefixed with double-underscore).  Add #error
> so the build terminates if it is included directly.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---

How about we add a get_all_the_clks_for_the_device() sort of API
that uses the bulk clk code but also allocates the array by
reading the number of clks from DT? Please think of a better name
of course. We can figure out how to handle non-DT platforms if we
need to. With clkdev we could probably handle it with some sort
of lookup table search given a device name. Don't add that part
until we have a user though. I assume simplefb is all DT
platforms?

Either way, it looks like what we really want here is a way to
get every clk for a device and not look at the details. USB has a
similar case, which I think Shawn Guo/Dong Aisheng was trying to
add an OF based bulk clk_get() API called of_clk_bulk_get()
for[1]. If this get all clks API works there too then we should
use it. If it can be non-DT specific, even better.

[1] http://lkml.kernel.org/r/1506415441-4435-1-git-send-email-aisheng.dong@nxp.com

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Masahiro Yamada]

Hi Stephen,


2017-10-13 8:17 GMT+09:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 10/05, Masahiro Yamada wrote:
>> The clock consumer, drivers/video/fbdev/simplefb.c, includes
>> <linux/clk-provider.h> just for calling of_clk_get_parent_count().
>> This is ugly.
>>
>> Looking at simplefb_clocks_get(), of_clk_get_parent_count() seems
>> useful for clock consumers as well as for clock providers.
>>
>> Unfortunately, we do not have a good home for declarations shared
>> between consumers and providers.
>>
>> Create a new header <linux/__clk.h>, and move it over to it.  This
>> header must be included via <linux/clk.h> or <linux/clk-provider.h>
>> (this is why it is prefixed with double-underscore).  Add #error
>> so the build terminates if it is included directly.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>
> How about we add a get_all_the_clks_for_the_device() sort of API
> that uses the bulk clk code but also allocates the array by
> reading the number of clks from DT? Please think of a better name
> of course. We can figure out how to handle non-DT platforms if we
> need to. With clkdev we could probably handle it with some sort
> of lookup table search given a device name. Don't add that part
> until we have a user though. I assume simplefb is all DT
> platforms?

of_reset_control_array_get() is implemented like that.

It gets all the resets specified in DT, and automatically
allocates the array of necessary number of pointers.

clk-bulk and reset-array were implemented in a different way.



> Either way, it looks like what we really want here is a way to
> get every clk for a device and not look at the details.

Yes.  This is helpful for generic drivers.


>  USB has a
> similar case, which I think Shawn Guo/Dong Aisheng was trying to
> add an OF based bulk clk_get() API called of_clk_bulk_get()
> for[1]. If this get all clks API works there too then we should
> use it. If it can be non-DT specific, even better.
>
> [1] http://lkml.kernel.org/r/1506415441-4435-1-git-send-email-aisheng.dong@nxp.com

OK, we can implement it based on Shawn/Dong's work.

My concern is clk-bulk.c still needs to include #<linux/clk-provider.h>,
but it is a bit odd since clk-bulk.c is a helper for clk consumers.


-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Masahiro Yamada]

Hi Stephen,


2017-10-13 8:17 GMT+09:00 Stephen Boyd <sboyd@codeaurora.org>:
> On 10/05, Masahiro Yamada wrote:
>> The clock consumer, drivers/video/fbdev/simplefb.c, includes
>> <linux/clk-provider.h> just for calling of_clk_get_parent_count().
>> This is ugly.
>>
>> Looking at simplefb_clocks_get(), of_clk_get_parent_count() seems
>> useful for clock consumers as well as for clock providers.
>>
>> Unfortunately, we do not have a good home for declarations shared
>> between consumers and providers.
>>
>> Create a new header <linux/__clk.h>, and move it over to it.  This
>> header must be included via <linux/clk.h> or <linux/clk-provider.h>
>> (this is why it is prefixed with double-underscore).  Add #error
>> so the build terminates if it is included directly.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>
> How about we add a get_all_the_clks_for_the_device() sort of API
> that uses the bulk clk code but also allocates the array by
> reading the number of clks from DT? Please think of a better name
> of course. We can figure out how to handle non-DT platforms if we
> need to. With clkdev we could probably handle it with some sort
> of lookup table search given a device name. Don't add that part
> until we have a user though. I assume simplefb is all DT
> platforms?

of_reset_control_array_get() is implemented like that.

It gets all the resets specified in DT, and automatically
allocates the array of necessary number of pointers.

clk-bulk and reset-array were implemented in a different way.



> Either way, it looks like what we really want here is a way to
> get every clk for a device and not look at the details.

Yes.  This is helpful for generic drivers.


>  USB has a
> similar case, which I think Shawn Guo/Dong Aisheng was trying to
> add an OF based bulk clk_get() API called of_clk_bulk_get()
> for[1]. If this get all clks API works there too then we should
> use it. If it can be non-DT specific, even better.
>
> [1] http://lkml.kernel.org/r/1506415441-4435-1-git-send-email-aisheng.dong@nxp.com

OK, we can implement it based on Shawn/Dong's work.

My concern is clk-bulk.c still needs to include #<linux/clk-provider.h>,
but it is a bit odd since clk-bulk.c is a helper for clk consumers.


-- 
Best Regards
Masahiro Yamada

Re: [RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Stephen Boyd]

On 10/29, Masahiro Yamada wrote:
> 2017-10-13 8:17 GMT+09:00 Stephen Boyd <sboyd@codeaurora.org>:
> 
> >  USB has a
> > similar case, which I think Shawn Guo/Dong Aisheng was trying to
> > add an OF based bulk clk_get() API called of_clk_bulk_get()
> > for[1]. If this get all clks API works there too then we should
> > use it. If it can be non-DT specific, even better.
> >
> > [1] http://lkml.kernel.org/r/1506415441-4435-1-git-send-email-aisheng.dong@nxp.com
> 
> OK, we can implement it based on Shawn/Dong's work.
> 
> My concern is clk-bulk.c still needs to include #<linux/clk-provider.h>,
> but it is a bit odd since clk-bulk.c is a helper for clk consumers.
> 
> 

Understood. I'm not concerned though, it's one place where this
happens, not in a bunch of random drivers. What exactly do you
need from clk-provider.h in clk-bulk.c though?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC PATCH] clk: move of_clk_get_parent_count() declaration to <linux/__clk.h>

[Stephen Boyd]

On 10/29, Masahiro Yamada wrote:
> 2017-10-13 8:17 GMT+09:00 Stephen Boyd <sboyd@codeaurora.org>:
> 
> >  USB has a
> > similar case, which I think Shawn Guo/Dong Aisheng was trying to
> > add an OF based bulk clk_get() API called of_clk_bulk_get()
> > for[1]. If this get all clks API works there too then we should
> > use it. If it can be non-DT specific, even better.
> >
> > [1] http://lkml.kernel.org/r/1506415441-4435-1-git-send-email-aisheng.dong@nxp.com
> 
> OK, we can implement it based on Shawn/Dong's work.
> 
> My concern is clk-bulk.c still needs to include #<linux/clk-provider.h>,
> but it is a bit odd since clk-bulk.c is a helper for clk consumers.
> 
> 

Understood. I'm not concerned though, it's one place where this
happens, not in a bunch of random drivers. What exactly do you
need from clk-provider.h in clk-bulk.c though?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project