Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1420639 > unrolled thread

[RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT

Back to article view | Back to linux.kernel

Contents

  [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT Jongsung Kim <neidhard.kim@lge.com> - 2016-06-13 11:10 +0200
    Re: [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT Stephen Boyd <sboyd@codeaurora.org> - 2016-06-21 03:00 +0200
      [PATCH] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim <neidhard.kim@lge.com> - 2016-06-24 06:00 +0200
        Re: [PATCH] clk: fixed-factor: add optional dt-binding clock-flags kbuild test robot <lkp@intel.com> - 2016-06-24 07:20 +0200
        Re: [PATCH] clk: fixed-factor: add optional dt-binding clock-flags kbuild test robot <lkp@intel.com> - 2016-06-24 08:00 +0200
      [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim <neidhard.kim@lge.com> - 2016-06-24 06:20 +0200
        Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Rob Herring <robh@kernel.org> - 2016-06-28 23:00 +0200
          Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Michael Turquette <mturquette@baylibre.com> - 2016-06-28 23:20 +0200
            Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim <neidhard.kim@lge.com> - 2016-06-29 09:30 +0200
              Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Stephen Boyd <sboyd@codeaurora.org> - 2016-07-02 02:30 +0200
                Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim <neidhard.kim@lge.com> - 2016-07-04 03:50 +0200
          Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags Jongsung Kim <neidhard.kim@lge.com> - 2016-06-29 07:10 +0200

#1420639 — [RESEND][PATCH] clk: fixed-factor: set CLK_SET_RATE_PARENT

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

[toc] | [next] | [standalone]

#1427168

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

[toc] | [prev] | [next] | [standalone]

#1430336 — [PATCH] clk: fixed-factor: add optional dt-binding clock-flags

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

[toc] | [prev] | [next] | [standalone]

#1430364 — Re: [PATCH] clk: fixed-factor: add optional dt-binding clock-flags

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1430378 — Re: [PATCH] clk: fixed-factor: add optional dt-binding clock-flags

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1430340 — [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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

[toc] | [prev] | [next] | [standalone]

#1433300 — Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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

[toc] | [prev] | [next] | [standalone]

#1433327 — Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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

[toc] | [prev] | [next] | [standalone]

#1433517 — Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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?

[toc] | [prev] | [next] | [standalone]

#1435698 — Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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

[toc] | [prev] | [next] | [standalone]

#1436107 — Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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.

[toc] | [prev] | [next] | [standalone]

#1433445 — Re: [PATCH v2] clk: fixed-factor: add optional dt-binding clock-flags

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web