* [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
* 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
* [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 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 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
* 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
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).