* [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT
@ 2016-06-13 9:08 Jongsung Kim
2016-06-21 0:59 ` Stephen Boyd
0 siblings, 1 reply; 13+ messages in thread
From: Jongsung Kim @ 2016-06-13 9:08 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-clk, linux-kernel, Chanho Min, Jongsung Kim
Without CLK_SET_RATE_PARENT-flag set, clk_factor_round_rate() just
returns the current frequency. A fixed-factor-clock initialzed via
of_fixed_factor_clk_set() (ie, by device-tree) can't have the flag
set. It can be problematic when the parent of a fixed-factor-clock
is rate-controllable clk, because the rounding can't be propagated
to parent, the rounded target frequency is always the current, and
finally the frequency can't be changed.
This patch sets the flags CLK_SET_RATE_PARENT for all fixed-factor-
clocks, from clk_register_fixed_factor(), and removes checking the
flag from clk_factor_round_rate().
Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
---
drivers/clk/clk-fixed-factor.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..9568306 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -38,13 +38,10 @@ static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
unsigned long *prate)
{
struct clk_fixed_factor *fix = to_clk_fixed_factor(hw);
+ unsigned long best_parent;
- if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
- unsigned long best_parent;
-
- best_parent = (rate / fix->mult) * fix->div;
- *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
- }
+ best_parent = (rate / fix->mult) * fix->div;
+ *prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
return (*prate / fix->div) * fix->mult;
}
@@ -88,7 +85,7 @@ struct clk_hw *clk_hw_register_fixed_factor(struct device *dev,
init.name = name;
init.ops = &clk_fixed_factor_ops;
- init.flags = flags | CLK_IS_BASIC;
+ init.flags = flags | CLK_IS_BASIC | CLK_SET_RATE_PARENT;
init.parent_names = &parent_name;
init.num_parents = 1;
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT
2016-06-13 9:08 [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT Jongsung Kim
@ 2016-06-21 0:59 ` Stephen Boyd
2016-06-24 3:49 ` [PATCH] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim
2016-06-24 4:12 ` [PATCH v2] " Jongsung Kim
0 siblings, 2 replies; 13+ messages in thread
From: Stephen Boyd @ 2016-06-21 0:59 UTC (permalink / raw)
To: Jongsung Kim
Cc: Michael Turquette, linux-clk, linux-kernel, Chanho Min, Maxime Ripard
On 06/13, Jongsung Kim wrote:
> Without CLK_SET_RATE_PARENT-flag set, clk_factor_round_rate() just
> returns the current frequency. A fixed-factor-clock initialzed via
> of_fixed_factor_clk_set() (ie, by device-tree) can't have the flag
> set. It can be problematic when the parent of a fixed-factor-clock
> is rate-controllable clk, because the rounding can't be propagated
> to parent, the rounded target frequency is always the current, and
> finally the frequency can't be changed.
>
> This patch sets the flags CLK_SET_RATE_PARENT for all fixed-factor-
> clocks, from clk_register_fixed_factor(), and removes checking the
> flag from clk_factor_round_rate().
>
> Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
> ---
Hmm there's another patch for the same problem from Maxime[1]
Mike replied there and said that it's worrisome to do this for
all users of fixed factors clks, so perhaps you can set this flag
when you register the clk instead? Or prove that all the users of
this code in mainline are ok to change behavior.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-May/429726.html
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-21 0:59 ` Stephen Boyd
@ 2016-06-24 3:49 ` Jongsung Kim
2016-06-24 5:11 ` kbuild test robot
2016-06-24 5:57 ` kbuild test robot
2016-06-24 4:12 ` [PATCH v2] " Jongsung Kim
1 sibling, 2 replies; 13+ messages in thread
From: Jongsung Kim @ 2016-06-24 3:49 UTC (permalink / raw)
To: Maxime Ripard, Mike Turquette, Stephen Boyd
Cc: linux-clk, devicetree, linux-kernel, Chanho Min, Jongsung Kim
There is no way to set additional flags for a DT-initialized fixed-
factor-clock, and it can be problematic i.e., when the clock rate
needs to be changed. [1][2]
This patch introduces an optional dt-binding named "clock-flags" to
be used for passing any needed flags from dts.
[1] http://www.spinics.net/lists/linux-clk/msg09040.html
[2] https://lkml.org/lkml/2016/6/20/1025
Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Mike Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
---
.../bindings/clock/fixed-factor-clock.txt | 4 ++++
drivers/clk/clk-fixed-factor.c | 4 +++-
include/dt-bindings/clk/clk.h | 22 ++++++++++++++++++++++
3 files changed, 29 insertions(+), 1 deletion(-)
create mode 100644 include/dt-bindings/clk/clk.h
diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
index 1bae8527..3e1b79e 100644
--- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
@@ -13,12 +13,16 @@ Required properties:
Optional properties:
- clock-output-names : From common clock binding.
+- clock-flags : Additional flags to be used.
Example:
+ #include <dt-bindings/clk/clk.h>
+
clock {
compatible = "fixed-factor-clock";
clocks = <&parentclk>;
#clock-cells = <0>;
clock-div = <2>;
clock-mult = <1>;
+ clock-flags = <CLK_SET_RATE_PARENT>;
};
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..da3cd9c 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -150,6 +150,7 @@ void __init of_fixed_factor_clk_setup(struct device_node *node)
struct clk *clk;
const char *clk_name = node->name;
const char *parent_name;
+ unsigned long flags = 0;
u32 div, mult;
if (of_property_read_u32(node, "clock-div", &div)) {
@@ -166,8 +167,9 @@ void __init of_fixed_factor_clk_setup(struct device_node *node)
of_property_read_string(node, "clock-output-names", &clk_name);
parent_name = of_clk_get_parent_name(node, 0);
+ of_property_read_u32(node, "clock-flags", &flags);
- clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+ clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
mult, div);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
diff --git a/include/dt-bindings/clk/clk.h b/include/dt-bindings/clk/clk.h
new file mode 100644
index 0000000..1834933
--- /dev/null
+++ b/include/dt-bindings/clk/clk.h
@@ -0,0 +1,22 @@
+/*
+ * See include/linux/clk-provider.h for more information.
+ */
+
+#ifndef __DT_BINDINGS_CLK_CLK_H
+#define __DT_BINDINGS_CLK_CLK_H
+
+#define BIT(nr) (1UL << (nr))
+
+#define CLK_SET_RATE_GATE BIT(0)
+#define CLK_SET_PARENT_GATE BIT(1)
+#define CLK_SET_RATE_PARENT BIT(2)
+#define CLK_IGNORE_UNUSED BIT(3)
+#define CLK_IS_BASIC BIT(5)
+#define CLK_GET_RATE_NOCACHE BIT(6)
+#define CLK_SET_RATE_NO_REPARENT BIT(7)
+#define CLK_GET_ACCURACY_NOCACHE BIT(8)
+#define CLK_RECALC_NEW_RATES BIT(9)
+#define CLK_SET_RATE_UNGATE BIT(10)
+#define CLK_IS_CRITICAL BIT(11)
+
+#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-21 0:59 ` Stephen Boyd
2016-06-24 3:49 ` [PATCH] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim
@ 2016-06-24 4:12 ` Jongsung Kim
2016-06-28 20:55 ` Rob Herring
1 sibling, 1 reply; 13+ messages in thread
From: Jongsung Kim @ 2016-06-24 4:12 UTC (permalink / raw)
To: Maxime Ripard, Mike Turquette, Stephen Boyd
Cc: linux-clk, devicetree, linux-kernel, Chanho Min, Jongsung Kim
There is no way to set additional flags for a DT-initialized fixed-
factor-clock, and it can be problematic i.e., when the clock rate
needs to be changed. [1][2]
This patch introduces an optional dt-binding named "clock-flags" to
be used for passing any needed flags from dts.
[1] http://www.spinics.net/lists/linux-clk/msg09040.html
[2] https://lkml.org/lkml/2016/6/20/1025
Changes since v1:
- fix possible build failure when using gcc-5 or gcc-6
Signed-off-by: Jongsung Kim <neidhard.kim@lge.com>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Mike Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
---
.../bindings/clock/fixed-factor-clock.txt | 4 ++++
drivers/clk/clk-fixed-factor.c | 4 +++-
include/dt-bindings/clk/clk.h | 22 ++++++++++++++++++++++
3 files changed, 29 insertions(+), 1 deletion(-)
create mode 100644 include/dt-bindings/clk/clk.h
diff --git a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
index 1bae8527..3e1b79e 100644
--- a/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
+++ b/Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
@@ -13,12 +13,16 @@ Required properties:
Optional properties:
- clock-output-names : From common clock binding.
+- clock-flags : Additional flags to be used.
Example:
+ #include <dt-bindings/clk/clk.h>
+
clock {
compatible = "fixed-factor-clock";
clocks = <&parentclk>;
#clock-cells = <0>;
clock-div = <2>;
clock-mult = <1>;
+ clock-flags = <CLK_SET_RATE_PARENT>;
};
diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c
index 75cd6c7..e626cad 100644
--- a/drivers/clk/clk-fixed-factor.c
+++ b/drivers/clk/clk-fixed-factor.c
@@ -150,6 +150,7 @@ void __init of_fixed_factor_clk_setup(struct device_node *node)
struct clk *clk;
const char *clk_name = node->name;
const char *parent_name;
+ u32 flags = 0;
u32 div, mult;
if (of_property_read_u32(node, "clock-div", &div)) {
@@ -166,8 +167,9 @@ void __init of_fixed_factor_clk_setup(struct device_node *node)
of_property_read_string(node, "clock-output-names", &clk_name);
parent_name = of_clk_get_parent_name(node, 0);
+ of_property_read_u32(node, "clock-flags", &flags);
- clk = clk_register_fixed_factor(NULL, clk_name, parent_name, 0,
+ clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
mult, div);
if (!IS_ERR(clk))
of_clk_add_provider(node, of_clk_src_simple_get, clk);
diff --git a/include/dt-bindings/clk/clk.h b/include/dt-bindings/clk/clk.h
new file mode 100644
index 0000000..1834933
--- /dev/null
+++ b/include/dt-bindings/clk/clk.h
@@ -0,0 +1,22 @@
+/*
+ * See include/linux/clk-provider.h for more information.
+ */
+
+#ifndef __DT_BINDINGS_CLK_CLK_H
+#define __DT_BINDINGS_CLK_CLK_H
+
+#define BIT(nr) (1UL << (nr))
+
+#define CLK_SET_RATE_GATE BIT(0)
+#define CLK_SET_PARENT_GATE BIT(1)
+#define CLK_SET_RATE_PARENT BIT(2)
+#define CLK_IGNORE_UNUSED BIT(3)
+#define CLK_IS_BASIC BIT(5)
+#define CLK_GET_RATE_NOCACHE BIT(6)
+#define CLK_SET_RATE_NO_REPARENT BIT(7)
+#define CLK_GET_ACCURACY_NOCACHE BIT(8)
+#define CLK_RECALC_NEW_RATES BIT(9)
+#define CLK_SET_RATE_UNGATE BIT(10)
+#define CLK_IS_CRITICAL BIT(11)
+
+#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-24 3:49 ` [PATCH] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim
@ 2016-06-24 5:11 ` kbuild test robot
2016-06-24 5:57 ` kbuild test robot
1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2016-06-24 5:11 UTC (permalink / raw)
To: Jongsung Kim
Cc: kbuild-all, Maxime Ripard, Mike Turquette, Stephen Boyd,
linux-clk, devicetree, linux-kernel, Chanho Min, Jongsung Kim
[-- Attachment #1: Type: text/plain, Size: 1977 bytes --]
Hi,
[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.7-rc4 next-20160623]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jongsung-Kim/clk-fixed-factor-add-optional-dt-binding-clock-flags/20160624-115201
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: x86_64-randconfig-s4-06241247 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/clk/clk-fixed-factor.c: In function 'of_fixed_factor_clk_setup':
>> drivers/clk/clk-fixed-factor.c:170:44: error: passing argument 3 of 'of_property_read_u32' from incompatible pointer type [-Werror=incompatible-pointer-types]
of_property_read_u32(node, "clock-flags", &flags);
^
In file included from include/linux/clk-provider.h:15:0,
from drivers/clk/clk-fixed-factor.c:11:
include/linux/of.h:916:19: note: expected 'u32 * {aka unsigned int *}' but argument is of type 'long unsigned int *'
static inline int of_property_read_u32(const struct device_node *np,
^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/of_property_read_u32 +170 drivers/clk/clk-fixed-factor.c
164 __func__, node->name);
165 return;
166 }
167
168 of_property_read_string(node, "clock-output-names", &clk_name);
169 parent_name = of_clk_get_parent_name(node, 0);
> 170 of_property_read_u32(node, "clock-flags", &flags);
171
172 clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
173 mult, div);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 26890 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-24 3:49 ` [PATCH] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim
2016-06-24 5:11 ` kbuild test robot
@ 2016-06-24 5:57 ` kbuild test robot
1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2016-06-24 5:57 UTC (permalink / raw)
To: Jongsung Kim
Cc: kbuild-all, Maxime Ripard, Mike Turquette, Stephen Boyd,
linux-clk, devicetree, linux-kernel, Chanho Min, Jongsung Kim
[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]
Hi,
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.7-rc4 next-20160623]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jongsung-Kim/clk-fixed-factor-add-optional-dt-binding-clock-flags/20160624-115201
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux for-next
config: microblaze-mmu_defconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 4.9.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=microblaze
All warnings (new ones prefixed by >>):
drivers/clk/clk-fixed-factor.c: In function 'of_fixed_factor_clk_setup':
>> drivers/clk/clk-fixed-factor.c:170:2: warning: passing argument 3 of 'of_property_read_u32' from incompatible pointer type
of_property_read_u32(node, "clock-flags", &flags);
^
In file included from include/linux/clk-provider.h:15:0,
from drivers/clk/clk-fixed-factor.c:11:
include/linux/of.h:916:19: note: expected 'u32 *' but argument is of type 'long unsigned int *'
static inline int of_property_read_u32(const struct device_node *np,
^
vim +/of_property_read_u32 +170 drivers/clk/clk-fixed-factor.c
154 u32 div, mult;
155
156 if (of_property_read_u32(node, "clock-div", &div)) {
157 pr_err("%s Fixed factor clock <%s> must have a clock-div property\n",
158 __func__, node->name);
159 return;
160 }
161
162 if (of_property_read_u32(node, "clock-mult", &mult)) {
163 pr_err("%s Fixed factor clock <%s> must have a clock-mult property\n",
164 __func__, node->name);
165 return;
166 }
167
168 of_property_read_string(node, "clock-output-names", &clk_name);
169 parent_name = of_clk_get_parent_name(node, 0);
> 170 of_property_read_u32(node, "clock-flags", &flags);
171
172 clk = clk_register_fixed_factor(NULL, clk_name, parent_name, flags,
173 mult, div);
174 if (!IS_ERR(clk))
175 of_clk_add_provider(node, of_clk_src_simple_get, clk);
176 }
177 EXPORT_SYMBOL_GPL(of_fixed_factor_clk_setup);
178 CLK_OF_DECLARE(fixed_factor_clk, "fixed-factor-clock",
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 12638 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-24 4:12 ` [PATCH v2] " Jongsung Kim
@ 2016-06-28 20:55 ` Rob Herring
2016-06-28 21:18 ` Michael Turquette
2016-06-29 5:04 ` Jongsung Kim
0 siblings, 2 replies; 13+ messages in thread
From: Rob Herring @ 2016-06-28 20:55 UTC (permalink / raw)
To: Jongsung Kim
Cc: Maxime Ripard, Mike Turquette, Stephen Boyd, linux-clk,
devicetree, linux-kernel, Chanho Min
On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
> There is no way to set additional flags for a DT-initialized fixed-
> factor-clock, and it can be problematic i.e., when the clock rate
> needs to be changed. [1][2]
>
> This patch introduces an optional dt-binding named "clock-flags" to
> be used for passing any needed flags from dts.
I don't think we want this in DT. If we did, the flags would need some
documentation about what the flags mean.
Rob
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-28 20:55 ` Rob Herring
@ 2016-06-28 21:18 ` Michael Turquette
2016-06-29 7:06 ` Jongsung Kim
2016-06-29 5:04 ` Jongsung Kim
1 sibling, 1 reply; 13+ messages in thread
From: Michael Turquette @ 2016-06-28 21:18 UTC (permalink / raw)
To: Rob Herring, Jongsung Kim
Cc: Maxime Ripard, Stephen Boyd, linux-clk, devicetree, linux-kernel,
Chanho Min
Quoting Rob Herring (2016-06-28 13:55:18)
> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
> > There is no way to set additional flags for a DT-initialized fixed-
> > factor-clock, and it can be problematic i.e., when the clock rate
> > needs to be changed. [1][2]
> >
> > This patch introduces an optional dt-binding named "clock-flags" to
> > be used for passing any needed flags from dts.
>
> I don't think we want this in DT. If we did, the flags would need some
> documentation about what the flags mean.
Flags are specific to Linux implementation, so I agree with Rob. Better
to create a compatible string for your hardware that bakes in the flags.
Regards,
Mike
>
> Rob
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-28 20:55 ` Rob Herring
2016-06-28 21:18 ` Michael Turquette
@ 2016-06-29 5:04 ` Jongsung Kim
1 sibling, 0 replies; 13+ messages in thread
From: Jongsung Kim @ 2016-06-29 5:04 UTC (permalink / raw)
To: Rob Herring
Cc: Maxime Ripard, Mike Turquette, Stephen Boyd, linux-clk,
devicetree, linux-kernel, Chanho Min
On 2016년 06월 29일 05:55, Rob Herring wrote:
> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>> There is no way to set additional flags for a DT-initialized fixed-
>> factor-clock, and it can be problematic i.e., when the clock rate
>> needs to be changed. [1][2]
>>
>> This patch introduces an optional dt-binding named "clock-flags" to
>> be used for passing any needed flags from dts.
> I don't think we want this in DT. If we did, the flags would need some
> documentation about what the flags mean.
Thanks for your comment. It looks not that big deal to provide a little documentation..? Some of the flags looks safe to be removed. Thinking of only fixed-factor-clock, most of them can be removed.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-28 21:18 ` Michael Turquette
@ 2016-06-29 7:06 ` Jongsung Kim
2016-07-02 0:20 ` Stephen Boyd
0 siblings, 1 reply; 13+ messages in thread
From: Jongsung Kim @ 2016-06-29 7:06 UTC (permalink / raw)
To: Michael Turquette, Rob Herring
Cc: Maxime Ripard, Stephen Boyd, linux-clk, devicetree, linux-kernel,
Chanho Min
On 2016년 06월 29일 06:18, Michael Turquette wrote:
> Quoting Rob Herring (2016-06-28 13:55:18)
>> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>>> There is no way to set additional flags for a DT-initialized fixed-
>>> factor-clock, and it can be problematic i.e., when the clock rate
>>> needs to be changed. [1][2]
>>>
>>> This patch introduces an optional dt-binding named "clock-flags" to
>>> be used for passing any needed flags from dts.
>> I don't think we want this in DT. If we did, the flags would need some
>> documentation about what the flags mean.
> Flags are specific to Linux implementation, so I agree with Rob. Better
> to create a compatible string for your hardware that bakes in the flags.
Thank you for your comment, Mike. This conversation starts from lacking method to set CLK_SET_RATE_PARENT from DT. I understand compatible string can be a solution. But.. if someone starts talking about lacking method to set another flag, i.e., CLK_SET_PARENT_GATE, then we'll need another compatible string list.
How do you think about defining possible required subset of the flags and using some more neutral flag-names acceptable in DT?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-06-29 7:06 ` Jongsung Kim
@ 2016-07-02 0:20 ` Stephen Boyd
2016-07-04 1:48 ` Jongsung Kim
0 siblings, 1 reply; 13+ messages in thread
From: Stephen Boyd @ 2016-07-02 0:20 UTC (permalink / raw)
To: Jongsung Kim
Cc: Michael Turquette, Rob Herring, Maxime Ripard, linux-clk,
devicetree, linux-kernel, Chanho Min
On 06/29, Jongsung Kim wrote:
> On 2016년 06월 29일 06:18, Michael Turquette wrote:
> > Quoting Rob Herring (2016-06-28 13:55:18)
> >> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
> >>> There is no way to set additional flags for a DT-initialized fixed-
> >>> factor-clock, and it can be problematic i.e., when the clock rate
> >>> needs to be changed. [1][2]
> >>>
> >>> This patch introduces an optional dt-binding named "clock-flags" to
> >>> be used for passing any needed flags from dts.
> >> I don't think we want this in DT. If we did, the flags would need some
> >> documentation about what the flags mean.
> > Flags are specific to Linux implementation, so I agree with Rob. Better
> > to create a compatible string for your hardware that bakes in the flags.
>
> Thank you for your comment, Mike. This conversation starts from lacking method to set CLK_SET_RATE_PARENT from DT. I understand compatible string can be a solution. But.. if someone starts talking about lacking method to set another flag, i.e., CLK_SET_PARENT_GATE, then we'll need another compatible string list.
> How do you think about defining possible required subset of the flags and using some more neutral flag-names acceptable in DT?
Do you actually have an IC on the board that is doing some fixed
factor calculation? Or is this a clk driver design where we are
listing out each piece of an SoC's clk controller in DT?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-07-02 0:20 ` Stephen Boyd
@ 2016-07-04 1:48 ` Jongsung Kim
2016-08-25 0:52 ` Stephen Boyd
0 siblings, 1 reply; 13+ messages in thread
From: Jongsung Kim @ 2016-07-04 1:48 UTC (permalink / raw)
To: Stephen Boyd
Cc: Michael Turquette, Rob Herring, Maxime Ripard, linux-clk,
devicetree, linux-kernel, Chanho Min
On 2016년 07월 02일 09:20, Stephen Boyd wrote:
> On 06/29, Jongsung Kim wrote:
>> On 2016년 06월 29일 06:18, Michael Turquette wrote:
>>> Quoting Rob Herring (2016-06-28 13:55:18)
>>>> On Fri, Jun 24, 2016 at 01:12:52PM +0900, Jongsung Kim wrote:
>>>>> There is no way to set additional flags for a DT-initialized fixed-
>>>>> factor-clock, and it can be problematic i.e., when the clock rate
>>>>> needs to be changed. [1][2]
>>>>>
>>>>> This patch introduces an optional dt-binding named "clock-flags" to
>>>>> be used for passing any needed flags from dts.
>>>> I don't think we want this in DT. If we did, the flags would need some
>>>> documentation about what the flags mean.
>>> Flags are specific to Linux implementation, so I agree with Rob. Better
>>> to create a compatible string for your hardware that bakes in the flags.
>> Thank you for your comment, Mike. This conversation starts from lacking method to set CLK_SET_RATE_PARENT from DT. I understand compatible string can be a solution. But.. if someone starts talking about lacking method to set another flag, i.e., CLK_SET_PARENT_GATE, then we'll need another compatible string list.
>> How do you think about defining possible required subset of the flags and using some more neutral flag-names acceptable in DT?
> Do you actually have an IC on the board that is doing some fixed
> factor calculation? Or is this a clk driver design where we are
> listing out each piece of an SoC's clk controller in DT?
>
The SoC has several PLLs of identical design, and one of them is divided
to half and used for CPUs. The fixed-factor-clock represents the divider.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags
2016-07-04 1:48 ` Jongsung Kim
@ 2016-08-25 0:52 ` Stephen Boyd
0 siblings, 0 replies; 13+ messages in thread
From: Stephen Boyd @ 2016-08-25 0:52 UTC (permalink / raw)
To: Jongsung Kim
Cc: Michael Turquette, Rob Herring, Maxime Ripard, linux-clk,
devicetree, linux-kernel, Chanho Min
On 07/04, Jongsung Kim wrote:
> On 2016년 07월 02일 09:20, Stephen Boyd wrote:
> > Do you actually have an IC on the board that is doing some fixed
> > factor calculation? Or is this a clk driver design where we are
> > listing out each piece of an SoC's clk controller in DT?
> >
> The SoC has several PLLs of identical design, and one of them is divided
> to half and used for CPUs. The fixed-factor-clock represents the divider.
>
Ok, so it sounds like we can have the driver that registers the
CPU PLL also register the fixed factor clk? I fail to see why we
need this from DT in that case.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-08-25 5:08 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 9:08 [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT Jongsung Kim
2016-06-21 0:59 ` Stephen Boyd
2016-06-24 3:49 ` [PATCH] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim
2016-06-24 5:11 ` kbuild test robot
2016-06-24 5:57 ` kbuild test robot
2016-06-24 4:12 ` [PATCH v2] " Jongsung Kim
2016-06-28 20:55 ` Rob Herring
2016-06-28 21:18 ` Michael Turquette
2016-06-29 7:06 ` Jongsung Kim
2016-07-02 0:20 ` Stephen Boyd
2016-07-04 1:48 ` Jongsung Kim
2016-08-25 0:52 ` Stephen Boyd
2016-06-29 5:04 ` Jongsung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).